diff mbox series

[v3,1/2] kvm/cpuid: remove GuestPhysBits code.

Message ID 20240311104118.284054-2-kraxel@redhat.com (mailing list archive)
State New, archived
Headers show
Series kvm/cpuid: set proper GuestPhysBits in CPUID.0x80000008 | expand

Commit Message

Gerd Hoffmann March 11, 2024, 10:41 a.m. UTC
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>
---
 arch/x86/kvm/cpuid.c | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

Comments

Xiaoyao Li March 12, 2024, 2:38 a.m. UTC | #1
On 3/11/2024 6:41 PM, Gerd Hoffmann wrote:
 > [PATCH v3 1/2] kvm/cpuid: remove GuestPhysBits code.

No need for a ending . in the title.

> 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>
> ---
>   arch/x86/kvm/cpuid.c | 32 ++++++++++++++------------------
>   1 file changed, 14 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index adba49afb5fe..c638b5fb2144 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -1221,26 +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: {
> -		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
> -		 * the guest operates in the same PA space as the host, i.e.
> -		 * 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_enabled)
> -			g_phys_as = boot_cpu_data.x86_phys_bits;
> -		else if (!g_phys_as)
> -			g_phys_as = phys_as;
> +		if (!tdp_enabled) {
> +			/*
> +			 * If TDP (NPT) is disabled use the adjusted host
> +			 * MAXPHYADDR as the guest operates in the same PA
> +			 * space as the host, i.e.  reductions in MAXPHYADDR
> +			 * for memory encryption affect shadow paging, too.
> +			 */

I suggest keeping as the original comment to stay out of if check, and 
keeping the second paragraph with guest MAXPHYADDR thing removed as 
below, because the second paragraph is also useful.

   If TDP is enabled, use the raw bare metal MAXPHYADDR as reductions to
   the HPAs do not affect GPAs.

Otherwise,

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

> +			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);
diff mbox series

Patch

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index adba49afb5fe..c638b5fb2144 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -1221,26 +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: {
-		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
-		 * the guest operates in the same PA space as the host, i.e.
-		 * 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_enabled)
-			g_phys_as = boot_cpu_data.x86_phys_bits;
-		else if (!g_phys_as)
-			g_phys_as = phys_as;
+		if (!tdp_enabled) {
+			/*
+			 * If TDP (NPT) is disabled use the adjusted host
+			 * MAXPHYADDR as the guest operates in the same PA
+			 * space as the host, i.e.  reductions in MAXPHYADDR
+			 * for memory encryption affect shadow paging, too.
+			 */
+			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);