[for-4.18][PATCH 0/2] Fix AMD erratum #1485 on AMD Zen4

[for-4.18][PATCH 0/2] Fix AMD erratum #1485 on AMD Zen4

[Alejandro Vallejo]

This patch should make it to 4.18, as it prevents random crashes on AMD
Zen4 running outdated microcode.

Under certain conditions Zen4 may corrupt its own code stream when SMT is
enabled and STIBP is not. The Linux thread in which this was highlighted is
in patch2's commit message.

NOTE: Still running in CI as of now, but tested locally. Pipeline here.
    https://gitlab.com/xen-project/people/agvallejo/xen/-/pipelines/1034895039

Patch 1: is just a minor refactor to ensure we don't get microarchitectures
         of different families mixed up now that we have 3 different
         families involved (Fam17h, Fam18h and Fam19h)
Patch 2: The actual fix. It involves setting a bit in an MSR if it's a non
         virtualized zen4. It's not a direct copy of the Linux patch, as we
         have started using macros to find out microarchitectures from
         CPUID bits, rather than relying on models.

Alejandro Vallejo (2):
  xen/x86: Add family guards to the is_zen[12]_uarch() macros
  x86/amd: Prevent potentially fetching wrong instruction bytes on Zen4

 xen/arch/x86/cpu/amd.c               | 16 +++++++++++++---
 xen/arch/x86/include/asm/amd.h       | 17 +++++++++++++----
 xen/arch/x86/include/asm/msr-index.h |  2 ++
 xen/arch/x86/spec_ctrl.c             |  3 ---
 4 files changed, 28 insertions(+), 10 deletions(-)

-- 
2.34.1

[for-4.18][PATCH 1/2] xen/x86: Add family guards to the is_zen[12]_uarch() macros

[Alejandro Vallejo]

It slightly simplifies the code that uses them at no real cost because the
code is very rarely executed. This makes it harder to confuse zen uarches
due to missing or mistaken family checks.

No functional change intended.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
 xen/arch/x86/cpu/amd.c         | 5 ++---
 xen/arch/x86/include/asm/amd.h | 9 +++++----
 xen/arch/x86/spec_ctrl.c       | 3 ---
 3 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index 4f27187f92..22aa8c0085 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -1056,8 +1056,7 @@ static void cf_check init_amd(struct cpuinfo_x86 *c)
 
 	amd_init_ssbd(c);
 
-	if (c->x86 == 0x17)
-		amd_init_spectral_chicken();
+	amd_init_spectral_chicken();
 
 	/* Probe for NSCB on Zen2 CPUs when not virtualised */
 	if (!cpu_has_hypervisor && !cpu_has_nscb && c == &boot_cpu_data &&
@@ -1293,7 +1292,7 @@ static int __init cf_check zen2_c6_errata_check(void)
 	 */
 	s_time_t delta;
 
-	if (cpu_has_hypervisor || boot_cpu_data.x86 != 0x17 || !is_zen2_uarch())
+	if (cpu_has_hypervisor || !is_zen2_uarch())
 		return 0;
 
 	/*
diff --git a/xen/arch/x86/include/asm/amd.h b/xen/arch/x86/include/asm/amd.h
index d862cb7972..5a40bcc2ba 100644
--- a/xen/arch/x86/include/asm/amd.h
+++ b/xen/arch/x86/include/asm/amd.h
@@ -145,11 +145,12 @@
  * Hygon (Fam18h) but without simple model number rules.  Instead, use STIBP
  * as a heuristic that distinguishes the two.
  *
- * The caller is required to perform the appropriate vendor/family checks
- * first.
+ * The caller is required to perform the appropriate vendor check first.
  */
-#define is_zen1_uarch() (!boot_cpu_has(X86_FEATURE_AMD_STIBP))
-#define is_zen2_uarch()   boot_cpu_has(X86_FEATURE_AMD_STIBP)
+#define is_zen1_uarch() ((boot_cpu_data.x86 == 0x17 || boot_cpu_data.x86 == 0x18) && \
+                         !boot_cpu_has(X86_FEATURE_AMD_STIBP))
+#define is_zen2_uarch() (boot_cpu_data.x86 == 0x17 && \
+                         boot_cpu_has(X86_FEATURE_AMD_STIBP))
 
 struct cpuinfo_x86;
 int cpu_has_amd_erratum(const struct cpuinfo_x86 *, int, ...);
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 6fd7d44ce4..508c454516 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -976,9 +976,6 @@ static bool __init has_div_vuln(void)
            (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
         return false;
 
-    if ( boot_cpu_data.x86 != 0x17 && boot_cpu_data.x86 != 0x18 )
-        return false;
-
     return is_zen1_uarch();
 }
 
