Re: [PATCH v3 2/7] pci: Do not ignore device's PXM information

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 09/02/15 20:04, Boris Ostrovsky wrote:
> If ACPI provides PXM data for IO devices then dom0 will pass it to
> hypervisor during PHYSDEVOP_pci_device_add call. This information,
> however, is currently ignored.
>
> We will store this information (in the form of nodeID) in pci_dev
> structure so that we can provide it, for example, to the toolstack
> when it adds support (in the following patches) for querying the
> hypervisor about device topology
>
> We will also print it when user requests device information dump.
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

> ---
>  xen/arch/x86/physdev.c        |   23 ++++++++++++++++++++---
>  xen/drivers/passthrough/pci.c |   13 +++++++++----
>  xen/include/public/physdev.h  |    6 ++++++
>  xen/include/xen/pci.h         |    5 ++++-
>  4 files changed, 39 insertions(+), 8 deletions(-)
>
> diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
> index 6b3201b..ab92fa5 100644
> --- a/xen/arch/x86/physdev.c
> +++ b/xen/arch/x86/physdev.c
> @@ -565,7 +565,8 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>          if ( copy_from_guest(&manage_pci, arg, 1) != 0 )
>              break;
>  
> -        ret = pci_add_device(0, manage_pci.bus, manage_pci.devfn, NULL);
> +        ret = pci_add_device(0, manage_pci.bus, manage_pci.devfn,
> +                             NULL, NUMA_NO_NODE);
>          break;
>      }
>  
> @@ -597,13 +598,14 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>          pdev_info.physfn.devfn = manage_pci_ext.physfn.devfn;
>          ret = pci_add_device(0, manage_pci_ext.bus,
>                               manage_pci_ext.devfn,
> -                             &pdev_info);
> +                             &pdev_info, NUMA_NO_NODE);
>          break;
>      }
>  
>      case PHYSDEVOP_pci_device_add: {
>          struct physdev_pci_device_add add;
>          struct pci_dev_info pdev_info;
> +        u8 node;
>  
>          ret = -EFAULT;
>          if ( copy_from_guest(&add, arg, 1) != 0 )
> @@ -618,7 +620,22 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>          }
>          else
>              pdev_info.is_virtfn = 0;
> -        ret = pci_add_device(add.seg, add.bus, add.devfn, &pdev_info);
> +
> +        if ( add.flags & XEN_PCI_DEV_PXM )
> +        {
> +            uint32_t pxm;
> +            size_t optarr_off = offsetof(struct physdev_pci_device_add, optarr) /
> +                                sizeof(add.optarr[0]);
> +
> +            if ( copy_from_guest_offset(&pxm, arg, optarr_off, 1) )
> +                break;
> +
> +            node = pxm_to_node(pxm);
> +        }
> +        else
> +            node = NUMA_NO_NODE;
> +
> +        ret = pci_add_device(add.seg, add.bus, add.devfn, &pdev_info, node);
>          break;
>      }
>  
> diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
> index 78c6977..43b4384 100644
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -568,7 +568,8 @@ static void pci_enable_acs(struct pci_dev *pdev)
>      pci_conf_write16(seg, bus, dev, func, pos + PCI_ACS_CTRL, ctrl);
>  }
>  
> -int pci_add_device(u16 seg, u8 bus, u8 devfn, const struct pci_dev_info *info)
> +int pci_add_device(u16 seg, u8 bus, u8 devfn,
> +                   const struct pci_dev_info *info, u8 node)
>  {
>      struct pci_seg *pseg;
>      struct pci_dev *pdev;
> @@ -586,7 +587,8 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn, const struct pci_dev_info *info)
>          pdev = pci_get_pdev(seg, info->physfn.bus, info->physfn.devfn);
>          spin_unlock(&pcidevs_lock);
>          if ( !pdev )
> -            pci_add_device(seg, info->physfn.bus, info->physfn.devfn, NULL);
> +            pci_add_device(seg, info->physfn.bus, info->physfn.devfn,
> +                           NULL, node);
>          pdev_type = "virtual function";
>      }
>      else
> @@ -609,6 +611,8 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn, const struct pci_dev_info *info)
>      if ( !pdev )
>          goto out;
>  
> +    pdev->node = node;
> +
>      if ( info )
>          pdev->info = *info;
>      else if ( !pdev->vf_rlen[0] )
> @@ -1191,10 +1195,11 @@ static int _dump_pci_devices(struct pci_seg *pseg, void *arg)
>  
>      list_for_each_entry ( pdev, &pseg->alldevs_list, alldevs_list )
>      {
> -        printk("%04x:%02x:%02x.%u - dom %-3d - MSIs < ",
> +        printk("%04x:%02x:%02x.%u - dom %-3d - node %-3d - MSIs < ",
>                 pseg->nr, pdev->bus,
>                 PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn),
> -               pdev->domain ? pdev->domain->domain_id : -1);
> +               pdev->domain ? pdev->domain->domain_id : -1,
> +               (pdev->node != NUMA_NO_NODE) ? pdev->node : -1);
>          list_for_each_entry ( msi, &pdev->msi_list, list )
>                 printk("%d ", msi->irq);
>          printk(">\n");
> diff --git a/xen/include/public/physdev.h b/xen/include/public/physdev.h
> index d547928..309346b 100644
> --- a/xen/include/public/physdev.h
> +++ b/xen/include/public/physdev.h
> @@ -293,6 +293,12 @@ struct physdev_pci_device_add {
>          uint8_t bus;
>          uint8_t devfn;
>      } physfn;
> +
> +    /*
> +     * Optional parameters array.
> +     * First element ([0]) is PXM domain associated with the device (if
> +     * XEN_PCI_DEV_PXM is set)
> +     */
>  #if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
>      uint32_t optarr[];
>  #elif defined(__GNUC__)
> diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
> index 5f295f3..1acab53 100644
> --- a/xen/include/xen/pci.h
> +++ b/xen/include/xen/pci.h
> @@ -56,6 +56,8 @@ struct pci_dev {
>  
>      u8 phantom_stride;
>  
> +    u8 node; /* NUMA node */
> +
>      enum pdev_type {
>          DEV_TYPE_PCI_UNKNOWN,
>          DEV_TYPE_PCIe_ENDPOINT,
> @@ -102,7 +104,8 @@ void setup_hwdom_pci_devices(struct domain *,
>  int pci_release_devices(struct domain *d);
>  int pci_add_segment(u16 seg);
>  const unsigned long *pci_get_ro_map(u16 seg);
> -int pci_add_device(u16 seg, u8 bus, u8 devfn, const struct pci_dev_info *);
> +int pci_add_device(u16 seg, u8 bus, u8 devfn,
> +                   const struct pci_dev_info *, u8 node);
>  int pci_remove_device(u16 seg, u8 bus, u8 devfn);
>  int pci_ro_device(int seg, int bus, int devfn);
>  void arch_pci_ro_device(int seg, int bdf);