Re: [PATCH] x86: Don't ack_APIC_irq() if lapic is disabled in GENERIC_INTERRUPT_VECTOR handler

[Sheng Yang <sheng@linux.intel.com>]

On Monday 21 September 2009 02:49:06 Cyrill Gorcunov wrote:
> [Cyrill Gorcunov - Sun, Sep 20, 2009 at 10:42:03PM +0400]
>
> | [Cyrill Gorcunov - Sun, Sep 20, 2009 at 10:30:11PM +0400]
> | ...
> |
> | | | > iirc it was Xen related patch. So it's not that simple.
> | | | >
> | | | > I've pointed out Sheng about disable_apic. I'm not Xen
> | | | > specialist but Xen team seem to use specific apic setup
> | | | > so our "dummy" operations are not involved (case they
> | | | > set disable_apic=1 without "turn off" apic ops in real).
> | | | > Something like that.
> | | |
> | | | They should then set a NOP function in that case. We really dont want
> | | | to slow down hotpath functions like smp_generic_interrupt() with
> | | | flaggery.
> | | |
> | | | 	Ingo
> | |
> | | Well, I suppose we should wait for Sheng's comments.
> | | I wish I would answer you but I simply don't know Xen
> | | code :)
> | |
> | | 	-- Cyrill
> |
> | Wait a bit Ingo, please. It seems I'm having different
> | patch series in mind. Need to restore mail thread.
> | Will back soon :)
> |
> | 	-- Cyrill
>
> yeah, it comes from Xen RFC series. Here is a quote from
> conversation.
>
> > Sheng Yan
> >
> > | | is there was some problem with it? I'm asking you
> > | | because if disable_apic=1 then any apic write/read
> > | | operations become NOPs. So I don't see how it may
> > | | hurt. But I could be missing something.
> > | |
> > | |   -- Cyrill
> > |
> > | Ah, I see -- it's due to your other patch...
> > | Hmm this makes all "disable apic" idea less
> > | general. And safety of ack_APIC_irq is now
> > | under suspicious.
> >
> > Um, probably. I've seen a ack_APIC_irq() in do_IRQ when handle_irq()
> > fail. Seems the assumption that ack_APIC_irq() always safe is there. I
> > will check if I can make it more elegant - maybe disable the warning in
> > the Xen code...
>
> Personally, I think "out-of-xen-thread" this patch is not needed.
> And if this apic-ack operation causes any kind of problems --
> this problem should be fixed without disable_apic involved.

In fact Xen set it to functional nop, but print warning when processing APIC 
access. So Xen assumed that no apic access would in the Xen path. But now this 
assumption have a little trouble with current hybrid implementation. I am 
working on to fix this issue now, so please ignore this patch temporarily, I 
would try to figure out a more elegant way to handle this issue and minimal  
the native impact.

Thanks.

-- 
regards
Yang, Sheng