[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 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?

Thanks,
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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.