When support was originally introduced, ebx, ecx and edx were reserved and
should have been zeroed in recalculate_misc() to avoid leaking into guests.

Since then, fields have been added into ebx.  Guests can't load microcode, so
shouldn't see ucode_size, and while in principle we do want to support larger
RAP sizes in guests, virtualising this for guests depends on AMD procuding any
official documentation for ERAPS, which is long overdue and with no ETA.

This patch will cause a difference in guests on Zen5 CPUs, but as the main
ERAPS feature is hidden, guests should be ignoring the rap_size field too.

Fixes: e9b4fe263649 ("x86/cpuid: support LFENCE always serialising CPUID bit")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/cpu-policy.c            | 3 +++
 xen/include/xen/lib/x86/cpu-policy.h | 5 ++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index 518f9c9e5409..c3aaac861d15 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -329,6 +329,9 @@ static void recalculate_misc(struct cpu_policy *p)
         p->extd.raw[0x1e] = EMPTY_LEAF; /* TopoExt APIC ID/Core/Node */
         p->extd.raw[0x1f] = EMPTY_LEAF; /* SEV */
         p->extd.raw[0x20] = EMPTY_LEAF; /* Platform QoS */
+        p->extd.raw[0x21].b = 0;
+        p->extd.raw[0x21].c = 0;
+        p->extd.raw[0x21].d = 0;
         break;
     }
 }
diff --git a/xen/include/xen/lib/x86/cpu-policy.h b/xen/include/xen/lib/x86/cpu-policy.h
index f43e1a3b21e9..aeaa16bbc732 100644
--- a/xen/include/xen/lib/x86/cpu-policy.h
+++ b/xen/include/xen/lib/x86/cpu-policy.h
@@ -325,7 +325,10 @@ struct cpu_policy
                 uint32_t e21a;
                 struct { DECL_BITFIELD(e21a); };
             };
-            uint32_t /* b */:32, /* c */:32, /* d */:32;
+            uint32_t ucode_size:12, /* Units of 16 bytes */
+                     rap_size:8,    /* Units of 8 entries */
+                     :12;
+            uint32_t /* c */:32, /* d */:32;
         };
     } extd;
 
-- 
2.39.5

On 01/07/2025 11:53 am, Andrew Cooper wrote:
> diff --git a/xen/include/xen/lib/x86/cpu-policy.h b/xen/include/xen/lib/x86/cpu-policy.h
> index f43e1a3b21e9..aeaa16bbc732 100644
> --- a/xen/include/xen/lib/x86/cpu-policy.h
> +++ b/xen/include/xen/lib/x86/cpu-policy.h
> @@ -325,7 +325,10 @@ struct cpu_policy
>                  uint32_t e21a;
>                  struct { DECL_BITFIELD(e21a); };
>              };
> -            uint32_t /* b */:32, /* c */:32, /* d */:32;
> +            uint32_t ucode_size:12, /* Units of 16 bytes */
> +                     rap_size:8,    /* Units of 8 entries */
> +                     :12;

Having tried this out on a real CPU, it's not correct.

The APM and Genona PPR say that ucode_size is 12 bits wide, with the
rest of the register reserved.

However, the Turin PPR says it's 16 bits wide, with rap_size being 8
bits starting at bit 16.  The raw value is 0x00080382, which matches the
PPR.

Therefore I'm going to turn this into a plain uint16_t and uint8_t as
there's no need for bitfields any more.  (Which I suspect might be why
the 12->16 change was made.)

~Andrew

On 01.07.2025 12:53, Andrew Cooper wrote:
> When support was originally introduced, ebx, ecx and edx were reserved and
> should have been zeroed in recalculate_misc() to avoid leaking into guests.
> 
> Since then, fields have been added into ebx.  Guests can't load microcode, so
> shouldn't see ucode_size, and while in principle we do want to support larger
> RAP sizes in guests, virtualising this for guests depends on AMD procuding any
> official documentation for ERAPS, which is long overdue and with no ETA.
> 
> This patch will cause a difference in guests on Zen5 CPUs, but as the main
> ERAPS feature is hidden, guests should be ignoring the rap_size field too.
> 
> Fixes: e9b4fe263649 ("x86/cpuid: support LFENCE always serialising CPUID bit")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

I notice I have similar omissions in the respective AVX10.2 and KL patches.

Jan

On 01/07/2025 1:11 pm, Jan Beulich wrote:
> On 01.07.2025 12:53, Andrew Cooper wrote:
>> When support was originally introduced, ebx, ecx and edx were reserved and
>> should have been zeroed in recalculate_misc() to avoid leaking into guests.
>>
>> Since then, fields have been added into ebx.  Guests can't load microcode, so
>> shouldn't see ucode_size, and while in principle we do want to support larger
>> RAP sizes in guests, virtualising this for guests depends on AMD procuding any
>> official documentation for ERAPS, which is long overdue and with no ETA.
>>
>> This patch will cause a difference in guests on Zen5 CPUs, but as the main
>> ERAPS feature is hidden, guests should be ignoring the rap_size field too.
>>
>> Fixes: e9b4fe263649 ("x86/cpuid: support LFENCE always serialising CPUID bit")
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks.

> I notice I have similar omissions in the respective AVX10.2 and KL patches.

The handling is clearly fragile.  I reviewed e9b4fe263649 and failed to
spot it.

I think it will be better when we're not having Xen fix up behind the
back of an unwitting toolstack.  At least then we'll have all the logic
in libx86 and can unit test it properly with real policies.

Until then, I think we'll just have to stay vigilant.

~Andrew