Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Firmware upgrade failed on headless devices #77

Open
santoshyadav30 opened this issue May 14, 2024 · 24 comments
Open

Firmware upgrade failed on headless devices #77

santoshyadav30 opened this issue May 14, 2024 · 24 comments

Comments

@santoshyadav30
Copy link

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 fwupd/fwupd#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
Copy link
Member

hughsie commented May 14, 2024

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
Copy link
Author

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
Copy link
Member

hughsie commented May 14, 2024

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
Copy link
Member

superm1 commented May 14, 2024

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

@santoshyadav30
Copy link
Author

@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
Copy link
Member

hughsie commented May 15, 2024

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
Copy link
Author

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
Copy link
Author

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

@mohdAnsariTariq
Copy link

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
Copy link
Author

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

@hughsie
Copy link
Member

hughsie commented Jun 13, 2024

@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
Copy link

@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
Copy link
Member

hughsie commented Jun 13, 2024

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
Copy link
Contributor

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

@superm1
Copy link
Member

superm1 commented Jun 13, 2024

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
Copy link
Member

hughsie commented Jun 13, 2024

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
Copy link
Member

superm1 commented Jun 13, 2024

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
Copy link

@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
Copy link
Member

superm1 commented Jun 14, 2024

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
Copy link
Member

hughsie commented Jun 14, 2024

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
Copy link
Member

superm1 commented Jun 14, 2024

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
Copy link
Author

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
Copy link
Member

hughsie commented Jun 18, 2024

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

@superm1
Copy link
Member

superm1 commented Jul 4, 2024

That sounds really counterintuitive to me too. No idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants