[PATCH] firmware: arm_ffa: Handle compatibility with different firmware versions

[Sudeep Holla]

On Wed, Oct 13, 2021 at 03:10:50PM +0200, Jens Wiklander wrote:
> On Wed, Oct 13, 2021 at 11:11 AM Sudeep Holla <sudeep.holla at arm.com> wrote:
> >
> > The driver currently just support v1.0 of Arm FFA specification. It also
> > expects the firmware implementation to match the same and bail out if it
> > doesn't match. This is causing issue when running with higher version of
> > firmware implementation(e.g. v1.1 which will released soon).
> >
> > In order to support compatibility with different firmware versions, let
> > us add additional checks and find the compatible version the driver can
> > work with.
> >
> > Signed-off-by: Sudeep Holla <sudeep.holla at arm.com>
> > ---
> >  drivers/firmware/arm_ffa/driver.c | 37 ++++++++++++++++++++++++++-----
> >  1 file changed, 32 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> > index c9fb56afbcb4..6e0c883ab708 100644
> > --- a/drivers/firmware/arm_ffa/driver.c
> > +++ b/drivers/firmware/arm_ffa/driver.c
> > @@ -167,6 +167,28 @@ struct ffa_drv_info {
> >
> >  static struct ffa_drv_info *drv_info;
> >
> > +/*
> > + * The driver must be able to support all the versions from the earliest
> > + * supported FFA_MIN_VERSION to the latest supported FFA_DRIVER_VERSION.
> > + * The specification states that if firmware supports a FFA implementation
> > + * that is incompatible with and at a greater version number than specified
> > + * by the caller(FFA_DRIVER_VERSION passed as parameter to FFA_VERSION),
> > + * it must return the NOT_SUPPORTED error code.
>
> From what I understand from the specification (v1.1 beta0) it may also
> return its version in this case.
>

Yes we are fixing that in the spec. Basically return a version higher than the
caller version only if it can work just fine at the caller version. It must
return NOT_SUPPORTED otherwise.

> > + */
> > +static u32 ffa_compatible_version_find(u32 version)
> > +{
> > +       u32 compat_version;
> > +       u16 major = MAJOR_VERSION(version), minor = MINOR_VERSION(version);
> > +       u16 drv_major = MAJOR_VERSION(FFA_DRIVER_VERSION);
> > +       u16 drv_minor = MINOR_VERSION(FFA_DRIVER_VERSION);
> > +
> > +       if ((major < drv_major) || (major == drv_major && minor <= drv_minor))
> > +               return version;
> 
> A mismatch in the major version number makes the versions
> incompatible. There's no recovery from this, an error code must be
> returned instead.
>

The point is if the firmware can operate at reduced functionality, why just
return error. Most other specs operate with that assumption.
e.g. if we just add few extra functionality in say v2.0 vs v1.y, would
you not want the v1.y driver and firmware v2.0 work together ? Especially
if we give the flexibility for the firmware to decide that and return
the version as v2.0 in response to v1.y by the caller.

If firmware is completely incompatible, it still has option top return
NOT_SUPPORTED.

--
Regards,
Sudeep