All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/2] kvm/cpuid: set proper GuestPhysBits in CPUID.0x80000008
@ 2024-03-13 12:58 Gerd Hoffmann
  2024-03-13 12:58 ` [PATCH v4 1/2] kvm/cpuid: remove GuestPhysBits code Gerd Hoffmann
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Gerd Hoffmann @ 2024-03-13 12:58 UTC (permalink / raw)
  To: kvm; +Cc: Tom Lendacky, Gerd Hoffmann

Use the GuestPhysBits field (EAX[23:16]) to communicate the max
addressable GPA to the guest.  Typically this is identical to the max
effective GPA, except in case the CPU supports MAXPHYADDR > 48 but does
not support 5-level TDP.

See commit messages and source code comments for details.

v4 changes:
 - comment fixups.
 - picked up reviewed-by tags,

Gerd Hoffmann (2):
  kvm/cpuid: remove GuestPhysBits code.
  kvm/cpuid: set proper GuestPhysBits in CPUID.0x80000008

 arch/x86/kvm/mmu.h     |  2 ++
 arch/x86/kvm/cpuid.c   | 41 +++++++++++++++++++++++++++++++----------
 arch/x86/kvm/mmu/mmu.c |  5 +++++
 3 files changed, 38 insertions(+), 10 deletions(-)

-- 
2.44.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4 1/2] kvm/cpuid: remove GuestPhysBits code.
  2024-03-13 12:58 [PATCH v4 0/2] kvm/cpuid: set proper GuestPhysBits in CPUID.0x80000008 Gerd Hoffmann
@ 2024-03-13 12:58 ` Gerd Hoffmann
  2024-03-13 12:58 ` [PATCH v4 2/2] kvm/cpuid: set proper GuestPhysBits in CPUID.0x80000008 Gerd Hoffmann
  2024-04-10  0:19 ` [PATCH v4 0/2] " Sean Christopherson
  2 siblings, 0 replies; 10+ messages in thread
From: Gerd Hoffmann @ 2024-03-13 12:58 UTC (permalink / raw)
  To: kvm
  Cc: Tom Lendacky, Gerd Hoffmann, Xiaoyao Li, Sean Christopherson,
	Paolo Bonzini, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	H. Peter Anvin, open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)