-- 
2.34.1

[for-4.18][PATCH 2/2] x86/amd: Prevent potentially fetching wrong instruction bytes on Zen4

[Alejandro Vallejo]

If Zen4 runs with SMT and without STIBP, then it may corrupt its own code
stream. This is erratum #1485 in the AMD specs.

Fix adapted off Linux's mailing list:
  https://lore.kernel.org/lkml/D99589F4-BC5D-430B-87B2-72C20370CF57@exactcode.com/T/#u

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
 xen/arch/x86/cpu/amd.c               | 11 +++++++++++
 xen/arch/x86/include/asm/amd.h       |  8 ++++++++
 xen/arch/x86/include/asm/msr-index.h |  2 ++
 3 files changed, 21 insertions(+)

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index 22aa8c0085..2426e4cf15 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -1166,6 +1166,17 @@ static void cf_check init_amd(struct cpuinfo_x86 *c)
 	if (c->x86 == 0x10)
 		__clear_bit(X86_FEATURE_MONITOR, c->x86_capability);
 
+	/*
+	 * Erratum #1485: Outdated microcode on Zen4 may cause corruption
+	 * in the code stream if SMT is enabled and STIBP is not. The fix
+	 * is cheap, so it's applied irrespective of the loaded microcode.
+	 */
+	if (!cpu_has_hypervisor && is_zen4_uarch()) {
+		rdmsrl(MSR_AMD64_BP_CFG, value);
+		wrmsrl(MSR_AMD64_BP_CFG,
+		       value | AMD64_BP_CFG_SHARED_BTB_FIX);
+	}
+
 	if (!cpu_has_amd_erratum(c, AMD_ERRATUM_121))
 		opt_allow_unsafe = 1;
 	else if (opt_allow_unsafe < 0)
diff --git a/xen/arch/x86/include/asm/amd.h b/xen/arch/x86/include/asm/amd.h
index 5a40bcc2ba..7d18f7c66b 100644
--- a/xen/arch/x86/include/asm/amd.h
+++ b/xen/arch/x86/include/asm/amd.h
@@ -145,12 +145,20 @@
  * Hygon (Fam18h) but without simple model number rules.  Instead, use STIBP
  * as a heuristic that distinguishes the two.
  *
+ * Zen3 and Zen4 are easier to spot by model number, but it's still not a
+ * single range. We use AutoIBRS to to discriminate instead, as that's a
+ * Zen4-specific feature.
+ *
  * The caller is required to perform the appropriate vendor check first.
  */
 #define is_zen1_uarch() ((boot_cpu_data.x86 == 0x17 || boot_cpu_data.x86 == 0x18) && \
                          !boot_cpu_has(X86_FEATURE_AMD_STIBP))
 #define is_zen2_uarch() (boot_cpu_data.x86 == 0x17 && \
                          boot_cpu_has(X86_FEATURE_AMD_STIBP))
+#define is_zen3_uarch() (boot_cpu_data.x86 == 0x19 && \
+                         !boot_cpu_has(X86_FEATURE_AUTO_IBRS))
+#define is_zen4_uarch() (boot_cpu_data.x86 == 0x19 && \
+                         boot_cpu_has(X86_FEATURE_AUTO_IBRS))
 
 struct cpuinfo_x86;
 int cpu_has_amd_erratum(const struct cpuinfo_x86 *, int, ...);
diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
index 11ffed543a..4437e8a63e 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -403,6 +403,8 @@
 #define MSR_AMD64_DE_CFG		0xc0011029
 #define AMD64_DE_CFG_LFENCE_SERIALISE	(_AC(1, ULL) << 1)
 #define MSR_AMD64_EX_CFG		0xc001102c
+#define MSR_AMD64_BP_CFG		0xc001102e
+#define AMD64_BP_CFG_SHARED_BTB_FIX	(_AC(1, ULL) << 5)
 #define MSR_AMD64_DE_CFG2		0xc00110e3
 
 #define MSR_AMD64_DR0_ADDRESS_MASK	0xc0011027
-- 
2.34.1

Re: [for-4.18][PATCH 0/2] Fix AMD erratum #1485 on AMD Zen4

[Henry Wang]

Hi Alejandro,

