Received-SPF: pass (google.com: domain of linux-kernel+bounces-116464-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Mon, 25 Mar 2024 11:43:02 +0800
From: Chao Gao <chao.gao@intel.com>
To: Sean Christopherson <seanjc@google.com>
CC: Paolo Bonzini <pbonzini@redhat.com>, Lai Jiangshan
	<jiangshanlai@gmail.com>, "Paul E. McKenney" <paulmck@kernel.org>, "Josh
 Triplett" <josh@joshtriplett.org>, <kvm@vger.kernel.org>,
	<rcu@vger.kernel.org>, <linux-kernel@vger.kernel.org>, Kevin Tian
	<kevin.tian@intel.com>, Yan Zhao <yan.y.zhao@intel.com>, Yiwei Zhang
	<zzyiwei@google.com>
Subject: Re: [PATCH 5/5] KVM: VMX: Always honor guest PAT on CPUs that
 support self-snoop
Message-ID: <ZgDyxpaf+HgQzYDp@chao-email>
References: <20240309010929.1403984-1-seanjc@google.com>
 <20240309010929.1403984-6-seanjc@google.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240309010929.1403984-6-seanjc@google.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?gz0u3SZD2Rlu8Wxa89MQDGdxS+wRXkddxMvli4Q7QTG6rY9SAhFiwObNvaJ0?=
 =?us-ascii?Q?rcKjS1GEVU8VxxtNiGfpch5AmlP4Jz57Lbeh3MQq4qgGDhzi4UHxQAkUcQTC?=
 =?us-ascii?Q?HxhfNuWc8uvcGm+p/5ZSyf8sSDEh/JWB5OfpmfuNfUwaBpuUYss3N6qHqxB1?=
 =?us-ascii?Q?gL7353uuhS0Zj4Z8zOQ7ly7+nYu0hnd4oAvHbxK55x1HN0iVfhGMIARulNQp?=
 =?us-ascii?Q?0Lpwk3s1fG2TE0839zZhedFqvQRICuL7sHVUZawXmuaPLyfFITv3d1hglxLZ?=
 =?us-ascii?Q?WMMEerlhOEQcERRA2lPKE8bAkDEQ4ZdnKde/Ktp4S2i3tQkhXMsLTEYI4Fw/?=
 =?us-ascii?Q?4Sh/vHu9l3ra2OAJH09Fga4t23lkesOa6Gn0RL1J2XWXDnVnj7SOvuAKRshH?=
 =?us-ascii?Q?yPbFFAW6P0+lRSKCj7Y+8pbm3Qi19H0zxZAC7mxRZKD1Nexb8j/ZRFYdWqWB?=
 =?us-ascii?Q?LroviPbcLZaV1MiSABDxR51IatK2EiacMRPkUWeCPMK5u8tEZHF4h+YeugEg?=
 =?us-ascii?Q?90NTnpBDyjvd1yfEeS3NKD0oW1lnKZ81npdBYyV7612pN5hISlSgvZ+s8Jqx?=
 =?us-ascii?Q?ycwdsovBoGi/0fHtXra+6fw9C9sPuZ8TjA7NFomnst5Nbd1gLIb+O7qbSMOA?=
 =?us-ascii?Q?Q1KyFijk8s+2QF+ohzpPYA6OxEAKANLdc+ehws7WJypIk/5gfHrS+K/OXo4Q?=
 =?us-ascii?Q?4uTxtAnnngMhM6UQu1+kkPvPEI3BLnhKmg7m/M6cmgLerh8WXCB9oktnHkIK?=
 =?us-ascii?Q?HcAnwH41BavTye3pFy5tsBIySLui2l/XE1za1C/omkGyG6W+C3+W5Duv4+9+?=
 =?us-ascii?Q?UE6wZW4AO779z39goXfmwBTtEXz4FFJFBnhpZaMzebaRd45c/vDZcFmMtoSh?=
 =?us-ascii?Q?03iZMvgzDWneX9795o8LRlBtKBTqeiF0P+q1BZA1qsaI4Nk2QVu2iu6PaJPr?=
 =?us-ascii?Q?VJTGKRCrzaWB1ymWoim8999nX7SDODXxoRFq2xxlYKmUk6RbtKHDVZ6F8l8s?=
 =?us-ascii?Q?RPzE3tBK8jM8gawbZjeikdenB4rQLk3PnmeR3bnpOwVJ9OOC2AmWc4H8Z2Z8?=
 =?us-ascii?Q?ofn7HN5/X1EFygdwfFrZh2eR0zIkVqGyKeHo9TkbtNtW8XZnjWuKePV6BHhJ?=
 =?us-ascii?Q?LLq4q7KGUwCA7bpmOKgCp6kP5CioEKDebmnY/PH4wTfHmr0kbTvvekzyVu/k?=
 =?us-ascii?Q?tEd0751BTpXD2xUU2GF/Qb+rMt/J0cu+/c01ZqoDClpp5Pu3ItRWAnGKQ5ml?=
 =?us-ascii?Q?V3N1bEuLtR4CytEw+ZbjHBzdJiq6VcphCjD318KXo2K/o+zoQB24IVcAL100?=
 =?us-ascii?Q?wgM9VfdmldqYO1uhEWCD2MVJwpPVbveJYRmaFanxtzYSBdF6aFdzJa6EJTvj?=
 =?us-ascii?Q?oONEkjrcdsGVZww9uTTyhd0OMkCwqOuEn/cmNid0g5lMr05OlKXUB2D7dD6g?=
 =?us-ascii?Q?C78sCcGUJDNUAe4VuijHZqkrpHINQt1oMFRs/YCCIMSv9MWBSdsMkZ9AuGsU?=
 =?us-ascii?Q?K6avV5beeBzKnqyJdFdoajMFfZ1wDlOjpPtObTl9/LEQ28v1e24neVq+touJ?=
 =?us-ascii?Q?GCgwAOVPDgUEs9Tqp+EcSFozsLQ0XSnbYfuxPapC?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 8887c484-a786-44bd-8e81-08dc4c7db20d
X-MS-Exchange-CrossTenant-AuthSource: CH3PR11MB8660.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Mar 2024 03:43:12.5904
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 4wPtm/QKXjXiuaXaSmnulV/9A1qMOO6Lu3WKohjc4xpd9zPnh6SixIjWj9FzcjxjeUGQaY1bSI2xqDvDJn+SqQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB4952
X-OriginatorOrg: intel.com

On Fri, Mar 08, 2024 at 05:09:29PM -0800, Sean Christopherson wrote:
>Unconditionally honor guest PAT on CPUs that support self-snoop, as
>Intel has confirmed that CPUs that support self-snoop always snoop caches
>and store buffers.  I.e. CPUs with self-snoop maintain cache coherency
>even in the presence of aliased memtypes, thus there is no need to trust
>the guest behaves and only honor PAT as a last resort, as KVM does today.
>
>Honoring guest PAT is desirable for use cases where the guest has access
>to non-coherent DMA _without_ bouncing through VFIO, e.g. when a virtual
>(mediated, for all intents and purposes) GPU is exposed to the guest, along
>with buffers that are consumed directly by the physical GPU, i.e. which
>can't be proxied by the host to ensure writes from the guest are performed
>with the correct memory type for the GPU.
>
>Cc: Yiwei Zhang <zzyiwei@google.com>
>Suggested-by: Yan Zhao <yan.y.zhao@intel.com>
>Suggested-by: Kevin Tian <kevin.tian@intel.com>
>Signed-off-by: Sean Christopherson <seanjc@google.com>
>---
> arch/x86/kvm/mmu/mmu.c |  8 +++++---
> arch/x86/kvm/vmx/vmx.c | 10 ++++++----
> 2 files changed, 11 insertions(+), 7 deletions(-)
>
>diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
>index 403cd8f914cd..7fa514830628 100644
>--- a/arch/x86/kvm/mmu/mmu.c
>+++ b/arch/x86/kvm/mmu/mmu.c
>@@ -4622,14 +4622,16 @@ static int kvm_tdp_mmu_page_fault(struct kvm_vcpu *vcpu,
> bool kvm_mmu_may_ignore_guest_pat(void)
> {
> 	/*
>-	 * When EPT is enabled (shadow_memtype_mask is non-zero), and the VM
>+	 * When EPT is enabled (shadow_memtype_mask is non-zero), the CPU does
>+	 * not support self-snoop (or is affected by an erratum), and the VM
> 	 * has non-coherent DMA (DMA doesn't snoop CPU caches), KVM's ABI is to
> 	 * honor the memtype from the guest's PAT so that guest accesses to
> 	 * memory that is DMA'd aren't cached against the guest's wishes.  As a
> 	 * result, KVM _may_ ignore guest PAT, whereas without non-coherent DMA,
>-	 * KVM _always_ ignores guest PAT (when EPT is enabled).
>+	 * KVM _always_ ignores or honors guest PAT, i.e. doesn't toggle SPTE
>+	 * bits in response to non-coherent device (un)registration.
> 	 */
>-	return shadow_memtype_mask;
>+	return !static_cpu_has(X86_FEATURE_SELFSNOOP) && shadow_memtype_mask;
> }
> 
> int kvm_tdp_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
>diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>index 17a8e4fdf9c4..5dc4c24ae203 100644
>--- a/arch/x86/kvm/vmx/vmx.c
>+++ b/arch/x86/kvm/vmx/vmx.c
>@@ -7605,11 +7605,13 @@ static u8 vmx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
> 
> 	/*
> 	 * Force WB and ignore guest PAT if the VM does NOT have a non-coherent
>-	 * device attached.  Letting the guest control memory types on Intel
>-	 * CPUs may result in unexpected behavior, and so KVM's ABI is to trust
>-	 * the guest to behave only as a last resort.
>+	 * device attached and the CPU doesn't support self-snoop.  Letting the
>+	 * guest control memory types on Intel CPUs without self-snoop may
>+	 * result in unexpected behavior, and so KVM's (historical) ABI is to
>+	 * trust the guest to behave only as a last resort.
> 	 */
>-	if (!kvm_arch_has_noncoherent_dma(vcpu->kvm))
>+	if (!static_cpu_has(X86_FEATURE_SELFSNOOP) &&
>+	    !kvm_arch_has_noncoherent_dma(vcpu->kvm))
> 		return (MTRR_TYPE_WRBACK << VMX_EPT_MT_EPTE_SHIFT) | VMX_EPT_IPAT_BIT;

W/ this change, guests w/o pass-thru devices can also access UC memory. Locking
UC memory leads to bus lock. So, guests w/o pass-thru devices can potentially
launch DOS attacks on other CPUs on host. isn't it a problem?