RE: [Xen-devel] bogus HPET initialization order on x86

["Wei, Gang" <gang.wei@xxxxxxxxx>]

Jan Beulich wrote on 2011-03-11:
>>>> "Wei, Gang"  03/10/11 4:36 AM >>> Jan Beulich wrote on 2011-03-09:
>>>> Further, with the channel getting enabled (down the
>>>> hpet_fsb_cap_lookup() call tree) before hpet_events[] gets fully
>>>> initialized, I'd also think it should be possible to hit the spurious
>>>> warning in hpet_interrupt_handler() just because of improper
>>>> initialization order.
>>>> 
>>>> If that's all impossible in practice, adding some meaningful
>>>> comments to the code to describe why this is so would be much
>>>> appreciated.
>> 
>> For normal booting case, hpet interrupts should not come before dom0
>> start booting and pass ACPI tables to hypervisor, so that's impossible in
>> practice in this case.
> 
> The legacy code path gets called from the timer interrupt, so what
> would prevent this path being taken before Dom0 boots?

Oh, yes, you are right. The legacy code path may get called before PIT 
interrupt being turned off. If we put the initialization of 
legacy_hpet_event.event_handler at the end of hpet_broadcast_init(), things 
should be ok. Further, it would be better if we could disable PIT interrupt 
before calling hpet_broadcast_init(), and re-enable PIT interrupt only while 
PIT broadcast is needed.

> The non-legacy interrupts also get fully set up before Dom0 boots - I
> don't think I saw anything that specifically enables these interrupts
> upon arrival of some ACPI data from Dom0.

The non-legacy interrupts will only come after some hpet channels was 
programmed. Before hypervisor got ACPI data from Dom0 and be capable to enter 
deep C states, hpet channels were not programmed.

>> Do I miss any other cases? If not, I will cook a patch to add the
>> required comments along with pulling spinlock/rwlock
>> initialization before .event_handler settings.
> 
> Yes, please, though I could also fold this in my bigger cleanup patch
> (properly separating init a resume paths) if I still didn't understand
> some aspect of it and this turns out a mere cleanup.

Oh, if you like, you can definitely include these change in your cleanup patch. 
It is your finding. You deserve the credit. I can just help to review & ack.

Jimmy



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel