|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v4 03/14] xen/arm: Add PHYSDEVOP_pci_device_(*add/remove) support for ARM
On Mon, 4 Oct 2021, Rahul Singh wrote:
> Hardware domain is in charge of doing the PCI enumeration and will
> discover the PCI devices and then will communicate to XEN via hyper
> call PHYSDEVOP_pci_device_add(..) to add the PCI devices in XEN.
>
> Also implement PHYSDEVOP_pci_device_remove(..) to remove the PCI device.
>
> As most of the code for PHYSDEVOP_pci_device_* is the same between x86
> and ARM, move the code to a common file to avoid duplication.
>
> There are other PHYSDEVOP_pci_device_* operations to add PCI devices.
> Currently implemented PHYSDEVOP_pci_device_remove(..) and
> PHYSDEVOP_pci_device_add(..) only as those are minimum required to
> support PCI passthrough on ARM.
>
> Signed-off-by: Rahul Singh <rahul.singh@xxxxxxx>
> ---
> Change in v4:
> - Move file commom/physdev.c to drivers/pci/physdev.c
> - minor comments.
> Change in v3: Fixed minor comment.
> Change in v2:
> - Add support for PHYSDEVOP_pci_device_remove()
> - Move code to common code
> ---
> ---
> xen/arch/arm/physdev.c | 5 +--
> xen/arch/x86/physdev.c | 52 +----------------------
> xen/arch/x86/x86_64/physdev.c | 2 +-
> xen/drivers/pci/Makefile | 1 +
> xen/drivers/pci/physdev.c | 80 +++++++++++++++++++++++++++++++++++
> xen/include/public/arch-arm.h | 4 +-
> xen/include/xen/hypercall.h | 11 +++++
> 7 files changed, 100 insertions(+), 55 deletions(-)
> create mode 100644 xen/drivers/pci/physdev.c
>
> diff --git a/xen/arch/arm/physdev.c b/xen/arch/arm/physdev.c
> index e91355fe22..d766978629 100644
> --- a/xen/arch/arm/physdev.c
> +++ b/xen/arch/arm/physdev.c
> @@ -8,13 +8,12 @@
> #include <xen/lib.h>
> #include <xen/errno.h>
> #include <xen/sched.h>
> -#include <asm/hypercall.h>
> +#include <xen/hypercall.h>
>
>
> int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> {
> - gdprintk(XENLOG_DEBUG, "PHYSDEVOP cmd=%d: not implemented\n", cmd);
> - return -ENOSYS;
> + return pci_physdev_op(cmd, arg);
> }
>
> /*
> diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
> index 23465bcd00..ea38be8b79 100644
> --- a/xen/arch/x86/physdev.c
> +++ b/xen/arch/x86/physdev.c
> @@ -12,7 +12,7 @@
> #include <asm/io_apic.h>
> #include <asm/msi.h>
> #include <asm/hvm/irq.h>
> -#include <asm/hypercall.h>
> +#include <xen/hypercall.h>
> #include <public/xen.h>
> #include <public/physdev.h>
> #include <xsm/xsm.h>
> @@ -480,54 +480,6 @@ ret_t do_physdev_op(int cmd,
> XEN_GUEST_HANDLE_PARAM(void) arg)
> break;
> }
>
> - case PHYSDEVOP_pci_device_add: {
> - struct physdev_pci_device_add add;
> - struct pci_dev_info pdev_info;
> - nodeid_t node;
> -
> - ret = -EFAULT;
> - if ( copy_from_guest(&add, arg, 1) != 0 )
> - break;
> -
> - pdev_info.is_extfn = !!(add.flags & XEN_PCI_DEV_EXTFN);
> - if ( add.flags & XEN_PCI_DEV_VIRTFN )
> - {
> - pdev_info.is_virtfn = 1;
> - pdev_info.physfn.bus = add.physfn.bus;
> - pdev_info.physfn.devfn = add.physfn.devfn;
> - }
> - else
> - pdev_info.is_virtfn = 0;
> -
> - 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;
> - }
> -
> - case PHYSDEVOP_pci_device_remove: {
> - struct physdev_pci_device dev;
> -
> - ret = -EFAULT;
> - if ( copy_from_guest(&dev, arg, 1) != 0 )
> - break;
> -
> - ret = pci_remove_device(dev.seg, dev.bus, dev.devfn);
> - break;
> - }
> -
> case PHYSDEVOP_prepare_msix:
> case PHYSDEVOP_release_msix: {
> struct physdev_pci_device dev;
> @@ -663,7 +615,7 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void)
> arg)
> }
>
> default:
> - ret = -ENOSYS;
> + ret = pci_physdev_op(cmd, arg);
> break;
> }
>
> diff --git a/xen/arch/x86/x86_64/physdev.c b/xen/arch/x86/x86_64/physdev.c
> index 0a50cbd4d8..e3cbd5ebcb 100644
> --- a/xen/arch/x86/x86_64/physdev.c
> +++ b/xen/arch/x86/x86_64/physdev.c
> @@ -9,7 +9,7 @@ EMIT_FILE;
> #include <compat/xen.h>
> #include <compat/event_channel.h>
> #include <compat/physdev.h>
> -#include <asm/hypercall.h>
> +#include <xen/hypercall.h>
>
> #define do_physdev_op compat_physdev_op
>
> diff --git a/xen/drivers/pci/Makefile b/xen/drivers/pci/Makefile
> index a98035df4c..972c923db0 100644
> --- a/xen/drivers/pci/Makefile
> +++ b/xen/drivers/pci/Makefile
> @@ -1 +1,2 @@
> obj-y += pci.o
> +obj-y += physdev.o
> diff --git a/xen/drivers/pci/physdev.c b/xen/drivers/pci/physdev.c
> new file mode 100644
> index 0000000000..4f3e1a96c0
> --- /dev/null
> +++ b/xen/drivers/pci/physdev.c
> @@ -0,0 +1,80 @@
> +
> +#include <xen/guest_access.h>
> +#include <xen/hypercall.h>
> +#include <xen/init.h>
> +
> +#ifndef COMPAT
> +typedef long ret_t;
> +#endif
> +
> +ret_t pci_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> +{
> + ret_t ret;
> +
> + switch ( cmd )
> + {
> + case PHYSDEVOP_pci_device_add: {
> + struct physdev_pci_device_add add;
> + struct pci_dev_info pdev_info;
> + nodeid_t node = NUMA_NO_NODE;
> +
> + ret = -EFAULT;
> + if ( copy_from_guest(&add, arg, 1) != 0 )
> + break;
> +
> + pdev_info.is_extfn = (add.flags & XEN_PCI_DEV_EXTFN);
> + if ( add.flags & XEN_PCI_DEV_VIRTFN )
> + {
> + pdev_info.is_virtfn = true;
> + pdev_info.physfn.bus = add.physfn.bus;
> + pdev_info.physfn.devfn = add.physfn.devfn;
> + }
> + else
> + pdev_info.is_virtfn = false;
> +
> +#ifdef CONFIG_NUMA
> + 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);
> + }
> +#endif
> +
> + ret = pci_add_device(add.seg, add.bus, add.devfn, &pdev_info, node);
> + break;
> + }
> +
> + case PHYSDEVOP_pci_device_remove: {
> + struct physdev_pci_device dev;
> +
> + ret = -EFAULT;
> + if ( copy_from_guest(&dev, arg, 1) != 0 )
> + break;
> +
> + ret = pci_remove_device(dev.seg, dev.bus, dev.devfn);
> + break;
> + }
> +
> + default:
> + ret = -ENOSYS;
> + break;
> + }
> +
> + return ret;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index 6b5a5f818a..d46c61fca9 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -107,7 +107,9 @@
> * All generic sub-operations
> *
> * HYPERVISOR_physdev_op
> - * No sub-operations are currenty supported
> + * Exactly these sub-operations are supported:
> + * PHYSDEVOP_pci_device_add
> + * PHYSDEVOP_pci_device_remove
> *
> * HYPERVISOR_sysctl
> * All generic sub-operations, with the exception of:
> diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
> index 3771487a30..7096cc4fe4 100644
> --- a/xen/include/xen/hypercall.h
> +++ b/xen/include/xen/hypercall.h
> @@ -45,6 +45,17 @@ extern long
> do_platform_op(
> XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op);
>
> +#ifdef CONFIG_HAS_PCI
> +extern long
> +pci_physdev_op(
> + int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
> +#else
> +static inline long pci_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> +{
> + gdprintk(XENLOG_DEBUG, "PHYSDEVOP cmd=%d: not implemented\n", cmd);
> + return -ENOSYS;
> +}
> +#endif
Everything looks good up until here and you seemed to have addressed
Jan's comments well.
However, for this last change to hypercall.h: hypercall.h doesn't seem
to be the right place to add the static inline stub for the
!CONFIG_HAS_PCI case.
Given that only ARM needs the !CONFIG_HAS_PCI stub, I would add it
directly to xen/arch/arm/physdev.c. Or just add an #ifdef directly
within do_physdev_op in xen/arch/arm/physdev.c.
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |