Add cmdline boot option "pci-passthrough = = <boolean>" to enable or
disable the PCI passthrough support on ARM.

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
---
Change in v3:
- Remove "define pci_passthrough_enabled (false)"
- Fixed coding style and minor comment
Change in v2:
- Add option in xen-command-line.pandoc
- Change pci option to pci-passthrough
- modify option from custom_param to boolean param
---
 docs/misc/xen-command-line.pandoc |  7 +++++++
 xen/arch/arm/pci/pci.c            | 14 ++++++++++++++
 xen/common/physdev.c              |  6 ++++++
 xen/include/asm-arm/pci.h         | 11 +++++++++++
 xen/include/asm-x86/pci.h         |  8 ++++++++
 5 files changed, 46 insertions(+)

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index 177e656f12..c8bf96ccf8 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -1808,6 +1808,13 @@ All numbers specified must be hexadecimal ones.
 
 This option can be specified more than once (up to 8 times at present).
 
+### pci-passthrough (arm)
+> `= <boolean>`
+
+> Default: `false`
+
+Flag to enable or disable support for PCI passthrough
+
 ### pcid (x86)
 > `= <boolean> | xpti=<bool>`
 
diff --git a/xen/arch/arm/pci/pci.c b/xen/arch/arm/pci/pci.c
index e359bab9ea..84d8f0d634 100644
--- a/xen/arch/arm/pci/pci.c
+++ b/xen/arch/arm/pci/pci.c
@@ -16,6 +16,7 @@
 #include <xen/device_tree.h>
 #include <xen/errno.h>
 #include <xen/init.h>
