fwupd / fwupd-efi

EFI Application used by uefi-capsule plugin in fwupd
GNU Lesser General Public License v2.1
37 stars 18 forks source link

Firmware upgrade failed on headless devices #77

Open santoshyadav30 opened 4 months ago

santoshyadav30 commented 4 months ago

Using fwupd-efi for firmware upgrade by copying fwupdx64.efi and capsule files ( system fw capsule and ux-capsule ) to ESP.

Upgrade happens successfully if display monitor is connected to the device but it fails (stuck) if no display device is connected.

As per https://github.com/fwupd/fwupd/issues/6187 , this is a known limitation in fwupd.

Is this a limitation of fwupd-efi module or it is not supported by UpdateCapsule RT BIOS service itself ? Do we have any other workaround available for this except connecting HDMI dummy plugin?

hughsie commented 4 months ago

Is this a limitation of fwupd-efi module or it is not supported by UpdateCapsule RT BIOS service itself

IIUC, it's limitation of the capsule updater itself -- according to Dell the workaround is one of those dummy plugs -- which is a shame. If you find there's any way to override the display requirement (e.g. by setting an EFI key) please let us know and we can add it as a workaround.

santoshyadav30 commented 4 months ago

Thanks @hughsie for quick reply. Although, we are not using the Dell machine and having our own board with BIOS provided by AMI but the behavior is same for headless devices. Moreover, the same HDMI dummy plug workaround works on it too.

hughsie commented 4 months ago

Could you try not uploading an UX capsule and see if that affects anything perhaps? See https://github.com/fwupd/fwupd/blob/main/plugins/uefi-capsule/uefi-capsule.quirk#L7 for an example.

superm1 commented 4 months ago

For Dell it shouldn't affect anything as I understand. Dell doesn't use the UX capsule.

santoshyadav30 commented 4 months ago

@hughsie - I had already tried this by not uploading UX capsule. It works and opens the default "Firmware Upgrade" UI of the BIOS to show the progress. But if BIOS is password protected then it prompts for the password before starting the upgrade and we don't require any manual intervention. UX capsule solves this but it doesn't work for headless devices. Our requirement is that BIOS upgrade should happen for headless devices without prompting for BIOS password.

hughsie commented 4 months ago

It works and opens the default "Firmware Upgrade" UI

Ohh, so maybe we shouldn't upload a UX capsule if there is no display detected? I'm a bit confused why a UX capsule would make the password prompt go away tho.

santoshyadav30 commented 4 months ago

I think password will be prompted to show default bios progress UI as it can be opened only after authentication. But if we provide UX capsule then a new progress bar UI (present in id capsule) is displayed which doesn’t require BIOS authentication as it is not part of BIOS UI.

santoshyadav30 commented 4 months ago

@hughsie - Thanks for your quick replies. I think we can close this ticket.

mohdAnsariTariq commented 3 months ago

There is a workaround in fwupd-efi which can make it work for headless devices too.

So the problem with headless devices is that it gets stuck when it calls uefi_call_wrapper() for BS->OpenProtocol to get gop_guid and iface from the function fwup_get_gop_mode().

fwup_get_gop_mode() function stores the gop mode in ux_capsule_header's mode. For the solution, if device is headless, skip the call to fwup_get_gop_mode() and set ux_capsule_header's mode as 0.

Below is the change in file 'efi/fwupdate.c'

fwup_check_gop_for_ux_capsule(EFI_HANDLE loaded_image,
        UX_CAPSULE_HEADER *payload_hdr;
        EFI_STATUS rc;

+#ifdef DEVICE_HEADLESS
+       payload_hdr->mode = 0;
+#else
        payload_hdr = (UX_CAPSULE_HEADER *) (((UINT8 *) capsule) + capsule->HeaderSize);
        rc = fwup_get_gop_mode(&payload_hdr->mode, loaded_image);
        if (EFI_ERROR(rc))
                return EFI_UNSUPPORTED;

        fwup_update_ux_capsule_checksum(payload_hdr);
+#endif

        return EFI_SUCCESS;
 }
santoshyadav30 commented 3 months ago

Can we have this fix provided by @mohdAnsariTariq in repo ?

hughsie commented 3 months ago

@mohdAnsariTariq we can't add an ifdef in the upstream code -- but great detective work getting to a workaround. As an alternative, could we call fwup_get_gop_mode() and ignore the error on failure? e.g. something like:

