[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Xen-devel] RE: [PATCH] passthrough: update bus2bridge mapping as PCI devices get added/removed
- To: Jan Beulich <JBeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
- 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
- List-id: Xen developer discussion <xen-devel.lists.xensource.com>
- 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 ---
>>> 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
|
