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.

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
---
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          | 50 +-------------------
 xen/arch/x86/x86_64/physdev.c   |  4 +-
 xen/common/Makefile             |  1 +
 xen/common/physdev.c            | 81 +++++++++++++++++++++++++++++++++
 xen/include/asm-arm/hypercall.h |  2 -
 xen/include/asm-arm/numa.h      |  5 ++
 xen/include/asm-x86/hypercall.h |  9 ++--
 xen/include/xen/hypercall.h     |  8 ++++
 9 files changed, 106 insertions(+), 59 deletions(-)
 create mode 100644 xen/common/physdev.c

diff --git a/xen/arch/arm/physdev.c b/xen/arch/arm/physdev.c
index e91355fe22..4e00b03aab 100644
--- a/xen/arch/arm/physdev.c
+++ b/xen/arch/arm/physdev.c
@@ -8,10 +8,9 @@
 #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)
+long arch_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     gdprintk(XENLOG_DEBUG, "PHYSDEVOP cmd=%d: not implemented\n", cmd);
     return -ENOSYS;
diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
index 23465bcd00..c00cc99404 100644
--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -174,7 +174,7 @@ int physdev_unmap_pirq(domid_t domid, int pirq)
 }
 #endif /* COMPAT */
 
-ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
+ret_t arch_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     int irq;
     ret_t ret;
@@ -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;
diff --git a/xen/arch/x86/x86_64/physdev.c b/xen/arch/x86/x86_64/physdev.c
index 0a50cbd4d8..5f72652ff7 100644
--- a/xen/arch/x86/x86_64/physdev.c
+++ b/xen/arch/x86/x86_64/physdev.c
@@ -9,9 +9,10 @@ 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
+#define arch_physdev_op arch_compat_physdev_op
 
 #define physdev_apic               compat_physdev_apic
 #define physdev_apic_t             physdev_apic_compat_t
@@ -82,6 +83,7 @@ CHECK_physdev_pci_device
 typedef int ret_t;
 
 #include "../physdev.c"
+#include "../../../common/physdev.c"
 
 /*
  * Local variables:
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 54de70d422..bcb1c8fb03 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -29,6 +29,7 @@ obj-y += notifier.o
 obj-y += page_alloc.o
 obj-$(CONFIG_HAS_PDX) += pdx.o
 obj-$(CONFIG_PERF_COUNTERS) += perfc.o
+obj-y += physdev.o
 obj-y += preempt.o
 obj-y += random.o
 obj-y += rangeset.o
diff --git a/xen/common/physdev.c b/xen/common/physdev.c
new file mode 100644
index 0000000000..20a5530269
--- /dev/null
+++ b/xen/common/physdev.c
@@ -0,0 +1,81 @@
+
+#include <xen/guest_access.h>
+#include <xen/hypercall.h>
+#include <xen/init.h>
+
+#ifndef COMPAT
+typedef long ret_t;
+#endif
+
+ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
+{
+    ret_t ret;
+
+    switch ( cmd )
+    {
+#ifdef CONFIG_HAS_PCI
+    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;
+    }
+#endif
+    default:
+        ret = arch_physdev_op(cmd, arg);
+        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/asm-arm/hypercall.h b/xen/include/asm-arm/hypercall.h
index a0c5a31a2f..c6e4a04fd6 100644
--- a/xen/include/asm-arm/hypercall.h
+++ b/xen/include/asm-arm/hypercall.h
@@ -2,8 +2,6 @@
 #define __ASM_ARM_HYPERCALL_H__
 
 #include <public/domctl.h> /* for arch_do_domctl */
