Re: [PATCH v2 10/17] xen/arm: Add cmdline boot option "pci-passthrough = <boolean>"

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 22 Sep 2021, Rahul Singh wrote:
> Add cmdline boot option "pci-passthrough = = <boolean>" to enable
> disable the PCI passthrough support on ARM.
 ^ or disable

> 
> Signed-off-by: Rahul Singh <rahul.singh@xxxxxxx>
> ---
> 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         | 13 +++++++++++++
>  xen/include/asm-x86/pci.h         |  8 ++++++++
>  5 files changed, 48 insertions(+)
> 
> diff --git a/docs/misc/xen-command-line.pandoc 
> b/docs/misc/xen-command-line.pandoc
> index b175645fde..c867f1cf58 100644
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -1783,6 +1783,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 71fa532842..fe96a9b135 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>
>  
>  /*
> @@ -65,8 +66,21 @@ static inline int __init acpi_pci_init(void)
>  }
>  #endif
>  
> +/*
> + * By default pci passthrough is disabled
> + */
> +bool_t __read_mostly pci_passthrough_enabled = 0;

I think we are using bool rather than bool_t nowadays. Also could do =
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)

missing space


> +        return 0;
> +
>      pci_segments_init();
>  
>      if ( acpi_disabled )
> diff --git a/xen/common/physdev.c b/xen/common/physdev.c
> index 8d44b20db8..7390d5d584 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..f2f86be9bc 100644
> --- a/xen/include/asm-arm/pci.h
> +++ b/xen/include/asm-arm/pci.h
> @@ -19,14 +19,27 @@
>  
>  #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*/
>  
> +#define pci_passthrough_enabled (false)

We don't need to define it, do we?


>  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..0e160c6d01 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 always enabled for x86.
                                 ^ is


> + */
> +static always_inline bool is_pci_passthrough_enabled(void)
> +{
> +    return true;
> +}
> +
>  #endif /* __X86_PCI_H__ */
> -- 
> 2.17.1
>