diff --git a/efi/fwupdate.c b/efi/fwupdate.c
index ad5a6b1..8056355 100644
--- a/efi/fwupdate.c
+++ b/efi/fwupdate.c
@@ -385,8 +385,11 @@ fwup_check_gop_for_ux_capsule(EFI_HANDLE loaded_image,

    payload_hdr = (UX_CAPSULE_HEADER *) (((UINT8 *) capsule) + capsule->HeaderSize);
    rc = fwup_get_gop_mode(&payload_hdr->mode, loaded_image);
-       if (EFI_ERROR(rc))
-               return EFI_UNSUPPORTED;
+       if (EFI_ERROR(rc)) {
+               fwup_warning(L"Getting GOP mode failed, assuming mode=0: %r", rc);
+               payload_hdr->mode = 0;
+               return EFI_SUCCESS;
+       }

    fwup_update_ux_capsule_checksum(payload_hdr);
mohdAnsariTariq commented 3 months ago

@hughsie yes it would've been better if we could just ignore the error but the problem here is that it doesn't throw any error, it just gets stucked when it calls uefi_call_wrapper() for BS->OpenProtocol to get gop_guid and iface from the function fwup_get_gop_mode().

In case of headless device, we need to avoid the call to uefi_call_wrapper() ( or fwup_get_gop_mode()) function.

hughsie commented 3 months ago

In case of headless device

So how do we know it's headless before calling into BS->OpenProtocol? When headless is the handles not a zero sized array? @vathpela do you know this is supposed to work when headless perhaps?

vathpela commented 3 months ago

I really don't. I can't see why there should be any difference at all.

superm1 commented 3 months ago

TBH it sounds like a bug in the GOP driver.

But how about as a workaround we can detect if monitors connected in Linux and as the NVRAM variable set by Linux side we can indicate whether or not monitors were plugged in? We can know to skip all this from that information and maybe get headless working.

hughsie commented 3 months ago

Wouldn't that mean bumping the EfiUpdateInfo structure so we could pass quirk flags like this? We'd need to check the fwupd-efi version before sending a version==8 struct, and I guess also look at the FuDisplayState in the context. @mohdAnsariTariq does fwupd detect no monitors and put the display state in Disconnected?

superm1 commented 3 months ago

Wouldn't that mean bumping the EfiUpdateInfo structure so we could pass quirk flags like this? We'd need to check the fwupd-efi version before sending a version==8 struct, and I guess also look at the FuDisplayState in the context. @mohdAnsariTariq does fwupd detect no monitors and put the display state in Disconnected?

Yeah we would have to bump the structure. Remember we "install" the binary to the ESP though.

I think we can parse fwupd-efi binary to find out what version it supports to know whether to use this.

mohdAnsariTariq commented 3 months ago

@mohdAnsariTariq does fwupd detect no monitors and put the display state in Disconnected?

fwupd doesn't provide any direct method to detect if a monitor is connected or not.

But how about as a workaround we can detect if monitors connected in Linux and as the NVRAM variable set by Linux side we can indicate whether or not monitors were plugged in? We can know to skip all this from that information and maybe get headless working.

It seems a good workaround, going to try it. I'll let you know how it goes.

Wouldn't that mean bumping the EfiUpdateInfo structure so we could pass quirk flags like this? We'd need to check the fwupd-efi version before sending a version==8 struct, and I guess also look at the FuDisplayState in the context.

I couldn't get that, could you please explain it a little bit?

superm1 commented 3 months ago

I couldn't get that, could you please explain it a little bit?

The way that fwupdx64.efi knows what capsule to flash is because the uefi-capsule fwupd plugin writes an NVRAM variable describing it. The structure for this variable needs to be updated. Here is where it's written:

https://github.com/fwupd/fwupd/blob/main/plugins/uefi-capsule/fu-uefi-nvram-device.c#L111

Here is where it's parsed:

https://github.com/fwupd/fwupd-efi/blob/main/efi/fwupdate.c#L70

For a PoC you can just update the schema on both of them to have an extra variable about displays connected and then conditionalize what fwupdx64.efi does. If it works then the uefi-capsule plugin will also need to be updated to detect whether the fwupdx64.efi on the system supports the schema and decide which schema to write during an update.

hughsie commented 3 months ago

Back a bit; why are we setting a UX capsule at all if we have no displays connected? Could we avoid doing the GOP stuff if we have no UX capsule?

superm1 commented 3 months ago

Back a bit; why are we setting a UX capsule at all if we have no displays connected? Could we avoid doing the GOP stuff if we have no UX capsule?

Ooh that sounds even better and can be done entirely in fwupd plugin.

santoshyadav30 commented 3 months ago

Yes but if BIOS is password protected then it will get stuck on password prompt screen as device is headless. Setting UX capsule skips this.

hughsie commented 3 months ago

@superm1 do you know why setting the UX capsule would stop the password prompt? It's all a bit confusing.

superm1 commented 3 months ago

That sounds really counterintuitive to me too. No idea.