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

Re: [Xen-devel] [Xen-unstable] regression in pci passthrough to HVM guests due to commit 568da4f8c43d2e5b614964c6aefd768de3e3af14 "pt-irq fixes and improvements".



Tuesday, August 5, 2014, 10:54:02 AM, you wrote:

>>>> On 05.08.14 at 10:30, <JBeulich@xxxxxxxx> wrote:
>> I'm testing a fix...

> Mind giving this a try?

> Jan

Hi Jan,

That seems to work .. i now get:

(XEN) [2014-08-05 09:25:03.463] io.c:320: d17: unbind: m_gsi=87 g_gsi=16 
dev=00:00.0 intx=0
(XEN) [2014-08-05 09:25:03.463] io.c:404: d17 final unmap: m_irq=87 dev=00:00.0 
intx=0
(XEN) [2014-08-05 09:25:03.463] io.c:320: d17: unbind: m_gsi=86 g_gsi=30 
dev=00:06.0 intx=8
(XEN) [2014-08-05 09:25:03.463] io.c:404: d17 final unmap: m_irq=86 dev=00:06.0 
intx=8
(XEN) [2014-08-05 09:25:03.463] io.c:320: d17: unbind: m_gsi=85 g_gsi=30 
dev=00:06.0 intx=8
(XEN) [2014-08-05 09:25:03.463] io.c:404: d17 final unmap: m_irq=85 dev=00:06.0 
intx=8
(XEN) [2014-08-05 09:25:03.463] io.c:320: d17: unbind: m_gsi=84 g_gsi=30 
dev=00:06.0 intx=8
(XEN) [2014-08-05 09:25:03.463] io.c:404: d17 final unmap: m_irq=84 dev=00:06.0 
intx=8

But 2 questions:

1) the:
   (XEN) [2014-08-05 09:25:03.463] io.c:320: d17: unbind: m_gsi=87 g_gsi=16 
dev=00:00.0 intx=0
   (XEN) [2014-08-05 09:25:03.463] io.c:404: d17 final unmap: m_irq=87 
dev=00:00.0 intx=0 

   That's not the device that is passed through, is that the unbind message you 
   where expecting due to the "pci_msitranslate=1" you were referring to before 
?

2) And the second is about the logging:

   - It doesn't seem to print the binding of the msi irq's of pt-devices, i 
only get 2 
     messages of bindings which seem to be for the emulated xen pci device:
     (XEN) [2014-08-05 09:46:05.355] io.c:280: d20: bind: m_gsi=37 g_gsi=36 
dev=00.00.5 intx=0
     (XEN) [2014-08-05 09:46:06.433] io.c:280: d20: bind: m_gsi=47 g_gsi=40 
dev=00.00.6 intx=0
     but in the guest i don't see any devices with those BDF's, looks like it 
is 
     printing some wrong values ?  

     at least it's not symmetric with what it is saying to unbind:
    (XEN) [2014-08-05 09:47:51.658] io.c:320: d20: unbind: m_gsi=87 g_gsi=16 
dev=00:00.0 intx=0
    (XEN) [2014-08-05 09:47:51.658] io.c:404: d20 final unmap: m_irq=87 
dev=00:00.0 intx=0
    (XEN) [2014-08-05 09:47:51.658] io.c:320: d20: unbind: m_gsi=86 g_gsi=45 
dev=00:02.0 intx=59
    (XEN) [2014-08-05 09:47:51.658] io.c:404: d20 final unmap: m_irq=86 
dev=00:02.0 intx=59
    (XEN) [2014-08-05 09:47:51.658] io.c:320: d20: unbind: m_gsi=85 g_gsi=45 
dev=00:02.0 intx=59
    (XEN) [2014-08-05 09:47:51.658] io.c:404: d20 final unmap: m_irq=85 
dev=00:02.0 intx=59
    (XEN) [2014-08-05 09:47:51.658] io.c:320: d20: unbind: m_gsi=84 g_gsi=45 
dev=00:02.0 intx=59
    (XEN) [2014-08-05 09:47:51.658] io.c:404: d20 final unmap: m_irq=84 
dev=00:02.0 intx=59
    (XEN) [2014-08-05 09:47:51.658] io.c:320: d20: unbind: m_gsi=83 g_gsi=45 
dev=00:02.0 intx=59
    (XEN) [2014-08-05 09:47:51.658] io.c:404: d20 final unmap: m_irq=83 
dev=00:02.0 intx=59

   - There are no mapping/unmapping messages for non-msi irq's ?

Thanks for the fast fix !

--
Sander


> pass-through: fix unbinding of MSI interrupts

> Commit 568da4f8 ("pt-irq fixes and improvements") went a little too far
> in its cleaning up of pt_irq_destroy_bind(): While neither of the two
> lists need any maintenance, the actualunbinding still needs to be done.
> Fix this and at once enhance the final (optional) log message to make
> clear how much cleaning up was actually done.

