[PATCH] board: rpi: Enable capsule updates

[Ilias Apalodimas]

Apologies for responding to myself here, but clarifying things a bit more...

On Fri, 13 Sept 2024 at 14:15, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Hi Matthias
>
> On Fri, 13 Sept 2024 at 13:40, Matthias Brugger <mbrugger at suse.com> wrote:
> >
> >
> >
> > On 11/09/2024 12:05, Ilias Apalodimas wrote:
> > > On Wed, 11 Sept 2024 at 12:50, Peter Robinson <pbrobinson at gmail.com> wrote:
> > >>
> > >> On Tue, 10 Sept 2024 at 08:29, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > >>>
> > >>> On Tue, 10 Sept 2024 at 12:09, Ilias Apalodimas
> > >>> <ilias.apalodimas at linaro.org> wrote:
> > >>>>
> > >>>> Since RPI works well using EFI and has no size limitations with regards
> > >>>> to U-Boot, add the needed structures and Kconfig options needed to
> > >>>> enable capsule updates
> > >>>> ---
> > >>>>   board/raspberrypi/rpi/rpi.c | 22 ++++++++++++++++++++++
> > >>>>   configs/rpi_4_defconfig     |  2 ++
> > >>>>   2 files changed, 24 insertions(+)
> > >>>
> > >>> Tested-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > >>>
> > >>> A couple of nits below.
> > >>>
> > >>>>
> > >>>> diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
> > >>>> index ab5ea85cf9f8..1f342eee12b2 100644
> > >>>> --- a/board/raspberrypi/rpi/rpi.c
> > >>>> +++ b/board/raspberrypi/rpi/rpi.c
> > >>>> @@ -63,6 +63,28 @@ struct msg_get_clock_rate {
> > >>>>          u32 end_tag;
> > >>>>   };
> > >>>>
> > >>>> +struct efi_fw_image fw_images[] = {
> > >>>> +       {
> > >>>> +               .fw_name = u"RPI_UBOOT",
> > >>>> +               .image_index = 1,
> > >>>> +       },
> > >>>> +};
> > >>>> +
> > >>>> +struct efi_capsule_update_info update_info = {
> > >>>> +       .dfu_string = "mmc 0=u-boot.bin fat 0 1",
> > >>>> +       .num_images = ARRAY_SIZE(fw_images),
> > >>>> +       .images = fw_images,
> > >>>> +};
> > >>>> +
> > >>>> +#if IS_ENABLED(CONFIG_SET_DFU_ALT_INFO)
> > >>>> +void set_dfu_alt_info(char *interface, char *devstr)
> > >>>> +{
> > >>>> +       if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
> > >>>> +               env_set("dfu_alt_info", update_info.dfu_string);
> > >>>> +}
> > >>>> +#endif
> > >>>
> > >>> Is this really needed? We have a weak function in efi_firrmware.c
> > >>> which is doing exactly this.
> > >>>
> > >>>> +
> > >>>> +
> > >>>>   #ifdef CONFIG_ARM64
> > >>>>   #define DTB_DIR "broadcom/"
> > >>>>   #else
> > >>>> diff --git a/configs/rpi_4_defconfig b/configs/rpi_4_defconfig
> > >>>> index f5fb322aa8fc..c70414e6fcaf 100644
> > >>>> --- a/configs/rpi_4_defconfig
> > >>>> +++ b/configs/rpi_4_defconfig
> > >>>> @@ -65,3 +65,5 @@ CONFIG_SYS_WHITE_ON_BLACK=y
> > >>>>   CONFIG_VIDEO_BCM2835=y
> > >>>>   CONFIG_CONSOLE_SCROLL_LINES=10
> > >>>>   CONFIG_PHYS_TO_BUS=y
> > >>>> +CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
> > >>>> +CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> > >>>
> > >>> Can we also add the efidebug and efi nvedit commands here. They are
> > >>> pretty handy especially when it comes to capsule updates. Thanks.
> > >>
> > >> Are they pretty handy for capsule updates for general users when
> > >> things should "just work", as a user applies them from Linux with
> > >> fwupdmgr and reboots, or for firmwre developers trying to debug
> > >> things? If it's the later I don't think we should be enabling them by
> > >> default :)
> > >
> > > nvedit lets you print and see EFI variables, which is pretty basic if
> > > you want to boot with efi.
> > > efidebug is supposed to be a debug tool mostly, but unfortunately, we
> > > havent plugged in EFI HTTP boot into the 'eficonfig' command yet. So
> > > the only way you can add a boot option for HTTP is via efidebug
> > >
> >
> > I don't really understand why you are mentioning EFI HTTP. >
>
> Because I misread Peters's question and assumed he was asking if it
> was for capsules only or useful overall as a tool.
>
> > From what I can see
> > we need efidebug to be able to do capsule updates from U-Boot command line.
>
> Updating via the command line is for debugging only. The capsule
> updates execute automatically after a reboot if a variable
> (OsIndications) is set properly or you enable
> CONFIG_EFI_IGNORE_OSINDICATIONS and have a boot entry pointing to the
> same ESP where the capsule is located. But that's a bit complicated
> atm, look below.

The above is only true if CONFIG_EFI_CAPSULE_ON_DISK, which I've
intentionally left out. Only runtime capsule update is selected, which
requires efidebug or an EFI app.

>
> > It is also needed for EFI HTTP boot but this needs CONFIG_EFI_HTTP_BOOT which we
> > don't enable (and I think is out-of-scope for capsule update).
> >
> > /me is puzzled :)
>
> Fair enough. We could make it default tbh. It depends on how much
> 'efi' you prefer in the defconfig. But that belongs on a different
> patchset.
>
> Something completely irrelevant to efidebug, but related to capsule updates.
> Capsule updates,  in order to work with tools like fwupd etc, either
> need commits 00da8d65a3ba and bc3dd2493ef8 and c28d32f946f0 and
> https://github.com/rhboot/efivar/pull/267 or enable
> CONFIG_EFI_IGNORE_OSINDICATIONS. Since the latter is a hack I was
> going to enable SetVariable at Runtime once the userspace patches get
> merged and sorted. Keep in mind there's a v3 out which I managed to
> test with rpi5. FWIW still think efidebug is useful and needs to be
> around regardless of capsules or not.
> What we could do in v4 is enable CONFIG_EFI_IGNORE_OSINDICATIONS, but
> I would prefer to wait for the efivar pathes to merge and just enable
> setvariable at runtime.
>

Also, all these apply to CONFIG_EFI_CAPSULE_ON_DISK only, which I will
enable once the efivar are merged

Sorry for the confusion!
/Ilias

> Thanks
> /Ilias
>
> >
> > Regards,
> > Matthias