Re: [PATCH 1/4] pci: Do not ignore device's PXM information

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

On 02/12/14 21:34, 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 should remember it (in the form of nodeID). We will also print it
> when user requests device information dump.
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> ---
>  xen/arch/x86/physdev.c        | 20 +++++++++++++++++---
>  xen/drivers/passthrough/pci.c | 13 +++++++++----
>  xen/include/xen/pci.h         |  5 ++++-
>  3 files changed, 30 insertions(+), 8 deletions(-)
>
> diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
> index 6b3201b..7775f80 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;
> +        int node;
>  
>          ret = -EFAULT;
>          if ( copy_from_guest(&add, arg, 1) != 0 )
> @@ -618,7 +620,19 @@ 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 ) {
> +            int optarr_off = offsetof(struct physdev_pci_device_add, optarr) /
> +                             sizeof(add.optarr[0]);
> +
> +            if ( copy_from_guest_offset(&add.optarr[0], arg, optarr_off, 1) )
> +                break;

This will clobber the hypervisor stack, attempting to put the PXM
information into what is probably pdev_info.

How is one expected to use XEN_PCI_DEV_PXM ? There is currently no
specification of how to use the variable length structure.

~Andrew