[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] linux-3.9-rc0 regression from 3.8 SATA controller not detected under xen



On Wed, Feb 27, 2013 at 08:56:07PM +0100, Sander Eikelenboom wrote:
> 
> Wednesday, February 27, 2013, 8:28:10 PM, you wrote:
> 
> > On Wed, Feb 27, 2013 at 06:50:59PM +0100, Sander Eikelenboom wrote:
> >> 
> >> Wednesday, February 27, 2013, 1:54:31 PM, you wrote:
> >> 
> >> >>>> On 27.02.13 at 12:46, Sander Eikelenboom <linux@xxxxxxxxxxxxxx> wrote:
> >> >>   [   89.338827] ahci: probe of 0000:00:11.0 failed with error -22
> >> 
> >> > Which is -EINVAL. With nothing else printed, I'm afraid you need to
> >> > find the origin of this return value by instrumenting the involved
> >> > call tree.
> >> 
> >> Just wondering, is multiple msi's per device actually supported by xen ?
> 
> > That is very good question. I know we support MSI-X b/c 1GB or 10GB NICs
> > use them and they work great with Xen.
> 
> > BTW, this is merge:
> > ommit 5800700f66678ea5c85e7d62b138416070bf7f60
> > Merge: 266d7ad af8d102
> > Author: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> > Date:   Tue Feb 19 19:07:27 2013 -0800
> 
> >     Merge branch 'x86-apic-for-linus' of 
> > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
> >     
> >     Pull x86/apic changes from Ingo Molnar:
> >      "Main changes:
> >     
> >        - Multiple MSI support added to the APIC, PCI and AHCI code - acked
> >          by all relevant maintainers, by Alexander Gordeev.
> >     
> >          The advantage is that multiple AHCI ports can have multiple MSI
> >          irqs assigned, and can thus spread to multiple CPUs.
> >     
> >          [ Drivers can make use of this new facility via the
> >            pci_enable_msi_block_auto() method ]
> 
> 
> Ahh yes, i have added some debug info to ahci.c:
> 
>   [   36.778395] SE | bus: 'pci': really_probe: probing driver ahci with 
> device 0000:00:11.0
>   [   36.809777] really_probe: pinctrl_bind_pins(0000:00:11.0) ret: 0
>   [   36.835136] ahci 0000:00:11.0: SE | ahci_init_one start 
>   [   36.858284] ahci 0000:00:11.0: version 3.0
>   [   36.877840] xen: registering gsi 19 triggering 0 polarity 1
>   [   36.901791] xen: --> pirq=19 -> irq=19 (gsi=19)
>   (XEN) [2013-02-27 19:43:07] IOAPIC[0]: Set PCI routing entry (6-19 -> 0x99 
> -> IRQ 19 Mode:1 Active:1)
>  [   36.949293] ahci 0000:00:11.0: SE | pcim_enable_device(pdev) rc:0
>   [   36.974714] ahci 0000:00:11.0: SE pcim_iomap_regions_request_all(pdev, 1 
> << ahci_pci_bar, DRV_NAME)  rc:0
>   [   37.010706] ahci 0000:00:11.0: SE pci_enable_msi_block_auto(pdev, 
> &maxvec) rc:4
>   [   37.039878] ahci 0000:00:11.0: SE | n_msis: 4
>   [   37.060115] ahci 0000:00:11.0: SE | ahci_configure_dma_masks(pdev, 
> hpriv->cap & HOST_CAP_64)  rc:0
>   [   37.094135] ahci 0000:00:11.0: SE | ahci_pci_reset_controller(host)  rc:0
>   [   37.121658] ahci 0000:00:11.0: AHCI 0001.0200 32 slots 4 ports 6 Gbps 
> 0xf impl SATA mode
>   [   37.153118] ahci 0000:00:11.0: flags: 64bit ncq sntf ilck pm led clo pmp 
> pio slum part 
>   [   37.184265] ahci 0000:00:11.0: SE | me here 1 
>   [   37.204568] ahci 0000:00:11.0: SE | n_msis(4) host->n_ports(4) irq:121
>   [   37.231748] ahci 0000:00:11.0: SE | ata_host_start(host) rc:0
>   [   37.256222] ahci 0000:00:11.0: SE | devm_request_threaded_irq i:0  rc:0
>   [   37.283023] ahci 0000:00:11.0: SE | devm_request_threaded_irq i:1  rc:-22
>   [   37.310344] really_probe: dev->bus->probe(0000:00:11.0) ret: -22
>   [   37.335467] ahci: probe of 0000:00:11.0 failed with error -22
>   [   37.359552] really_probe: 0000:00:11.0 done ret: 0
> 
> So it bails out at the second devm_request_threaded_irq in:
> 
> int ahci_host_activate(struct ata_host *host, int irq, unsigned int n_msis)
> {
>         int i, rc;
> 
>         dev_err(host->dev, "SE | n_msis(%d) host->n_ports(%d) 
> irq:%d\n",n_msis , host->n_ports,irq);
>         /* Sharing Last Message among several ports is not supported */
>         if (n_msis < host->n_ports){
>                 dev_err(host->dev, "SE | uhoh n_msis(%d) < host->n_ports(%d) 
> irq:%d\n",n_msis , host->n_ports,irq);
>                 return -EINVAL;
>         }
>         rc = ata_host_start(host);
>         dev_err(host->dev, "SE | ata_host_start(host) rc:%d\n",rc);
>         if (rc)
>                 return rc;
> 
>         for (i = 0; i < host->n_ports; i++) {
>                 rc = devm_request_threaded_irq(host->dev,
>                         irq + i, ahci_hw_interrupt, ahci_thread_fn, 
> IRQF_SHARED,
>                         dev_driver_string(host->dev), host->ports[i]);
>                 dev_err(host->dev, "SE | devm_request_threaded_irq i:%d  
> rc:%d\n",i,rc);
>                 if (rc)
>                         goto out_free_irqs;
>         }
> 
This is what I have for the patch, OK with me sending it tomorrow to Linus?

>From f4dce2c2114ef623dc6d931b5ea950a08e80057b Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Date: Thu, 28 Feb 2013 09:05:41 -0500
Subject: [PATCH] xen/pci: We don't do multiple MSI's.

There is no hypercall to setup multiple MSI per PCI device.
As such with these two new commits:
-  08261d87f7d1b6253ab3223756625a5c74532293
   PCI/MSI: Enable multiple MSIs with pci_enable_msi_block_auto()
- 5ca72c4f7c412c2002363218901eba5516c476b1
   AHCI: Support multiple MSIs

we would call the PHYSDEVOP_map_pirq 'nvec' times with the same
contents of the PCI device. Sander discovered that we would get
the same PIRQ value 'nvec' times and return said values to the
caller. That of course meant that the device was configured only
with one MSI and AHCI would fail with:

ahci 0000:00:11.0: version 3.0
xen: registering gsi 19 triggering 0 polarity 1
xen: --> pirq=19 -> irq=19 (gsi=19)
(XEN) [2013-02-27 19:43:07] IOAPIC[0]: Set PCI routing entry (6-19 -> 0x99 -> 
IRQ 19 Mode:1 Active:1)
ahci 0000:00:11.0: AHCI 0001.0200 32 slots 4 ports 6 Gbps 0xf impl SATA mode
ahci 0000:00:11.0: flags: 64bit ncq sntf ilck pm led clo pmp pio slum part
ahci: probe of 0000:00:11.0 failed with error -22

That is b/c in ahci_host_activate the second call to
devm_request_threaded_irq  would return -EINVAL as we passed in
(on the second run) an IRQ value that was never initialized.

Reported-and-Tested-by: Sander Eikelenboom <linux@xxxxxxxxxxxxxx>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
---
 arch/x86/pci/xen.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index 56ab749..94e7662 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -162,6 +162,9 @@ static int xen_setup_msi_irqs(struct pci_dev *dev, int 
nvec, int type)
        struct msi_desc *msidesc;
        int *v;
 
+       if (type == PCI_CAP_ID_MSI && nvec > 1)
+               return 1;
+
        v = kzalloc(sizeof(int) * max(1, nvec), GFP_KERNEL);
        if (!v)
                return -ENOMEM;
@@ -220,6 +223,9 @@ static int xen_hvm_setup_msi_irqs(struct pci_dev *dev, int 
nvec, int type)
        struct msi_desc *msidesc;
        struct msi_msg msg;
 
+       if (type == PCI_CAP_ID_MSI && nvec > 1)
+               return 1;
+
        list_for_each_entry(msidesc, &dev->msi_list, list) {
                __read_msi_msg(msidesc, &msg);
                pirq = MSI_ADDR_EXT_DEST_ID(msg.address_hi) |
@@ -263,6 +269,9 @@ static int xen_initdom_setup_msi_irqs(struct pci_dev *dev, 
int nvec, int type)
        int ret = 0;
        struct msi_desc *msidesc;
 
+       if (type == PCI_CAP_ID_MSI && nvec > 1)
+               return 1;
+
        list_for_each_entry(msidesc, &dev->msi_list, list) {
                struct physdev_map_pirq map_irq;
                domid_t domid;
-- 
1.8.0.2


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.