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]Add MSI support to PV_dom0

To: "Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH]Add MSI support to PV_dom0
From: Jeremy Fitzhardinge <jeremy@xxxxxxxx>
Date: Wed, 13 May 2009 10:31:03 -0700
Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Matthew Wilcox <matthew@xxxxxx>
Delivery-date: Wed, 13 May 2009 10:31:35 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <E2263E4A5B2284449EEBD0AAB751098402C4E343A4@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
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: <E2263E4A5B2284449EEBD0AAB751098401D1F443F2@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <4A09D69F.4010109@xxxxxxxx> <E2263E4A5B2284449EEBD0AAB751098402C4E343A4@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Thunderbird 2.0.0.21 (X11/20090320)
Jiang, Yunhong wrote:
moment, a Linux irq == the gsi, which is a convention I kept for Xen

I suspect if this will always work, because
1) It may not work on x86_32, because gsi is compressed in x86_32, or we will 
not support that?

Yinghai Lu has posted patches to remove gsi compression, now that dynamic irq arrays mean we have no practical limit on irq numbers.

2) Even in x86_64, I suspect it may not work still. Currently Xen defined NR_IRQS 
as 256, unless specified in Rules.mk. So what will happen if the gsi > 256? I 
suspect the PHYSDEVOP_alloc_irq_vector hypercall will fail.

The reason is, the pirq is virtual interrupt line between Xen/guest, and is 
determined by Xen HV, while irq/gsi is defined by guest/dom0. Current Xen's 
dom0 has a mapping between pirq/irq, so I'm not sure if that logic is needed in 
pv_dom0 still.

Well, we could decouple Xen pirq from Linux irq fairly easily if needed. Or just limit the range of identity mapped irqs (by adjusting get_nr_hw_irqs()/identity_mapped_irq()) so that the common case will still get identities but we can leave room in the sub-256 irq space for pirqs.

Sure, I will do like that. In fact, my draft implemented this way.
BTW, what do you mean of "an ops vec"???

A struct msi_ops (or something) with function pointers to appropriate functions. But I think a better approach is to just pull everything out into xen_setup_msi_irqs().

void arch_teardown_msi_irq(unsigned int irq) { - destroy_irq(irq); +
if (xen_destroy_irq(irq)) + destroy_irq(irq);
Yes, this little pattern should be put into a single place.

What do you mean of put into a single place?

Have a destroy_msi_irq() which contains this logic, and call it from whenever it is needed.

(base + 0x04)
Isn't this defined somewhere else?  If not, is there a better
place to define it?

It is defined in drivers/pci/msi.h. As I don't want to touch anything in 
drivers/pci/ directory, so I put it here (sorry that I should place it under 
some .h file).

It is always best to put generically useful stuff in the most appropriate common place.

commit 134186e0b4526908b4c64c1454caff5db6ddf972 Author: Jiang,
Yunhong <yunhong.jiang@xxxxxxxxx> Date: Thu May 7 21:22:26 2009
+0800 Add the pci wrap function, to notify Xen of all PCI
information. Currently Xen depends on Dom0 to notify all PCI
information. When Xen try to setup MSI for a pci device, it will
depends on this information. This method need more discussion, since
it add some ugly hook to pci_bus_type.
Yes, this is a bit unpleasant.  I can't see any immediately
satisfying solution, partly because
I don't fully understand what needs to be done in these hooks.
Why does it need to do this at probe
time rather than when setting up the interrupts?

The main purpose of this hook is to keep Xen have the list of PCI device in the 
system, include those hot plug/unpluged devices, before the device begin 
function,  I think the main purpose is for VT-d support in Xen, so it is in 
probe time.

In fact, MSI should works without this list. Unfortunately, currently Xen will 
check if the device exists or not when enabling MSI/MSI-X, and will return 
-ENODEV if the device is not in the list.

Well, couldn't we quietly add it to the list just before setting up the first interrupt for the device? I guess that wouldn't work for VT-d support, unless we can defer it until we first see the device in some Xen-specific code.

Matthew, do you have any thoughts about a cleaner way to hook into probe/remove? Would a arch_pci_device_probe() hook in pci_device_probe() be a reasonable way to handle it?

But I do think this code is very ugly, and I suspect if we can push this code 
into upstream successfully. I know Winston is working on PCI hotplug in Xen 
side, I will talk with him to see if any plan to change this logic.

Either way we're eventually going to need something like it for VT-d, won't we?

+static int pci_ari_enabled(struct pci_bus *bus) +{ + return
bus->self && bus->self->ari_enabled; +}
Is this a generally useful predicate?

I just copied from Xen's dom0 code. I will check it .

It looks like something that could be in a common pci header.


   J

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

<Prev in Thread] Current Thread [Next in Thread>