RE: [Xen-devel] RE: Resend: RE: enable_ats_device() call site

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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;



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