Re: [PATCH] firmware: bluefield: add boot control driver

[Leif Lindholm]

On Tue, Oct 10, 2017 at 11:23:32AM +0100, Mark Rutland wrote:
> On Tue, Oct 10, 2017 at 11:15:39AM +0100, Sudeep Holla wrote:
> > (+Mark, Grant)
> > 
> > On 09/10/17 18:16, Chris Metcalf wrote:
> > > The Mellanox BlueField SoC firmware supports a safe upgrade mode as
> > > part of the flow where users put new firmware on the secondary eMMC
> > > boot partition (the one not currently in use), tell the eMMC to make
> > > the secondary boot partition primary, and reset.
> 
> When you say "firmware", are you referreind to actual firmware, or a
> platform-specific OS image?
> 
> For the former, the preferred update mechanism would be UpdateCapsule().
> 
> For the latter, I'm not sure what the usual mechanism for doing this
> with EFI would be.
> 
> Ard, Leif?

This sounds to me very much like something we'd want to keep out of
the kernel. Assuming ACPI means UEFI, there should be less invasive
solutions to this problem.

I've added linaro-uefi to cc. Chris - if you'd be willing to have a
side discussion on the overall aproach, please drop the kernel lists
from cc and let's have a chat there.

Best Regards,

Leif

> Thanks,
> Mark.
> 
> > > This driver is
> > > used to request that the firmware start the ARM watchdog after the
> > > next reset, and also request that the firmware swap the eMMC boot
> > > partition back again on the reset after that (the second reset).
> > > This means that if anything goes wrong, the watchdog will fire, the
> > > system will reset, and the firmware will switch back to the original
> > > boot partition.  If the boot is successful, the user will use this
> > > driver to put the firmware back into the state where it doesn't touch
> > > the eMMC boot partition at reset, and turn off the ARM watchdog.
> > > 
> > > The firmware allows for more configurability than that, as can
> > > be seen in the code, but the use case above is what the driver
> > > primarily supports.
> > > 
> > > It is structured as a simple sysfs driver that is loaded based on
> > > an ACPI table entry, and allows reading/writing text strings to
> > > various /sys/bus/platform/drivers/mlx-bootctl/* files.
> > > 
> > > Signed-off-by: Chris Metcalf <cmetcalf@xxxxxxxxxxxx>
> > > ---
> > > Ingo, since there isn't an overall maintainer for drivers/firmware,
> > > does it make sense for this to go through your tree?  Thanks!
> > > 
> > >  drivers/firmware/Kconfig       |  12 +++
> > >  drivers/firmware/Makefile      |   1 +
> > >  drivers/firmware/mlx-bootctl.c | 222 +++++++++++++++++++++++++++++++++++++++++
> > >  drivers/firmware/mlx-bootctl.h | 103 +++++++++++++++++++
> > >  4 files changed, 338 insertions(+)
> > >  create mode 100644 drivers/firmware/mlx-bootctl.c
> > >  create mode 100644 drivers/firmware/mlx-bootctl.h
> > > 
> > > diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> > > index 6e4ed5a9c6fd..1f2adbcc5acc 100644
> > > --- a/drivers/firmware/Kconfig
> > > +++ b/drivers/firmware/Kconfig
> > > @@ -230,6 +230,18 @@ config TI_SCI_PROTOCOL
> > >  	  This protocol library is used by client drivers to use the features
> > >  	  provided by the system controller.
> > >  
> > > +config MLX_BOOTCTL
> > > +	tristate "Mellanox BlueField Firmware Boot Control"
> > > +	depends on ARM64
> > > +	help
> > > +	  The Mellanox BlueField firmware implements functionality to
> > > +	  request swapping the primary and alternate eMMC boot
> > > +	  partition, and to set up a watchdog that can undo that swap
> > > +	  if the system does not boot up correctly.  This driver
> > > +	  provides sysfs access to the firmware, to be used in
> > > +	  conjunction with the eMMC device driver to do any necessary
> > > +	  initial swap of the boot partition.
> > > +
> > For me, this looks like any other distro upgrade use-case which requires
> > to set some variable that must persist across reset (i.e. in non
> > volatile memory)
> > 
> > ARM is proposing Embedded Base Boot Requirements (EBBR)[1] and it covers
> > this IIUC. I am not sure if we need to achieve that using SMCCC as
> > proposed in the patch. I may be wrong, but just wanted to check.
> > 
> > -- 
> > [1]
> > https://developer.arm.com/products/architecture/system-architecture/embedded-system-architecture
> > -- 
> > Regards,
> > Sudeep