Re: [PATCH v2 1/3] firmware: arm_ffa: Skip creation of the notification bitmaps

[Sudeep Holla <sudeep.holla@arm.com>]

On Fri, Apr 12, 2024 at 10:42:07AM +0000, Sebastian Ene wrote:
> On Thu, Apr 11, 2024 at 01:57:32PM +0100, Sudeep Holla wrote:
> > From: Jens Wiklander <jens.wiklander@linaro.org>
> > 
> > When the FF-A driver is running inside a guest VM under an hypervisor,
> > the driver/guest VM doesn't have the permission/capability to request
> > the creation of notification bitmaps. For a VM, the hypervisor reserves
> 
> Who restrains them from having this capability ? (to use the
> FFA_NOTIFICATION_BITMAP_CREATE API).

The hypervisor has to IIUC from the spec as it is responsible for creating
the bitmaps on its behalf. It can trap and handle those calls if it prefers
to fail for FFA_FEATURES(FFA_NOTIFICATION_BITMAP_CREATE).

> > memory for its VM and hypervisor framework notification bitmaps and the
> > SPMC reserves memory for its SP and SPMC framework notification bitmaps
> > before the hypervisor initializes it.
> > 
> > The hypervisor does not initialize a VM if memory cannot be reserved
> > for all its notification bitmaps. So the creation of all the necessary
> > bitmaps are already done when the driver initialises and hence it can be
> > skipped. We rely on FFA_FEATURES(FFA_NOTIFICATION_BITMAP_CREATE) to fail
> > when running in the guest to handle this in the driver.
> > 
> 
> This implies that the hypervisor reserves memory in advance to keep the
> notification bitmaps even though the guest might decide not to use them.
> For pKVM this assumption does not work because we might have guests that will not be
> allowed to talk to TZ.
>

Sure, in such case pKVM can always trap and fail for FFA_VERSION call so
that no further FFA transactions happen from that guest or just fail for
notification APIs if the notifications alone are not needed.

Hope that clarifies.

> > Signed-off-by:Jens Wiklander <jens.wiklander@linaro.org>
> > [sudeep.holla: Updated the commit message]
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> >  drivers/firmware/arm_ffa/driver.c | 16 +++++++---------
> >  1 file changed, 7 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> > index f2556a8e9401..4a576af7b8fd 100644
> > --- a/drivers/firmware/arm_ffa/driver.c
> > +++ b/drivers/firmware/arm_ffa/driver.c
> > @@ -1442,17 +1442,15 @@ static void ffa_notifications_setup(void)
> >  	int ret, irq;
> >  
> >  	ret = ffa_features(FFA_NOTIFICATION_BITMAP_CREATE, 0, NULL, NULL);
> > -	if (ret) {
> > -		pr_info("Notifications not supported, continuing with it ..\n");
> > -		return;
> > -	}
> > +	if (!ret) {
> > +		ret = ffa_notification_bitmap_create();
> > +		if (ret) {
> > +			pr_info("Notification bitmap create error %d\n", ret);
> 
> Not a big deal but shouldn't this be a pr_err(..) 
>

Sure I can fixup when I apply.

-- 
Regards,
Sudeep

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