GuestPhysBits (cpuid leaf 80000008, eax[23:16]) is intended for software
use.  Physical CPUs do not set that field.  The current code which
propagates the host's GuestPhysBits to the guest's PhysBits does not
make sense.  Remove it.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 arch/x86/kvm/cpuid.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index adba49afb5fe..3235254724cf 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -1221,9 +1221,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
 		entry->eax = entry->ebx = entry->ecx = 0;
 		break;
 	case 0x80000008: {
-		unsigned g_phys_as = (entry->eax >> 16) & 0xff;
-		unsigned virt_as = max((entry->eax >> 8) & 0xff, 48U);
-		unsigned phys_as = entry->eax & 0xff;
+		unsigned int virt_as = max((entry->eax >> 8) & 0xff, 48U);
+		unsigned int phys_as;
 
 		/*
 		 * If TDP (NPT) is disabled use the adjusted host MAXPHYADDR as
@@ -1231,16 +1230,16 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
 		 * reductions in MAXPHYADDR for memory encryption affect shadow
 		 * paging, too.
 		 *
-		 * If TDP is enabled but an explicit guest MAXPHYADDR is not
-		 * provided, use the raw bare metal MAXPHYADDR as reductions to
-		 * the HPAs do not affect GPAs.
+		 * If TDP is enabled, use the raw bare metal MAXPHYADDR as
+		 * reductions to the HPAs do not affect GPAs.
 		 */
-		if (!tdp_enabled)
-			g_phys_as = boot_cpu_data.x86_phys_bits;
-		else if (!g_phys_as)
-			g_phys_as = phys_as;
+		if (!tdp_enabled) {
+			phys_as = boot_cpu_data.x86_phys_bits;
+		} else {
+			phys_as = entry->eax & 0xff;
+		}
 
-		entry->eax = g_phys_as | (virt_as << 8);
+		entry->eax = phys_as | (virt_as << 8);
 		entry->ecx &= ~(GENMASK(31, 16) | GENMASK(11, 8));
 		entry->edx = 0;
 		cpuid_entry_override(entry, CPUID_8000_0008_EBX);
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 2/2] kvm/cpuid: set proper GuestPhysBits in CPUID.0x80000008
  2024-03-13 12:58 [PATCH v4 0/2] kvm/cpuid: set proper GuestPhysBits in CPUID.0x80000008 Gerd Hoffmann
  2024-03-13 12:58 ` [PATCH v4 1/2] kvm/cpuid: remove GuestPhysBits code Gerd Hoffmann
@ 2024-03-13 12:58 ` Gerd Hoffmann
  2024-04-10  0:19 ` [PATCH v4 0/2] " Sean Christopherson
  2 siblings, 0 replies; 10+ messages in thread
From: Gerd Hoffmann @ 2024-03-13 12:58 UTC (permalink / raw)
  To: kvm
  Cc: Tom Lendacky, Gerd Hoffmann, Xiaoyao Li, Sean Christopherson,
	Paolo Bonzini, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	H. Peter Anvin, open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)

The AMD APM (3.35) defines GuestPhysBits (EAX[23:16]) as:

  Maximum guest physical address size in bits.  This number applies
  only to guests using nested paging.  When this field is zero, refer
  to the PhysAddrSize field for the maximum guest physical address size.

Tom Lendacky confirmed that the purpose of GuestPhysBits is software use
and KVM can use it as described below.  Hardware always returns zero
here.

Use the GuestPhysBits field to communicate the max addressable GPA to
the guest.  Typically this is identical to the max effective GPA, except
in case the CPU supports MAXPHYADDR > 48 but does not support 5-level
TDP.

GuestPhysBits is set only in case TDP is enabled, otherwise it is left
at zero.

GuestPhysBits will be used by the guest firmware to make sure resources
like PCI bars are mapped into the addressable GPA.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 arch/x86/kvm/mmu.h     |  2 ++
 arch/x86/kvm/cpuid.c   | 28 +++++++++++++++++++++++++---
 arch/x86/kvm/mmu/mmu.c |  5 +++++
 3 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index 60f21bb4c27b..b410a227c601 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -100,6 +100,8 @@ static inline u8 kvm_get_shadow_phys_bits(void)
 	return boot_cpu_data.x86_phys_bits;
 }
 
+u8 kvm_mmu_get_max_tdp_level(void);
+
 void kvm_mmu_set_mmio_spte_mask(u64 mmio_value, u64 mmio_mask, u64 access_mask);
 void kvm_mmu_set_me_spte_mask(u64 me_value, u64 me_mask);
 void kvm_mmu_set_ept_masks(bool has_ad_bits, bool has_exec_only);
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 3235254724cf..b54a85d7387e 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -1221,8 +1221,22 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
 		entry->eax = entry->ebx = entry->ecx = 0;
 		break;
 	case 0x80000008: {
+		/*
+		 * GuestPhysAddrSize (EAX[23:16]) is intended for software
+		 * use.
+		 *
+		 * KVM's ABI is to report the effective MAXPHYADDR for the
+		 * guest in PhysAddrSize (phys_as), and the maximum
+		 * *addressable* GPA in GuestPhysAddrSize (g_phys_as).
+		 *
+		 * GuestPhysAddrSize is valid if and only if TDP is enabled,
+		 * in which case the max GPA that can be addressed by KVM may
+		 * be less than the max GPA that can be legally generated by
+		 * the guest, e.g. if MAXPHYADDR>48 but the CPU doesn't
+		 * support 5-level TDP.
+		 */
 		unsigned int virt_as = max((entry->eax >> 8) & 0xff, 48U);
-		unsigned int phys_as;
+		unsigned int phys_as, g_phys_as;
 
 		/*
 		 * If TDP (NPT) is disabled use the adjusted host MAXPHYADDR as
@@ -1231,15 +1245,23 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
 		 * paging, too.
 		 *
 		 * If TDP is enabled, use the raw bare metal MAXPHYADDR as
-		 * reductions to the HPAs do not affect GPAs.
+		 * reductions to the HPAs do not affect GPAs.  The max
+		 * addressable GPA is the same as the max effective GPA, except
+		 * that it's capped at 48 bits if 5-level TDP isn't supported
+		 * (hardware processes bits 51:48 only when walking the fifth
+		 * level page table).
 		 */
 		if (!tdp_enabled) {
 			phys_as = boot_cpu_data.x86_phys_bits;
+			g_phys_as = 0;
 		} else {
 			phys_as = entry->eax & 0xff;
+			g_phys_as = phys_as;
+			if (kvm_mmu_get_max_tdp_level() < 5)
+				g_phys_as = min(g_phys_as, 48);
 		}
 
-		entry->eax = phys_as | (virt_as << 8);
+		entry->eax = phys_as | (virt_as << 8) | (g_phys_as << 16);
 		entry->ecx &= ~(GENMASK(31, 16) | GENMASK(11, 8));
 		entry->edx = 0;
 		cpuid_entry_override(entry, CPUID_8000_0008_EBX);
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 2b515acd8e72..74b9d0354bff 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5309,6 +5309,11 @@ static inline int kvm_mmu_get_tdp_level(struct kvm_vcpu *vcpu)
 	return max_tdp_level;
 }
 
+u8 kvm_mmu_get_max_tdp_level(void)
+{
+	return tdp_root_level ? tdp_root_level : max_tdp_level;
+}
+
 static union kvm_mmu_page_role
 kvm_calc_tdp_mmu_root_page_role(struct kvm_vcpu *vcpu,
 				union kvm_cpu_role cpu_role)
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/2] kvm/cpuid: set proper GuestPhysBits in CPUID.0x80000008
  2024-03-13 12:58 [PATCH v4 0/2] kvm/cpuid: set proper GuestPhysBits in CPUID.0x80000008 Gerd Hoffmann
  2024-03-13 12:58 ` [PATCH v4 1/2] kvm/cpuid: remove GuestPhysBits code Gerd Hoffmann
  2024-03-13 12:58 ` [PATCH v4 2/2] kvm/cpuid: set proper GuestPhysBits in CPUID.0x80000008 Gerd Hoffmann
@ 2024-04-10  0:19 ` Sean Christopherson
  2024-04-12  7:03   ` Xiaoyao Li
  2 siblings, 1 reply; 10+ messages in thread
From: Sean Christopherson @ 2024-04-10  0:19 UTC (permalink / raw)
  To: Sean Christopherson, kvm, Gerd Hoffmann; +Cc: Tom Lendacky

On Wed, 13 Mar 2024 13:58:41 +0100, Gerd Hoffmann wrote:
> Use the GuestPhysBits field (EAX[23:16]) to communicate the max
> addressable GPA to the guest.  Typically this is identical to the max
> effective GPA, except in case the CPU supports MAXPHYADDR > 48 but does
> not support 5-level TDP.
> 
> See commit messages and source code comments for details.
> 
> [...]

Applied to kvm-x86 misc, with massaged changelogs to be more verbose when
describing the impact of each change, e.g. to call out that patch 2 isn't an
urgent fix because guest firmware can simply limit itself to using GPAs that
can be addressed with 4-level paging.

I also tagged patch 1 for stable@, as KVM-on-KVM will do the wrong thing when
patch 2 lands, i.e. KVM will incorrectly advertise the addressable MAXPHYADDR
as the raw/real MAXPHYADDR.

Please holler if you (or anyone) disagrees with the changes or my analysis on
the KVM-on-KVM issue.

Thanks!

[1/2] KVM: x86: Don't advertise guest.MAXPHYADDR as host.MAXPHYADDR in CPUID
      https://github.com/kvm-x86/linux/commit/6f5c9600621b
[2/2] KVM: x86: Advertise max mappable GPA in CPUID.0x80000008.GuestPhysBits
      https://github.com/kvm-x86/linux/commit/b628cb523c65

--
https://github.com/kvm-x86/linux/tree/next

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/2] kvm/cpuid: set proper GuestPhysBits in CPUID.0x80000008
  2024-04-10  0:19 ` [PATCH v4 0/2] " Sean Christopherson
@ 2024-04-12  7:03   ` Xiaoyao Li
  2024-04-12 15:48     ` Sean Christopherson
  0 siblings, 1 reply; 10+ messages in thread
From: Xiaoyao Li @ 2024-04-12  7:03 UTC (permalink / raw)
  To: Sean Christopherson, kvm, Gerd Hoffmann; +Cc: Tom Lendacky

On 4/10/2024 8:19 AM, Sean Christopherson wrote:
> On Wed, 13 Mar 2024 13:58:41 +0100, Gerd Hoffmann wrote:
>> Use the GuestPhysBits field (EAX[23:16]) to communicate the max
>> addressable GPA to the guest.  Typically this is identical to the max
>> effective GPA, except in case the CPU supports MAXPHYADDR > 48 but does
>> not support 5-level TDP.
>>
>> See commit messages and source code comments for details.
>>
>> [...]
> 
> Applied to kvm-x86 misc, with massaged changelogs to be more verbose when
> describing the impact of each change, e.g. to call out that patch 2 isn't an
> urgent fix because guest firmware can simply limit itself to using GPAs that
> can be addressed with 4-level paging.
> 
> I also tagged patch 1 for stable@, as KVM-on-KVM will do the wrong thing when
> patch 2 lands, i.e. KVM will incorrectly advertise the addressable MAXPHYADDR
> as the raw/real MAXPHYADDR.

