Re: [PATCH 1/2] firmware: arm_ffa: Move the FF-A v1.0 NULL UUID workaround to bus notifier

[Sebastian Ene <sebastianene@google.com>]

On Fri, May 10, 2024 at 07:38:10PM +0100, Sudeep Holla wrote:
> Currently FF-A bus ffa_device_match() handles the workaround for the
> FF-A v1.0 devices which are not populated with UUID to match. The FF-A
> bus layer calls into FF-A driver and populates the device UUID if it
> matches with the driver attempting to match.
> 
> But this forces to have both FF-A bus and core driver to be bundled into
> a single module. However, keep it as a single module adds problems for
> the FF-A driver registrations and their initcall levels.
> 
> In preparation to split the FF-A bus and the core driver into distinct
> modules, we need to move the workaround away from the FF-A bus layer.
> We can add it into the FF-A core driver as a bus notifier.
> 
> In order to do so, we need to always match any driver with the device if
> the UUID is NULL and then during the driver binding phase, we can populate
> the UUID if it matches with the driver UUID table using the bus notifiers.
> We also need to add a check for NULL UUID before calling the device/driver
> probe as devices with NULL UUID is possible since we match all for that
> case.
> 
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/firmware/arm_ffa/driver.c | 33 ++++++++++++++++++++++++++++++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> index 9f89ee0aaa6b..159f2106a541 100644
> --- a/drivers/firmware/arm_ffa/driver.c
> +++ b/drivers/firmware/arm_ffa/driver.c
> @@ -1235,6 +1235,29 @@ void ffa_device_match_uuid(struct ffa_device *ffa_dev, const uuid_t *uuid)
>  	kfree(pbuf);
>  }

Hello Sudeep,

> 
> +static int
> +ffa_bus_notifier(struct notifier_block *nb, unsigned long action, void *data)
> +{
> +	struct device *dev = data;
> +	struct ffa_device *fdev = to_ffa_dev(dev);
> +
> +	if (action == BUS_NOTIFY_BIND_DRIVER) {
> +		struct ffa_driver *ffa_drv = to_ffa_driver(dev->driver);
> +		const struct ffa_device_id *id_table = ffa_drv->id_table;
> +
> +		if (uuid_is_null(&fdev->uuid))
> +			ffa_device_match_uuid(fdev, &id_table->uuid);
> +
> +		return NOTIFY_OK;
> +	}
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block ffa_bus_nb = {
> +	.notifier_call = ffa_bus_notifier,
> +};
> +
>  static int ffa_setup_partitions(void)
>  {
>  	int count, idx, ret;
> @@ -1243,6 +1266,14 @@ static int ffa_setup_partitions(void)
>  	struct ffa_dev_part_info *info;
>  	struct ffa_partition_info *pbuf, *tpbuf;
> 
> +	if (drv_info->version == FFA_VERSION_1_0) {

Should we remove the version check inside the ffa_device_match_uuid
function as this function will only be called from the bus notifier ?

> +		ret = bus_register_notifier(&ffa_bus_type, &ffa_bus_nb);
> +		if (ret) {
> +			pr_err("Failed to register FF-A bus notifiers\n");
> +			return ret;
> +		}
> +	}
> +
>  	count = ffa_partition_probe(&uuid_null, &pbuf);
>  	if (count <= 0) {
>  		pr_info("%s: No partitions found, error %d\n", __func__, count);
> @@ -1254,7 +1285,7 @@ static int ffa_setup_partitions(void)
>  		import_uuid(&uuid, (u8 *)tpbuf->uuid);
> 
>  		/* Note that if the UUID will be uuid_null, that will require
> -		 * ffa_device_match() to find the UUID of this partition id
> +		 * ffa_bus_notifier() to find the UUID of this partition id
>  		 * with help of ffa_device_match_uuid(). FF-A v1.1 and above
>  		 * provides UUID here for each partition as part of the
>  		 * discovery API and the same is passed.
> --
> 2.43.2
> 

Also I wonder if we should have the change from patch 2 (the one which
removes the ffa_device_match_uuid call from ffa_device_match) inside
this. Wdyth ?


Thank you,
Sebastian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel