[Xen-devel] Re: [PATCH 07/12] xen: events: separate MSI PIRQ allocation from PIRQ binding to IRQ

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 18 Feb 2011, Ian Campbell wrote:
> Split the binding aspect of xen_allocate_pirq_msi out into a new
> xen_bind_pirq_to_irq function.
> 
> In xen_hvm_setup_msi_irq when allocating a pirq write the MSI message
> to signal the PIRQ as soon as the pirq is obtained. There is no way to
> free the pirq back so if the subsequent binding to an IRQ fails we
> want to ensure that we will reuse the PIRQ next time rather than leak
> it.
> 
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> ---
>  arch/x86/pci/xen.c   |   68 +++++++++++++++++++------------------------------
>  drivers/xen/events.c |   30 ++++++++++-----------
>  include/xen/events.h |    4 ++-
>  3 files changed, 43 insertions(+), 59 deletions(-)
> 
> diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
> index b734358..2b915c1 100644
> --- a/arch/x86/pci/xen.c
> +++ b/arch/x86/pci/xen.c
> @@ -86,7 +86,7 @@ static void xen_msi_compose_msg(struct pci_dev *pdev, 
> unsigned int pirq,
>  
>  static int xen_hvm_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
>  {
> -     int irq, pirq, ret = 0;
> +     int irq, pirq;
>       struct msi_desc *msidesc;
>       struct msi_msg msg;
>  
> @@ -94,39 +94,32 @@ static int xen_hvm_setup_msi_irqs(struct pci_dev *dev, 
> int nvec, int type)
>               __read_msi_msg(msidesc, &msg);
>               pirq = MSI_ADDR_EXT_DEST_ID(msg.address_hi) |
>                       ((msg.address_lo >> MSI_ADDR_DEST_ID_SHIFT) & 0xff);
> -             if (xen_irq_from_pirq(pirq) >= 0 && msg.data == 
> XEN_PIRQ_MSI_DATA) {
> -                     irq = xen_allocate_pirq_msi((type == PCI_CAP_ID_MSIX) ?
> -                                                 "msi-x" : "msi", &pirq, 0);
> -                     if (irq < 0)
> +             if (msg.data != XEN_PIRQ_MSI_DATA ||
> +                 xen_irq_from_pirq(pirq) < 0) {
> +                     pirq = xen_allocate_pirq_msi(dev, msidesc);
> +                     if (pirq < 0)
>                               goto error;
> -                     ret = set_irq_msi(irq, msidesc);
> -                     if (ret < 0)
> -                             goto error_while;
> -                     printk(KERN_DEBUG "xen: msi already setup: msi --> 
> irq=%d"
> -                                     " pirq=%d\n", irq, pirq);
> -                     return 0;
> +                     xen_msi_compose_msg(dev, pirq, &msg);
> +                     __write_msi_msg(msidesc, &msg);
> +                     dev_dbg(&dev->dev, "xen: msi bound to pirq=%d\n", pirq);
> +             } else {
> +                     dev_dbg(&dev->dev,
> +                             "xen: msi already bound to pirq=%d\n", pirq);
>               }
> -             irq = xen_allocate_pirq_msi((type == PCI_CAP_ID_MSIX) ?
> -                                         "msi-x" : "msi", &pirq, 1);
> -             if (irq < 0 || pirq < 0)
> +             irq = xen_bind_pirq_msi_to_irq(dev, msidesc, pirq,
> +                                            (type == PCI_CAP_ID_MSIX) ?
> +                                            "msi-x" : "msi");
> +             if (irq < 0)
>                       goto error;
> -             printk(KERN_DEBUG "xen: msi --> irq=%d, pirq=%d\n", irq, pirq);
> -             xen_msi_compose_msg(dev, pirq, &msg);
> -             ret = set_irq_msi(irq, msidesc);
> -             if (ret < 0)
> -                     goto error_while;
> -             write_msi_msg(irq, &msg);
> +             dev_dbg(&dev->dev,
> +                     "xen: msi --> pirq=%d --> irq=%d\n", pirq, irq);
>       }
>       return 0;

my only concern is about the order of the calls: on native and on xen
before this patch the order is

msi_compose_msg
set_irq_msi
write_msi_msg

while after this patch the order is:

msi_compose_msg
write_msi_msg
set_irq_msi

however I don't think it makes a difference because msi (and msix)
are not enabled yet anyway on the pci device in question (they are
enabled by msi(x)_capability_init right after calling
arch_setup_msi_irqs).

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