you mean old KVM on new KVM?

As far as I see, it seems no harm. e.g., if the userspace and L0 KVM 
have the new implementation. On Intel SRF platform, L1 KVM sees 
EAX[23:16]=48, EAX[7:0]=52. And when L1 KVM is old, it reports EAX[7:0] 
= 48 to L1 userspace.

right, 48 is not the raw/real MAXPHYADDR. But I think there is not 
statement on KVM that CPUID.0x8000_0008.EAX[7:0] of 
KVM_GET_SUPPORTED_CPUID reports the raw/real MAXPHYADDR.

> Please holler if you (or anyone) disagrees with the changes or my analysis on
> the KVM-on-KVM issue.
> 
> Thanks!
> 
> [1/2] KVM: x86: Don't advertise guest.MAXPHYADDR as host.MAXPHYADDR in CPUID
>        https://github.com/kvm-x86/linux/commit/6f5c9600621b
> [2/2] KVM: x86: Advertise max mappable GPA in CPUID.0x80000008.GuestPhysBits
>        https://github.com/kvm-x86/linux/commit/b628cb523c65
> 
> --
> https://github.com/kvm-x86/linux/tree/next
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/2] kvm/cpuid: set proper GuestPhysBits in CPUID.0x80000008
  2024-04-12  7:03   ` Xiaoyao Li
@ 2024-04-12 15:48     ` Sean Christopherson
  2024-04-15  6:17       ` Xiaoyao Li
  0 siblings, 1 reply; 10+ messages in thread
From: Sean Christopherson @ 2024-04-12 15:48 UTC (permalink / raw)
  To: Xiaoyao Li; +Cc: kvm, Gerd Hoffmann, Tom Lendacky

On Fri, Apr 12, 2024, Xiaoyao Li wrote:
> On 4/10/2024 8:19 AM, Sean Christopherson wrote:
> > On Wed, 13 Mar 2024 13:58:41 +0100, Gerd Hoffmann wrote:
> > > Use the GuestPhysBits field (EAX[23:16]) to communicate the max
> > > addressable GPA to the guest.  Typically this is identical to the max
> > > effective GPA, except in case the CPU supports MAXPHYADDR > 48 but does
> > > not support 5-level TDP.
> > > 
> > > See commit messages and source code comments for details.
> > > 
> > > [...]
> > 
> > Applied to kvm-x86 misc, with massaged changelogs to be more verbose when
> > describing the impact of each change, e.g. to call out that patch 2 isn't an
> > urgent fix because guest firmware can simply limit itself to using GPAs that
> > can be addressed with 4-level paging.
> > 
> > I also tagged patch 1 for stable@, as KVM-on-KVM will do the wrong thing when
> > patch 2 lands, i.e. KVM will incorrectly advertise the addressable MAXPHYADDR
> > as the raw/real MAXPHYADDR.
> 
> you mean old KVM on new KVM?

Yep.

> As far as I see, it seems no harm. e.g., if the userspace and L0 KVM have
> the new implementation. On Intel SRF platform, L1 KVM sees EAX[23:16]=48,
> EAX[7:0]=52. And when L1 KVM is old, it reports EAX[7:0] = 48 to L1
> userspace.

Yep.

> right, 48 is not the raw/real MAXPHYADDR. But I think there is not statement
> on KVM that CPUID.0x8000_0008.EAX[7:0] of KVM_GET_SUPPORTED_CPUID reports
> the raw/real MAXPHYADDR.

If we go deep enough, it becomes a functional problem.  It's not even _that_
ridiculous/contrived :-)

L1 KVM is still aware that the real MAXPHYADDR=52, and so there are no immediate
issues with reserved bits at that level.

But L1 userspace will unintentionally configure L2 with CPUID.0x8000_0008.EAX[7:0]=48,
and so L2 KVM will incorrectly think bits 51:48 are reserved.  If both L0 and L1
are using TDP, neither L0 nor L1 will intercept #PF.  And because L1 userspace
was told MAXPHYADDR=48, it won't know that KVM needs to be configured with
allow_smaller_maxphyaddr=true in order for the setup to function correctly.

If L2 runs an L3, and does not use EPT, L2 will think it can generate a RSVD #PF
to accelerate emulated MMIO.  The GPA with bits 51:48!=0 created by L2 generates
an EPT violation in L1.  Because L1 doesn't have allow_smaller_maxphyaddr, L1
installs an EPT mapping for the wrong GPA (effectively drops bits 51:48), and
L3 hangs because L1 will keep doing nothing on the resulting EPT violation (L1
thinks there's already a valid mapping).

With patch 1 and the OVMF fixes backported, L1 KVM will enumerate MAXPHYADDR=52,
L1 userspace creates L2 with MAXPHYADDR=52, and L2 OVMF restricts its mappings to
bits 47:0.

At least, I think that's what will happen.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/2] kvm/cpuid: set proper GuestPhysBits in CPUID.0x80000008
  2024-04-12 15:48     ` Sean Christopherson
