|
xen-devel
Re: [Xen-devel] [PATCH]Add MSI support to PV_dom0
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
|
|
|
Home