> On Oct 13, 2023, at 01:25, Alejandro Vallejo <alejandro.vallejo@cloud.com> wrote:
> 
> This patch should make it to 4.18, as it prevents random crashes on AMD
> Zen4 running outdated microcode.

Yes I agree, so for this series:

Release-acked-by: Henry Wang <Henry.Wang@arm.com>

Kind regards,
Henry


> 
> Under certain conditions Zen4 may corrupt its own code stream when SMT is
> enabled and STIBP is not. The Linux thread in which this was highlighted is
> in patch2's commit message.
> 
> NOTE: Still running in CI as of now, but tested locally. Pipeline here.
>    https://gitlab.com/xen-project/people/agvallejo/xen/-/pipelines/1034895039
> 
> Patch 1: is just a minor refactor to ensure we don't get microarchitectures
>         of different families mixed up now that we have 3 different
>         families involved (Fam17h, Fam18h and Fam19h)
> Patch 2: The actual fix. It involves setting a bit in an MSR if it's a non
>         virtualized zen4. It's not a direct copy of the Linux patch, as we
>         have started using macros to find out microarchitectures from
>         CPUID bits, rather than relying on models.
> 
> Alejandro Vallejo (2):
>  xen/x86: Add family guards to the is_zen[12]_uarch() macros
>  x86/amd: Prevent potentially fetching wrong instruction bytes on Zen4
> 
> xen/arch/x86/cpu/amd.c               | 16 +++++++++++++---
> xen/arch/x86/include/asm/amd.h       | 17 +++++++++++++----
> xen/arch/x86/include/asm/msr-index.h |  2 ++
> xen/arch/x86/spec_ctrl.c             |  3 ---
> 4 files changed, 28 insertions(+), 10 deletions(-)
> 
> -- 
> 2.34.1
> 

Re: [for-4.18][PATCH 1/2] xen/x86: Add family guards to the is_zen[12]_uarch() macros

[Andrew Cooper]

On 13/10/2023 1:26 am, Alejandro Vallejo wrote:
> It slightly simplifies the code that uses them at no real cost because the
> code is very rarely executed. This makes it harder to confuse zen uarches
> due to missing or mistaken family checks.

I'm afraid I disagree.

It's bogus to do a family check without a vendor check.  By making this
change, you're separating (spatially in code, and therefore cognitively)
the two checks that it's important to be able to cross-check.

> diff --git a/xen/arch/x86/include/asm/amd.h b/xen/arch/x86/include/asm/amd.h
> index d862cb7972..5a40bcc2ba 100644
> --- a/xen/arch/x86/include/asm/amd.h
> +++ b/xen/arch/x86/include/asm/amd.h
> @@ -145,11 +145,12 @@
>   * Hygon (Fam18h) but without simple model number rules.  Instead, use STIBP
>   * as a heuristic that distinguishes the two.
>   *
> - * The caller is required to perform the appropriate vendor/family checks
> - * first.
> + * The caller is required to perform the appropriate vendor check first.
>   */
> -#define is_zen1_uarch() (!boot_cpu_has(X86_FEATURE_AMD_STIBP))
> -#define is_zen2_uarch()   boot_cpu_has(X86_FEATURE_AMD_STIBP)
> +#define is_zen1_uarch() ((boot_cpu_data.x86 == 0x17 || boot_cpu_data.x86 == 0x18) && \
> +                         !boot_cpu_has(X86_FEATURE_AMD_STIBP))
> +#define is_zen2_uarch() (boot_cpu_data.x86 == 0x17 && \
> +                         boot_cpu_has(X86_FEATURE_AMD_STIBP))

What leads you to believe there aren't Hygon Zen2's ?

~Andrew

Re: [for-4.18][PATCH 2/2] x86/amd: Prevent potentially fetching wrong instruction bytes on Zen4

[Andrew Cooper]

On 13/10/2023 1:26 am, Alejandro Vallejo wrote:
> If Zen4 runs with SMT and without STIBP, then it may corrupt its own code
> stream. This is erratum #1485 in the AMD specs.

I'm afraid this description isn't fully accurate, and I can't elaborate
any further at this juncture.

Just say "address AMD erratum #1485".  When the revision guides do get
published, things will become clearer.

> diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
> index 22aa8c0085..2426e4cf15 100644
> --- a/xen/arch/x86/cpu/amd.c
> +++ b/xen/arch/x86/cpu/amd.c
> @@ -1166,6 +1166,17 @@ static void cf_check init_amd(struct cpuinfo_x86 *c)
>  	if (c->x86 == 0x10)
>  		__clear_bit(X86_FEATURE_MONITOR, c->x86_capability);
>  
> +	/*
> +	 * Erratum #1485: Outdated microcode on Zen4 may cause corruption
> +	 * in the code stream if SMT is enabled and STIBP is not. The fix
> +	 * is cheap, so it's applied irrespective of the loaded microcode.

Again, unfortunately not accurate.  Just link to the erratum here.

> +	 */
> +	if (!cpu_has_hypervisor && is_zen4_uarch()) {
> +		rdmsrl(MSR_AMD64_BP_CFG, value);
> +		wrmsrl(MSR_AMD64_BP_CFG,
> +		       value | AMD64_BP_CFG_SHARED_BTB_FIX);
> +	}
> +
>  	if (!cpu_has_amd_erratum(c, AMD_ERRATUM_121))
>  		opt_allow_unsafe = 1;
>  	else if (opt_allow_unsafe < 0)
> diff --git a/xen/arch/x86/include/asm/amd.h b/xen/arch/x86/include/asm/amd.h
> index 5a40bcc2ba..7d18f7c66b 100644
> --- a/xen/arch/x86/include/asm/amd.h
> +++ b/xen/arch/x86/include/asm/amd.h
> @@ -145,12 +145,20 @@
>   * Hygon (Fam18h) but without simple model number rules.  Instead, use STIBP
>   * as a heuristic that distinguishes the two.
>   *
> + * Zen3 and Zen4 are easier to spot by model number, but it's still not a
> + * single range. We use AutoIBRS to to discriminate instead, as that's a
> + * Zen4-specific feature.

I'd strongly advise not passing commentary on whether Zen3/4 are easier
or harder to spot.  Just discuss the technical aspect.

> + *
>   * The caller is required to perform the appropriate vendor check first.
>   */
>  #define is_zen1_uarch() ((boot_cpu_data.x86 == 0x17 || boot_cpu_data.x86 == 0x18) && \
>                           !boot_cpu_has(X86_FEATURE_AMD_STIBP))
>  #define is_zen2_uarch() (boot_cpu_data.x86 == 0x17 && \
>                           boot_cpu_has(X86_FEATURE_AMD_STIBP))
> +#define is_zen3_uarch() (boot_cpu_data.x86 == 0x19 && \
> +                         !boot_cpu_has(X86_FEATURE_AUTO_IBRS))
> +#define is_zen4_uarch() (boot_cpu_data.x86 == 0x19 && \
> +                         boot_cpu_has(X86_FEATURE_AUTO_IBRS))
>  
>  struct cpuinfo_x86;
>  int cpu_has_amd_erratum(const struct cpuinfo_x86 *, int, ...);
> diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
> index 11ffed543a..4437e8a63e 100644
> --- a/xen/arch/x86/include/asm/msr-index.h
> +++ b/xen/arch/x86/include/asm/msr-index.h
> @@ -403,6 +403,8 @@
>  #define MSR_AMD64_DE_CFG		0xc0011029
>  #define AMD64_DE_CFG_LFENCE_SERIALISE	(_AC(1, ULL) << 1)
>  #define MSR_AMD64_EX_CFG		0xc001102c
> +#define MSR_AMD64_BP_CFG		0xc001102e
> +#define AMD64_BP_CFG_SHARED_BTB_FIX	(_AC(1, ULL) << 5)

MSR_AMD64_BP_CFG is fine to have in msr-index.h (it's consistent across
generations), but SHARED_BTB_FIX is not.  It's model specific, so keep
it local to the errata workaround.

LFENCE_SERIALISE was special.  It happened to have always been
consistent, and was retroactively declared to be architectural.

~Andrew

Re: [for-4.18][PATCH 1/2] xen/x86: Add family guards to the is_zen[12]_uarch() macros

[Alejandro Vallejo]

Hi,

I'll just remove this patch (and amend the next) in the interest of having
it committed early. That said...

On Fri, Oct 13, 2023 at 10:14:45AM +0800, Andrew Cooper wrote:
> On 13/10/2023 1:26 am, Alejandro Vallejo wrote:
> > It slightly simplifies the code that uses them at no real cost because the
> > code is very rarely executed. This makes it harder to confuse zen uarches
> > due to missing or mistaken family checks.
> 
> I'm afraid I disagree.
As it stands, it's a matter of time before a bug of this form creeps up. Particularly
because it reads very innocent.

   if (is_zen1_uarch())
      fun1();
   else if (is_zen2_uarch())
      fun2();
   else if (is_zen3_uarch())
      fun3();
   else if (is_zen4_uarch())
      fun4();