@ 2024-04-15  6:17       ` Xiaoyao Li
  2024-04-15 14:58         ` Sean Christopherson
  0 siblings, 1 reply; 10+ messages in thread
From: Xiaoyao Li @ 2024-04-15  6:17 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: kvm, Gerd Hoffmann, Tom Lendacky

On 4/12/2024 11:48 PM, Sean Christopherson wrote:
> On Fri, Apr 12, 2024, Xiaoyao Li wrote:
>> On 4/10/2024 8:19 AM, Sean Christopherson wrote:
>>> On Wed, 13 Mar 2024 13:58:41 +0100, Gerd Hoffmann wrote:
>>>> Use the GuestPhysBits field (EAX[23:16]) to communicate the max
>>>> addressable GPA to the guest.  Typically this is identical to the max
>>>> effective GPA, except in case the CPU supports MAXPHYADDR > 48 but does
>>>> not support 5-level TDP.
>>>>
>>>> See commit messages and source code comments for details.
>>>>
>>>> [...]
>>>
>>> Applied to kvm-x86 misc, with massaged changelogs to be more verbose when
>>> describing the impact of each change, e.g. to call out that patch 2 isn't an
>>> urgent fix because guest firmware can simply limit itself to using GPAs that
>>> can be addressed with 4-level paging.
>>>
>>> I also tagged patch 1 for stable@, as KVM-on-KVM will do the wrong thing when
>>> patch 2 lands, i.e. KVM will incorrectly advertise the addressable MAXPHYADDR
>>> as the raw/real MAXPHYADDR.
>>
>> you mean old KVM on new KVM?
> 
> Yep.
> 
>> As far as I see, it seems no harm. e.g., if the userspace and L0 KVM have
>> the new implementation. On Intel SRF platform, L1 KVM sees EAX[23:16]=48,
>> EAX[7:0]=52. And when L1 KVM is old, it reports EAX[7:0] = 48 to L1
>> userspace.
> 
> Yep.
> 
>> right, 48 is not the raw/real MAXPHYADDR. But I think there is not statement
>> on KVM that CPUID.0x8000_0008.EAX[7:0] of KVM_GET_SUPPORTED_CPUID reports
>> the raw/real MAXPHYADDR.
> 
> If we go deep enough, it becomes a functional problem.  It's not even _that_
> ridiculous/contrived :-)
> 
> L1 KVM is still aware that the real MAXPHYADDR=52, and so there are no immediate
> issues with reserved bits at that level.
> 
> But L1 userspace will unintentionally configure L2 with CPUID.0x8000_0008.EAX[7:0]=48,
> and so L2 KVM will incorrectly think bits 51:48 are reserved.  If both L0 and L1
> are using TDP, neither L0 nor L1 will intercept #PF.  And because L1 userspace
> was told MAXPHYADDR=48, it won't know that KVM needs to be configured with
> allow_smaller_maxphyaddr=true in order for the setup to function correctly.

In this case, a) L1 userspace was told by L1 KVM that MAXPHYADDR = 48 
via KVM_GET_SUPPORTED_CPUID. But b) L1 userspace gets MAXPHYADDR = 52 by 
executing CPUID itself.

So if L1 userspace decides to configure MAXPHYADDR to 48 for L2, 
according to a). It is supposed to check if KVM is configured with 
allow_smaller_maxphyaddr=y. Otherwise, it cannot expect it works 
function correctly.

> If L2 runs an L3, and does not use EPT, L2 will think it can generate a RSVD #PF
> to accelerate emulated MMIO.  The GPA with bits 51:48!=0 created by L2 generates
> an EPT violation in L1.  Because L1 doesn't have allow_smaller_maxphyaddr, L1
> installs an EPT mapping for the wrong GPA (effectively drops bits 51:48), and
> L3 hangs because L1 will keep doing nothing on the resulting EPT violation (L1
> thinks there's already a valid mapping).
> 
> With patch 1 and the OVMF fixes backported, L1 KVM will enumerate MAXPHYADDR=52,
> L1 userspace creates L2 with MAXPHYADDR=52, and L2 OVMF restricts its mappings to
> bits 47:0.
> 
> At least, I think that's what will happen.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/2] kvm/cpuid: set proper GuestPhysBits in CPUID.0x80000008
  2024-04-15  6:17       ` Xiaoyao Li
@ 2024-04-15 14:58         ` Sean Christopherson
  2024-04-16  8:47           ` Xiaoyao Li
  0 siblings, 1 reply; 10+ messages in thread
From: Sean Christopherson @ 2024-04-15 14:58 UTC (permalink / raw)
  To: Xiaoyao Li; +Cc: kvm, Gerd Hoffmann, Tom Lendacky

On Mon, Apr 15, 2024, Xiaoyao Li wrote:
> On 4/12/2024 11:48 PM, Sean Christopherson wrote:
> > On Fri, Apr 12, 2024, Xiaoyao Li wrote:
> > If we go deep enough, it becomes a functional problem.  It's not even _that_
> > ridiculous/contrived :-)
> > 
> > L1 KVM is still aware that the real MAXPHYADDR=52, and so there are no immediate
> > issues with reserved bits at that level.
> > 
> > But L1 userspace will unintentionally configure L2 with CPUID.0x8000_0008.EAX[7:0]=48,
> > and so L2 KVM will incorrectly think bits 51:48 are reserved.  If both L0 and L1
> > are using TDP, neither L0 nor L1 will intercept #PF.  And because L1 userspace
> > was told MAXPHYADDR=48, it won't know that KVM needs to be configured with
> > allow_smaller_maxphyaddr=true in order for the setup to function correctly.
> 
> In this case, a) L1 userspace was told by L1 KVM that MAXPHYADDR = 48 via
> KVM_GET_SUPPORTED_CPUID. But b) L1 userspace gets MAXPHYADDR = 52 by
> executing CPUID itself.

KVM can't assume userspace will do raw CPUID.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/2] kvm/cpuid: set proper GuestPhysBits in CPUID.0x80000008
  2024-04-15 14:58         ` Sean Christopherson
@ 2024-04-16  8:47           ` Xiaoyao Li
  2024-04-16 14:14             ` Sean Christopherson
  0 siblings, 1 reply; 10+ messages in thread
From: Xiaoyao Li @ 2024-04-16  8:47 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: kvm, Gerd Hoffmann, Tom Lendacky

On 4/15/2024 10:58 PM, Sean Christopherson wrote:
> On Mon, Apr 15, 2024, Xiaoyao Li wrote:
>> On 4/12/2024 11:48 PM, Sean Christopherson wrote:
>>> On Fri, Apr 12, 2024, Xiaoyao Li wrote:
>>> If we go deep enough, it becomes a functional problem.  It's not even _that_
>>> ridiculous/contrived :-)
>>>
>>> L1 KVM is still aware that the real MAXPHYADDR=52, and so there are no immediate
>>> issues with reserved bits at that level.
>>>
>>> But L1 userspace will unintentionally configure L2 with CPUID.0x8000_0008.EAX[7:0]=48,
>>> and so L2 KVM will incorrectly think bits 51:48 are reserved.  If both L0 and L1
>>> are using TDP, neither L0 nor L1 will intercept #PF.  And because L1 userspace
>>> was told MAXPHYADDR=48, it won't know that KVM needs to be configured with
>>> allow_smaller_maxphyaddr=true in order for the setup to function correctly.
>>
>> In this case, a) L1 userspace was told by L1 KVM that MAXPHYADDR = 48 via
>> KVM_GET_SUPPORTED_CPUID. But b) L1 userspace gets MAXPHYADDR = 52 by
>> executing CPUID itself.
> 
> KVM can't assume userspace will do raw CPUID.

