Re: [PATCH] x86/amd: extend CPU errata #1474 affected models

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 20/12/2023 2:22 pm, Roger Pau Monne wrote:
> Errata #1474 does also affect models from family 17h ranges 00-2Fh, so the
> errata now covers all the models released under Family 17h (Zen, Zen+ and
> Zen2).

Perhaps "has now been extended to cover models from ..." ?

> Additionally extend the workaround to Family 18h (Hygon), since it's based on
> the Zen architecture and very likely affected.
>
> Rename all the zen2 related symbols to plain zen, since the errata doesn't
> exclusively affect Zen2 anymore.
>
> Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks for doing this.

Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

I was going to suggest linking to an example revision guide but I see
the AMD website is still broken.

> diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
> index 0f305312ff2a..099646dc3994 100644
> --- a/xen/arch/x86/cpu/amd.c
> +++ b/xen/arch/x86/cpu/amd.c
> @@ -54,7 +54,7 @@ bool __read_mostly amd_acpi_c1e_quirk;
>  bool __ro_after_init amd_legacy_ssbd;
>  bool __initdata amd_virt_spec_ctrl;
>  
> -static bool __read_mostly zen2_c6_disabled;
> +static bool __read_mostly zen_c6_disabled;

amd_1474_c6_disable ?

That's about as general as I can make it, without losing precision.


>  
>  static inline int rdmsr_amd_safe(unsigned int msr, unsigned int *lo,
>                                unsigned int *hi)
> @@ -978,24 +978,24 @@ void amd_check_zenbleed(void)
>                      val & chickenbit ? "chickenbit" : "microcode");
>  }
>  
> -static void cf_check zen2_disable_c6(void *arg)
> +static void cf_check zen_disable_c6(void *arg)

fam17_disable_c6() ?  I know Hygon is 0x18 but it's also reasonably well
know to be the same uarch.

This particular algorithm is good for all Fam17 uarches, irrespective of
#1474, even if they happen to be the same set of CPUs in practice.

~Andrew