[PATCH v1 1/2] drivers: firmware: introduce Meson Secure Monitor driver

[Simon Glass]

Hi Alexey,

On Mon, 10 Jul 2023 at 02:34, Alexey Romanov <AVRomanov at sberdevices.ru> wrote:
>
>
> Hello!
>
> On Fri, Jul 07, 2023 at 11:35:47AM -0600, Simon Glass wrote:
> > Hi Alexey,
> >
> > On Fri, 7 Jul 2023 at 09:43, Alexey Romanov <AVRomanov at sberdevices.ru> wrote:
> > >
> > > Hello, Simon!
> > >
> > > On Thu, Jul 06, 2023 at 09:58:02AM -0600, Simon Glass wrote:
> > > > Hi Alexey,
> > > >
> > > > On Thu, 6 Jul 2023 at 14:16, Alexey Romanov <avromanov at sberdevices.ru> wrote:
> > > > >
> > > > > At the moment, only smc API is a set of functions in
> > > > > arch/arm/mach-meson/sm.c. This approach is hard to configure
> > > > > and also doesni't contain any generic API for calling smc.
> > > > >
> > > > > This patch add Meson SM driver with generic API (struct meson_sm_ops):
> > > > >
> > > > > - sm_call()
> > > > > - sm_call_write()
> > > > > - sm_call_read()
> > > > >
> > > > > A typical driver usage example is shown here:
> > > > >
> > > > > 1. uclass_get_device_by_driver(UCLASS_FIRMWARE, "secure-monitor", &dev);
> > > > > 2. handle = meson_sm_get_handle(dev);
> > > > > 3. handle->ops.sm_call(dev, cmd, ...);
> > > > >
> > > > > Signed-off-by: Alexey Romanov <avromanov at sberdevices.ru>
> > > > > ---
> > > > >  arch/arm/mach-meson/Kconfig       |   1 +
> > > > >  drivers/firmware/Kconfig          |  10 ++
> > > > >  drivers/firmware/Makefile         |   1 +
> > > > >  drivers/firmware/meson/Kconfig    |   6 +
> > > > >  drivers/firmware/meson/Makefile   |   3 +
> > > > >  drivers/firmware/meson/meson_sm.c | 217 ++++++++++++++++++++++++++++++
> > > > >  include/meson/sm_handle.h         |  38 ++++++
> > > > >  7 files changed, 276 insertions(+)
> > > > >  create mode 100644 drivers/firmware/meson/Kconfig
> > > > >  create mode 100644 drivers/firmware/meson/Makefile
> > > > >  create mode 100644 drivers/firmware/meson/meson_sm.c
> > > > >  create mode 100644 include/meson/sm_handle.h
> > > >
> > > > Please can you use the remoteproc uclass for this and add a proper driver?
> > > >
> > >
> > > I don't see it architecturally well. Can you explain please?
> > >
> > > This driver is just ARM SMC fw interface. There seems to be nothing to
> > > do here for remoteproc uclass.
> >
> > Well you seem to be implementing a remote CPU interface, which is what
> > remoteproc is for. How does Linux do this?
>
> The main idea of the patchset is to abstract the smc calls (which run on
> the same CPU) and make a request to the firmware that runs on a higher
> EL. UCLASS_REMOTEPROC may give the impression that we are
> interacting with another CPU, although this is not the case.
>
> Also, UCLASS_REMOTEPROC requires two mandatory interfaces: load() and
> start(), and I don't even know how to apply my current changes to them.

You can return -ENOSYS if not implemented.

>
> My implementation is very close to the Linux implementation, they
> also use the firmware driver without remoteproc:
>
> https://elixir.bootlin.com/linux/latest/source/drivers/firmware/meson/meson_sm.c

Yes that seems like it doesn't use any common infra. I don't think it
is a great model for U-Boot though.

>
> I spoke to Neil on IRC and he said UCLASS_FIRMWARE for such driver is
> OK.
>
> >
> > Also there is a pending series on FFA - is that related? It uses smc
> > from what I can tell.
>
> Not entirely clear, my changes don't seem to be related to this
> patchset.

So perhaps this needs a new UCLASS_SMC? I see various other SMC calls
in U-Boot but no one has taken the initiative to think about this in
terms of driver model.

It might just need one operation, to make a call, passing a regs
struct, perhaps? The uclass would presumably be ARM-specific.

I really don't think this is UCLASS_FIRMWARE. That seems to be for
loading firmware. It doesn't even have a firmware.h header.

Also instead of:

1. uclass_get_device_by_driver(UCLASS_FIRMWARE, "secure-monitor", &dev);

use:

ret = uclass_first_device_err(UCLASS_SMC, &dev)

using device tree to find the device.

Regards,
Simon