Hi Michal, On Thu, Jul 27, 2023 at 10:53:44AM +0200, Michal Simek wrote: > > > On 7/27/23 02:38, AKASHI Takahiro wrote: > > While UPDATE_CAPSULE api is not fully implemented, this interface and > > capsule-on-disk feature should behave in the same way, especially in > > handling an empty capsule for fwu multibank, for future enhancement. > > > > So move the guid check into efi_capsule_update_firmware(). > > > > Fixed: commit a6aafce494ab ("efi_loader: use efi_update_capsule_firmware() > > for capsule on disk") > > just fyi: b4 mess this. > You should likely put it on the same line and ignore line limit. > > This is how this ends up. > > handling an empty capsule for fwu multibank, for future enhancement. > > So move the guid check into efi_capsule_update_firmware(). > > for capsule on disk") > > Fixed: commit a6aafce494ab ("efi_loader: use efi_update_capsule_firmware() > Reported-by: Michal Simek <michal.si...@amd.com> > Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org> > Link: > https://lore.kernel.org/r/20230727003800.25105-1-takahiro.aka...@linaro.org > > > > > > Reported-by: Michal Simek <michal.si...@amd.com> > > Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org> > > --- > > lib/efi_loader/efi_capsule.c | 17 ++++++++--------- > > 1 file changed, 8 insertions(+), 9 deletions(-) > > > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > > index 7a6f195cbc02..ddf8153e0982 100644 > > --- a/lib/efi_loader/efi_capsule.c > > +++ b/lib/efi_loader/efi_capsule.c > > @@ -581,6 +581,13 @@ static efi_status_t efi_capsule_update_firmware( > > fw_accept_os = capsule_data->flags & FW_ACCEPT_OS ? 0x1 : 0x0; > > } > > + if (guidcmp(&capsule_data->capsule_guid, > > + &efi_guid_firmware_management_capsule_id)) { > > + log_err("Unsupported capsule type: %pUs\n", > > + &capsule_data->capsule_guid); > > + return EFI_UNSUPPORTED; > > + } > > + > > /* sanity check */ > > if (capsule_data->header_size < sizeof(*capsule) || > > capsule_data->header_size >= capsule_data->capsule_image_size) > > @@ -751,15 +758,7 @@ efi_status_t EFIAPI efi_update_capsule( > > log_debug("Capsule[%d] (guid:%pUs)\n", > > i, &capsule->capsule_guid); > > - if (!guidcmp(&capsule->capsule_guid, > > - &efi_guid_firmware_management_capsule_id)) { > > - ret = efi_capsule_update_firmware(capsule); > > - } else { > > - log_err("Unsupported capsule type: %pUs\n", > > - &capsule->capsule_guid); > > - ret = EFI_UNSUPPORTED; > > - } > > - > > + ret = efi_capsule_update_firmware(capsule); > > if (ret != EFI_SUCCESS) > > goto out; > > } > > I have no problem with this patch because it works as the previous one. When > commit message is fixed feel free to add > Tested-by: Michal Simek <michal.si...@amd.com>
I hope that the maintainer would take care of it when he tries to merge the patch. > And regarding empty capsule functionality with A/B. > I boot from A. Download capsules and run disk-update to get to Image B and > trial state and I can download and apply acceptance capsule by hand via > efidebug capsule update <addr>. That works fine for acceptance capsule is > reflected via fwu in mdata. > When I apply revert capsule there is nothing visible in mdata and I think it > should. The only visibility is that it resets to A system. Is this the only > intention of revert capsules? > (keep in mind that I use two images per bank). > > Empty capsules are just accepted only in trial state which is understandable. > > And I also see that with latest master branch capsule on disk feature is not > working properly. Capsule are not processed at all. Can you please double > check it? I locally ran the pytest with v2023.10-rc1, and - test_capsule_firmware_raw passed - test_capsule_firmware_signed_raw failed but it seems to me that 'signed_raw' failure is not directly related to efi implementation (I didn't dig into details, though). Can you describe more about your environment? -Takahiro Akashi > Thanks, > Michal > >