So the KVM ABI is that, KVM_GET_SUPPORTED_CPUID always reports the 
host's MAXPHYADDR, if userspace wants to configure a smaller one than it 
for guest and expect it functioning, it needs to set 
kvm_intel.allower_smaller_maxphyaddr ?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/2] kvm/cpuid: set proper GuestPhysBits in CPUID.0x80000008
  2024-04-16  8:47           ` Xiaoyao Li
@ 2024-04-16 14:14             ` Sean Christopherson
  0 siblings, 0 replies; 10+ messages in thread
From: Sean Christopherson @ 2024-04-16 14:14 UTC (permalink / raw)
  To: Xiaoyao Li; +Cc: kvm, Gerd Hoffmann, Tom Lendacky

On Tue, Apr 16, 2024, Xiaoyao Li wrote:
> On 4/15/2024 10:58 PM, Sean Christopherson wrote:
> > On Mon, Apr 15, 2024, Xiaoyao Li wrote:
> > > On 4/12/2024 11:48 PM, Sean Christopherson wrote:
> > > > On Fri, Apr 12, 2024, Xiaoyao Li wrote:
> > > > If we go deep enough, it becomes a functional problem.  It's not even _that_
> > > > ridiculous/contrived :-)
> > > > 
> > > > L1 KVM is still aware that the real MAXPHYADDR=52, and so there are no immediate
> > > > issues with reserved bits at that level.
> > > > 
> > > > But L1 userspace will unintentionally configure L2 with CPUID.0x8000_0008.EAX[7:0]=48,
> > > > and so L2 KVM will incorrectly think bits 51:48 are reserved.  If both L0 and L1
> > > > are using TDP, neither L0 nor L1 will intercept #PF.  And because L1 userspace
> > > > was told MAXPHYADDR=48, it won't know that KVM needs to be configured with
> > > > allow_smaller_maxphyaddr=true in order for the setup to function correctly.
> > > 
> > > In this case, a) L1 userspace was told by L1 KVM that MAXPHYADDR = 48 via
> > > KVM_GET_SUPPORTED_CPUID. But b) L1 userspace gets MAXPHYADDR = 52 by
> > > executing CPUID itself.
> > 
> > KVM can't assume userspace will do raw CPUID.
> 
> So the KVM ABI is that, KVM_GET_SUPPORTED_CPUID always reports the host's
> MAXPHYADDR,

Not precisely, because KVM will report a reduced value when something, e.g. MKTME,
is stealing physical address bits and KVM is using shadow paging.  I.e. when the
host's effective address width is also the guest's effective address width.

> if userspace wants to configure a smaller one than it for guest and expect it
> functioning, it needs to set kvm_intel.allower_smaller_maxphyaddr ?

Yep.  The interaction with allow_smaller_maxphyaddr is what I want to get "right",
in that I don't want KVM_GET_SUPPORTED_CPUID to report a MAXPHYADDR value that
won't work for KVM's default configuration.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-04-16 14:14 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-13 12:58 [PATCH v4 0/2] kvm/cpuid: set proper GuestPhysBits in CPUID.0x80000008 Gerd Hoffmann
2024-03-13 12:58 ` [PATCH v4 1/2] kvm/cpuid: remove GuestPhysBits code Gerd Hoffmann
2024-03-13 12:58 ` [PATCH v4 2/2] kvm/cpuid: set proper GuestPhysBits in CPUID.0x80000008 Gerd Hoffmann
2024-04-10  0:19 ` [PATCH v4 0/2] " Sean Christopherson
2024-04-12  7:03   ` Xiaoyao Li
2024-04-12 15:48     ` Sean Christopherson
2024-04-15  6:17       ` Xiaoyao Li
2024-04-15 14:58         ` Sean Christopherson
2024-04-16  8:47           ` Xiaoyao Li
2024-04-16 14:14             ` Sean Christopherson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.