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

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

To: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH 07/12] xen: events: separate MSI PIRQ allocation from PIRQ binding to IRQ
From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Date: Mon, 21 Feb 2011 12:50:45 +0000
Cc: Jeremy Fitzhardinge <jeremy@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <Stefano.Stabellini@xxxxxxxxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Delivery-date: Mon, 21 Feb 2011 04:52:42 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1298047417-7445-7-git-send-email-ian.campbell@xxxxxxxxxx>
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: <1298047386.16356.3620.camel@xxxxxxxxxxxxxxxxxxxxxx> <1298047417-7445-7-git-send-email-ian.campbell@xxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Alpine 2.00 (DEB 1167 2008-08-23)
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

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