Patchwork ioh3420: Add a map_irq function

login
register
mail settings
Submitter Alex Williamson
Date Feb. 28, 2013, 6:49 p.m.
Message ID <20130228184803.23865.84744.stgit@bling.home>
Download mbox | patch
Permalink /patch/224136/
State New
Headers show

Comments

Alex Williamson - Feb. 28, 2013, 6:49 p.m.
Every bridge needs to know how to map IRQs from it's secondary bus to
the primary bus.  We seem to be direct mapped on ioh3420.  This avoids
segfaults when trying to put assigned devices behind root ports.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/ioh3420.c |    7 +++++++
 1 file changed, 7 insertions(+)
Alexander Duyck - Sept. 27, 2013, 10:10 p.m.
On 02/28/2013 10:49 AM, Alex Williamson wrote:
> Every bridge needs to know how to map IRQs from it's secondary bus to
> the primary bus.  We seem to be direct mapped on ioh3420.  This avoids
> segfaults when trying to put assigned devices behind root ports.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> 
> ---
> hw/ioh3420.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/hw/ioh3420.c b/hw/ioh3420.c
> index 95bceb5..6ac4fe7 100644
> --- a/hw/ioh3420.c
> +++ b/hw/ioh3420.c
> @@ -90,6 +90,11 @@ static void ioh3420_reset(DeviceState *qdev)
>      pci_bridge_disable_base_limit(d);
>  }
>  
> +static int ioh3420_map_irq(PCIDevice *pci_dev, int irq_num)
> +{
> +    return irq_num;
> +}
> +
>  static int ioh3420_initfn(PCIDevice *d)
>  {
>      PCIBridge* br = DO_UPCAST(PCIBridge, dev, d);
> @@ -97,6 +102,8 @@ static int ioh3420_initfn(PCIDevice *d)
>      PCIESlot *s = DO_UPCAST(PCIESlot, port, p);
>      int rc;
>  
> +    pci_bridge_map_irq(br, NULL, ioh3420_map_irq);
> +
>      rc = pci_bridge_initfn(d);
>      if (rc < 0) {
>          return rc;
> 

What became of this patch?  I believe I am seeing the issue described
when I assign a MSI-X capable virtual device to the secondary bus of an
ioh3420, and applying this patch seems to resolve the issue.  Was there
an alternate patch proposed and if so where can I find it?

Thanks,

Alex
Alex Williamson - Oct. 2, 2013, 5:12 p.m.
On Fri, 2013-09-27 at 15:10 -0700, Alexander Duyck wrote:
> On 02/28/2013 10:49 AM, Alex Williamson wrote:
> > Every bridge needs to know how to map IRQs from it's secondary bus to
> > the primary bus.  We seem to be direct mapped on ioh3420.  This avoids
> > segfaults when trying to put assigned devices behind root ports.
> > 
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > 
> > ---
> > hw/ioh3420.c |    7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/hw/ioh3420.c b/hw/ioh3420.c
> > index 95bceb5..6ac4fe7 100644
> > --- a/hw/ioh3420.c
> > +++ b/hw/ioh3420.c
> > @@ -90,6 +90,11 @@ static void ioh3420_reset(DeviceState *qdev)
> >      pci_bridge_disable_base_limit(d);
> >  }
> >  
> > +static int ioh3420_map_irq(PCIDevice *pci_dev, int irq_num)
> > +{
> > +    return irq_num;
> > +}
> > +
> >  static int ioh3420_initfn(PCIDevice *d)
> >  {
> >      PCIBridge* br = DO_UPCAST(PCIBridge, dev, d);
> > @@ -97,6 +102,8 @@ static int ioh3420_initfn(PCIDevice *d)
> >      PCIESlot *s = DO_UPCAST(PCIESlot, port, p);
> >      int rc;
> >  
> > +    pci_bridge_map_irq(br, NULL, ioh3420_map_irq);
> > +
> >      rc = pci_bridge_initfn(d);
> >      if (rc < 0) {
> >          return rc;
> > 
> 
> What became of this patch?  I believe I am seeing the issue described
> when I assign a MSI-X capable virtual device to the secondary bus of an
> ioh3420, and applying this patch seems to resolve the issue.  Was there
> an alternate patch proposed and if so where can I find it?

Hmm, I thought it was no longer necessary with
pci_device_route_intx_to_irq(), especially if you're assigning a VF that
doesn't support INTx.  Are you using the latest qemu bits?  pci-assign
or vfio-pci?  Maybe provide a backtrace?  Thanks,

Alex
Alexander Duyck - Oct. 2, 2013, 5:30 p.m.
On 10/02/2013 10:12 AM, Alex Williamson wrote:
> On Fri, 2013-09-27 at 15:10 -0700, Alexander Duyck wrote:
>> On 02/28/2013 10:49 AM, Alex Williamson wrote:
>>> Every bridge needs to know how to map IRQs from it's secondary bus to
>>> the primary bus.  We seem to be direct mapped on ioh3420.  This avoids
>>> segfaults when trying to put assigned devices behind root ports.
>>>
>>> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
>>>
>>> ---
>>> hw/ioh3420.c |    7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/hw/ioh3420.c b/hw/ioh3420.c
>>> index 95bceb5..6ac4fe7 100644
>>> --- a/hw/ioh3420.c
>>> +++ b/hw/ioh3420.c
>>> @@ -90,6 +90,11 @@ static void ioh3420_reset(DeviceState *qdev)
>>>      pci_bridge_disable_base_limit(d);
>>>  }
>>>  
>>> +static int ioh3420_map_irq(PCIDevice *pci_dev, int irq_num)
>>> +{
>>> +    return irq_num;
>>> +}
>>> +
>>>  static int ioh3420_initfn(PCIDevice *d)
>>>  {
>>>      PCIBridge* br = DO_UPCAST(PCIBridge, dev, d);
>>> @@ -97,6 +102,8 @@ static int ioh3420_initfn(PCIDevice *d)
>>>      PCIESlot *s = DO_UPCAST(PCIESlot, port, p);
>>>      int rc;
>>>  
>>> +    pci_bridge_map_irq(br, NULL, ioh3420_map_irq);
>>> +
>>>      rc = pci_bridge_initfn(d);
>>>      if (rc < 0) {
>>>          return rc;
>>>
>> What became of this patch?  I believe I am seeing the issue described
>> when I assign a MSI-X capable virtual device to the secondary bus of an
>> ioh3420, and applying this patch seems to resolve the issue.  Was there
>> an alternate patch proposed and if so where can I find it?
> Hmm, I thought it was no longer necessary with
> pci_device_route_intx_to_irq(), especially if you're assigning a VF that
> doesn't support INTx.  Are you using the latest qemu bits?  pci-assign
> or vfio-pci?  Maybe provide a backtrace?  Thanks,
>
> Alex

I'm seeing it with an emulated interface using MSI-X, not with a VF or
assigned device.

I'll try to get you a backtrack when I have some time.  I'm currently
chasing down another issue and will try to remember to follow up on this
one.

Thanks,

Alex

Patch

diff --git a/hw/ioh3420.c b/hw/ioh3420.c
index 95bceb5..6ac4fe7 100644
--- a/hw/ioh3420.c
+++ b/hw/ioh3420.c
@@ -90,6 +90,11 @@  static void ioh3420_reset(DeviceState *qdev)
     pci_bridge_disable_base_limit(d);
 }
 
+static int ioh3420_map_irq(PCIDevice *pci_dev, int irq_num)
+{
+    return irq_num;
+}
+
 static int ioh3420_initfn(PCIDevice *d)
 {
     PCIBridge* br = DO_UPCAST(PCIBridge, dev, d);
@@ -97,6 +102,8 @@  static int ioh3420_initfn(PCIDevice *d)
     PCIESlot *s = DO_UPCAST(PCIESlot, port, p);
     int rc;
 
+    pci_bridge_map_irq(br, NULL, ioh3420_map_irq);
+
     rc = pci_bridge_initfn(d);
     if (rc < 0) {
         return rc;