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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Dec 20, 2023 at 02:46:43PM +0000, Andrew Cooper wrote:
> 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 ?

Maybe just fam17h_c6_disabled, since the main usage of that variable
is to force calling fam17_disable_c6().

> 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.

Yeah, I was about to use fam17h prefix, but that wouldn't cover Hygon.
I we are fine with it I can send an adjusted v2 using fam17h prefix.

Thanks, Roger.