> Reported-by: Sander Eikelenboom <linux@xxxxxxxxxxxxxx>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

> --- a/xen/drivers/passthrough/io.c
> +++ b/xen/drivers/passthrough/io.c
> @@ -300,17 +300,15 @@ int pt_irq_destroy_bind(
>      unsigned int intx = pt_irq_bind->u.pci.intx;
>      unsigned int guest_gsi = hvm_pci_intx_gsi(device, intx);
>      unsigned int link = hvm_pci_intx_link(device, intx);
> -    struct dev_intx_gsi_link *digl, *tmp;
> -    struct hvm_girq_dpci_mapping *girq;
>      struct pirq *pirq;
> +    const char *what;
>  
>      switch ( pt_irq_bind->irq_type )
>      {
>      case PT_IRQ_TYPE_PCI:
> +    case PT_IRQ_TYPE_MSI:
>      case PT_IRQ_TYPE_MSI_TRANSLATE:
>          break;
> -    case PT_IRQ_TYPE_MSI:
> -        return 0;
>      default:
>          return -EOPNOTSUPP;
>      }
> @@ -331,27 +329,32 @@ int pt_irq_destroy_bind(
>          return -EINVAL;
>      }
>  
> -    list_for_each_entry ( girq, &hvm_irq_dpci->girq[guest_gsi], list )
> +    if ( pt_irq_bind->irq_type != PT_IRQ_TYPE_MSI )
>      {
-        if ( girq->>bus         == bus &&
-             girq->>device      == device &&
-             girq->>intx        == intx &&
-             girq->>machine_gsi == machine_gsi )
> +        struct hvm_girq_dpci_mapping *girq;
> +
> +        list_for_each_entry ( girq, &hvm_irq_dpci->girq[guest_gsi], list )
>          {
> -            list_del(&girq->list);
> -            xfree(girq);
> -            girq = NULL;
> -            break;
> +            if ( girq->bus         == bus &&
> +                 girq->device      == device &&
> +                 girq->intx        == intx &&
> +                 girq->machine_gsi == machine_gsi )
> +            {
> +                list_del(&girq->list);
> +                xfree(girq);
> +                girq = NULL;
> +                break;
> +            }
>          }
> -    }
>  
> -    if ( girq )
> -    {
> -        spin_unlock(&d->event_lock);
> -        return -EINVAL;
> -    }
> +        if ( girq )
> +        {
> +            spin_unlock(&d->event_lock);
> +            return -EINVAL;
> +        }
>  
-    hvm_irq_dpci->>link_cnt[link]--;
> +        hvm_irq_dpci->link_cnt[link]--;
> +    }
>  
>      pirq = pirq_info(d, machine_gsi);
>      pirq_dpci = pirq_dpci(pirq);
> @@ -359,14 +362,19 @@ int pt_irq_destroy_bind(
>      /* clear the mirq info */
>      if ( pirq_dpci && (pirq_dpci->flags & HVM_IRQ_DPCI_MAPPED) )
>      {
> -        list_for_each_entry_safe ( digl, tmp, &pirq_dpci->digl_list, list )
> +        if ( pt_irq_bind->irq_type != PT_IRQ_TYPE_MSI )
>          {
> -            if ( digl->bus    == bus &&
> -                 digl->device == device &&
> -                 digl->intx   == intx )
> +            struct dev_intx_gsi_link *digl, *tmp;
> +
> +            list_for_each_entry_safe ( digl, tmp, &pirq_dpci->digl_list, 
> list )
>              {
> -                list_del(&digl->list);
> -                xfree(digl);
> +                if ( digl->bus    == bus &&
> +                     digl->device == device &&
> +                     digl->intx   == intx )
> +                {
> +                    list_del(&digl->list);
> +                    xfree(digl);
> +                }
>              }
>          }
>  
> @@ -379,14 +387,20 @@ int pt_irq_destroy_bind(
>              pirq_dpci->dom   = NULL;
>              pirq_dpci->flags = 0;
>              pirq_cleanup_check(pirq, d);
> +            what = "final";
>          }
> +        else
> +            what = "partial";
>      }
> +    else
> +        what = "bogus";
> +
>      spin_unlock(&d->event_lock);
>  
>      if ( iommu_verbose )
>          dprintk(XENLOG_G_INFO,
> -                "d%d unmap: m_irq=%u dev=%02x:%02x.%u intx=%u\n",
-                d->>domain_id, machine_gsi, bus,
> +                "d%d %s unmap: m_irq=%u dev=%02x:%02x.%u intx=%u\n",
+                d->>domain_id, what, machine_gsi, bus,
>                  PCI_SLOT(device), PCI_FUNC(device), intx);
>  
>      return 0;




_______________________________________________
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®.