+#include <xen/param.h>
 #include <xen/pci.h>
 
 /*
@@ -62,8 +63,21 @@ static int __init acpi_pci_init(void)
 }
 #endif
 
+/*
+ * By default pci passthrough is disabled
+ */
+bool __read_mostly pci_passthrough_enabled = false;
+boolean_param("pci-passthrough", pci_passthrough_enabled);
+
 static int __init pci_init(void)
 {
+    /*
+     * Enable PCI passthrough when has been enabled explicitly
+     * (pci-passthrough=on)
+     */
+    if ( !pci_passthrough_enabled )
+        return 0;
+
     pci_segments_init();
 
     if ( acpi_disabled )
diff --git a/xen/common/physdev.c b/xen/common/physdev.c
index 20a5530269..2d5fc886fc 100644
--- a/xen/common/physdev.c
+++ b/xen/common/physdev.c
@@ -19,6 +19,9 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         struct pci_dev_info pdev_info;
         nodeid_t node;
 
+        if ( !is_pci_passthrough_enabled() )
+            return -ENOSYS;
+
         ret = -EFAULT;
         if ( copy_from_guest(&add, arg, 1) != 0 )
             break;
@@ -54,6 +57,9 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
     case PHYSDEVOP_pci_device_remove: {
         struct physdev_pci_device dev;
 
+        if ( !is_pci_passthrough_enabled() )
+            return -ENOSYS;
+
         ret = -EFAULT;
         if ( copy_from_guest(&dev, arg, 1) != 0 )
             break;
diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h
index 7dd9eb3dba..0cf849e26f 100644
--- a/xen/include/asm-arm/pci.h
+++ b/xen/include/asm-arm/pci.h
@@ -19,14 +19,25 @@
 
 #define pci_to_dev(pcidev) (&(pcidev)->arch.dev)
 
+extern bool_t pci_passthrough_enabled;
+
 /* Arch pci dev struct */
 struct arch_pci_dev {
     struct device dev;
 };
 
+static always_inline bool is_pci_passthrough_enabled(void)
+{
+    return pci_passthrough_enabled;
+}
 #else   /*!CONFIG_HAS_PCI*/
 
 struct arch_pci_dev { };
 
+static always_inline bool is_pci_passthrough_enabled(void)
+{
+    return false;
+}
+
 #endif  /*!CONFIG_HAS_PCI*/
 #endif /* __ARM_PCI_H__ */
diff --git a/xen/include/asm-x86/pci.h b/xen/include/asm-x86/pci.h
index cc05045e9c..3f806ce7a8 100644
--- a/xen/include/asm-x86/pci.h
+++ b/xen/include/asm-x86/pci.h
@@ -32,4 +32,12 @@ bool_t pci_ro_mmcfg_decode(unsigned long mfn, unsigned int *seg,
 extern int pci_mmcfg_config_num;
 extern struct acpi_mcfg_allocation *pci_mmcfg_config;
 
+/*
+ * Unlike ARM, PCI passthrough is always enabled for x86.
+ */
+static always_inline bool is_pci_passthrough_enabled(void)
+{
+    return true;
+}
+
 #endif /* __X86_PCI_H__ */
-- 
2.17.1

On Tue, 28 Sep 2021, Rahul Singh wrote:
> Add cmdline boot option "pci-passthrough = = <boolean>" to enable or
> disable the PCI passthrough support on ARM.
> 
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>

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


> ---
> Change in v3:
> - Remove "define pci_passthrough_enabled (false)"
> - Fixed coding style and minor comment
> Change in v2:
> - Add option in xen-command-line.pandoc
> - Change pci option to pci-passthrough
> - modify option from custom_param to boolean param
> ---
>  docs/misc/xen-command-line.pandoc |  7 +++++++
>  xen/arch/arm/pci/pci.c            | 14 ++++++++++++++
>  xen/common/physdev.c              |  6 ++++++
>  xen/include/asm-arm/pci.h         | 11 +++++++++++
>  xen/include/asm-x86/pci.h         |  8 ++++++++
>  5 files changed, 46 insertions(+)
> 
> diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
> index 177e656f12..c8bf96ccf8 100644
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -1808,6 +1808,13 @@ All numbers specified must be hexadecimal ones.
>  
>  This option can be specified more than once (up to 8 times at present).
>  
> +### pci-passthrough (arm)
> +> `= <boolean>`
> +
> +> Default: `false`
> +
> +Flag to enable or disable support for PCI passthrough
> +
>  ### pcid (x86)
>  > `= <boolean> | xpti=<bool>`
>  
> diff --git a/xen/arch/arm/pci/pci.c b/xen/arch/arm/pci/pci.c
> index e359bab9ea..84d8f0d634 100644
> --- a/xen/arch/arm/pci/pci.c
> +++ b/xen/arch/arm/pci/pci.c
> @@ -16,6 +16,7 @@
>  #include <xen/device_tree.h>
>  #include <xen/errno.h>
>  #include <xen/init.h>
> +#include <xen/param.h>
>  #include <xen/pci.h>
>  
>  /*
> @@ -62,8 +63,21 @@ static int __init acpi_pci_init(void)
>  }
>  #endif
>  
> +/*
> + * By default pci passthrough is disabled
> + */
> +bool __read_mostly pci_passthrough_enabled = false;
> +boolean_param("pci-passthrough", pci_passthrough_enabled);
> +
>  static int __init pci_init(void)
>  {
> +    /*
> +     * Enable PCI passthrough when has been enabled explicitly
> +     * (pci-passthrough=on)
> +     */
> +    if ( !pci_passthrough_enabled )
> +        return 0;
> +
>      pci_segments_init();
>  
>      if ( acpi_disabled )
> diff --git a/xen/common/physdev.c b/xen/common/physdev.c
> index 20a5530269..2d5fc886fc 100644
> --- a/xen/common/physdev.c
> +++ b/xen/common/physdev.c
> @@ -19,6 +19,9 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>          struct pci_dev_info pdev_info;
>          nodeid_t node;
>  
> +        if ( !is_pci_passthrough_enabled() )
> +            return -ENOSYS;
> +
>          ret = -EFAULT;
>          if ( copy_from_guest(&add, arg, 1) != 0 )
>              break;
> @@ -54,6 +57,9 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>      case PHYSDEVOP_pci_device_remove: {
>          struct physdev_pci_device dev;
>  
> +        if ( !is_pci_passthrough_enabled() )
> +            return -ENOSYS;
> +
>          ret = -EFAULT;
>          if ( copy_from_guest(&dev, arg, 1) != 0 )
>              break;
> diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h
> index 7dd9eb3dba..0cf849e26f 100644
> --- a/xen/include/asm-arm/pci.h
> +++ b/xen/include/asm-arm/pci.h
> @@ -19,14 +19,25 @@
>  
>  #define pci_to_dev(pcidev) (&(pcidev)->arch.dev)
>  
> +extern bool_t pci_passthrough_enabled;
> +
>  /* Arch pci dev struct */
>  struct arch_pci_dev {
>      struct device dev;
>  };
>  
> +static always_inline bool is_pci_passthrough_enabled(void)
> +{
> +    return pci_passthrough_enabled;
> +}
>  #else   /*!CONFIG_HAS_PCI*/
>  
>  struct arch_pci_dev { };
>  
> +static always_inline bool is_pci_passthrough_enabled(void)
> +{
> +    return false;
> +}
> +
>  #endif  /*!CONFIG_HAS_PCI*/
>  #endif /* __ARM_PCI_H__ */
> diff --git a/xen/include/asm-x86/pci.h b/xen/include/asm-x86/pci.h
> index cc05045e9c..3f806ce7a8 100644
> --- a/xen/include/asm-x86/pci.h
> +++ b/xen/include/asm-x86/pci.h
> @@ -32,4 +32,12 @@ bool_t pci_ro_mmcfg_decode(unsigned long mfn, unsigned int *seg,
>  extern int pci_mmcfg_config_num;
>  extern struct acpi_mcfg_allocation *pci_mmcfg_config;
>  
> +/*
> + * Unlike ARM, PCI passthrough is always enabled for x86.
> + */
> +static always_inline bool is_pci_passthrough_enabled(void)
> +{
> +    return true;
> +}
> +
>  #endif /* __X86_PCI_H__ */
> -- 
> 2.17.1
> 

Hi Rahul,

> On 28 Sep 2021, at 19:18, Rahul Singh <rahul.singh@arm.com> wrote:
> 
> Add cmdline boot option "pci-passthrough = = <boolean>" to enable or
> disable the PCI passthrough support on ARM.
> 
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> ---
> Change in v3:
> - Remove "define pci_passthrough_enabled (false)"
> - Fixed coding style and minor comment
> Change in v2:
> - Add option in xen-command-line.pandoc
> - Change pci option to pci-passthrough
> - modify option from custom_param to boolean param
> ---
> docs/misc/xen-command-line.pandoc |  7 +++++++
> xen/arch/arm/pci/pci.c            | 14 ++++++++++++++
> xen/common/physdev.c              |  6 ++++++
> xen/include/asm-arm/pci.h         | 11 +++++++++++
> xen/include/asm-x86/pci.h         |  8 ++++++++
> 5 files changed, 46 insertions(+)
> 
> diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
> index 177e656f12..c8bf96ccf8 100644
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -1808,6 +1808,13 @@ All numbers specified must be hexadecimal ones.
> 
> This option can be specified more than once (up to 8 times at present).
> 
> +### pci-passthrough (arm)
> +> `= <boolean>`
> +
> +> Default: `false`
> +
> +Flag to enable or disable support for PCI passthrough
> +
> ### pcid (x86)
>> `= <boolean> | xpti=<bool>`
> 
> diff --git a/xen/arch/arm/pci/pci.c b/xen/arch/arm/pci/pci.c
> index e359bab9ea..84d8f0d634 100644
> --- a/xen/arch/arm/pci/pci.c
> +++ b/xen/arch/arm/pci/pci.c
> @@ -16,6 +16,7 @@
> #include <xen/device_tree.h>
> #include <xen/errno.h>
> #include <xen/init.h>
> +#include <xen/param.h>
> #include <xen/pci.h>
> 
> /*
> @@ -62,8 +63,21 @@ static int __init acpi_pci_init(void)
> }
> #endif
> 
> +/*
> + * By default pci passthrough is disabled
> + */
> +bool __read_mostly pci_passthrough_enabled = false;
> +boolean_param("pci-passthrough", pci_passthrough_enabled);
> +
> static int __init pci_init(void)
> {
> +    /*
> +     * Enable PCI passthrough when has been enabled explicitly
> +     * (pci-passthrough=on)
> +     */
> +    if ( !pci_passthrough_enabled )
> +        return 0;
> +
>     pci_segments_init();
> 
>     if ( acpi_disabled )
> diff --git a/xen/common/physdev.c b/xen/common/physdev.c
> index 20a5530269..2d5fc886fc 100644
> --- a/xen/common/physdev.c
> +++ b/xen/common/physdev.c
> @@ -19,6 +19,9 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>         struct pci_dev_info pdev_info;
>         nodeid_t node;
> 
> +        if ( !is_pci_passthrough_enabled() )
> +            return -ENOSYS;
> +
>         ret = -EFAULT;
>         if ( copy_from_guest(&add, arg, 1) != 0 )
>             break;
> @@ -54,6 +57,9 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>     case PHYSDEVOP_pci_device_remove: {
>         struct physdev_pci_device dev;
> 
> +        if ( !is_pci_passthrough_enabled() )
> +            return -ENOSYS;
> +
>         ret = -EFAULT;
>         if ( copy_from_guest(&dev, arg, 1) != 0 )
>             break;
> diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h
> index 7dd9eb3dba..0cf849e26f 100644
> --- a/xen/include/asm-arm/pci.h
> +++ b/xen/include/asm-arm/pci.h
> @@ -19,14 +19,25 @@
> 
> #define pci_to_dev(pcidev) (&(pcidev)->arch.dev)
> 
> +extern bool_t pci_passthrough_enabled;
> +
> /* Arch pci dev struct */
> struct arch_pci_dev {
>     struct device dev;
> };
> 
> +static always_inline bool is_pci_passthrough_enabled(void)
> +{
> +    return pci_passthrough_enabled;
> +}
> #else   /*!CONFIG_HAS_PCI*/
> 
> struct arch_pci_dev { };
> 
> +static always_inline bool is_pci_passthrough_enabled(void)
> +{
> +    return false;
> +}
> +
> #endif  /*!CONFIG_HAS_PCI*/
> #endif /* __ARM_PCI_H__ */
> diff --git a/xen/include/asm-x86/pci.h b/xen/include/asm-x86/pci.h
> index cc05045e9c..3f806ce7a8 100644
> --- a/xen/include/asm-x86/pci.h
> +++ b/xen/include/asm-x86/pci.h
> @@ -32,4 +32,12 @@ bool_t pci_ro_mmcfg_decode(unsigned long mfn, unsigned int *seg,
> extern int pci_mmcfg_config_num;
> extern struct acpi_mcfg_allocation *pci_mmcfg_config;
> 
> +/*
> + * Unlike ARM, PCI passthrough is always enabled for x86.
> + */
> +static always_inline bool is_pci_passthrough_enabled(void)
> +{
> +    return true;
> +}
> +
> #endif /* __X86_PCI_H__ */
> -- 
> 2.17.1
> 

On 28.09.2021 20:18, Rahul Singh wrote:
> @@ -62,8 +63,21 @@ static int __init acpi_pci_init(void)
>  }
>  #endif
>  
> +/*
> + * By default pci passthrough is disabled
> + */

Nit: This is a single line comment.

> +bool __read_mostly pci_passthrough_enabled = false;

Nit: Unnecessary initializer.

> --- a/xen/common/physdev.c
> +++ b/xen/common/physdev.c
> @@ -19,6 +19,9 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>          struct pci_dev_info pdev_info;
>          nodeid_t node;
>  
> +        if ( !is_pci_passthrough_enabled() )
> +            return -ENOSYS;

Here and ...

> @@ -54,6 +57,9 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>      case PHYSDEVOP_pci_device_remove: {
>          struct physdev_pci_device dev;
>  
> +        if ( !is_pci_passthrough_enabled() )
> +            return -ENOSYS;

... here -EOPNOTSUPP (or any other suitable value, which -ENOSYS is not
from all I can tell) please.

> --- a/xen/include/asm-x86/pci.h
> +++ b/xen/include/asm-x86/pci.h
> @@ -32,4 +32,12 @@ bool_t pci_ro_mmcfg_decode(unsigned long mfn, unsigned int *seg,
>  extern int pci_mmcfg_config_num;
>  extern struct acpi_mcfg_allocation *pci_mmcfg_config;
>  
> +/*
> + * Unlike ARM, PCI passthrough is always enabled for x86.
> + */

Nit: This again is a single line comment.

Jan

Hi Jan,

> On 30 Sep 2021, at 4:00 pm, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 28.09.2021 20:18, Rahul Singh wrote:
>> @@ -62,8 +63,21 @@ static int __init acpi_pci_init(void)
>> }
>> #endif
>> 
>> +/*
>> + * By default pci passthrough is disabled
>> + */
> 
> Nit: This is a single line comment.
Ack.
> 
>> +bool __read_mostly pci_passthrough_enabled = false;
> 
> Nit: Unnecessary initializer.

Ack. 
> 
>> --- a/xen/common/physdev.c
>> +++ b/xen/common/physdev.c
>> @@ -19,6 +19,9 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>         struct pci_dev_info pdev_info;
>>         nodeid_t node;
>> 
>> +        if ( !is_pci_passthrough_enabled() )
>> +            return -ENOSYS;
> 
> Here and ...
> 
>> @@ -54,6 +57,9 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>     case PHYSDEVOP_pci_device_remove: {
>>         struct physdev_pci_device dev;
>> 
>> +        if ( !is_pci_passthrough_enabled() )
>> +            return -ENOSYS;
> 
> ... here -EOPNOTSUPP (or any other suitable value, which -ENOSYS is not
> from all I can tell) please.

I checked all the possible errors and came to the conclusion that
 -EOPNOTSUPP is the right choice here.

> 
>> --- a/xen/include/asm-x86/pci.h
>> +++ b/xen/include/asm-x86/pci.h
>> @@ -32,4 +32,12 @@ bool_t pci_ro_mmcfg_decode(unsigned long mfn, unsigned int *seg,
>> extern int pci_mmcfg_config_num;
>> extern struct acpi_mcfg_allocation *pci_mmcfg_config;
>> 
>> +/*
>> + * Unlike ARM, PCI passthrough is always enabled for x86.
>> + */
> 
> Nit: This again is a single line comment.

Ack.

Regards,
Rahul
> 
> Jan
>