Particularly if the bodies of each conditional are big enough that you lose
track of the family you're dealing with.

> 
> It's bogus to do a family check without a vendor check.
I can get behind that. I didn't include the vendor check because by and
large these macros are used in vendor-specific areas. Would that appease
your concerns? Whenever the macros are used we're in glacially cold paths
anyway.

> By making this
> change, you're separating (spatially in code, and therefore cognitively)
> the two checks that it's important to be able to cross-check.
IMO, It's no different from separating the heuristic from the vendor/family
check. What causes definite cognitive load is (as a reader) having to
remember what particular fields must be read off boot_cpu_data in order to
discriminate zenN, which of them are co-familiar and so on.

> 
> > diff --git a/xen/arch/x86/include/asm/amd.h b/xen/arch/x86/include/asm/amd.h
> > index d862cb7972..5a40bcc2ba 100644
> > --- a/xen/arch/x86/include/asm/amd.h
> > +++ b/xen/arch/x86/include/asm/amd.h
> > @@ -145,11 +145,12 @@
> >   * Hygon (Fam18h) but without simple model number rules.  Instead, use STIBP
> >   * as a heuristic that distinguishes the two.
> >   *
> > - * The caller is required to perform the appropriate vendor/family checks
> > - * first.
> > + * The caller is required to perform the appropriate vendor check first.
> >   */
> > -#define is_zen1_uarch() (!boot_cpu_has(X86_FEATURE_AMD_STIBP))
> > -#define is_zen2_uarch()   boot_cpu_has(X86_FEATURE_AMD_STIBP)
> > +#define is_zen1_uarch() ((boot_cpu_data.x86 == 0x17 || boot_cpu_data.x86 == 0x18) && \
> > +                         !boot_cpu_has(X86_FEATURE_AMD_STIBP))
> > +#define is_zen2_uarch() (boot_cpu_data.x86 == 0x17 && \
> > +                         boot_cpu_has(X86_FEATURE_AMD_STIBP))
> 
> What leads you to believe there aren't Hygon Zen2's ?
The same argument that a Hygon zen2 supports STIBP. Having seen neither HW
nor docs all that's left is divination skills :)

> 
> ~Andrew

As I said, let's move this discussion away from the errata fix (I'll remove
this patch and adapt the next one to remove the family check), as it's
unrelated and can be done later if I manage to convince you.

Thanks,
Alejandro

Re: [for-4.18][PATCH 1/2] xen/x86: Add family guards to the is_zen[12]_uarch() macros

[Roger Pau Monné]

On Fri, Oct 13, 2023 at 12:23:43PM +0100, Alejandro Vallejo wrote:
> Hi,
> 
> I'll just remove this patch (and amend the next) in the interest of having
> it committed early. That said...
> 
> On Fri, Oct 13, 2023 at 10:14:45AM +0800, Andrew Cooper wrote:
> > On 13/10/2023 1:26 am, Alejandro Vallejo wrote:
> > > It slightly simplifies the code that uses them at no real cost because the
> > > code is very rarely executed. This makes it harder to confuse zen uarches
> > > due to missing or mistaken family checks.
> > 
> > I'm afraid I disagree.
> As it stands, it's a matter of time before a bug of this form creeps up. Particularly
> because it reads very innocent.
> 
>    if (is_zen1_uarch())
>       fun1();
>    else if (is_zen2_uarch())
>       fun2();
>    else if (is_zen3_uarch())
>       fun3();
>    else if (is_zen4_uarch())
>       fun4();
> 
> Particularly if the bodies of each conditional are big enough that you lose
> track of the family you're dealing with.
> 
> > 
> > It's bogus to do a family check without a vendor check.
> I can get behind that. I didn't include the vendor check because by and
> large these macros are used in vendor-specific areas. Would that appease
> your concerns? Whenever the macros are used we're in glacially cold paths
> anyway.
> 
> > By making this
> > change, you're separating (spatially in code, and therefore cognitively)
> > the two checks that it's important to be able to cross-check.
> IMO, It's no different from separating the heuristic from the vendor/family
> check. What causes definite cognitive load is (as a reader) having to
> remember what particular fields must be read off boot_cpu_data in order to
> discriminate zenN, which of them are co-familiar and so on.
> 
> > 
> > > diff --git a/xen/arch/x86/include/asm/amd.h b/xen/arch/x86/include/asm/amd.h
> > > index d862cb7972..5a40bcc2ba 100644
> > > --- a/xen/arch/x86/include/asm/amd.h
> > > +++ b/xen/arch/x86/include/asm/amd.h
> > > @@ -145,11 +145,12 @@
> > >   * Hygon (Fam18h) but without simple model number rules.  Instead, use STIBP
> > >   * as a heuristic that distinguishes the two.
> > >   *
> > > - * The caller is required to perform the appropriate vendor/family checks
> > > - * first.
> > > + * The caller is required to perform the appropriate vendor check first.
> > >   */
> > > -#define is_zen1_uarch() (!boot_cpu_has(X86_FEATURE_AMD_STIBP))
> > > -#define is_zen2_uarch()   boot_cpu_has(X86_FEATURE_AMD_STIBP)
> > > +#define is_zen1_uarch() ((boot_cpu_data.x86 == 0x17 || boot_cpu_data.x86 == 0x18) && \
> > > +                         !boot_cpu_has(X86_FEATURE_AMD_STIBP))
> > > +#define is_zen2_uarch() (boot_cpu_data.x86 == 0x17 && \
> > > +                         boot_cpu_has(X86_FEATURE_AMD_STIBP))
> > 
> > What leads you to believe there aren't Hygon Zen2's ?
> The same argument that a Hygon zen2 supports STIBP. Having seen neither HW
> nor docs all that's left is divination skills :)

