| Message ID | 20240319144802.3894710-3-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/boot: More watchdog fixes | expand |
On 19.03.2024 15:48, Andrew Cooper wrote: > The MSRs used by setup_k7_watchdog() are architectural in 64bit. The Unit > Select (0x76, cycles not in halt state) isn't, but it hasn't changed in 23 > years, making this a trend likely to continue. > > Drop the family check. If the Unit Select does happen to change meaning in > the future, check_nmi_watchdog() will still notice the watchdog not operating > as expected. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> > --- a/xen/arch/x86/nmi.c > +++ b/xen/arch/x86/nmi.c > @@ -387,15 +387,12 @@ void setup_apic_nmi_watchdog(void) > if ( nmi_watchdog == NMI_NONE ) > return; > > - switch (boot_cpu_data.x86_vendor) { > + switch ( boot_cpu_data.x86_vendor ) > + { > case X86_VENDOR_AMD: > - switch (boot_cpu_data.x86) { > - case 6: Just to mention it - this case label has been dead code anyway for about 10 years. Jan
On 19/03/2024 4:51 pm, Jan Beulich wrote: > On 19.03.2024 15:48, Andrew Cooper wrote: >> The MSRs used by setup_k7_watchdog() are architectural in 64bit. The Unit >> Select (0x76, cycles not in halt state) isn't, but it hasn't changed in 23 >> years, making this a trend likely to continue. >> >> Drop the family check. If the Unit Select does happen to change meaning in >> the future, check_nmi_watchdog() will still notice the watchdog not operating >> as expected. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Reviewed-by: Jan Beulich <jbeulich@suse.com> Thanks. > >> --- a/xen/arch/x86/nmi.c >> +++ b/xen/arch/x86/nmi.c >> @@ -387,15 +387,12 @@ void setup_apic_nmi_watchdog(void) >> if ( nmi_watchdog == NMI_NONE ) >> return; >> >> - switch (boot_cpu_data.x86_vendor) { >> + switch ( boot_cpu_data.x86_vendor ) >> + { >> case X86_VENDOR_AMD: >> - switch (boot_cpu_data.x86) { >> - case 6: > Just to mention it - this case label has been dead code anyway for about 10 > years. Yeah... I noticed that after writing the commit message, although it's not very easy to slip in. Also it's 25 years since the K7 was released (June 1999), because I apparently can't count. ~Andrew
diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c index 33f77a92047f..f6329cb0270e 100644 --- a/xen/arch/x86/nmi.c +++ b/xen/arch/x86/nmi.c @@ -387,15 +387,12 @@ void setup_apic_nmi_watchdog(void) if ( nmi_watchdog == NMI_NONE ) return; - switch (boot_cpu_data.x86_vendor) { + switch ( boot_cpu_data.x86_vendor ) + { case X86_VENDOR_AMD: - switch (boot_cpu_data.x86) { - case 6: - case 0xf ... 0x19: - setup_k7_watchdog(); - break; - } + setup_k7_watchdog(); break; + case X86_VENDOR_INTEL: switch (boot_cpu_data.x86) { case 6:
The MSRs used by setup_k7_watchdog() are architectural in 64bit. The Unit Select (0x76, cycles not in halt state) isn't, but it hasn't changed in 23 years, making this a trend likely to continue. Drop the family check. If the Unit Select does happen to change meaning in the future, check_nmi_watchdog() will still notice the watchdog not operating as expected. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/nmi.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)