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

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

Jan Beulich wrote on 2011-03-11:
>>>> On 11.03.11 at 03:43, "Wei, Gang" <gang.wei@xxxxxxxxx> wrote:
>> 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.
> 
> But I'll leave that part to you.

I will send a patch for this.

>>> 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.
> 
> Could you point out *where* this happens? My reading of the code is
> that all channels get set fully up during hpet_broadcast_init()
> (->hpet_fsb_cap_lookup() ->hpet_assign_irq() ->hpet_setup_msi_irq()
> ->request_irq()). The only thing done later are adjustments of the
> interrupts' affinities.

What you read is just the configuration of the hpet interrupt. The hpet 
programming is happen in below places:
1. hpet_broadcast_enter()->reprogram_hpet_evt_channel()
2. handle_hpet_broadcast()->reprogram_hpet_evt_channel()

So the hpet interrupt should come after the first calling to 
hpet_broadcast_enter() (i.e. lapic_timer_off()).

> 
>>>> 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.
> 
> As it's a (latent) bug fix, I'll do it separately, so that it can also
> make it into 4.0 and 4.1. Independent of the above, I'll probably do
> the adjustment to both legacy and MSI paths, though, even if the latter 
> should turn out benign.

It is fine. I will have a look at it when the patch is ready.

Jimmy



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