WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH] x86/IRQ: prevent vector sharing within IO-APICs

To: Jan Beulich <JBeulich@xxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] x86/IRQ: prevent vector sharing within IO-APICs
From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Date: Mon, 14 Nov 2011 13:59:07 +0000
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Mon, 14 Nov 2011 06:03:40 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4EC0E6010200007800060B29@xxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <4EBD54B80200007800060776@xxxxxxxxxxxxxxxxxxxx> <4EBD579F.70901@xxxxxxxxxx> <4EC0E6010200007800060B29@xxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.23) Gecko/20110921 Lightning/1.0b2 Thunderbird/3.1.15
On 14/11/11 08:57, Jan Beulich wrote:
>>>> On 11.11.11 at 18:13, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 11/11/11 16:00, Jan Beulich wrote:
>>> Following the prevention of vector sharing for MSIs, this change
>>> enforces the same within IO-APICs: Pin based interrupts use the IO-APIC
>>> as their identifying device under the AMD IOMMU (and just like for
>>> MSIs, only the identifying device is used to remap interrupts here,
>>> with no regard to an interrupt's destination).
>>>
>>> Additionally, LAPIC initiated EOIs (for level triggered interrupts) too
>>> use only the vector for identifying which interrupts to end. While this
>>> generally causes no significant problem (at worst an interrupt would be
>>> re-raised without a new interrupt event actually having occurred)
>> At worst, hardware asserts a line interrupt, deasserts it later, and an
>> EOI broadcast gets rid of any record that the IRQ was ever raised. 
>> While I would classify this as buggy behavior, I believe I have seen
>> some hardware doing this when investigating the line level IRQ migration
>> bug, as clearing the IRR did not immediately cause another interrupt to
>> be generated.
>>
>>> , it
>>> still seems better to avoid the situation.
>>>
>>> For this second aspect, a distinction is being made between the
>>> traditional and the directed-EOI cases: In the former, vectors should
>>> not be shared throughout all IO-APICs in the system, while in the
>>> latter case only individual IO-APICs need to be contrained (or, if the
>>> firmware indicates so, sub- groups of them having the same GSI appear
>>> at multiple pins).
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> Provisional nack because it is my understanding that under all
>> circumstances, you must maintain a vector exclusivity map across all
>> IO-APICs because of the broadcast problem.  Or have I made a mistake in
>> my reasoning?
> With directed EOI there's no broadcasting involved, which is why
> global sharing prevention is not necessary.
>
> However, after some more thinking over the weekend I think we need
> to also/first adjust end_level_ioapic_irq()'s call to io_apic_eoi_vector():
> It shouldn't really iterate over all IO-APICs, but instead call
> eoi_IO_APIC_irq(). Thoughts? (That would also eliminate the need to
> look up pin or vector in __io_apic_eoi(), as all remaining call sites pass
> both.)
>
> Jan
>

I believe that should work.  By the point end_level_ioapic_irq() is
called, all the irq_desc information should point to the new vector, so
eoi_IO_APIC_irq() should get it correct.  At the time I made that patch,
I was not so familiar with the IO-APIC code so decided that calling
io_apic_eoi was the safer bet.

Having had the weekend to consider the logic in your patch, I now think
I was wrong, and your reasoning is correct.  Therefore, Ack.

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com


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