[PATCH 1/3] kvm: wire up KVM_CAP_VM_GPA_BITS for x86

[PATCH 1/3] kvm: wire up KVM_CAP_VM_GPA_BITS for x86

[Gerd Hoffmann]

Add new guest_phys_bits field to kvm_caps, return the value to
userspace when asked for KVM_CAP_VM_GPA_BITS capability.

Initialize guest_phys_bits with boot_cpu_data.x86_phys_bits.
Vendor modules (i.e. vmx and svm) can adjust this field in case
additional restrictions apply, for example in case EPT has no
support for 5-level paging.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 arch/x86/kvm/x86.h | 2 ++
 arch/x86/kvm/x86.c | 5 +++++
 2 files changed, 7 insertions(+)

diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 2f7e19166658..e03aec3527f8 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -24,6 +24,8 @@ struct kvm_caps {
 	bool has_bus_lock_exit;
 	/* notify VM exit supported? */
 	bool has_notify_vmexit;
+	/* usable guest phys bits */
+	u32  guest_phys_bits;
 
 	u64 supported_mce_cap;
 	u64 supported_xcr0;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 48a61d283406..e270b9b708d1 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4784,6 +4784,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 		if (kvm_is_vm_type_supported(KVM_X86_SW_PROTECTED_VM))
 			r |= BIT(KVM_X86_SW_PROTECTED_VM);
 		break;
+	case KVM_CAP_VM_GPA_BITS:
+		r = kvm_caps.guest_phys_bits;
+		break;
 	default:
 		break;
 	}
@@ -9706,6 +9709,8 @@ static int __kvm_x86_vendor_init(struct kvm_x86_init_ops *ops)
 	if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES))
 		rdmsrl(MSR_IA32_ARCH_CAPABILITIES, host_arch_capabilities);
 
+	kvm_caps.guest_phys_bits = boot_cpu_data.x86_phys_bits;
+
 	r = ops->hardware_setup();
 	if (r != 0)
 		goto out_mmu_exit;
-- 
2.44.0

Re: [PATCH 1/3] kvm: wire up KVM_CAP_VM_GPA_BITS for x86

[Tao Su]

On Fri, Mar 01, 2024 at 11:14:07AM +0100, Gerd Hoffmann wrote:
> Add new guest_phys_bits field to kvm_caps, return the value to
> userspace when asked for KVM_CAP_VM_GPA_BITS capability.
> 
> Initialize guest_phys_bits with boot_cpu_data.x86_phys_bits.
> Vendor modules (i.e. vmx and svm) can adjust this field in case
> additional restrictions apply, for example in case EPT has no
> support for 5-level paging.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  arch/x86/kvm/x86.h | 2 ++
>  arch/x86/kvm/x86.c | 5 +++++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index 2f7e19166658..e03aec3527f8 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -24,6 +24,8 @@ struct kvm_caps {
>  	bool has_bus_lock_exit;
>  	/* notify VM exit supported? */
>  	bool has_notify_vmexit;
> +	/* usable guest phys bits */
> +	u32  guest_phys_bits;
>  
>  	u64 supported_mce_cap;
>  	u64 supported_xcr0;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 48a61d283406..e270b9b708d1 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4784,6 +4784,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  		if (kvm_is_vm_type_supported(KVM_X86_SW_PROTECTED_VM))
>  			r |= BIT(KVM_X86_SW_PROTECTED_VM);
>  		break;
> +	case KVM_CAP_VM_GPA_BITS:
> +		r = kvm_caps.guest_phys_bits;
> +		break;
>  	default:
>  		break;
>  	}
> @@ -9706,6 +9709,8 @@ static int __kvm_x86_vendor_init(struct kvm_x86_init_ops *ops)
>  	if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES))
>  		rdmsrl(MSR_IA32_ARCH_CAPABILITIES, host_arch_capabilities);
>  
> +	kvm_caps.guest_phys_bits = boot_cpu_data.x86_phys_bits;

When KeyID_bits is non-zero, MAXPHYADDR != boot_cpu_data.x86_phys_bits
here, you can check in detect_tme().

Thanks,
Tao

> +
>  	r = ops->hardware_setup();
>  	if (r != 0)
>  		goto out_mmu_exit;
> -- 
> 2.44.0
> 
> 

Re: [PATCH 1/3] kvm: wire up KVM_CAP_VM_GPA_BITS for x86

[Gerd Hoffmann]

> > +	kvm_caps.guest_phys_bits = boot_cpu_data.x86_phys_bits;
> 
> When KeyID_bits is non-zero, MAXPHYADDR != boot_cpu_data.x86_phys_bits
> here, you can check in detect_tme().

from detect_tme():

        /*
         * KeyID bits effectively lower the number of physical address
         * bits.  Update cpuinfo_x86::x86_phys_bits accordingly.
         */
        c->x86_phys_bits -= keyid_bits;

This looks like x86_phys_bits gets adjusted if needed.

take care,
  Gerd

Re: [PATCH 1/3] kvm: wire up KVM_CAP_VM_GPA_BITS for x86

[Tao Su]

On Mon, Mar 04, 2024 at 09:43:53AM +0100, Gerd Hoffmann wrote:
> > > +	kvm_caps.guest_phys_bits = boot_cpu_data.x86_phys_bits;
> > 
> > When KeyID_bits is non-zero, MAXPHYADDR != boot_cpu_data.x86_phys_bits
> > here, you can check in detect_tme().
> 
> from detect_tme():
> 
>         /*
>          * KeyID bits effectively lower the number of physical address
>          * bits.  Update cpuinfo_x86::x86_phys_bits accordingly.
>          */
>         c->x86_phys_bits -= keyid_bits;
> 
> This looks like x86_phys_bits gets adjusted if needed.

If TDP is enabled and supports 5-level, we want kvm_caps.guest_phys_bits=52,
but c->x86_phys_bits!=52 here. Maybe we need to set kvm_caps.guest_phys_bits
according to whether TDP is enabled or not, like leaf 0x80000008 in
__do_cpuid_func().

Thanks,
Tao

Re: [PATCH 1/3] kvm: wire up KVM_CAP_VM_GPA_BITS for x86

[Gerd Hoffmann]

On Mon, Mar 04, 2024 at 04:59:32PM +0800, Tao Su wrote:
> On Mon, Mar 04, 2024 at 09:43:53AM +0100, Gerd Hoffmann wrote:
> > > > +	kvm_caps.guest_phys_bits = boot_cpu_data.x86_phys_bits;
> > > 
> > > When KeyID_bits is non-zero, MAXPHYADDR != boot_cpu_data.x86_phys_bits
> > > here, you can check in detect_tme().
> > 
> > from detect_tme():
> > 
> >         /*
> >          * KeyID bits effectively lower the number of physical address
> >          * bits.  Update cpuinfo_x86::x86_phys_bits accordingly.
> >          */
> >         c->x86_phys_bits -= keyid_bits;
> > 
> > This looks like x86_phys_bits gets adjusted if needed.
> 
> If TDP is enabled and supports 5-level, we want kvm_caps.guest_phys_bits=52,
> but c->x86_phys_bits!=52 here.

Do you talk about EPT or NPT or both?

> Maybe we need to set kvm_caps.guest_phys_bits
> according to whether TDP is enabled or not, like leaf 0x80000008 in
> __do_cpuid_func().

See patches 2+3 of this series.

Maybe it is better to just not set kvm_caps.guest_phys_bits in generic
kvm code and leave that completely to vmx / svm vendor modules.  Or let
the generic code handle the !tdp_enabled case and have the vendor
modules override (considering EPT / NPT limitations) in case tdp is
enabled.

take care,
  Gerd

Re: [PATCH 1/3] kvm: wire up KVM_CAP_VM_GPA_BITS for x86

[Sean Christopherson]

