Message ID | 20230503182852.3431281-3-seanjc@google.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp1534925vqo; Wed, 3 May 2023 11:40:48 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ54Q4cB06F7MYu2Un30lII4lWy9OqZubBgkBGnM4LvcCP6ZD3c+gkkbdGmpCjJKUtO363EY X-Received: by 2002:a05:6a20:9185:b0:da:8917:bdc5 with SMTP id v5-20020a056a20918500b000da8917bdc5mr29276408pzd.45.1683139247975; Wed, 03 May 2023 11:40:47 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1683139247; cv=none; d=google.com; s=arc-20160816; b=yoD9Ni+feKz4NOgs2ENfrGk4pN2rKWnVU6UzS9YgVM59XtHNxB2XsLJClL4xK3KHAn 46jf/8JzaBqJTe+lOqlPUO5DQjr/oEmyoEhUhJGR8pFOCMCtyfA7/FcQLB2qBQPfu1gS oTWcT2n/WI5ZX7IKrKArWlb+60q6jPf0wDgGM+Rigs7Wj34WyCZQG147ZuoMl4vMzngp 0Gat04aWvDEvGUzNna38dekUoJfyIuzyVMxd+K3Vz9jCYDVEkebKh2fDW3IMyVEz8z45 8RT7hBzjhSJ76ZdK8O0vip5s9HlB8Od8sg0pHLh9bR9AmgchkizugB/jG2PpwYCkXX1R BgBQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:reply-to:dkim-signature; bh=rU7RFo/Qi6sAFl76yDK5utbxvptYWkIyT3bH8yXhY1Y=; b=O1Xg95ltODAR026/w1uUS8y5x2+e+2yFGWT9x9Ca3l9Oz0/eA2k8NYGuZbakOcMhn7 uwKxZlKEYrBf4mfPm2sIr/lWoGbnPGJHLsxplKDyCRpEndEUM7+pT5OS2akGCbZCQKj4 rktzeVWr19bNClXHSj8IDlF3EyI/hP++YlcxOn6GBA4wvWENTK7AiGorCRkKE1vyR6tA 7gm3l9MS7k1I8vWy5k75S37n84LJqSHS0TKrI3CqlcuCbvx4DIQAHf45kDK92bxb+o+0 T4nMZDuN+46xhdOflciSJvKkWIl2WoC89GK1+i6ZoA8rtU50rqVO6E5qHCMJnvOxr4zz 6NwA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20221208 header.b="RPZaNW/A"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id z18-20020aa79592000000b0063b716685fasi33224633pfj.235.2023.05.03.11.40.33; Wed, 03 May 2023 11:40:47 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20221208 header.b="RPZaNW/A"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230085AbjECS3H (ORCPT <rfc822;lhua1029@gmail.com> + 99 others); Wed, 3 May 2023 14:29:07 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60982 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229993AbjECS3D (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 3 May 2023 14:29:03 -0400 Received: from mail-yb1-xb4a.google.com (mail-yb1-xb4a.google.com [IPv6:2607:f8b0:4864:20::b4a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5840F5275 for <linux-kernel@vger.kernel.org>; Wed, 3 May 2023 11:29:02 -0700 (PDT) Received: by mail-yb1-xb4a.google.com with SMTP id 3f1490d57ef6-b9a75194eebso6787835276.1 for <linux-kernel@vger.kernel.org>; Wed, 03 May 2023 11:29:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1683138541; x=1685730541; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:from:to:cc:subject:date:message-id:reply-to; bh=rU7RFo/Qi6sAFl76yDK5utbxvptYWkIyT3bH8yXhY1Y=; b=RPZaNW/A8d2jFPzO5Uu6mxCjBBnsw736c6iwc2fjyW8mE2OJXdJEsKQGpx06e2NqDO JB46FqXGZ7WGezRBcWe8jWL+JiX+IDEHxOAK6jYXZGOxh+xnykTev40tymd/GSAU4hf9 4hqcrlWTCBRW1FoJ7KItL1u+2KGt3wl+wWTQZyzcRM8aOTxDcCqnfopfrkBEGtDQnudd dA/MoBxmvAVZV10G61+rwJGi14dXUX2nVFJKeqdK7EjhTDtEmX0DrSPcJnWxY8HE9ScQ hERe1vFVfwavgFKEXQa4qM3xTjcaZGISPvPubszeUY0/QlqyDUMKDQlKc6BKIaQq1neS 6HBg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683138541; x=1685730541; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=rU7RFo/Qi6sAFl76yDK5utbxvptYWkIyT3bH8yXhY1Y=; b=DX+icj9JKa1HvMPXEX1qvigtQxJqm1WFdHhnjrLnG1Qgi3QA4Ikb782Q1eYSa0oWAf lMx8MYJr67t/RLNykGNoh0h/Ltg0efCzwmnaekXNRbpxF5DbAnkNsGiGBvMjm9B5t3yY It7RE/1vWWFlpAINHzJwlEQi8zXHHYAL54Bge5z+HmYZCG/fiBTUCtHNW9d6cOGJQxni HEvLz7MJ3nqUB+FhUcV3yTCGh+ZJVddp4XYNAQQoy1iBHxHkLbr+ruUSj8KSodBRICih YqNZ0DZs0Kp7YBCI4+hdUCjYtmnYSuIxSxnbR/u6wTmjesK5K2c/vJukr8MtlbZuXp3E NQUA== X-Gm-Message-State: AC+VfDygyuTP0Rj4gdrQvSDLl7pRz54uL535sBNykDXIX3uSYC8pyItC gjAa65K9QjTe9Z4E3F1TlsTY5Rr+sAY= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a25:5305:0:b0:b92:5a75:9f55 with SMTP id h5-20020a255305000000b00b925a759f55mr13450125ybb.1.1683138541455; Wed, 03 May 2023 11:29:01 -0700 (PDT) Reply-To: Sean Christopherson <seanjc@google.com> Date: Wed, 3 May 2023 11:28:49 -0700 In-Reply-To: <20230503182852.3431281-1-seanjc@google.com> Mime-Version: 1.0 References: <20230503182852.3431281-1-seanjc@google.com> X-Mailer: git-send-email 2.40.1.495.gc816e09b53d-goog Message-ID: <20230503182852.3431281-3-seanjc@google.com> Subject: [PATCH 2/5] KVM: SVM: Use kvm_pat_valid() directly instead of kvm_mtrr_valid() From: Sean Christopherson <seanjc@google.com> To: Sean Christopherson <seanjc@google.com>, Paolo Bonzini <pbonzini@redhat.com> Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Wenyao Hai <haiwenyao@uniontech.com>, Ke Guo <guoke@uniontech.com> Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1764899419930872830?= X-GMAIL-MSGID: =?utf-8?q?1764899419930872830?= |
Series |
KVM: x86: Clean up MSR PAT handling
|
|
Commit Message
Sean Christopherson
May 3, 2023, 6:28 p.m. UTC
From: Ke Guo <guoke@uniontech.com> Use kvm_pat_valid() directly instead of bouncing through kvm_mtrr_valid(). The PAT is not an MTRR, and kvm_mtrr_valid() just redirects to kvm_pat_valid(), i.e. for better or worse, KVM doesn't apply the "zap SPTEs" logic to guest PAT changes when the VM has a passthrough device with non-coherent DMA. Signed-off-by: Ke Guo <guoke@uniontech.com> [sean: massage changelog] Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/svm/svm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Comments
> for better or worse, KVM doesn't apply the "zap > SPTEs" logic to guest PAT changes when the VM has a passthrough device > with non-coherent DMA. Is it a bug? > > Signed-off-by: Ke Guo <guoke@uniontech.com> > [sean: massage changelog] > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/svm/svm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > index eb308c9994f9..db237ccdc957 100644 > --- a/arch/x86/kvm/svm/svm.c > +++ b/arch/x86/kvm/svm/svm.c > @@ -2935,7 +2935,7 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr) > > break; > case MSR_IA32_CR_PAT: > - if (!kvm_mtrr_valid(vcpu, MSR_IA32_CR_PAT, data)) > + if (!kvm_pat_valid(data)) > return 1; > vcpu->arch.pat = data; > svm->vmcb01.ptr->save.g_pat = data; Anyway for this change, Reviewed-by: Kai Huang <kai.huang@intel.com>
On Wed, May 03, 2023, Kai Huang wrote: > > for better or worse, KVM doesn't apply the "zap > > SPTEs" logic to guest PAT changes when the VM has a passthrough device > > with non-coherent DMA. > > Is it a bug? No. KVM's MTRR behavior is using a heuristic to try not to break the VM: if the VM has non-coherent DMA, then honor UC mapping in the MTRRs as such mappings may be coverage the non-coherent DMA. From vmx_get_mt_mask(): /* We wanted to honor guest CD/MTRR/PAT, but doing so could result in * memory aliases with conflicting memory types and sometimes MCEs. * We have to be careful as to what are honored and when. The PAT is problematic because it is referenced via the guest PTEs, versus the MTRRs being tied to the guest physical address, e.g. different virtual mappings for the same physical address can yield different memtypes via the PAT. My head hurts just thinking about how that might interact with shadow paging :-) Even the MTRRs are somewhat sketchy because they are technically per-CPU, i.e. two vCPUs could have different memtypes for the same physical address. But in practice, sane software/firmware uses consistent MTRRs across all CPUs.
On Thu, 2023-05-04 at 08:34 -0700, Sean Christopherson wrote: > On Wed, May 03, 2023, Kai Huang wrote: > > > for better or worse, KVM doesn't apply the "zap > > > SPTEs" logic to guest PAT changes when the VM has a passthrough device > > > with non-coherent DMA. > > > > Is it a bug? > > No. KVM's MTRR behavior is using a heuristic to try not to break the VM: if the > VM has non-coherent DMA, then honor UC mapping in the MTRRs as such mappings may > be coverage the non-coherent DMA. > > From vmx_get_mt_mask(): > > /* We wanted to honor guest CD/MTRR/PAT, but doing so could result in > * memory aliases with conflicting memory types and sometimes MCEs. > * We have to be careful as to what are honored and when. > > The PAT is problematic because it is referenced via the guest PTEs, versus the > MTRRs being tied to the guest physical address, e.g. different virtual mappings > for the same physical address can yield different memtypes via the PAT. My head > hurts just thinking about how that might interact with shadow paging :-) > > Even the MTRRs are somewhat sketchy because they are technically per-CPU, i.e. > two vCPUs could have different memtypes for the same physical address. But in > practice, sane software/firmware uses consistent MTRRs across all CPUs. Agreed on all above odds. But I think the answer to my question is actually we simply don't _need_ to zap SPTEs (with non-coherent DMA) when guest's IA32_PAT is changed: 1) If EPT is enabled, IIUC guest's PAT is already horned. VMCS's GUEST_IA32_PAT always reflects the IA32_PAT that guest wants to set. EPT's memtype bits are set according to guest's MTRR. That means guest changing IA32_PAT doesn't need to zap EPT PTEs as "EPT PTEs essentially only replaces guest's MTRRs". 2) If EPT is disabled, looking at the code, if I read correctly, the 'shadow_memtype_mask' is 0 for Intel, in which case KVM won't try to set any PAT memtype bit in shadow MMU PTE, which means the true PAT memtype is always WB and guest's memtype is never horned (guest's MTRRs are also never actually used by HW), which should be fine I guess?? My brain refused to go further :) But anyway back to my question, I think "changing guest's IA32_PAT" shouldn't result in needing to "zap SPTEs".
On Fri, May 05, 2023, Kai Huang wrote: > On Thu, 2023-05-04 at 08:34 -0700, Sean Christopherson wrote: > > On Wed, May 03, 2023, Kai Huang wrote: > > > > for better or worse, KVM doesn't apply the "zap > > > > SPTEs" logic to guest PAT changes when the VM has a passthrough device > > > > with non-coherent DMA. > > > > > > Is it a bug? > > > > No. KVM's MTRR behavior is using a heuristic to try not to break the VM: if the > > VM has non-coherent DMA, then honor UC mapping in the MTRRs as such mappings may > > be coverage the non-coherent DMA. > > > > From vmx_get_mt_mask(): > > > > /* We wanted to honor guest CD/MTRR/PAT, but doing so could result in > > * memory aliases with conflicting memory types and sometimes MCEs. > > * We have to be careful as to what are honored and when. > > > > The PAT is problematic because it is referenced via the guest PTEs, versus the > > MTRRs being tied to the guest physical address, e.g. different virtual mappings > > for the same physical address can yield different memtypes via the PAT. My head > > hurts just thinking about how that might interact with shadow paging :-) > > > > Even the MTRRs are somewhat sketchy because they are technically per-CPU, i.e. > > two vCPUs could have different memtypes for the same physical address. But in > > practice, sane software/firmware uses consistent MTRRs across all CPUs. > > Agreed on all above odds. > > But I think the answer to my question is actually we simply don't _need_ to zap > SPTEs (with non-coherent DMA) when guest's IA32_PAT is changed: > > 1) If EPT is enabled, IIUC guest's PAT is already horned. VMCS's GUEST_IA32_PAT > always reflects the IA32_PAT that guest wants to set. EPT's memtype bits are > set according to guest's MTRR. That means guest changing IA32_PAT doesn't need > to zap EPT PTEs as "EPT PTEs essentially only replaces guest's MTRRs". Ah, yes, you're correct. I thought KVM _always_ set the "ignore guest PAT" bit in the EPT PTEs, but KVM honors guest PAT when non-coherent DMA is present and CR0.CD=0. > 2) If EPT is disabled, looking at the code, if I read correctly, the > 'shadow_memtype_mask' is 0 for Intel, in which case KVM won't try to set any PAT > memtype bit in shadow MMU PTE, which means the true PAT memtype is always WB and > guest's memtype is never horned (guest's MTRRs are also never actually used by > HW), which should be fine I guess?? My brain refused to go further :) Yep. It's entirely possible that VT-d without snoop control simply doesn't work with shadow paging, but no one has ever cared. > But anyway back to my question, I think "changing guest's IA32_PAT" shouldn't > result in needing to "zap SPTEs".
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index eb308c9994f9..db237ccdc957 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -2935,7 +2935,7 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr) break; case MSR_IA32_CR_PAT: - if (!kvm_mtrr_valid(vcpu, MSR_IA32_CR_PAT, data)) + if (!kvm_pat_valid(data)) return 1; vcpu->arch.pat = data; svm->vmcb01.ptr->save.g_pat = data;