|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [BUG] panic: "IO-APIC + timer doesn't work" - several people have reproduced
On Wed, Mar 18, 2020 at 10:04 AM Jason Andryuk <jandryuk@xxxxxxxxx> wrote:
> On Wed, Mar 18, 2020 at 6:38 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
> > On 17.03.2020 16:23, Jason Andryuk wrote:
> > > On 17.03.2020 15:08, Jan Beulich wrote:
> > >> On 17.03.2020 15:08, Jan Beulich wrote:
> > >>> On 17.03.2020 14:48, Jason Andryuk wrote:
> > >>>> I got it to boot past "IO-APIC + timer doesn't work". I programmed
> > >>>> the HPET to provide a periodic timer in hpet_resume() on T0. When I
> > >>>> actually got it programmed properly, it worked to increment
> > >>>> pit0_ticks. I also made timer_interrupt() unconditionally
> > >>>> pit0_ticks++ though that may not matter.
> > >>>
> > >>> Hmm, at the first glance I would imply the system gets handed to Xen
> > >>> with a HPET state that we don't (and probably also shouldn't) expect.
> > >>> Could you provide HPET_CFG as well as all HPET_Tn_CFG and
> > >>> HPET_Tn_ROUTE values as hpet_resume() finds them before doing any
> > >>> adjustments to them? What are the components / parties involved in
> > >>> getting Xen loaded and started?
> > >>
> > >> Of course much depends on what exactly you mean you've done to
> > >> the HPET by saying "I programmed the HPET to provide ...".
> > >
> > > Below is the diff. It was messier and I tidied it up some.
> > >
> > > It's mainly the change to hpet_resume() to mimic Linux's legacy HPET
> > > setup on T0. It turns on HPET_CFG_LEGACY to ensure the timer interrupt
> > > is running. And it also includes the printing of the initial HPET
> > > config:
> > > HPET_CFG 00000001
> > > HPET_T0_CFG 00008030
> > > HPET_T0_ROUTE 0000016c
> > > HPET_T1_CFG 00008000
> > > HPET_T1_ROUTE 00000000
> > > HPET_T2_CFG 00008000
> > > HPET_T2_ROUTE 00000000
> > > HPET_T3_CFG 00008000
> > > HPET_T3_ROUTE 00000000
> > > HPET_T4_CFG 0000c000
> > > HPET_T4_ROUTE 00000000
> > > HPET_T5_CFG 0000c000
> > > HPET_T5_ROUTE 00000000
> > > HPET_T6_CFG 0000c000
> > > HPET_T6_ROUTE 00000000
> > > HPET_T7_CFG 0000c000
> > > HPET_T7_ROUTE 00000000
> > >
> > > Other changes are to try to prevent Xen from clobbering T0 as a periodic
> > > timer.
> >
> > Why "clobbering"? According to the values above T0 is neither enabled
> > nor set to periodic.
>
> I was trying to indicated the changes in hpet_broadcast_init() to
> preserve T0 as a periodic timer after it was set up in hpet_resume().
>
> > > --- a/xen/arch/x86/hpet.c
> > > +++ b/xen/arch/x86/hpet.c
> > > @@ -585,16 +585,27 @@ void __init hpet_broadcast_init(void)
> > > pv_rtc_handler = handle_rtc_once;
> > > }
> > >
> > > + printk(XENLOG_INFO "%s cfg %d\n", __func__, cfg);
> > > hpet_write32(cfg, HPET_CFG);
> > >
> > > for ( i = 0; i < n; i++ )
> > > {
> > > - if ( i == 0 && (cfg & HPET_CFG_LEGACY) )
> > > + printk(XENLOG_INFO "hpet cfg %d legacy %d\n", i, cfg &
> > > HPET_CFG_LEGACY);
> > > + if ( i == 1 && (cfg & HPET_CFG_LEGACY) )
> > > {
> > > /* set HPET T0 as oneshot */
> > > - cfg = hpet_read32(HPET_Tn_CFG(0));
> > > + cfg = hpet_read32(HPET_Tn_CFG(1));
> > > cfg &= ~(HPET_TN_LEVEL | HPET_TN_PERIODIC);
> > > cfg |= HPET_TN_ENABLE | HPET_TN_32BIT;
> > > + hpet_write32(cfg, HPET_Tn_CFG(1));
> > > + }
> > > +
> > > + if ( i == 0 && (cfg & HPET_CFG_LEGACY) )
> > > + {
> > > + /* set HPET T0 as periodic */
> > > + cfg = hpet_read32(HPET_Tn_CFG(0));
> > > + cfg |= (HPET_TN_LEVEL | HPET_TN_PERIODIC);
> >
> > A change like this of course won't be acceptable outside of
> > your own repo, but I assume you're clear about this.
>
> Of course. I was just providing the example that passes
> check_timer(). I'm not familiar with the Xen timer code or HPETs, so
> I was hoping this provides useful information to clarify the problem
> and find a cleaner solution.
>
> Locally, I minimized the HPET changes to just enable it during
> check_timer() and then disable it afterwards. That diff is below.
>
> Xen is still having issues booting dom0 with the HPET changes, so this
> change may be incorrect and break something else. Previously, I wrote
> about a failed assert in pv_destroy_gdt() during dom0 construction. I
> added a printk before the assert, and that issue disappeared and is
> also gone after removing it again. Since this is a tablet form
> factor, serial output is impossible. I added a delay to printk so I
> could more easily capture screen output without it scrolling by. I
> have since removed that delay which may have been shifted the problem
> as there is now a pagefault in emulate_forced_invalid_op().
>
> r12 is NULL in
> testb $0x1,0x4(%r12)
> which is:
> if ( msrs->misc_features_enables.cpuid_faulting &&
>
> So msrs is NULL? msrs = current->arch.msrs ealier in the function.
>
> The pv_destroy_gdt failed assert was:
> ASSERT(v == current || !vcpu_cpu_dirty(v));
>
> I wonder if the timer interrupt could be messing with current somehow?
>
Something was stale in my build tree. After cleaning and re-build
Xen, it boots into dom0 with the patch below.
Regards,
Jason
> diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c
> index 86929b9ba1..93a34792b2 100644
> --- a/xen/arch/x86/hpet.c
> +++ b/xen/arch/x86/hpet.c
> @@ -765,6 +765,15 @@ int hpet_legacy_irq_tick(void)
>
> static u32 *hpet_boot_cfg;
>
> +void hpet_disable_legacy(void)
> +{
> + u32 cfg = hpet_read32(HPET_CFG);
> + printk(XENLOG_INFO "%s HPET_CFG %08x\n", __func__, cfg);
> + cfg &= ~HPET_CFG_LEGACY;
> + printk(XENLOG_INFO "%s HPET_CFG %08x\n", __func__, cfg);
> + hpet_write32(cfg, HPET_CFG);
> +}
> +
> u64 __init hpet_setup(void)
> {
> static u64 __initdata hpet_rate;
> @@ -804,6 +813,8 @@ u64 __init hpet_setup(void)
> return hpet_rate + (last * 2 > hpet_period);
> }
>
> +#include <asm/delay.h>
> +
> void hpet_resume(u32 *boot_cfg)
> {
> static u32 system_reset_latch;
> @@ -842,11 +853,33 @@ void hpet_resume(u32 *boot_cfg)
> cfg & HPET_TN_RESERVED, i);
> cfg &= ~HPET_TN_RESERVED;
> }
> + if (i == 0) {
> + cfg |= HPET_TN_ENABLE | HPET_TN_PERIODIC | HPET_TN_SETVAL |
> + HPET_TN_32BIT;
> + }
> hpet_write32(cfg, HPET_Tn_CFG(i));
> + if (i == 0) {
> +#define NSEC_PER_SEC 1000000000L
> + uint64_t delta;
> + unsigned int now;
> + unsigned int cmp;
> + u64 hpet_rate = hpet_setup();
> + uint32_t mult = div_sc((unsigned long)hpet_rate,
> + 1000000000ul, 32);
> + uint32_t shift = 32;
> + printk(XENLOG_INFO "hpet mult %d shift %d\n", mult, shift);
> + delta = ((uint64_t)(NSEC_PER_SEC / HZ)) * mult;
> + delta >>= shift;
> + now = hpet_read32(HPET_COUNTER);
> + cmp = now + (unsigned int)delta;
> + hpet_write32(cmp, HPET_Tn_CMP(i));
> + udelay(1);
> + hpet_write32(delta, HPET_Tn_CMP(i));
> + }
> }
>
> cfg = hpet_read32(HPET_CFG);
> - cfg |= HPET_CFG_ENABLE;
> + cfg |= HPET_CFG_ENABLE | HPET_CFG_LEGACY;
> hpet_write32(cfg, HPET_CFG);
> }
>
> diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
> index e98e08e9c8..b62dea190a 100644
> --- a/xen/arch/x86/io_apic.c
> +++ b/xen/arch/x86/io_apic.c
> @@ -34,6 +34,7 @@
> #include <asm/desc.h>
> #include <asm/msi.h>
> #include <asm/setup.h>
> +#include <asm/hpet.h>
> #include <mach_apic.h>
> #include <io_ports.h>
> #include <irq_vectors.h>
> @@ -2047,6 +2048,7 @@ void __init setup_IO_APIC(void)
> setup_IO_APIC_irqs();
> init_IO_APIC_traps();
> check_timer();
> + hpet_disable_legacy();
> print_IO_APIC();
> ioapic_pm_state_alloc();
>
> diff --git a/xen/include/asm-x86/hpet.h b/xen/include/asm-x86/hpet.h
> index fb6bf05065..531e94e904 100644
> --- a/xen/include/asm-x86/hpet.h
> +++ b/xen/include/asm-x86/hpet.h
> @@ -82,6 +82,7 @@ void hpet_broadcast_enter(void);
> void hpet_broadcast_exit(void);
> int hpet_broadcast_is_available(void);
> void hpet_disable_legacy_broadcast(void);
> +void hpet_disable_legacy(void);
>
> extern void (*pv_rtc_handler)(uint8_t reg, uint8_t value);
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |