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

Baolu Lu
2023-08-12T04:57:33Z

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