Re: [PATCH v6 15/25] iommufd/selftest: Make the mock iommu driver into a real driver

[Baolu Lu <baolu.lu@linux.intel.com>]

On 2023/8/3 8:08, Jason Gunthorpe wrote:
> I've avoided doing this because there is no way to make this happen
> without an intrusion into the core code. Up till now this has avoided
> needing the core code's probe path with some hackery - but now that
> default domains are becoming mandatory it is unavoidable. The core probe
> path must be run to set the default_domain, only it can do it. Without
> a default domain iommufd can't use the group.
> 
> Make it so that iommufd selftest can create a real iommu driver and bind
> it only to is own private bus. Add iommu_device_register_bus() as a core
> code helper to make this possible. It simply sets the right pointers and
> registers the notifier block. The mock driver then works like any normal
> driver should, with probe triggered by the bus ops
> 
> When the bus->iommu_ops stuff is fully unwound we can probably do better
> here and remove this special case.
> 
> Remove set_platform_dma_ops from selftest and make it use a BLOCKED
> default domain.
> 
> Signed-off-by: Jason Gunthorpe<jgg@nvidia.com>
> ---
>   drivers/iommu/iommu-priv.h              |  16 +++
>   drivers/iommu/iommu.c                   |  43 +++++++
>   drivers/iommu/iommufd/iommufd_private.h |   5 +-
>   drivers/iommu/iommufd/main.c            |   8 +-
>   drivers/iommu/iommufd/selftest.c        | 149 +++++++++++++-----------
>   5 files changed, 152 insertions(+), 69 deletions(-)
>   create mode 100644 drivers/iommu/iommu-priv.h
> 
> diff --git a/drivers/iommu/iommu-priv.h b/drivers/iommu/iommu-priv.h
> new file mode 100644
> index 00000000000000..1cbc04b9cf7297
> --- /dev/null
> +++ b/drivers/iommu/iommu-priv.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES.
> + */
> +#ifndef __IOMMU_PRIV_H
> +#define __IOMMU_PRIV_H
> +
> +#include <linux/iommu.h>
> +
> +int iommu_device_register_bus(struct iommu_device *iommu,
> +			      const struct iommu_ops *ops, struct bus_type *bus,
> +			      struct notifier_block *nb);
> +void iommu_device_unregister_bus(struct iommu_device *iommu,
> +				 struct bus_type *bus,
> +				 struct notifier_block *nb);
> +
> +#endif
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index a1a93990b3a211..7fae866af0db7a 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -36,6 +36,7 @@
>   #include "dma-iommu.h"
>   
>   #include "iommu-sva.h"
> +#include "iommu-priv.h"
>   
>   static struct kset *iommu_group_kset;
>   static DEFINE_IDA(iommu_group_ida);
> @@ -290,6 +291,48 @@ void iommu_device_unregister(struct iommu_device *iommu)
>   }
>   EXPORT_SYMBOL_GPL(iommu_device_unregister);
>   
> +#if IS_ENABLED(CONFIG_IOMMUFD_TEST)
> +void iommu_device_unregister_bus(struct iommu_device *iommu,
> +				 struct bus_type *bus,
> +				 struct notifier_block *nb)
> +{
> +	bus_unregister_notifier(bus, nb);
> +	iommu_device_unregister(iommu);
> +}
> +EXPORT_SYMBOL_GPL(iommu_device_unregister_bus);
> +
> +/*
> + * Register an iommu driver against a single bus. This is only used by iommufd
> + * selftest to create a mock iommu driver. The caller must provide
> + * some memory to hold a notifier_block.
> + */
> +int iommu_device_register_bus(struct iommu_device *iommu,
> +			      const struct iommu_ops *ops, struct bus_type *bus,
> +			      struct notifier_block *nb)
> +{
> +	int err;
> +
> +	iommu->ops = ops;
> +	nb->notifier_call = iommu_bus_notifier;
> +	err = bus_register_notifier(bus, nb);
> +	if (err)
> +		return err;
> +
> +	spin_lock(&iommu_device_lock);
> +	list_add_tail(&iommu->list, &iommu_device_list);
> +	spin_unlock(&iommu_device_lock);
> +
> +	bus->iommu_ops = ops;
> +	err = bus_iommu_probe(bus);
> +	if (err) {
> +		iommu_device_unregister_bus(iommu, bus, nb);
> +		return err;
> +	}
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(iommu_device_register_bus);
> +#endif

Although

#if IS_ENABLED(CONFIG_IOMMUFD_TEST)
... ...
#endif

doesn't look so comfortable, this is also the most concise method that I
can think of.

So,

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

Best regards,
baolu