Re: [Xen-devel] [PATCH 2/3][RFC] MSI/MSI-X support for dom0/driver domain

[Keir Fraser <Keir.Fraser@xxxxxxxxxxxx>]

On 28/5/07 09:01, "Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx> wrote:

> 1) Two phys_ops are added, First is physdev_msi_setup, to notify xen
> that the vector is msi-irq type. The second is PHYSDEVOP_msi_format, to
> get the msi format from xen side (mainly for target CPU), is it ok to
> add them?
> 
> 2) Current pirq is a global resource, how about make pirq be a
> per-domain resource?

Hi Jiang,

Yes, my main problem with this patchset is that it further overloads the
global pirq namespace inside Xen. Could we just specify vectors at the Xen
interface? Something like:

vector = PHYSDEVOP_alloc_irq_vector(MSI_IRQ)

Where MSI_IRQ is a specially-chosen value for the irq parameter to indicate
no presence in 'pirq' namespace (from Xen's point of view at least!).

Then you don't need physdev_msi_setup, as it's implicit in specifying
MSI_IRQ above. And then the fact that physdev_msi_format takes a vector
parameter makes more sense.

The new physdev_irq_permission can also provide a translation service into a
new per-domain pirq space. Takes parameters something like:
 @type: MSI, or INTx
 @idx: vector (MSI type), or irq number as specified to alloc_irq_vector
(INTx type)
 @domain: domain to remap to
 @pirq: 'pirq' in that domain (allow caller to specify, or -1 to
auto-allocate).

Probably call it PHYSDEVOP_map_irq instead. And provide a
PHYSDEVOP_unmap_irq too (takes domain/pirq pair).

Then EVTCHNOP_bind_pirq clearly maps into this neatly! The only question is
how to map the MSIs into the dom0 pirq space. We could probably hook a
special call to map_irq() for dom0 into pci_enable_msi() or similar for
dom0-bound devices.

Overall I think the patches are looking good. I thought about whether we
should push more knowledge of MSI-X into Xen (e.g., so that it can
automatically mask/unmask/set-affinity on MSI-X sources) but I think that's
a bad idea after all, especially with VT-d and PCI-IOV coming down the
pipeline which will allow remapping of interrupts in a much nicer manner.

One other general comment is that you need to keep an eye out for coding
style (e.g., make sure you use 4-space soft tabs in most places in Xen;
always use 8-space hard tabs in Linux). Also, where you modify previously
untainted Linux files, make sure they should still compile correctly in
native builds (you may need to sprinkle some ifdef CONFIG_XEN's around the
place).

Anyway, good patches for a first cut!

 -- Keir



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