Message ID | 20211026151233.57246-1-roger.pau@citrix.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | x86/hpet: setup HPET even when disabled due to stopping in deep C states | expand |
On 26/10/2021 16:12, Roger Pau Monne wrote: > Always allow the HPET to be setup, but don't report a frequency back > to the platform time source probe in order to avoid it from being > selected as a valid timer if it's not usable. > > Doing the setup even when not intended to be used as a platform timer > is required so that is can be used in legacy replacement mode in order > to assert the IO-APIC is capable of receiving interrupts. > > Fixes: c12731493a ('x86/hpet: Use another crystalball to evaluate HPET usability') > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Yup - does fix the regression. Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Roger Pau Monne writes ("[PATCH] x86/hpet: setup HPET even when disabled due to stopping in deep C states"): > Always allow the HPET to be setup, but don't report a frequency back > to the platform time source probe in order to avoid it from being > selected as a valid timer if it's not usable. > > Doing the setup even when not intended to be used as a platform timer > is required so that is can be used in legacy replacement mode in order > to assert the IO-APIC is capable of receiving interrupts. > > Fixes: c12731493a ('x86/hpet: Use another crystalball to evaluate HPET usability') > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Release-Acked-by: Ian Jackson <iwj@xenproject.org>
On 26.10.2021 17:12, Roger Pau Monne wrote: > Always allow the HPET to be setup, but don't report a frequency back > to the platform time source probe in order to avoid it from being > selected as a valid timer if it's not usable. > > Doing the setup even when not intended to be used as a platform timer > is required so that is can be used in legacy replacement mode in order > to assert the IO-APIC is capable of receiving interrupts. > > Fixes: c12731493a ('x86/hpet: Use another crystalball to evaluate HPET usability') I realize this has gone in already, but imo pointing at this commit is only part of the truth (maybe the larger one). e1de4c196a2e ("x86/timer: Fix boot on Intel systems using ITSSPRC static PIT clock gating") post- dates d5294a302c84 ("x86: avoid HPET use on certain Intel platforms") by over a year, yet it's that patch'es logic which the referenced commit merely extended. I am of the opinion that e1de4c196a2e should have made the adjustment you make now, and hence should (also) have been tagged. Jan
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c index cbadaa036f..a290aba3e8 100644 --- a/xen/arch/x86/time.c +++ b/xen/arch/x86/time.c @@ -379,6 +379,12 @@ static int64_t __init init_hpet(struct platform_timesource *pts) { uint64_t hpet_rate, start; uint32_t count, target; + /* + * Allow HPET to be setup, but report a frequency of 0 so it's not selected + * as a timer source. This is required so it can be used in legacy + * replacement mode in check_timer. + */ + bool disable_hpet = false; if ( hpet_address && strcmp(opt_clocksource, pts->id) && cpuidle_using_deep_cstate() ) @@ -391,7 +397,7 @@ static int64_t __init init_hpet(struct platform_timesource *pts) case 0x0f1c: /* HPET on Cherry Trail platforms will halt in deep C states. */ case 0x229c: - hpet_address = 0; + disable_hpet = true; break; } @@ -431,14 +437,14 @@ static int64_t __init init_hpet(struct platform_timesource *pts) else if ( !strcmp(opt_clocksource, pts->id) ) printk("HPET use requested via command line, but dysfunctional in PC10\n"); else - hpet_address = 0; + disable_hpet = true; } - if ( !hpet_address ) + if ( disable_hpet ) printk("Disabling HPET for being unreliable\n"); } - if ( (hpet_rate = hpet_setup()) == 0 ) + if ( (hpet_rate = hpet_setup()) == 0 || disable_hpet ) return 0; pts->frequency = hpet_rate;
Always allow the HPET to be setup, but don't report a frequency back to the platform time source probe in order to avoid it from being selected as a valid timer if it's not usable. Doing the setup even when not intended to be used as a platform timer is required so that is can be used in legacy replacement mode in order to assert the IO-APIC is capable of receiving interrupts. Fixes: c12731493a ('x86/hpet: Use another crystalball to evaluate HPET usability') Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/time.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)