| Message ID | 20200430095947.31958-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/amd: Initial support for Fam19h processors | expand |
On Thu, Apr 30, 2020 at 10:59:47AM +0100, Andrew Cooper wrote: > diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c > index c3f92ed231..014524486f 100644 > --- a/xen/arch/x86/nmi.c > +++ b/xen/arch/x86/nmi.c > @@ -398,7 +398,7 @@ void setup_apic_nmi_watchdog(void) > case X86_VENDOR_AMD: > switch (boot_cpu_data.x86) { > case 6: > - case 0xf ... 0x17: > + case 0xf ... 0x19: > setup_k7_watchdog(); > break; > default: > diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c > index 0bcf554e93..33e5d21ece 100644 > --- a/xen/arch/x86/traps.c > +++ b/xen/arch/x86/traps.c > @@ -1939,7 +1939,7 @@ static unsigned int calc_ler_msr(void) > switch ( boot_cpu_data.x86 ) > { > case 6: > - case 0xf ... 0x17: > + case 0xf ... 0x19: > return MSR_IA32_LASTINTFROMIP; You seem to also add support for Fam18h here and in the chunk above, is this intentional? Thanks, Roger.
On 30/04/2020 11:35, Roger Pau Monné wrote: > On Thu, Apr 30, 2020 at 10:59:47AM +0100, Andrew Cooper wrote: >> diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c >> index c3f92ed231..014524486f 100644 >> --- a/xen/arch/x86/nmi.c >> +++ b/xen/arch/x86/nmi.c >> @@ -398,7 +398,7 @@ void setup_apic_nmi_watchdog(void) >> case X86_VENDOR_AMD: >> switch (boot_cpu_data.x86) { >> case 6: >> - case 0xf ... 0x17: >> + case 0xf ... 0x19: >> setup_k7_watchdog(); >> break; >> default: >> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c >> index 0bcf554e93..33e5d21ece 100644 >> --- a/xen/arch/x86/traps.c >> +++ b/xen/arch/x86/traps.c >> @@ -1939,7 +1939,7 @@ static unsigned int calc_ler_msr(void) >> switch ( boot_cpu_data.x86 ) >> { >> case 6: >> - case 0xf ... 0x17: >> + case 0xf ... 0x19: >> return MSR_IA32_LASTINTFROMIP; > You seem to also add support for Fam18h here and in the chunk above, > is this intentional? Yes. Honestly, these details have never changed since the K7. I'm tempted to drop the family logic entirely. ~Andrew
On Thu, Apr 30, 2020 at 11:38:14AM +0100, Andrew Cooper wrote: > On 30/04/2020 11:35, Roger Pau Monné wrote: > > On Thu, Apr 30, 2020 at 10:59:47AM +0100, Andrew Cooper wrote: > >> diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c > >> index c3f92ed231..014524486f 100644 > >> --- a/xen/arch/x86/nmi.c > >> +++ b/xen/arch/x86/nmi.c > >> @@ -398,7 +398,7 @@ void setup_apic_nmi_watchdog(void) > >> case X86_VENDOR_AMD: > >> switch (boot_cpu_data.x86) { > >> case 6: > >> - case 0xf ... 0x17: > >> + case 0xf ... 0x19: > >> setup_k7_watchdog(); > >> break; > >> default: > >> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c > >> index 0bcf554e93..33e5d21ece 100644 > >> --- a/xen/arch/x86/traps.c > >> +++ b/xen/arch/x86/traps.c > >> @@ -1939,7 +1939,7 @@ static unsigned int calc_ler_msr(void) > >> switch ( boot_cpu_data.x86 ) > >> { > >> case 6: > >> - case 0xf ... 0x17: > >> + case 0xf ... 0x19: > >> return MSR_IA32_LASTINTFROMIP; > > You seem to also add support for Fam18h here and in the chunk above, > > is this intentional? > > Yes. Honestly, these details have never changed since the K7. I'm > tempted to drop the family logic entirely. Ack, just wanted to be sure the changes where intentional: Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Re dropping the logic - as you wish. Thanks, Roger.
On 30.04.2020 11:59, Andrew Cooper wrote: > Fam19h is very similar to Fam17h in these regards. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Nevertheless a question: > --- a/xen/arch/x86/cpu/microcode/amd.c > +++ b/xen/arch/x86/cpu/microcode/amd.c > @@ -125,6 +125,7 @@ static bool_t verify_patch_size(uint32_t patch_size) > max_size = F16H_MPB_MAX_SIZE; > break; > case 0x17: > + case 0x19: > max_size = F17H_MPB_MAX_SIZE; > break; Didn't you indicate to me the other day that the upper bound would grow? Jan
On 30/04/2020 12:09, Jan Beulich wrote: > On 30.04.2020 11:59, Andrew Cooper wrote: >> Fam19h is very similar to Fam17h in these regards. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Reviewed-by: Jan Beulich <jbeulich@suse.com> Thanks. > > Nevertheless a question: > >> --- a/xen/arch/x86/cpu/microcode/amd.c >> +++ b/xen/arch/x86/cpu/microcode/amd.c >> @@ -125,6 +125,7 @@ static bool_t verify_patch_size(uint32_t patch_size) >> max_size = F16H_MPB_MAX_SIZE; >> break; >> case 0x17: >> + case 0x19: >> max_size = F17H_MPB_MAX_SIZE; >> break; > Didn't you indicate to me the other day that the upper bound would > grow? That was a very non-specific patch to Linux. I've asked around, and the answer seems to be 4800. Are you happy for your review to stand with adding a new F19H_MPB_MAX_SIZE define to this effect? ~Andrew
On 30.04.2020 17:50, Andrew Cooper wrote: > On 30/04/2020 12:09, Jan Beulich wrote: >> On 30.04.2020 11:59, Andrew Cooper wrote: >>> --- a/xen/arch/x86/cpu/microcode/amd.c >>> +++ b/xen/arch/x86/cpu/microcode/amd.c >>> @@ -125,6 +125,7 @@ static bool_t verify_patch_size(uint32_t patch_size) >>> max_size = F16H_MPB_MAX_SIZE; >>> break; >>> case 0x17: >>> + case 0x19: >>> max_size = F17H_MPB_MAX_SIZE; >>> break; >> Didn't you indicate to me the other day that the upper bound would >> grow? > > That was a very non-specific patch to Linux. I've asked around, and the > answer seems to be 4800. > > Are you happy for your review to stand with adding a new > F19H_MPB_MAX_SIZE define to this effect? Yes. Jan
diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c index e00f2a82de..b83446e77d 100644 --- a/xen/arch/x86/acpi/cpu_idle.c +++ b/xen/arch/x86/acpi/cpu_idle.c @@ -1356,6 +1356,7 @@ static void amd_cpuidle_init(struct acpi_processor_power *power) switch ( c->x86 ) { + 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 13bf9f4dee..89b656bd2b 100644 --- a/xen/arch/x86/cpu/microcode/amd.c +++ b/xen/arch/x86/cpu/microcode/amd.c @@ -125,6 +125,7 @@ static bool_t verify_patch_size(uint32_t patch_size) max_size = F16H_MPB_MAX_SIZE; break; case 0x17: + case 0x19: max_size = F17H_MPB_MAX_SIZE; break; default: diff --git a/xen/arch/x86/cpu/vpmu_amd.c b/xen/arch/x86/cpu/vpmu_amd.c index 3c6799b42c..eba47cd2a0 100644 --- a/xen/arch/x86/cpu/vpmu_amd.c +++ b/xen/arch/x86/cpu/vpmu_amd.c @@ -576,6 +576,7 @@ int __init amd_vpmu_init(void) { case 0x15: case 0x17: + case 0x19: num_counters = F15H_NUM_COUNTERS; counters = AMD_F15H_COUNTERS; ctrls = AMD_F15H_CTRLS; diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c index c3f92ed231..014524486f 100644 --- a/xen/arch/x86/nmi.c +++ b/xen/arch/x86/nmi.c @@ -398,7 +398,7 @@ void setup_apic_nmi_watchdog(void) case X86_VENDOR_AMD: switch (boot_cpu_data.x86) { case 6: - case 0xf ... 0x17: + case 0xf ... 0x19: setup_k7_watchdog(); break; default: diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index 0bcf554e93..33e5d21ece 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -1939,7 +1939,7 @@ static unsigned int calc_ler_msr(void) switch ( boot_cpu_data.x86 ) { case 6: - case 0xf ... 0x17: + case 0xf ... 0x19: return MSR_IA32_LASTINTFROMIP; } break;
Fam19h is very similar to Fam17h in these regards. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Wei Liu <wl@xen.org> CC: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/acpi/cpu_idle.c | 1 + xen/arch/x86/cpu/microcode/amd.c | 1 + xen/arch/x86/cpu/vpmu_amd.c | 1 + xen/arch/x86/nmi.c | 2 +- xen/arch/x86/traps.c | 2 +- 5 files changed, 5 insertions(+), 2 deletions(-)