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] passthrough: update bus2bridge mapping as PCI de

To: Jan Beulich <JBeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] RE: [PATCH] passthrough: update bus2bridge mapping as PCI devices get added/removed
From: "Kay, Allen M" <allen.m.kay@xxxxxxxxx>
Date: Thu, 6 Oct 2011 19:00:55 -0700
Accept-language: en-US
Acceptlanguage: en-US
Cc:
Delivery-date: Thu, 06 Oct 2011 19:04:56 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4E8B08A10200007800059278@xxxxxxxxxxxxxxxxxxxx>
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: <4E8B08A10200007800059278@xxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcyCh/IrkHvXVCJUROekhAfvhSwM/ACDG17g
Thread-topic: [PATCH] passthrough: update bus2bridge mapping as PCI devices get added/removed
Hi Jan,

I'm not able to spot the difference between this patch and the earlier one you 
had second thoughts about in attached email.  Was there a change I missed?

Allen

-----Original Message-----
From: Jan Beulich [mailto:JBeulich@xxxxxxxx] 
Sent: Tuesday, October 04, 2011 4:23 AM
To: xen-devel@xxxxxxxxxxxxxxxxxxx
Cc: Kay, Allen M
Subject: [PATCH] passthrough: update bus2bridge mapping as PCI devices get 
added/removed

This deals with two limitations at once: On device removal, the
mapping did not get updated so far at all, and hotplugged devices as
well as such not discoverable by Xen's initial bus scan (including the
case where a non-zero PCI segment wasn't accessible during Xen boot,
but became accessible after Dom0 validated access information against
ACPI data) wouldn't cause updates to the mapping either.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -129,11 +129,67 @@ static struct pci_dev *alloc_pdev(struct
     list_add(&pdev->alldevs_list, &pseg->alldevs_list);
     spin_lock_init(&pdev->msix_table_lock);
 
+    /* update bus2bridge */
+    switch ( pdev_type(pseg->nr, bus, devfn) )
+    {
+        u8 sec_bus, sub_bus;
+
+        case DEV_TYPE_PCIe_BRIDGE:
+            break;
+
+        case DEV_TYPE_PCIe2PCI_BRIDGE:
+        case DEV_TYPE_LEGACY_PCI_BRIDGE:
+            sec_bus = pci_conf_read8(pseg->nr, bus, PCI_SLOT(devfn),
+                                     PCI_FUNC(devfn), PCI_SECONDARY_BUS);
+            sub_bus = pci_conf_read8(pseg->nr, bus, PCI_SLOT(devfn),
+                                     PCI_FUNC(devfn), PCI_SUBORDINATE_BUS);
+
+            spin_lock(&pseg->bus2bridge_lock);
+            for ( ; sec_bus <= sub_bus; sec_bus++ )
+            {
+                pseg->bus2bridge[sec_bus].map = 1;
+                pseg->bus2bridge[sec_bus].bus = bus;
+                pseg->bus2bridge[sec_bus].devfn = devfn;
+            }
+            spin_unlock(&pseg->bus2bridge_lock);
+            break;
+
+        case DEV_TYPE_PCIe_ENDPOINT:
+        case DEV_TYPE_PCI:
+            break;
+
+        default:
+            printk(XENLOG_WARNING "%s: unknown type: %04x:%02x:%02x.%u\n",
+                   __func__, pseg->nr, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
+            break;
+    }
+
     return pdev;
 }
 
-static void free_pdev(struct pci_dev *pdev)
+static void free_pdev(struct pci_seg *pseg, struct pci_dev *pdev)
 {
+    /* update bus2bridge */
+    switch ( pdev_type(pseg->nr, pdev->bus, pdev->devfn) )
+    {
+        u8 dev, func, sec_bus, sub_bus;
+
+        case DEV_TYPE_PCIe2PCI_BRIDGE:
+        case DEV_TYPE_LEGACY_PCI_BRIDGE:
+            dev = PCI_SLOT(pdev->devfn);
+            func = PCI_FUNC(pdev->devfn);
+            sec_bus = pci_conf_read8(pseg->nr, pdev->bus, dev, func,
+                                     PCI_SECONDARY_BUS);
+            sub_bus = pci_conf_read8(pseg->nr, pdev->bus, dev, func,
+                                     PCI_SUBORDINATE_BUS);
+
+            spin_lock(&pseg->bus2bridge_lock);
+            for ( ; sec_bus <= sub_bus; sec_bus++ )
+                pseg->bus2bridge[sec_bus] = pseg->bus2bridge[pdev->bus];
+            spin_unlock(&pseg->bus2bridge_lock);
+            break;
+    }
+
     list_del(&pdev->alldevs_list);
     xfree(pdev);
 }
