| Message ID | 20250106141929.615831-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [for-4.20] x86/amd: Misc setup for Fam1Ah processors | expand |
On 06.01.2025 15:19, Andrew Cooper wrote: > Fam1Ah is similar to Fam19h in these regards. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > > With this patch, I think we're in an ok position to declare support on Zen5 > CPUs. What about amd_log_freq(), where the 0x19 upper bound may need bumping? > --- a/xen/arch/x86/cpu/microcode/amd.c > +++ b/xen/arch/x86/cpu/microcode/amd.c > @@ -114,6 +114,7 @@ static bool verify_patch_size(uint32_t patch_size) > #define F16H_MPB_MAX_SIZE 3458 > #define F17H_MPB_MAX_SIZE 3200 > #define F19H_MPB_MAX_SIZE 5568 > +#define F1AH_MPB_MAX_SIZE 14368 Yet another pretty odd number. Are these actually documented anywhere? And what has come of their plan to make ucode size available via CPUID (for which I even sent a patch quite a long while ago)? Jan
On 06/01/2025 2:41 pm, Jan Beulich wrote: > On 06.01.2025 15:19, Andrew Cooper wrote: >> Fam1Ah is similar to Fam19h in these regards. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> --- >> CC: Jan Beulich <JBeulich@suse.com> >> CC: Roger Pau Monné <roger.pau@citrix.com> >> >> With this patch, I think we're in an ok position to declare support on Zen5 >> CPUs. > What about amd_log_freq(), where the 0x19 upper bound may need bumping? Ah, that's new since I did Fam19. I'll adjust. > >> --- a/xen/arch/x86/cpu/microcode/amd.c >> +++ b/xen/arch/x86/cpu/microcode/amd.c >> @@ -114,6 +114,7 @@ static bool verify_patch_size(uint32_t patch_size) >> #define F16H_MPB_MAX_SIZE 3458 >> #define F17H_MPB_MAX_SIZE 3200 >> #define F19H_MPB_MAX_SIZE 5568 >> +#define F1AH_MPB_MAX_SIZE 14368 > Yet another pretty odd number. Are these actually documented anywhere? In the PPRs. > And what has come of their plan to make ucode size available via CPUID > (for which I even sent a patch quite a long while ago)? This check in this function need to work for any microcode we find in the container. Knowing the size of the current CPU doesn't help parsing others. And talking of, I've just found another Fam1Ah processor with an even larger patch size. This limit needs bumping to 15296. ~Andrew
On 06.01.2025 16:12, Andrew Cooper wrote: > On 06/01/2025 2:41 pm, Jan Beulich wrote: >> On 06.01.2025 15:19, Andrew Cooper wrote: >>> --- a/xen/arch/x86/cpu/microcode/amd.c >>> +++ b/xen/arch/x86/cpu/microcode/amd.c >>> @@ -114,6 +114,7 @@ static bool verify_patch_size(uint32_t patch_size) >>> #define F16H_MPB_MAX_SIZE 3458 >>> #define F17H_MPB_MAX_SIZE 3200 >>> #define F19H_MPB_MAX_SIZE 5568 >>> +#define F1AH_MPB_MAX_SIZE 14368 >> Yet another pretty odd number. Are these actually documented anywhere? > > In the PPRs. So to find the number to use it's really ... >> And what has come of their plan to make ucode size available via CPUID >> (for which I even sent a patch quite a long while ago)? > > This check in this function need to work for any microcode we find in > the container. Knowing the size of the current CPU doesn't help parsing > others. > > And talking of, I've just found another Fam1Ah processor with an even > larger patch size. This limit needs bumping to 15296. ... digging through the PPRs (and hoping no later model will have yet larger size). Jan
On 06/01/2025 3:17 pm, Jan Beulich wrote: > On 06.01.2025 16:12, Andrew Cooper wrote: >> On 06/01/2025 2:41 pm, Jan Beulich wrote: >>> On 06.01.2025 15:19, Andrew Cooper wrote: >>>> --- a/xen/arch/x86/cpu/microcode/amd.c >>>> +++ b/xen/arch/x86/cpu/microcode/amd.c >>>> @@ -114,6 +114,7 @@ static bool verify_patch_size(uint32_t patch_size) >>>> #define F16H_MPB_MAX_SIZE 3458 >>>> #define F17H_MPB_MAX_SIZE 3200 >>>> #define F19H_MPB_MAX_SIZE 5568 >>>> +#define F1AH_MPB_MAX_SIZE 14368 >>> Yet another pretty odd number. Are these actually documented anywhere? >> In the PPRs. > So to find the number to use it's really ... > >>> And what has come of their plan to make ucode size available via CPUID >>> (for which I even sent a patch quite a long while ago)? >> This check in this function need to work for any microcode we find in >> the container. Knowing the size of the current CPU doesn't help parsing >> others. >> >> And talking of, I've just found another Fam1Ah processor with an even >> larger patch size. This limit needs bumping to 15296. > ... digging through the PPRs (and hoping no later model will have yet > larger size). Correct. I'd prefer a better approach, but alas. ~Andrew
On 06/01/2025 2:41 pm, Jan Beulich wrote: > On 06.01.2025 15:19, Andrew Cooper wrote: >> Fam1Ah is similar to Fam19h in these regards. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> --- >> CC: Jan Beulich <JBeulich@suse.com> >> CC: Roger Pau Monné <roger.pau@citrix.com> >> >> With this patch, I think we're in an ok position to declare support on Zen5 >> CPUs. > What about amd_log_freq(), where the 0x19 upper bound may need bumping? The Pstate MSRs are still there, but their layout is quite different. FID is 12 bits, and Vid is 9 bits in two split fields. As this is only informational for now, I think I'll leave it. This needs a bigger rework to make the code tractable. ~Andrew
On 06.01.2025 17:37, Andrew Cooper wrote: > On 06/01/2025 2:41 pm, Jan Beulich wrote: >> On 06.01.2025 15:19, Andrew Cooper wrote: >>> Fam1Ah is similar to Fam19h in these regards. >>> >>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >>> --- >>> CC: Jan Beulich <JBeulich@suse.com> >>> CC: Roger Pau Monné <roger.pau@citrix.com> >>> >>> With this patch, I think we're in an ok position to declare support on Zen5 >>> CPUs. >> What about amd_log_freq(), where the 0x19 upper bound may need bumping? > > The Pstate MSRs are still there, but their layout is quite different. > FID is 12 bits, and Vid is 9 bits in two split fields. Oh, okay. > As this is only informational for now, I think I'll leave it. This > needs a bigger rework to make the code tractable. Fair enough then. And with the adjusted ucode size: Acked-by: Jan Beulich <jbeulich@suse.com> Jan
diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c index 876317fad059..420198406def 100644 --- a/xen/arch/x86/acpi/cpu_idle.c +++ b/xen/arch/x86/acpi/cpu_idle.c @@ -1417,6 +1417,7 @@ static void amd_cpuidle_init(struct acpi_processor_power *power) switch ( c->x86 ) { + case 0x1a: case 0x19: case 0x18: if ( boot_cpu_data.x86_vendor != X86_VENDOR_HYGON ) diff --git a/xen/arch/x86/cpu/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c index ba7668a94670..58568f9aa024 100644 --- a/xen/arch/x86/cpu/microcode/amd.c +++ b/xen/arch/x86/cpu/microcode/amd.c @@ -114,6 +114,7 @@ static bool verify_patch_size(uint32_t patch_size) #define F16H_MPB_MAX_SIZE 3458 #define F17H_MPB_MAX_SIZE 3200 #define F19H_MPB_MAX_SIZE 5568 +#define F1AH_MPB_MAX_SIZE 14368 switch ( boot_cpu_data.x86 ) { @@ -132,6 +133,9 @@ static bool verify_patch_size(uint32_t patch_size) case 0x19: max_size = F19H_MPB_MAX_SIZE; break; + case 0x1a: + max_size = F1AH_MPB_MAX_SIZE; + break; default: max_size = F1XH_MPB_MAX_SIZE; break; diff --git a/xen/arch/x86/cpu/vpmu_amd.c b/xen/arch/x86/cpu/vpmu_amd.c index a082450e923a..a6117dfebf2a 100644 --- a/xen/arch/x86/cpu/vpmu_amd.c +++ b/xen/arch/x86/cpu/vpmu_amd.c @@ -568,6 +568,7 @@ const struct arch_vpmu_ops *__init amd_vpmu_init(void) case 0x15: case 0x17: case 0x19: + case 0x1a: num_counters = F15H_NUM_COUNTERS; counters = AMD_F15H_COUNTERS; ctrls = AMD_F15H_CTRLS;
Fam1Ah is similar to Fam19h in these regards. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> With this patch, I think we're in an ok position to declare support on Zen5 CPUs. I'm very disappointed that AMD don't have any documetation about ERAPS, but to the best of my (backchannel) knowledge, Xen should behave safely. --- xen/arch/x86/acpi/cpu_idle.c | 1 + xen/arch/x86/cpu/microcode/amd.c | 4 ++++ xen/arch/x86/cpu/vpmu_amd.c | 1 + 3 files changed, 6 insertions(+)