On 04.02.2025 14:54, Mykyta Poturai wrote:
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -27,6 +27,7 @@
> #include <xen/errno.h>
> #include <public/domctl.h>
> #include <public/hvm/ioreq.h>
> +#include <xen/acpi.h>
> #include <asm/device.h>
Please insert where the other xen/ #include-s are. (Those aren't sorted
yet, so there is more than one place where you could reasonably put it.
I'd recommend ahead of xen/errno.h.)
> @@ -241,8 +244,31 @@ int iommu_remove_dt_device(struct dt_device_node *np);
> /* Error code for reporting no IOMMU is present */
> #define NO_IOMMU 1
>
> +#else /* !HAS_DEVICE_TREE */
> +static inline int iommu_add_dt_pci_sideband_ids(struct pci_dev *pdev)
> +{
> + return -ENOSYS;
No abuse of ENOSYS please. EOPNOTSUPP if nothing better can be found.
> +}
> +
> #endif /* HAS_DEVICE_TREE */
>
> +#ifdef CONFIG_HAS_PCI
> +static inline int iommu_add_pci_sideband_ids(struct pci_dev *pdev)
>From its name it's unclear whether the function actually means to alter
the passed in pdev (my initial guess was that it wouldn't, but the call
tree from iommu_add_dt_pci_sideband_ids() is getting deep-ish). If not,
the parameter should be pointer-to-const.
> +{
> + int ret = -ENOSYS;
> +
> + if ( acpi_disabled )
> + ret = iommu_add_dt_pci_sideband_ids(pdev);
> +
> + return ret;
> +}
> +#else /* !HAS_PCI */
> +static inline int iommu_add_pci_sideband_ids(struct pci_dev *pdev)
> +{
> + return -ENOSYS;
> +}
> +#endif
Why the redundancy?
static inline int iommu_add_pci_sideband_ids(struct pci_dev *pdev)
{
int ret = -EOPNOTSUPP;
#ifdef CONFIG_HAS_PCI
if ( acpi_disabled )
ret = iommu_add_dt_pci_sideband_ids(pdev);
#endif
return ret;
}
Jan
