Re: [Xen-devel] Re: 4.0/4.1 requests - IO-APIC EOI v2 [RFC]

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.09.11 at 17:55, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> On 09/09/11 16:39, Jan Beulich wrote:
>>>>> On 09.09.11 at 17:06, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>>> @@ -397,18 +397,7 @@ static void clear_IO_APIC_pin(unsigned i
>>>             entry.trigger = 1;
>>>             __ioapic_write_entry(apic, pin, TRUE, entry);
>>>         }
>>> -        if (mp_ioapics[apic].mpc_apicver >= 0x20)
>>> -            io_apic_eoi(apic, entry.vector);
>>> -        else {
>>> -            /*
>>> -             * Mechanism by which we clear remoteIRR in this case is by
>>> -             * changing the trigger mode to edge and back to level.
>>> -             */
>>> -            entry.trigger = 0;
>>> -            __ioapic_write_entry(apic, pin, TRUE, entry);
>>> -            entry.trigger = 1;
>>> -            __ioapic_write_entry(apic, pin, TRUE, entry);
>>> -        }
>>> +        io_apic_eoi(apic, entry.vector, pin);
>> This should be __io_apic_eoi() - all other functions called here are the
>> non-locking ones, too.
> 
> Questionable - I traced the calls and at this point and cant see the
> ioapic lock being taken.  I guess it might be safer overall to use
> non-locking and leave the problem to whoever cleans up the irq code...

It indeed is not being taken, and as you say this is deliberate (we may be
cleaning up in a crashed environment here).

>>> +    {
>>> +        /* Else fake an EOI by switching to edge triggered mode
>>> +         * and back */
>>> +        struct IO_APIC_route_entry entry;
>>> +        bool_t need_to_unmask = 0;
>>> +
>> You may want to assert that at least one of vector and pin is not -1.
> 
> There is a BUG_ON at the top of the function if both vector and pin are -1.

Sorry, I must have missed that.

Jan


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