Re: [PATCH v2] xen: Remove dependency between pciback and privcmd

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 10 Oct 2024, Jiqian Chen wrote:
> Commit 2fae6bb7be32 ("xen/privcmd: Add new syscall to get gsi from dev")
> adds a weak reverse dependency to the config XEN_PRIVCMD definition, that
> dependency causes xen-privcmd can't be loaded on domU, because
> dependent xen-pciback is always not be loaded successfully on domU.
> 
> To solve above problem, remove that dependency, and do not call
> pcistub_get_gsi_from_sbdf() directly, instead add a hook in
> drivers/xen/apci.c, xen-pciback register the real call function, then in
> privcmd_ioctl_pcidev_get_gsi call that hook.
> 
> Fixes: 2fae6bb7be32 ("xen/privcmd: Add new syscall to get gsi from dev")
> Reported-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Jiqian Chen <Jiqian.Chen@xxxxxxx>
> ---
> v1->v2 changes:
> Added hook xen_acpi_get_gsi_from_sbdf.
> Changed pcistub_get_gsi_from_sbdf to static and pciback register it as the 
> real hook function.
> ---
>  drivers/xen/Kconfig                |  1 -
>  drivers/xen/acpi.c                 | 17 +++++++++++++++++
>  drivers/xen/privcmd.c              |  6 ++----
>  drivers/xen/xen-pciback/pci_stub.c |  4 ++--
>  include/xen/acpi.h                 | 12 ++++--------
>  5 files changed, 25 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> index 72ddee4c1544..f7d6f47971fd 100644
> --- a/drivers/xen/Kconfig
> +++ b/drivers/xen/Kconfig
> @@ -261,7 +261,6 @@ config XEN_SCSI_BACKEND
>  config XEN_PRIVCMD
>       tristate "Xen hypercall passthrough driver"
>       depends on XEN
> -     imply XEN_PCIDEV_BACKEND
>       default m
>       help
>         The hypercall passthrough driver allows privileged user programs to
> diff --git a/drivers/xen/acpi.c b/drivers/xen/acpi.c
> index 9e2096524fbc..0bff2f3a87d2 100644
> --- a/drivers/xen/acpi.c
> +++ b/drivers/xen/acpi.c
> @@ -125,3 +125,20 @@ int xen_acpi_get_gsi_info(struct pci_dev *dev,
>       return 0;
>  }
>  EXPORT_SYMBOL_GPL(xen_acpi_get_gsi_info);
> +
> +get_gsi_from_sbdf_t get_gsi_from_sbdf = NULL;

This can be static


> +void xen_acpi_register_get_gsi_func(get_gsi_from_sbdf_t func)
> +{
> +     get_gsi_from_sbdf = func;
> +}
> +EXPORT_SYMBOL_GPL(xen_acpi_register_get_gsi_func);
> +
> +int xen_acpi_get_gsi_from_sbdf(u32 sbdf)
> +{
> +     if (get_gsi_from_sbdf)
> +             return get_gsi_from_sbdf(sbdf);
> +
> +     return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(xen_acpi_get_gsi_from_sbdf);
> \ No newline at end of file
> diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
> index 3273cb8c2a66..4f75bc876454 100644
> --- a/drivers/xen/privcmd.c
> +++ b/drivers/xen/privcmd.c
> @@ -850,15 +850,13 @@ static long privcmd_ioctl_mmap_resource(struct file 
> *file,
>  static long privcmd_ioctl_pcidev_get_gsi(struct file *file, void __user 
> *udata)
>  {
>  #if defined(CONFIG_XEN_ACPI)
> -     int rc = -EINVAL;
> +     int rc;
>       struct privcmd_pcidev_get_gsi kdata;
>  
>       if (copy_from_user(&kdata, udata, sizeof(kdata)))
>               return -EFAULT;
>  
> -     if (IS_REACHABLE(CONFIG_XEN_PCIDEV_BACKEND))
> -             rc = pcistub_get_gsi_from_sbdf(kdata.sbdf);
> -
> +     rc = xen_acpi_get_gsi_from_sbdf(kdata.sbdf);
>       if (rc < 0)
>               return rc;
>  
> diff --git a/drivers/xen/xen-pciback/pci_stub.c 
> b/drivers/xen/xen-pciback/pci_stub.c
> index 2f3da5ac62cd..900e6199eec7 100644
> --- a/drivers/xen/xen-pciback/pci_stub.c
> +++ b/drivers/xen/xen-pciback/pci_stub.c
> @@ -227,7 +227,7 @@ static struct pci_dev *pcistub_device_get_pci_dev(struct 
> xen_pcibk_device *pdev,
>  }
>  
>  #ifdef CONFIG_XEN_ACPI
> -int pcistub_get_gsi_from_sbdf(unsigned int sbdf)
> +static int pcistub_get_gsi_from_sbdf(unsigned int sbdf)
>  {
>       struct pcistub_device *psdev;
>       int domain = (sbdf >> 16) & 0xffff;
> @@ -242,7 +242,6 @@ int pcistub_get_gsi_from_sbdf(unsigned int sbdf)
>  
>       return psdev->gsi;
>  }
> -EXPORT_SYMBOL_GPL(pcistub_get_gsi_from_sbdf);
>  #endif
>  
>  struct pci_dev *pcistub_get_pci_dev_by_slot(struct xen_pcibk_device *pdev,
> @@ -484,6 +483,7 @@ static int pcistub_init_device(struct pcistub_device 
> *psdev)
>               if (err)
>                       goto config_release;
>               psdev->gsi = gsi;
> +             xen_acpi_register_get_gsi_func(pcistub_get_gsi_from_sbdf);
>       }
>  #endif
>  
> diff --git a/include/xen/acpi.h b/include/xen/acpi.h
> index daa96a22d257..ef4f70e4a557 100644
> --- a/include/xen/acpi.h
> +++ b/include/xen/acpi.h
> @@ -91,13 +91,9 @@ static inline int xen_acpi_get_gsi_info(struct pci_dev 
> *dev,
>  }
>  #endif
>  
> -#ifdef CONFIG_XEN_PCI_STUB
> -int pcistub_get_gsi_from_sbdf(unsigned int sbdf);
> -#else
> -static inline int pcistub_get_gsi_from_sbdf(unsigned int sbdf)
> -{
> -     return -1;
> -}
> -#endif
> +typedef int (*get_gsi_from_sbdf_t)(u32 sbdf);
> +
> +void xen_acpi_register_get_gsi_func(get_gsi_from_sbdf_t func);
> +int xen_acpi_get_gsi_from_sbdf(unsigned int sbdf);
>  
>  #endif       /* _XEN_ACPI_H */
> -- 
> 2.34.1
>