Xen devel - May 2008 - [PATCH] bind passthroug pci device interrupt pins to INTA

This patch changes the virtual pci configuration space of passthrough
PCI devices so that INTA is unconditionally used, thus minimizes the
situation of guest gsi sharing, which doesn''t work. It also adds a
warning when such sharing is detected.

Signed-off-by: Qing He <qing.he@intel.com>
---

The original scheme is to use the interrupt pin in the physical pci
configuration space. However, the use of interrupt pins other than INTA
will likely cause problem when the number of assigned devices exceeds 8,
e.g. dev 3, INTB and dev 11, INTA share the same girq. In this case, one
machine may be left untracked and masked, any devices using the same
machine irq (including those owned by other domains) is then blocked.

Just wonder if there is any need to expose multifunction devices (i.e.
have to use INTB, etc.) to the guest in the future.

All comments and suggestions are welcomed.


 tools/ioemu/hw/pass-through.c |    6 ++++--
 xen/drivers/passthrough/io.c  |    9 +++++++++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff -r 86587698116d tools/ioemu/hw/pass-through.c
--- a/tools/ioemu/hw/pass-through.c	Wed May 14 14:12:53 2008 +0100
+++ b/tools/ioemu/hw/pass-through.c	Tue May 20 01:57:31 2008 +0800
@@ -563,9 +563,11 @@ struct pt_dev * register_real_device(PCI
     /* Handle real device''s MMIO/PIO BARs */
     pt_register_regions(assigned_device);
 
-    /* Bind interrupt */
+    /* Bind interrupt to INTA to minimize guest irq sharing */
     e_device = (assigned_device->dev.devfn >> 3) & 0x1f;
-    e_intx = assigned_device->dev.config[0x3d]-1;
+    if (assigned_device->dev.config[0x3d] > 0)
+        assigned_device->dev.config[0x3d] = 1;
+    e_intx = 0;
 
     if ( PT_MACHINE_IRQ_AUTO == machine_irq )
     {
diff -r 86587698116d xen/drivers/passthrough/io.c
--- a/xen/drivers/passthrough/io.c	Wed May 14 14:12:53 2008 +0100
+++ b/xen/drivers/passthrough/io.c	Tue May 20 18:52:26 2008 +0800
@@ -91,6 +91,15 @@ int pt_irq_create_bind_vtd(
         guest_gsi = hvm_pci_intx_gsi(device, intx);
         link = hvm_pci_intx_link(device, intx);
         hvm_irq_dpci->link_cnt[link]++;
+
+        if (hvm_irq_dpci->girq[guest_gsi].valid) {
+            gdprintk(XENLOG_WARNING VTDPREFIX,
+                     "pt_irq_create_bind_vtd: guest_gsi %d already in
use, "
+                     "device,intx = %d,%d\n",
+                     guest_gsi, hvm_irq_dpci->girq[guest_gsi].device,
+                     hvm_irq_dpci->girq[guest_gsi].intx);
+            return -EEXIST;
+        }
 
         digl = xmalloc(struct dev_intx_gsi_link);
         if ( !digl )


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

On 20/5/08 05:34, "He, Qing" <qing.he@intel.com> wrote:
> The original scheme is to use the interrupt pin in the physical pci
> configuration space. However, the use of interrupt pins other than INTA
> will likely cause problem when the number of assigned devices exceeds 8,
> e.g. dev 3, INTB and dev 11, INTA share the same girq. In this case, one
> machine may be left untracked and masked, any devices using the same
> machine irq (including those owned by other domains) is then blocked.
> 
> Just wonder if there is any need to expose multifunction devices (i.e.
> have to use INTB, etc.) to the guest in the future.
> 
> All comments and suggestions are welcomed.
Could you set the INT line to the function number? FN0->INTA, FN1->INTB,
...? This would then work for multi-fn devices, yet still most devices have
only fn0 and hence would use INTA as you desire.

 -- Keir



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

>-----Original Message-----
>From: Keir Fraser [mailto:keir.fraser@eu.citrix.com]
>Sent: 2008年5月20日 16:55
>To: He, Qing
>Cc: xen-devel@lists.xensource.com
>Subject: Re: [PATCH] bind passthroug pci device interrupt pins to INTA
>
>
>
>
>On 20/5/08 05:34, "He, Qing" <qing.he@intel.com> wrote:
>
>> The original scheme is to use the interrupt pin in the physical pci
>> configuration space. However, the use of interrupt pins other than INTA
>> will likely cause problem when the number of assigned devices exceeds
8,
>> e.g. dev 3, INTB and dev 11, INTA share the same girq. In this case,
one
>> machine may be left untracked and masked, any devices using the same
>> machine irq (including those owned by other domains) is then blocked.
>>
>> Just wonder if there is any need to expose multifunction devices (i.e.
>> have to use INTB, etc.) to the guest in the future.
>>
>> All comments and suggestions are welcomed.
>
>Could you set the INT line to the function number? FN0->INTA,
FN1->INTB,
>...? This would then work for multi-fn devices, yet still most devices have
>only fn0 and hence would use INTA as you desire.
The case that makes me want to change this is the USB assignment. If I assign
all USB stuff to a guest, that''s already 8 functions within 2 devices
using all the INTA to INTD. Guest gsi sharing will be very likely to happen if I
assign another one or two device.

There are other options I can think of, including
(a) support sharing of guest gsi. This may also be good for PIC guest where
sharing is more common. However, eoi and unmask of sharing guest gsi would be a
pain, if they have different machine irqs. The implementation is subjected to
careful considerations of corner cases.
(b) change the device model slot allocation policy, to ensure that guest gsi
sharing devices also have the same machine irq. This solution reduces the
flexibility.
(c) dynamic routing table, which I assume is not desirable.


(a) may be the better way to go, but at the cost of additional complexity. Also,
it does not look very good to me that a guest gsi has to link to multiple
machine irqs, especially in EOI handling part. While simply pins interrupt pins
to INTA works fine in most cases and it''s stupidly simple compared with
other options.

Thanks,
Qing
>
> -- Keir
>

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

>>> Just wonder if there is any need to expose multifunction devices
(i.e.
>>> have to use INTB, etc.) to the guest in the future.
>>>
>>> All comments and suggestions are welcomed.
>>
Keir,
I just checked the documentation. It seems that USB device does need to
be assigned to guest as a single multifunction device, since the
interrupt pin actually used by a single function may not be the same as
what shows in the configuration space.

So I''m sorry but please ignore this patch at the moment. I''ll
have it
reworked for multifunction device support.

-Qing
>>Could you set the INT line to the function number? FN0->INTA,
FN1->INTB,
>>...? This would then work for multi-fn devices, yet still most devices
have
>>only fn0 and hence would use INTA as you desire.
>
>The case that makes me want to change this is the USB assignment. If I
assign all
>USB stuff to a guest, that''s already 8 functions within 2 devices
using
all the INTA to
>INTD. Guest gsi sharing will be very likely to happen if I assign
another one or two
>device.
>
>There are other options I can think of, including
>(a) support sharing of guest gsi. This may also be good for PIC guest
where sharing
>is more common. However, eoi and unmask of sharing guest gsi would be a
pain, if
>they have different machine irqs. The implementation is subjected to
careful
>considerations of corner cases.
>(b) change the device model slot allocation policy, to ensure that
guest gsi sharing
>devices also have the same machine irq. This solution reduces the
flexibility.
>(c) dynamic routing table, which I assume is not desirable.
>
>
>(a) may be the better way to go, but at the cost of additional
complexity. Also, it does
>not look very good to me that a guest gsi has to link to multiple
machine irqs,
>especially in EOI handling part. While simply pins interrupt pins to
INTA works fine in
>most cases and it''s stupidly simple compared with other options.
>
>Thanks,
>Qing
>>
>> -- Keir
>>
>
>
>_______________________________________________
>Xen-devel mailing list
>Xen-devel@lists.xensource.com
>http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel