On Fri, Mar 26, 2021 at 06:59:47PM +0000, Andrew Cooper wrote: > If Legacy Replacement mode doesn't help in check_timer(), restore the old > configuration before falling back to other workarounds.
Oh, I guess this answers my question from the previous patch. > Signed-off-by: Andrew Cooper <[email protected]> Reviewed-by: Roger Pau Monné <[email protected]> > --- > CC: Jan Beulich <[email protected]> > CC: Roger Pau Monné <[email protected]> > CC: Wei Liu <[email protected]> > CC: Ian Jackson <[email protected]> > CC: Marek Marczykowski-Górecki <[email protected]> > CC: Frédéric Pierret <[email protected]> > > v2: > * New. > > For 4.15: Attempt to unbreak AMD Ryzen 1800X systems. Is this really the fix for AMD 1800X? I assumed not setting the HPET into legacy replacement mode unconditionally was the fix for those systems? > > I'm tempted to fold this into the previous patch, but its presented here in > isolation for ease of review. > > Tested by repositioning the timer_irq_works() calls on a system with a working > PIT. > > (XEN) ENABLING IO-APIC IRQs > (XEN) -> Using old ACK method > (XEN) ..TIMER: vector=0xF0 apic1=0 pin1=2 apic2=0 pin2=0 > (XEN) ..no 8254 timer found - trying HPET Legacy Replacement Mode > (XEN) ..no HPET timer found - reverting Legacy Replacement Mode > (XEN) TSC deadline timer enabled > --- > xen/arch/x86/hpet.c | 27 ++++++++++++++++++++++++++- > xen/arch/x86/io_apic.c | 4 ++++ > xen/include/asm-x86/hpet.h | 6 ++++++ > 3 files changed, 36 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c > index bfa75f135a..afe104dc93 100644 > --- a/xen/arch/x86/hpet.c > +++ b/xen/arch/x86/hpet.c > @@ -783,6 +783,9 @@ int hpet_legacy_irq_tick(void) > > static u32 *hpet_boot_cfg; > static uint64_t __initdata hpet_rate; > +static __initdata struct { > + uint32_t cmp, cfg; > +} pre_legacy_c0; > > bool __init hpet_enable_legacy_replacement_mode(void) > { > @@ -796,8 +799,11 @@ bool __init hpet_enable_legacy_replacement_mode(void) > /* Stop the main counter. */ > hpet_write32(cfg & ~HPET_CFG_ENABLE, HPET_CFG); > > + /* Stash channel 0's old CFG/CMP incase we need to undo. */ > + pre_legacy_c0.cfg = c0_cfg = hpet_read32(HPET_Tn_CFG(0)); > + pre_legacy_c0.cmp = hpet_read32(HPET_Tn_CMP(0)); > + > /* Reconfigure channel 0 to be 32bit periodic. */ > - c0_cfg = hpet_read32(HPET_Tn_CFG(0)); > c0_cfg |= (HPET_TN_ENABLE | HPET_TN_PERIODIC | HPET_TN_SETVAL | > HPET_TN_32BIT); > hpet_write32(c0_cfg, HPET_Tn_CFG(0)); > @@ -843,6 +849,25 @@ bool __init hpet_enable_legacy_replacement_mode(void) > return true; > } > > +void __init hpet_disable_legacy_replacement_mode(void) > +{ > + unsigned int cfg = hpet_read32(HPET_CFG); > + > + ASSERT(hpet_rate); > + > + cfg &= ~(HPET_CFG_LEGACY | HPET_CFG_ENABLE); > + > + /* Stop the main counter and disable legacy mode. */ > + hpet_write32(cfg, HPET_CFG); > + > + /* Restore pre-Legacy Replacement Mode settings. */ > + hpet_write32(pre_legacy_c0.cfg, HPET_Tn_CFG(0)); > + hpet_write32(pre_legacy_c0.cmp, HPET_Tn_CMP(0)); > + > + /* Restart the main counter. */ > + hpet_write32(cfg | HPET_CFG_ENABLE, HPET_CFG); > +} > + > u64 __init hpet_setup(void) > { > unsigned int hpet_id, hpet_period; > diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c > index 3f131a81fb..58b26d962c 100644 > --- a/xen/arch/x86/io_apic.c > +++ b/xen/arch/x86/io_apic.c > @@ -1955,6 +1955,10 @@ static void __init check_timer(void) > local_irq_restore(flags); > return; > } > + > + /* Legacy Replacement mode hasn't helped. Undo it. */ > + printk(XENLOG_ERR "..no HPET timer found - reverting Legacy > Replacement Mode\n"); > + hpet_disable_legacy_replacement_mode(); I think you could also get away just calling hpet_disable and hpet_resume? (bypassing the system_reset_counter check) Thanks, Roger.