-int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
-
 long do_arm_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg);
 
 long subarch_do_domctl(struct xen_domctl *domctl, struct domain *d,
diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h
index 31a6de4e23..dbd1c2467f 100644
--- a/xen/include/asm-arm/numa.h
+++ b/xen/include/asm-arm/numa.h
@@ -25,6 +25,11 @@ extern mfn_t first_valid_mfn;
 #define node_start_pfn(nid) (mfn_x(first_valid_mfn))
 #define __node_distance(a, b) (20)
 
+static inline nodeid_t pxm_to_node(unsigned pxm)
+{
+    return 0xff;
+}
+
 #endif /* __ARCH_ARM_NUMA_H */
 /*
  * Local variables:
diff --git a/xen/include/asm-x86/hypercall.h b/xen/include/asm-x86/hypercall.h
index 0ae3b8b043..7afba98186 100644
--- a/xen/include/asm-x86/hypercall.h
+++ b/xen/include/asm-x86/hypercall.h
@@ -105,10 +105,6 @@ do_update_va_mapping(
     u64 val64,
     unsigned long flags);
 
-extern long
-do_physdev_op(
-    int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
-
 extern long
 do_update_va_mapping_otherdomain(
     unsigned long va,
@@ -151,6 +147,11 @@ compat_physdev_op(
     int cmd,
     XEN_GUEST_HANDLE_PARAM(void) arg);
 
+extern int
+arch_compat_physdev_op(
+    int cmd,
+    XEN_GUEST_HANDLE_PARAM(void) arg);
+
 extern int
 arch_compat_vcpu_op(
     int cmd, struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg);
diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
index 3771487a30..5cc74f40b7 100644
--- a/xen/include/xen/hypercall.h
+++ b/xen/include/xen/hypercall.h
@@ -45,6 +45,14 @@ extern long
 do_platform_op(
     XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op);
 
+extern long
+do_physdev_op(
+    int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
+
+extern long
+arch_physdev_op(
+    int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
+
 /*
  * To allow safe resume of do_memory_op() after preemption, we need to know
  * at what point in the page list to resume. For this purpose I steal the
-- 
2.17.1

On Tue, 28 Sep 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.
> 
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> 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          | 50 +-------------------
>  xen/arch/x86/x86_64/physdev.c   |  4 +-
>  xen/common/Makefile             |  1 +
>  xen/common/physdev.c            | 81 +++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/hypercall.h |  2 -
>  xen/include/asm-arm/numa.h      |  5 ++
>  xen/include/asm-x86/hypercall.h |  9 ++--
>  xen/include/xen/hypercall.h     |  8 ++++
>  9 files changed, 106 insertions(+), 59 deletions(-)
>  create mode 100644 xen/common/physdev.c
> 
> diff --git a/xen/arch/arm/physdev.c b/xen/arch/arm/physdev.c
> index e91355fe22..4e00b03aab 100644
> --- a/xen/arch/arm/physdev.c
> +++ b/xen/arch/arm/physdev.c
> @@ -8,10 +8,9 @@
>  #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)
> +long arch_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>  {
>      gdprintk(XENLOG_DEBUG, "PHYSDEVOP cmd=%d: not implemented\n", cmd);
>      return -ENOSYS;
> diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
> index 23465bcd00..c00cc99404 100644
> --- a/xen/arch/x86/physdev.c
> +++ b/xen/arch/x86/physdev.c
> @@ -174,7 +174,7 @@ int physdev_unmap_pirq(domid_t domid, int pirq)
>  }
>  #endif /* COMPAT */
>  
> -ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> +ret_t arch_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>  {
>      int irq;
>      ret_t ret;
> @@ -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;
> diff --git a/xen/arch/x86/x86_64/physdev.c b/xen/arch/x86/x86_64/physdev.c
> index 0a50cbd4d8..5f72652ff7 100644
> --- a/xen/arch/x86/x86_64/physdev.c
> +++ b/xen/arch/x86/x86_64/physdev.c
> @@ -9,9 +9,10 @@ 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
> +#define arch_physdev_op arch_compat_physdev_op
>  
>  #define physdev_apic               compat_physdev_apic
>  #define physdev_apic_t             physdev_apic_compat_t
> @@ -82,6 +83,7 @@ CHECK_physdev_pci_device
>  typedef int ret_t;
>  
>  #include "../physdev.c"
> +#include "../../../common/physdev.c"
>  
>  /*
>   * Local variables:
> diff --git a/xen/common/Makefile b/xen/common/Makefile
> index 54de70d422..bcb1c8fb03 100644
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -29,6 +29,7 @@ obj-y += notifier.o
>  obj-y += page_alloc.o
>  obj-$(CONFIG_HAS_PDX) += pdx.o
>  obj-$(CONFIG_PERF_COUNTERS) += perfc.o
> +obj-y += physdev.o
>  obj-y += preempt.o
>  obj-y += random.o
>  obj-y += rangeset.o
> diff --git a/xen/common/physdev.c b/xen/common/physdev.c
> new file mode 100644
> index 0000000000..20a5530269
> --- /dev/null
> +++ b/xen/common/physdev.c
> @@ -0,0 +1,81 @@
> +
> +#include <xen/guest_access.h>
> +#include <xen/hypercall.h>
> +#include <xen/init.h>
> +
> +#ifndef COMPAT
> +typedef long ret_t;
> +#endif
> +
> +ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> +{
> +    ret_t ret;
> +
> +    switch ( cmd )
> +    {
> +#ifdef CONFIG_HAS_PCI
> +    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;
> +    }
> +#endif
> +    default:
> +        ret = arch_physdev_op(cmd, arg);
> +        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/asm-arm/hypercall.h b/xen/include/asm-arm/hypercall.h
> index a0c5a31a2f..c6e4a04fd6 100644
> --- a/xen/include/asm-arm/hypercall.h
> +++ b/xen/include/asm-arm/hypercall.h
> @@ -2,8 +2,6 @@
>  #define __ASM_ARM_HYPERCALL_H__
>  
>  #include <public/domctl.h> /* for arch_do_domctl */
> -int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
> -
>  long do_arm_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg);
>  
>  long subarch_do_domctl(struct xen_domctl *domctl, struct domain *d,
> diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h
> index 31a6de4e23..dbd1c2467f 100644
> --- a/xen/include/asm-arm/numa.h
> +++ b/xen/include/asm-arm/numa.h
> @@ -25,6 +25,11 @@ extern mfn_t first_valid_mfn;
>  #define node_start_pfn(nid) (mfn_x(first_valid_mfn))
>  #define __node_distance(a, b) (20)
>  
> +static inline nodeid_t pxm_to_node(unsigned pxm)
> +{
> +    return 0xff;
> +}
> +
>  #endif /* __ARCH_ARM_NUMA_H */
>  /*
>   * Local variables:
> diff --git a/xen/include/asm-x86/hypercall.h b/xen/include/asm-x86/hypercall.h
> index 0ae3b8b043..7afba98186 100644
> --- a/xen/include/asm-x86/hypercall.h
> +++ b/xen/include/asm-x86/hypercall.h
> @@ -105,10 +105,6 @@ do_update_va_mapping(
>      u64 val64,
>      unsigned long flags);
>  
> -extern long
> -do_physdev_op(
> -    int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
> -
>  extern long
>  do_update_va_mapping_otherdomain(
>      unsigned long va,
> @@ -151,6 +147,11 @@ compat_physdev_op(
>      int cmd,
>      XEN_GUEST_HANDLE_PARAM(void) arg);
>  
> +extern int
> +arch_compat_physdev_op(
> +    int cmd,
> +    XEN_GUEST_HANDLE_PARAM(void) arg);
> +
>  extern int
>  arch_compat_vcpu_op(
>      int cmd, struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg);
> diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
> index 3771487a30..5cc74f40b7 100644
> --- a/xen/include/xen/hypercall.h
> +++ b/xen/include/xen/hypercall.h
> @@ -45,6 +45,14 @@ extern long
>  do_platform_op(
>      XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op);
>  
> +extern long
> +do_physdev_op(
> +    int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
> +
> +extern long
> +arch_physdev_op(
> +    int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
> +
>  /*
>   * To allow safe resume of do_memory_op() after preemption, we need to know
>   * at what point in the page list to resume. For this purpose I steal the
> -- 
> 2.17.1
> 

Hi Rahul,

> On 28 Sep 2021, at 19:18, Rahul Singh <rahul.singh@arm.com> 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.
> 
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> ---
> 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          | 50 +-------------------
> xen/arch/x86/x86_64/physdev.c   |  4 +-
> xen/common/Makefile             |  1 +
> xen/common/physdev.c            | 81 +++++++++++++++++++++++++++++++++
> xen/include/asm-arm/hypercall.h |  2 -
> xen/include/asm-arm/numa.h      |  5 ++
> xen/include/asm-x86/hypercall.h |  9 ++--
> xen/include/xen/hypercall.h     |  8 ++++
> 9 files changed, 106 insertions(+), 59 deletions(-)
> create mode 100644 xen/common/physdev.c
> 
> diff --git a/xen/arch/arm/physdev.c b/xen/arch/arm/physdev.c
> index e91355fe22..4e00b03aab 100644
> --- a/xen/arch/arm/physdev.c
> +++ b/xen/arch/arm/physdev.c
> @@ -8,10 +8,9 @@
> #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)
> +long arch_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> {
>     gdprintk(XENLOG_DEBUG, "PHYSDEVOP cmd=%d: not implemented\n", cmd);
>     return -ENOSYS;
> diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
> index 23465bcd00..c00cc99404 100644
> --- a/xen/arch/x86/physdev.c
> +++ b/xen/arch/x86/physdev.c
> @@ -174,7 +174,7 @@ int physdev_unmap_pirq(domid_t domid, int pirq)
> }
> #endif /* COMPAT */
> 
> -ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> +ret_t arch_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> {
>     int irq;
>     ret_t ret;
> @@ -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;
> diff --git a/xen/arch/x86/x86_64/physdev.c b/xen/arch/x86/x86_64/physdev.c
> index 0a50cbd4d8..5f72652ff7 100644
> --- a/xen/arch/x86/x86_64/physdev.c
> +++ b/xen/arch/x86/x86_64/physdev.c
> @@ -9,9 +9,10 @@ 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
> +#define arch_physdev_op arch_compat_physdev_op
> 
> #define physdev_apic               compat_physdev_apic
> #define physdev_apic_t             physdev_apic_compat_t
> @@ -82,6 +83,7 @@ CHECK_physdev_pci_device
> typedef int ret_t;
> 
> #include "../physdev.c"
> +#include "../../../common/physdev.c"
> 
> /*
>  * Local variables:
> diff --git a/xen/common/Makefile b/xen/common/Makefile
> index 54de70d422..bcb1c8fb03 100644
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -29,6 +29,7 @@ obj-y += notifier.o
> obj-y += page_alloc.o
> obj-$(CONFIG_HAS_PDX) += pdx.o
> obj-$(CONFIG_PERF_COUNTERS) += perfc.o
> +obj-y += physdev.o
> obj-y += preempt.o
> obj-y += random.o
> obj-y += rangeset.o
> diff --git a/xen/common/physdev.c b/xen/common/physdev.c
> new file mode 100644
> index 0000000000..20a5530269
> --- /dev/null
> +++ b/xen/common/physdev.c
> @@ -0,0 +1,81 @@
> +
> +#include <xen/guest_access.h>
> +#include <xen/hypercall.h>
> +#include <xen/init.h>
> +
> +#ifndef COMPAT
> +typedef long ret_t;
> +#endif
> +
> +ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> +{
> +    ret_t ret;
> +
> +    switch ( cmd )
> +    {
> +#ifdef CONFIG_HAS_PCI
> +    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;
> +    }
> +#endif
> +    default:
> +        ret = arch_physdev_op(cmd, arg);
> +        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/asm-arm/hypercall.h b/xen/include/asm-arm/hypercall.h
> index a0c5a31a2f..c6e4a04fd6 100644
> --- a/xen/include/asm-arm/hypercall.h
> +++ b/xen/include/asm-arm/hypercall.h
> @@ -2,8 +2,6 @@
> #define __ASM_ARM_HYPERCALL_H__
> 
> #include <public/domctl.h> /* for arch_do_domctl */
> -int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
> -
> long do_arm_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg);
> 
> long subarch_do_domctl(struct xen_domctl *domctl, struct domain *d,
> diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h
> index 31a6de4e23..dbd1c2467f 100644
> --- a/xen/include/asm-arm/numa.h
> +++ b/xen/include/asm-arm/numa.h
> @@ -25,6 +25,11 @@ extern mfn_t first_valid_mfn;
> #define node_start_pfn(nid) (mfn_x(first_valid_mfn))
> #define __node_distance(a, b) (20)
> 
> +static inline nodeid_t pxm_to_node(unsigned pxm)
> +{
> +    return 0xff;
> +}
> +
> #endif /* __ARCH_ARM_NUMA_H */
> /*
>  * Local variables:
> diff --git a/xen/include/asm-x86/hypercall.h b/xen/include/asm-x86/hypercall.h
> index 0ae3b8b043..7afba98186 100644
> --- a/xen/include/asm-x86/hypercall.h
> +++ b/xen/include/asm-x86/hypercall.h
> @@ -105,10 +105,6 @@ do_update_va_mapping(
>     u64 val64,
>     unsigned long flags);
> 
> -extern long
> -do_physdev_op(
> -    int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
> -
> extern long
> do_update_va_mapping_otherdomain(
>     unsigned long va,
> @@ -151,6 +147,11 @@ compat_physdev_op(
>     int cmd,
>     XEN_GUEST_HANDLE_PARAM(void) arg);
> 
> +extern int
> +arch_compat_physdev_op(
> +    int cmd,
> +    XEN_GUEST_HANDLE_PARAM(void) arg);
> +
> extern int
> arch_compat_vcpu_op(
>     int cmd, struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg);
> diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
> index 3771487a30..5cc74f40b7 100644
> --- a/xen/include/xen/hypercall.h
> +++ b/xen/include/xen/hypercall.h
> @@ -45,6 +45,14 @@ extern long
> do_platform_op(
>     XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op);
> 
> +extern long
> +do_physdev_op(
> +    int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
> +
> +extern long
> +arch_physdev_op(
> +    int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
> +
> /*
>  * To allow safe resume of do_memory_op() after preemption, we need to know
>  * at what point in the page list to resume. For this purpose I steal the
> -- 
> 2.17.1
> 

On 28.09.2021 20:18, 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.
> 
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>

On v1 Julien said:

"There are other PHYSDEVOP operations to add PCI devices. I think it is 
 fine to only implement the latest (CC Jan for some opinion and confirm 
 this is the latest). However, this ought to be explained in the commit 
 message."

> @@ -82,6 +83,7 @@ CHECK_physdev_pci_device
>  typedef int ret_t;
>  
>  #include "../physdev.c"
> +#include "../../../common/physdev.c"

Please don't unless entirely unavoidable: common/ has its own
common/compat/.

> --- /dev/null
> +++ b/xen/common/physdev.c
> @@ -0,0 +1,81 @@
> +
> +#include <xen/guest_access.h>
> +#include <xen/hypercall.h>
> +#include <xen/init.h>
> +
> +#ifndef COMPAT
> +typedef long ret_t;
> +#endif
> +
> +ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> +{
> +    ret_t ret;
> +
> +    switch ( cmd )
> +    {
> +#ifdef CONFIG_HAS_PCI

All of the enclosed code should really be under drivers/pci/ or in
drivers/passthrough/pci.c, e.g. in a pci_physdev_op() function
called from both Arm and x86. Unless, I will admit, this would pose
undue problems for x86'es compat handling. But I'd like to know
whether that route was at least explored. (I.e. I'm afraid Julien's
request to move this code to "common" was understood too much to the
word, sorry.)

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

While I'm not going to insist (as you're merely moving this code), it
would be nice if the !!() was dropped here, ...

> +        if ( add.flags & XEN_PCI_DEV_VIRTFN )
> +        {
> +            pdev_info.is_virtfn = 1;

... "true" was used here, and ...

> +            pdev_info.physfn.bus = add.physfn.bus;
> +            pdev_info.physfn.devfn = add.physfn.devfn;
> +        }
> +        else
> +            pdev_info.is_virtfn = 0;

... "false" here while moving, as both fields are bool.

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

I think this code should become CONFIG_NUMA dependent, now that it
gets moved to common code. This would save you from (oddly; see
below) implementing pxm_to_node() on Arm.

> +            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;
> +    }
> +#endif
> +    default:

Blank line between non-fall-through case blocks please.

> --- a/xen/include/asm-arm/numa.h
> +++ b/xen/include/asm-arm/numa.h
> @@ -25,6 +25,11 @@ extern mfn_t first_valid_mfn;
>  #define node_start_pfn(nid) (mfn_x(first_valid_mfn))
>  #define __node_distance(a, b) (20)
>  
> +static inline nodeid_t pxm_to_node(unsigned pxm)
> +{
> +    return 0xff;

If you can use NUMA_NO_NODE in do_physdev_op(), why not also here?
(Assuming this function is going to survive in this series in the
first place, as per the earlier comment.)

Jan

Hi Jan,

> On 30 Sep 2021, at 3:51 pm, Jan Beulich <jbeulich@suse.com> wrote:
>
> On 28.09.2021 20:18, 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.
>>
>> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
>
> On v1 Julien said:
>
> "There are other PHYSDEVOP operations to add PCI devices. I think it is
> fine to only implement the latest (CC Jan for some opinion and confirm
> this is the latest). However, this ought to be explained in the commit
> message."

Ok.I will add that information in commit msg.
>
>> @@ -82,6 +83,7 @@ CHECK_physdev_pci_device
>> typedef int ret_t;
>>
>> #include "../physdev.c"
>> +#include "../../../common/physdev.c"
>
> Please don't unless entirely unavoidable: common/ has its own
> common/compat/.
>
>> --- /dev/null
>> +++ b/xen/common/physdev.c
>> @@ -0,0 +1,81 @@
>> +
>> +#include <xen/guest_access.h>
>> +#include <xen/hypercall.h>
>> +#include <xen/init.h>
>> +
>> +#ifndef COMPAT
>> +typedef long ret_t;
>> +#endif
>> +
>> +ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>> +{
>> +    ret_t ret;
>> +
>> +    switch ( cmd )
>> +    {
>> +#ifdef CONFIG_HAS_PCI
>
> All of the enclosed code should really be under drivers/pci/ or in
> drivers/passthrough/pci.c, e.g. in a pci_physdev_op() function
> called from both Arm and x86. Unless, I will admit, this would pose
> undue problems for x86'es compat handling. But I'd like to know
> whether that route was at least explored. (I.e. I'm afraid Julien's
> request to move this code to "common" was understood too much to the
> word, sorry.)

I tried to move the code to driver/pci/ and I also feel it is better than moving code to common/physdev.c
I attached the patch for early feedback please have a look once.


>
>> +    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);
>
> While I'm not going to insist (as you're merely moving this code), it
> would be nice if the !!() was dropped here, ...

Ack.
>
>> +        if ( add.flags & XEN_PCI_DEV_VIRTFN )
>> +        {
>> +            pdev_info.is_virtfn = 1;
>
> ... "true" was used here, and ...

Ack.
>
>> +            pdev_info.physfn.bus = add.physfn.bus;
>> +            pdev_info.physfn.devfn = add.physfn.devfn;
>> +        }
>> +        else
>> +            pdev_info.is_virtfn = 0;
>
> ... "false" here while moving, as both fields are bool.
Ack.
>
>> +        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
>
> I think this code should become CONFIG_NUMA dependent, now that it
> gets moved to common code. This would save you from (oddly; see
> below) implementing pxm_to_node() on Arm.

Ok.
>
>> +            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;
>> +    }
>> +#endif
>> +    default:
>
> Blank line between non-fall-through case blocks please.
Ack.
>
>> --- a/xen/include/asm-arm/numa.h
>> +++ b/xen/include/asm-arm/numa.h
>> @@ -25,6 +25,11 @@ extern mfn_t first_valid_mfn;
>> #define node_start_pfn(nid) (mfn_x(first_valid_mfn))
>> #define __node_distance(a, b) (20)
>>
>> +static inline nodeid_t pxm_to_node(unsigned pxm)
>> +{
>> +    return 0xff;
>
> If you can use NUMA_NO_NODE in do_physdev_op(), why not also here?
> (Assuming this function is going to survive in this series in the
> first place, as per the earlier comment.)

NUMA_NO_NODE is defined in  "xen/numa.h” but "asm/numa.h" is include in "xen/numa.h”
before defining NUMA_NO_NODE.

I will fix this like we fix for pci.  Move the "asm/numa.h" in "xen/numa.h"  after defining NUMA_NO_NODE


Regards,
Rahul
> Jan
>

From 7a723aebfda7cdb267a3683447a6383950868d11 Mon Sep 17 00:00:00 2001
Message-Id: <7a723aebfda7cdb267a3683447a6383950868d11.1633105061.git.rahul.singh@arm.com>
From: Rahul Singh <rahul.singh@arm.com>
Date: Tue, 27 Jul 2021 18:48:46 +0100
Subject: [PATCH] xen/arm: Add PHYSDEVOP_pci_device_(*add/remove) support for
 ARM

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.

Issue-ID: SCM-3171
Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Change-Id: I3599cef84b02b1393387dbae451a9d91fc460343
---
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/xen/hypercall.h   | 11 +++++
 6 files changed, 97 insertions(+), 54 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/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
 /*
  * To allow safe resume of do_memory_op() after preemption, we need to know
  * at what point in the page list to resume. For this purpose I steal the
-- 
2.25.1