@@ -377,7 +433,7 @@ int pci_remove_device(u16 seg, u8 bus, u
             if ( pdev->domain )
                 list_del(&pdev->domain_list);
             pci_cleanup_msi(pdev);
-            free_pdev(pdev);
+            free_pdev(pseg, pdev);
             printk(XENLOG_DEBUG "PCI remove device %04x:%02x:%02x.%u\n",
                    seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
             break;
@@ -545,8 +601,6 @@ static int __init _scan_pci_devices(stru
 {
     struct pci_dev *pdev;
     int bus, dev, func;
-    u8 sec_bus, sub_bus;
-    int type;
 
     for ( bus = 0; bus < 256; bus++ )
     {
@@ -564,41 +618,6 @@ static int __init _scan_pci_devices(stru
                     return -ENOMEM;
                 }
 
-                /* build bus2bridge */
-                type = pdev_type(pseg->nr, bus, PCI_DEVFN(dev, func));
-                switch ( type )
-                {
-                    case DEV_TYPE_PCIe_BRIDGE:
-                        break;
-
-                    case DEV_TYPE_PCIe2PCI_BRIDGE:
-                    case DEV_TYPE_LEGACY_PCI_BRIDGE:
-                        sec_bus = pci_conf_read8(pseg->nr, bus, dev, func,
-                                                 PCI_SECONDARY_BUS);
-                        sub_bus = pci_conf_read8(pseg->nr, bus, dev, func,
-                                                 PCI_SUBORDINATE_BUS);
-
-                        spin_lock(&pseg->bus2bridge_lock);
-                        for ( sub_bus &= 0xff; sec_bus <= sub_bus; sec_bus++ )
-                        {
-                            pseg->bus2bridge[sec_bus].map = 1;
-                            pseg->bus2bridge[sec_bus].bus = bus;
-                            pseg->bus2bridge[sec_bus].devfn =
-                                PCI_DEVFN(dev, func);
-                        }
-                        spin_unlock(&pseg->bus2bridge_lock);
-                        break;
-
-                    case DEV_TYPE_PCIe_ENDPOINT:
-                    case DEV_TYPE_PCI:
-                        break;
-
-                    default:
-                        printk("%s: unknown type: %04x:%02x:%02x.%u\n",
-                               __func__, pseg->nr, bus, dev, func);
-                        return -EINVAL;
-                }
-
                 if ( !func && !(pci_conf_read8(pseg->nr, bus, dev, func,
                                                PCI_HEADER_TYPE) & 0x80) )
                     break;


--- Begin Message ---
To: "Kay, Allen M" <allen.m.kay@xxxxxxxxx>
Subject: RE: [Xen-devel] RE: Resend: RE: enable_ats_device() call site
From: Jan Beulich <JBeulich@xxxxxxxx>
Date: Thu, 22 Sep 2011 01:40:24 -0700
Cc: Xen Devel <xen-devel@xxxxxxxxxxxxxxxxxxx>
In-reply-to: <987664A83D2D224EAE907B061CE93D5301EE256A2C@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
References: <4E79B56C0200007800056F8E@xxxxxxxxxxxxxxxxxxxx> <987664A83D2D224EAE907B061CE93D5301EE256A2C@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
Thread-index: Acx5AziVrlwBOCuGQNmG/OCZ1lEt9g==
Thread-topic: [Xen-devel] RE: Resend: RE: enable_ats_device() call site
>>> On 21.09.11 at 22:32, "Kay, Allen M" <allen.m.kay@xxxxxxxxx> wrote:
> Looks good to me.

On a second thought I'm not sure about the free_pdev() part anymore:
When having a bridge behind a bridge, and the downstream one gets
removed, wouldn't that mean the upstream one would now be covering
the bus range that got orphaned? Which means rather than flushing
.map we'd have to copy our parent's entry.

Jan

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Wednesday, September 21, 2011 12:59 AM
> To: Kay, Allen M
> Cc: Xen Devel
> Subject: RE: [Xen-devel] RE: Resend: RE: enable_ats_device() call site
>
>>>> On 20.09.11 at 20:02, "Kay, Allen M" <allen.m.kay@xxxxxxxxx> wrote:
>> I agree that the same problem exists for bus-to-bridge mapping function
>> pci_scan_device().  By the way, we probably should take the opportunity to
>> give it a proper name that reflects to the true purpose of this function.
>
> How about the below (applying only on top of the multi-seg patches,
> and not yet removing the scanning functions)?
>
> Jan
>
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -129,11 +129,67 @@ static struct pci_dev *alloc_pdev(struct
>      list_add(&pdev->alldevs_list, &pseg->alldevs_list);
>      spin_lock_init(&pdev->msix_table_lock);
>
> +    /* update bus2bridge */
> +    switch ( pdev_type(pseg->nr, bus, devfn) )
> +    {
> +        u8 sec_bus, sub_bus;
> +
> +        case DEV_TYPE_PCIe_BRIDGE:
> +            break;
> +
> +        case DEV_TYPE_PCIe2PCI_BRIDGE:
> +        case DEV_TYPE_LEGACY_PCI_BRIDGE:
> +            sec_bus = pci_conf_read8(pseg->nr, bus, PCI_SLOT(devfn),
> +                                     PCI_FUNC(devfn), PCI_SECONDARY_BUS);
> +            sub_bus = pci_conf_read8(pseg->nr, bus, PCI_SLOT(devfn),
> +                                     PCI_FUNC(devfn), PCI_SUBORDINATE_BUS);
> +
> +            spin_lock(&pseg->bus2bridge_lock);
> +            for ( ; sec_bus <= sub_bus; sec_bus++ )
> +            {
> +                pseg->bus2bridge[sec_bus].map = 1;
> +                pseg->bus2bridge[sec_bus].bus = bus;
> +                pseg->bus2bridge[sec_bus].devfn = devfn;
> +            }
> +            spin_unlock(&pseg->bus2bridge_lock);
> +            break;
> +
> +        case DEV_TYPE_PCIe_ENDPOINT:
> +        case DEV_TYPE_PCI:
> +            break;
> +
> +        default:
> +            printk(XENLOG_WARNING "%s: unknown type: %04x:%02x:%02x.%u\n",
> +                   __func__, pseg->nr, bus, PCI_SLOT(devfn),
> PCI_FUNC(devfn));
> +            break;
> +    }
> +
>      return pdev;
>  }
>
> -static void free_pdev(struct pci_dev *pdev)
> +static void free_pdev(struct pci_seg *pseg, struct pci_dev *pdev)
>  {
> +    /* update bus2bridge */
> +    switch ( pdev_type(pseg->nr, pdev->bus, pdev->devfn) )
> +    {
> +        u8 dev, func, sec_bus, sub_bus;
> +
> +        case DEV_TYPE_PCIe2PCI_BRIDGE:
> +        case DEV_TYPE_LEGACY_PCI_BRIDGE:
> +            dev = PCI_SLOT(pdev->devfn);
> +            func = PCI_FUNC(pdev->devfn);
> +            sec_bus = pci_conf_read8(pseg->nr, pdev->bus, dev, func,
> +                                     PCI_SECONDARY_BUS);
> +            sub_bus = pci_conf_read8(pseg->nr, pdev->bus, dev, func,
> +                                     PCI_SUBORDINATE_BUS);
> +
> +            spin_lock(&pseg->bus2bridge_lock);
> +            for ( ; sec_bus <= sub_bus; sec_bus++ )
> +                pseg->bus2bridge[sec_bus].map = 0;
> +            spin_unlock(&pseg->bus2bridge_lock);
> +            break;
> +    }
> +
>      list_del(&pdev->alldevs_list);
>      xfree(pdev);
>  }
> @@ -378,7 +434,7 @@ int pci_remove_device(u16 seg, u8 bus, u
>              if ( pdev->domain )
>                  list_del(&pdev->domain_list);
>              pci_cleanup_msi(pdev);
> -            free_pdev(pdev);
> +            free_pdev(pseg, pdev);
>              printk(XENLOG_DEBUG "PCI remove device %04x:%02x:%02x.%u\n",
>                     seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
>              break;
> @@ -546,8 +602,6 @@ static int __init _scan_pci_devices(stru
>  {
>      struct pci_dev *pdev;
>      int bus, dev, func;
> -    u8 sec_bus, sub_bus;
> -    int type;
>
>      for ( bus = 0; bus < 256; bus++ )
>      {
> @@ -565,41 +619,6 @@ static int __init _scan_pci_devices(stru
>                      return -ENOMEM;
>                  }
>
> -                /* build bus2bridge */
> -                type = pdev_type(pseg->nr, bus, PCI_DEVFN(dev, func));
> -                switch ( type )
> -                {
> -                    case DEV_TYPE_PCIe_BRIDGE:
> -                        break;
> -
> -                    case DEV_TYPE_PCIe2PCI_BRIDGE:
> -                    case DEV_TYPE_LEGACY_PCI_BRIDGE:
> -                        sec_bus = pci_conf_read8(pseg->nr, bus, dev, func,
> -                                                 PCI_SECONDARY_BUS);
> -                        sub_bus = pci_conf_read8(pseg->nr, bus, dev, func,
> -                                                 PCI_SUBORDINATE_BUS);
> -
> -                        spin_lock(&pseg->bus2bridge_lock);
> -                        for ( sub_bus &= 0xff; sec_bus <= sub_bus; sec_bus++ 
> )
> -                        {
> -                            pseg->bus2bridge[sec_bus].map = 1;
> -                            pseg->bus2bridge[sec_bus].bus = bus;
> -                            pseg->bus2bridge[sec_bus].devfn =
> -                                PCI_DEVFN(dev, func);
> -                        }
> -                        spin_unlock(&pseg->bus2bridge_lock);
> -                        break;
> -
> -                    case DEV_TYPE_PCIe_ENDPOINT:
> -                    case DEV_TYPE_PCI:
> -                        break;
> -
> -                    default:
> -                        printk("%s: unknown type: %04x:%02x:%02x.%u\n",
> -                               __func__, pseg->nr, bus, dev, func);
> -                        return -EINVAL;
> -                }
> -
>                  if ( !func && !(pci_conf_read8(pseg->nr, bus, dev, func,
>                                                 PCI_HEADER_TYPE) & 0x80) )
>                      break;



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