This was true in the K10 days, but even back then the match registers were
really payload data rather than header data.

But, it's really model specific data, and these days typically part of the
signature, so is random data for all intents and purposes.

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>

The single difference from this is:

  @@ -207587,7 +207587,7 @@
   ffff82d0402ad261:	4c 89 ce             	mov    %r9,%rsi
   ffff82d0402ad264:	4c 39 c8             	cmp    %r9,%rax
   ffff82d0402ad267:	0f 82 c2 11 f6 ff    	jb     ffff82d04020e42f <amd_ucode_parse.cold+0x55>
  -ffff82d0402ad26d:	41 83 f9 3f          	cmp    $0x3f,%r9d
  +ffff82d0402ad26d:	41 83 f9 1f          	cmp    $0x1f,%r9d
   ffff82d0402ad271:	0f 86 b8 11 f6 ff    	jbe    ffff82d04020e42f <amd_ucode_parse.cold+0x55>
   ffff82d0402ad277:	85 ed                	test   %ebp,%ebp
   ffff82d0402ad279:	75 55                	jne    ffff82d0402ad2d0 <amd_ucode_parse+0x170>

which is "mc->len < sizeof(struct microcode_patch)" expression in
amd_ucode_parse().
---
 xen/arch/x86/cpu/microcode/amd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/xen/arch/x86/cpu/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c
index 1845f51ba330..54acd6928781 100644
--- a/xen/arch/x86/cpu/microcode/amd.c
+++ b/xen/arch/x86/cpu/microcode/amd.c
@@ -48,7 +48,6 @@ struct microcode_patch {
     uint8_t  sb_rev_id;
     uint8_t  bios_api_rev;
     uint8_t  reserved1[3];
-    uint32_t match_reg[8];
 };
 
 #define UCODE_MAGIC                0x00414d44
-- 
2.39.5

On 24.10.2024 15:22, Andrew Cooper wrote:
> This was true in the K10 days, but even back then the match registers were
> really payload data rather than header data.
> 
> But, it's really model specific data, and these days typically part of the
> signature, so is random data for all intents and purposes.
> 
> 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>
> 
> The single difference from this is:
> 
>   @@ -207587,7 +207587,7 @@
>    ffff82d0402ad261:	4c 89 ce             	mov    %r9,%rsi
>    ffff82d0402ad264:	4c 39 c8             	cmp    %r9,%rax
>    ffff82d0402ad267:	0f 82 c2 11 f6 ff    	jb     ffff82d04020e42f <amd_ucode_parse.cold+0x55>
>   -ffff82d0402ad26d:	41 83 f9 3f          	cmp    $0x3f,%r9d
>   +ffff82d0402ad26d:	41 83 f9 1f          	cmp    $0x1f,%r9d
>    ffff82d0402ad271:	0f 86 b8 11 f6 ff    	jbe    ffff82d04020e42f <amd_ucode_parse.cold+0x55>
>    ffff82d0402ad277:	85 ed                	test   %ebp,%ebp
>    ffff82d0402ad279:	75 55                	jne    ffff82d0402ad2d0 <amd_ucode_parse+0x170>
> 
> which is "mc->len < sizeof(struct microcode_patch)" expression in
> amd_ucode_parse().

Yet is it correct to effectively relax that check, i.e. to accept something
we previously would have rejected?

Jan

On 2024-10-24 09:22, Andrew Cooper wrote:
> This was true in the K10 days, but even back then the match registers were
> really payload data rather than header data.
> 
> But, it's really model specific data, and these days typically part of the
> signature, so is random data for all intents and purposes.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>