There is at least one reference online, from:

https://www.tomshardware.com/news/amd-zen-china-x86-ip-license,39573.html

"THATIC* was a single-generation technology license, and there are no
additional technology licenses," Su explained, though she did not
clarify if the decision not to extend the technology transfers was a
direct result of the trade war. That means that the technology
transfer, which provided THATIC with access to the first-gen Zen
microarchitecture, will not be extended to allow the Chinese chipmaker
access to AMD's Zen 2 microarchitecture."

* Tianjin Haiguang Advanced Technology Investment Co. Ltd. (THATIC)

So I would expect there aren't any Hygon Zen2 given those claims, but
again that relies on how much you trust tomshardware.com.  The article
seems to have been reviewed by AMD as some changes were requested.

Regards, Roger.

Re: [for-4.18][PATCH 1/2] xen/x86: Add family guards to the is_zen[12]_uarch() macros

[Andrew Cooper]

[-- Attachment #1: Type: text/plain, Size: 1380 bytes --]

On 13/10/2023 7:36 pm, Roger Pau Monné wrote:
> On Fri, Oct 13, 2023 at 12:23:43PM +0100, Alejandro Vallejo wrote:
>>>> diff --git a/xen/arch/x86/include/asm/amd.h b/xen/arch/x86/include/asm/amd.h
>>>> index d862cb7972..5a40bcc2ba 100644
>>>> --- a/xen/arch/x86/include/asm/amd.h
>>>> +++ b/xen/arch/x86/include/asm/amd.h
>>>> @@ -145,11 +145,12 @@
>>>>   * Hygon (Fam18h) but without simple model number rules.  Instead, use STIBP
>>>>   * as a heuristic that distinguishes the two.
>>>>   *
>>>> - * The caller is required to perform the appropriate vendor/family checks
>>>> - * first.
>>>> + * The caller is required to perform the appropriate vendor check first.
>>>>   */
>>>> -#define is_zen1_uarch() (!boot_cpu_has(X86_FEATURE_AMD_STIBP))
>>>> -#define is_zen2_uarch()   boot_cpu_has(X86_FEATURE_AMD_STIBP)
>>>> +#define is_zen1_uarch() ((boot_cpu_data.x86 == 0x17 || boot_cpu_data.x86 == 0x18) && \
>>>> +                         !boot_cpu_has(X86_FEATURE_AMD_STIBP))
>>>> +#define is_zen2_uarch() (boot_cpu_data.x86 == 0x17 && \
>>>> +                         boot_cpu_has(X86_FEATURE_AMD_STIBP))
>>> What leads you to believe there aren't Hygon Zen2's ?
>> The same argument that a Hygon zen2 supports STIBP. Having seen neither HW
>> nor docs all that's left is divination skills :)

Hygon reported/fixed Zen2 uarch issues in Xen.  Something clearly exists.

~Andrew

[-- Attachment #2: Type: text/html, Size: 2027 bytes --]