On Fri, Mar 01, 2024, Gerd Hoffmann wrote:
> Add new guest_phys_bits field to kvm_caps, return the value to
> userspace when asked for KVM_CAP_VM_GPA_BITS capability.
> 
> Initialize guest_phys_bits with boot_cpu_data.x86_phys_bits.
> Vendor modules (i.e. vmx and svm) can adjust this field in case
> additional restrictions apply, for example in case EPT has no
> support for 5-level paging.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  arch/x86/kvm/x86.h | 2 ++
>  arch/x86/kvm/x86.c | 5 +++++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index 2f7e19166658..e03aec3527f8 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -24,6 +24,8 @@ struct kvm_caps {
>  	bool has_bus_lock_exit;
>  	/* notify VM exit supported? */
>  	bool has_notify_vmexit;
> +	/* usable guest phys bits */
> +	u32  guest_phys_bits;
>  
>  	u64 supported_mce_cap;
>  	u64 supported_xcr0;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 48a61d283406..e270b9b708d1 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4784,6 +4784,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  		if (kvm_is_vm_type_supported(KVM_X86_SW_PROTECTED_VM))
>  			r |= BIT(KVM_X86_SW_PROTECTED_VM);
>  		break;
> +	case KVM_CAP_VM_GPA_BITS:
> +		r = kvm_caps.guest_phys_bits;

This is not a fast path, just compute the effective guest.MAXPHYADDR on the fly
using tdp_root_level and max_tdp_level.  But as pointed out and discussed in the
previous thread, adverising a guest.MAXPHYADDR that is smaller than host.MAXPHYADDR
simply doesn't work[*].

I thought the plan was to add a way for KVM to advertise the maximum *addressable*
GPA, and figure out a way to communicate that to the guest, e.g. so that firmware
doesn't try to use legal GPAs that the host cannot address.

Paolo, any update on this?

[*] https://lore.kernel.org/all/CALMp9eTutnTxCjQjs-nxP=XC345vTmJJODr+PcSOeaQpBW0Skw@mail.gmail.com

Re: [PATCH 1/3] kvm: wire up KVM_CAP_VM_GPA_BITS for x86

[Xiaoyao Li]

On 3/4/2024 11:15 PM, Sean Christopherson wrote:
> On Fri, Mar 01, 2024, Gerd Hoffmann wrote:
>> Add new guest_phys_bits field to kvm_caps, return the value to
>> userspace when asked for KVM_CAP_VM_GPA_BITS capability.
>>
>> Initialize guest_phys_bits with boot_cpu_data.x86_phys_bits.
>> Vendor modules (i.e. vmx and svm) can adjust this field in case
>> additional restrictions apply, for example in case EPT has no
>> support for 5-level paging.
>>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> ---
>>   arch/x86/kvm/x86.h | 2 ++
>>   arch/x86/kvm/x86.c | 5 +++++
>>   2 files changed, 7 insertions(+)
>>
>> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
>> index 2f7e19166658..e03aec3527f8 100644
>> --- a/arch/x86/kvm/x86.h
>> +++ b/arch/x86/kvm/x86.h
>> @@ -24,6 +24,8 @@ struct kvm_caps {
>>   	bool has_bus_lock_exit;
>>   	/* notify VM exit supported? */
>>   	bool has_notify_vmexit;
>> +	/* usable guest phys bits */
>> +	u32  guest_phys_bits;
>>   
>>   	u64 supported_mce_cap;
>>   	u64 supported_xcr0;
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 48a61d283406..e270b9b708d1 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -4784,6 +4784,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>   		if (kvm_is_vm_type_supported(KVM_X86_SW_PROTECTED_VM))
>>   			r |= BIT(KVM_X86_SW_PROTECTED_VM);
>>   		break;
>> +	case KVM_CAP_VM_GPA_BITS:
>> +		r = kvm_caps.guest_phys_bits;
> 
> This is not a fast path, just compute the effective guest.MAXPHYADDR on the fly
> using tdp_root_level and max_tdp_level.  But as pointed out and discussed in the
> previous thread, adverising a guest.MAXPHYADDR that is smaller than host.MAXPHYADDR
> simply doesn't work[*].
> 
> I thought the plan was to add a way for KVM to advertise the maximum *addressable*
> GPA, and figure out a way to communicate that to the guest, e.g. so that firmware
> doesn't try to use legal GPAs that the host cannot address.

 From one off-list email thread, Paolo was proposing to change the 
definition of CPUID.0x80000008:EAX[23:16] to "Maximum usable physical 
address size in bits", in detail:

  Maximum usable physical address size in bits. Physical addresses
  above this size should not be used, but will not produce a "reserved"
  page fault. When this field is zero, all bits up to PhysAddrSize are
  usable. This field is expected to be nonzero only on guests where
  the hypervisor is using nesting paging.

As I understand it, it turns bit [23:16] of EAX of CPUID 0x80000008 into 
a PV field, that is set by VMM(e.g., KVM/QEMU) and consumed by guest.

So KVM can advertise maximum addressable/usable physical address bits in 
CPUID.0x80000008:EAX[23:16] via GET_SUPPORTED_CPUID.


> Paolo, any update on this?
> 
> [*] https://lore.kernel.org/all/CALMp9eTutnTxCjQjs-nxP=XC345vTmJJODr+PcSOeaQpBW0Skw@mail.gmail.com
> 

[PATCH 2/3] kvm/vmx: limit guest_phys_bits to 48 without 5-level ept

[Gerd Hoffmann]

If EPT has no support for 5-level paging the guest physical address
space is limited to 48 bits.  Adjust kvm_caps.guest_phys_bits
accordingly.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 arch/x86/kvm/vmx/vmx.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 1111d9d08903..8bd644a5022d 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7899,6 +7899,11 @@ static __init void vmx_set_cpu_caps(void)
 
 	if (cpu_has_vmx_waitpkg())
 		kvm_cpu_cap_check_and_set(X86_FEATURE_WAITPKG);
+
+	if (enable_ept &&
+	    !cpu_has_vmx_ept_5levels() &&
+	    kvm_caps.guest_phys_bits > 48)
+		kvm_caps.guest_phys_bits = 48;
 }
 
 static void vmx_request_immediate_exit(struct kvm_vcpu *vcpu)
-- 
2.44.0

[PATCH 3/3] kvm/svm: limit guest_phys_bits to 48 in 4-level paging mode

[Gerd Hoffmann]

If the host runs in 4-level paging mode NPT is restricted to 4 paging
levels too.  Adjust kvm_caps.guest_phys_bits accordingly.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 arch/x86/kvm/svm/svm.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index e90b429c84f1..8c3e2e3bd468 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -5229,6 +5229,11 @@ static __init int svm_hardware_setup(void)
 			  get_npt_level(), PG_LEVEL_1G);
 	pr_info("Nested Paging %sabled\n", npt_enabled ? "en" : "dis");
 
+	if (npt_enabled &&
+	    get_npt_level() == PT64_ROOT_4LEVEL &&
+	    kvm_caps.guest_phys_bits > 48)
+		kvm_caps.guest_phys_bits = 48;
+
 	/* Setup shadow_me_value and shadow_me_mask */
 	kvm_mmu_set_me_spte_mask(sme_me_mask, sme_me_mask);
 
-- 
2.44.0