[PATCH 3/3] x86/vtd: Switch model check to VFM

Andrew Cooper posted 3 patches 3 months, 2 weeks ago
[PATCH 3/3] x86/vtd: Switch model check to VFM
Posted by Andrew Cooper 3 months, 2 weeks ago
This form is shorer and more legible.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>

As this is the first transformation, an anlysis of the code generation change:

before:
<quirk_iommu_caps>:
	       8b 05 4a 7e 09 00    	mov    0x97e4a(%rip),%eax        # <boot_cpu_data>
	       25 00 ff ff 00       	and    $0xffff00,%eax
	       3d 00 06 01 00       	cmp    $0x10600,%eax
	   /-- 74 0e                	je     <quirk_iommu_caps+0x20>
	/--|-> e9 59 8a dc ff       	jmp    <__x86_return_thunk>
	|  |   66 0f 1f 84 00 00 00 	nopw   0x0(%rax,%rax,1)
	|  |   00 00
	|  \-> 0f b6 05 29 7e 09 00 	movzbl 0x97e29(%rip),%eax        # <boot_cpu_data>
	|      3c 2a                	cmp    $0x2a,%al
	|  /-- 74 04                	je     <quirk_iommu_caps+0x2f>
	|  |   3c 2d                	cmp    $0x2d,%al
	\--|-- 75 e3                	jne    <quirk_iommu_caps+0x12>
	   \-> 48 b8 ff ff ff ff c3 	movabs $0xffffffc3ffffffff,%rax
	       ff ff ff
	       48 21 47 20          	and    %rax,0x20(%rdi)
	       e9 2e 8a dc ff       	jmp    <__x86_return_thunk>

after:
<quirk_iommu_caps>:
	    8b 05 4a 7e 09 00    	mov    0x97e4a(%rip),%eax        # <boot_cpu_data>
	    3d 2a 06 01 00       	cmp    $0x1062a,%eax
	/-- 74 13                	je     <quirk_iommu_caps+0x20>
	|   3d 2d 06 01 00       	cmp    $0x1062d,%eax
	+-- 74 0c                	je     <quirk_iommu_caps+0x20>
	|   e9 57 8a dc ff       	jmp    <__x86_return_thunk>
	|   0f 1f 80 00 00 00 00 	nopl   0x0(%rax)
	\-> 48 b8 ff ff ff ff c3 	movabs $0xffffffc3ffffffff,%rax
	    ff ff ff
	    48 21 47 20          	and    %rax,0x20(%rdi)
	    e9 3d 8a dc ff       	jmp    <__x86_return_thunk>

Previously, GCC managed to merge the "x86_vendor == X86_VENDOR_INTEL && x86 ==
6" condition, but performed a second load for x86_model.

Afterwards, there's a single load, and two compares.

Neither managed to merge the model checks together by playing with bit 2, but
that's something that compilers could learn to and is not suitable to express
at the C level for this kind of logic.

Unrelated to this change, it would be rather better to do `andl $0xc3,
0x24(%rdi)` than to manifest a 64bit constant.
---
 xen/arch/x86/include/asm/cpufeature.h | 2 +-
 xen/drivers/passthrough/vtd/quirks.c  | 7 +++----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/include/asm/cpufeature.h b/xen/arch/x86/include/asm/cpufeature.h
index ba2c1c1c7416..f8b85c0f9520 100644
--- a/xen/arch/x86/include/asm/cpufeature.h
+++ b/xen/arch/x86/include/asm/cpufeature.h
@@ -28,7 +28,7 @@
 
 #define VFM_MAKE(v, f, m) (MASK_INSR(v, VFM_VENDOR_MASK) | \
                            MASK_INSR(f, VFM_FAMILY_MASK) | \
-                           MASK_INSR(f, VFM_MODEL_MASK))
+                           MASK_INSR(m, VFM_MODEL_MASK))
 
 #define VFM_MODEL(vfm)  MASK_EXTR(vfm, VFM_MODEL_MASK)
 #define VFM_FAMILY(vfm) MASK_EXTR(vfm, VFM_FAMILY_MASK)
diff --git a/xen/drivers/passthrough/vtd/quirks.c b/xen/drivers/passthrough/vtd/quirks.c
index dc3dac749ce6..0a10a46d90f7 100644
--- a/xen/drivers/passthrough/vtd/quirks.c
+++ b/xen/drivers/passthrough/vtd/quirks.c
@@ -30,6 +30,7 @@
 #include <xen/pci_regs.h>
 #include <xen/keyhandler.h>
 #include <asm/msi.h>
+#include <asm/intel-family.h>
 #include <asm/irq.h>
 #include <asm/pci.h>
 
@@ -630,9 +631,7 @@ void __init quirk_iommu_caps(struct vtd_iommu *iommu)
      * model because the client parts don't expose their IOMMUs as PCI devices
      * we could match with a Device ID.
      */
-    if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
-         boot_cpu_data.x86 == 6 &&
-         (boot_cpu_data.x86_model == 0x2a ||
-          boot_cpu_data.x86_model == 0x2d) )
+    if ( boot_cpu_data.vfm == INTEL_SANDYBRIDGE ||
+         boot_cpu_data.vfm == INTEL_SANDYBRIDGE_X )
         iommu->cap &= ~(0xful << 34);
 }
-- 
2.39.5


Re: [PATCH 3/3] x86/vtd: Switch model check to VFM
Posted by Andrew Cooper 3 months, 2 weeks ago
On 16/07/2025 2:28 pm, Andrew Cooper wrote:
> diff --git a/xen/arch/x86/include/asm/cpufeature.h b/xen/arch/x86/include/asm/cpufeature.h
> index ba2c1c1c7416..f8b85c0f9520 100644
> --- a/xen/arch/x86/include/asm/cpufeature.h
> +++ b/xen/arch/x86/include/asm/cpufeature.h
> @@ -28,7 +28,7 @@
>  
>  #define VFM_MAKE(v, f, m) (MASK_INSR(v, VFM_VENDOR_MASK) | \
>                             MASK_INSR(f, VFM_FAMILY_MASK) | \
> -                           MASK_INSR(f, VFM_MODEL_MASK))
> +                           MASK_INSR(m, VFM_MODEL_MASK))
>  

This hunk should be in the prior patch.  Fixed up locally.

~Andrew

Re: [PATCH 3/3] x86/vtd: Switch model check to VFM
Posted by Jan Beulich 3 months, 2 weeks ago
On 16.07.2025 15:32, Andrew Cooper wrote:
> On 16/07/2025 2:28 pm, Andrew Cooper wrote:
>> diff --git a/xen/arch/x86/include/asm/cpufeature.h b/xen/arch/x86/include/asm/cpufeature.h
>> index ba2c1c1c7416..f8b85c0f9520 100644
>> --- a/xen/arch/x86/include/asm/cpufeature.h
>> +++ b/xen/arch/x86/include/asm/cpufeature.h
>> @@ -28,7 +28,7 @@
>>  
>>  #define VFM_MAKE(v, f, m) (MASK_INSR(v, VFM_VENDOR_MASK) | \
>>                             MASK_INSR(f, VFM_FAMILY_MASK) | \
>> -                           MASK_INSR(f, VFM_MODEL_MASK))
>> +                           MASK_INSR(m, VFM_MODEL_MASK))
>>  
> 
> This hunk should be in the prior patch.  Fixed up locally.

Then:
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan