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

Warn on machines not booted in EFI mode #1220

Closed
jankratochvil opened this issue Jun 11, 2019 · 24 comments
Closed

Warn on machines not booted in EFI mode #1220

jankratochvil opened this issue Jun 11, 2019 · 24 comments

Comments

@jankratochvil
Copy link
Contributor

I even was installing MS-Windows (without success) to update my docking station firmware as fwupdmgr did not find any firmware update on it.
Only after I was suggested to boot in EFI I booted from EFI flashdrive and then I could easily update my docking station firmware.
Could fwupdmgr print some warning that some devices may not be listed when the system is no in EFI boot mode?

Please answer the following questions:

  • Operating system and version: Fedora 29
  • How did you install fwupd (ex: from source, pacman, apt-get, etc): dnf/rpm
  • Have you tried rebooting? yes
  • Are you using an NVMe disk? yes
  • Is secure boot enabled (only for the UEFI plugin)? no
@jankratochvil
Copy link
Contributor Author

The devices + were added in fwupdmgr get-devices after booting in EFI mode:
get-devices.txt

@hughsie
Copy link
Member

hughsie commented Jun 12, 2019

Could fwupdmgr print some warning that some devices may not be listed when the system is no in EFI boot mode?

How would we know that UEFI is available? When we're in compat-BIOS mode all the UEFI tables are completely invisible to us I think. Quite a few machines don't have UEFI available and I think it would be a mistake to show a warning on that kind of hardware.

@jankratochvil
Copy link
Contributor Author

I think people using fwupdmgr have rather newer machines anyway and with more time there will be no longer any non-UEFI machines. Sure feel free to close this issue, IMO it could provide such warning. Then sure there could be also a blacklist of known non-UEFI machines but that is already a lot of work.

@hughsie
Copy link
Member

hughsie commented Jun 12, 2019

@superm1 do you know if there's anything we can get from DMI/SMBIOS that says "this is a UEFI-capable machine running in legacy BIOS mode"? If you reply before you get back from leave I'll shake my head disapprovingly. :)

@JohnAZoidberg
Copy link
Collaborator

Is it bad to print:

This device is not booted in UEFI mode. If it supports UEFI booting, please enable that. Otherwise fwupdmgr cannot update the system firmware or show its update status.

@superm1
Copy link
Member

superm1 commented Jul 8, 2019

@JohnAZoidberg
Yes that would be bad to print "generally" for at least the following reasons:

  • Some UEFI machines don't support UEFI capsule updates in the first place
  • Some system architectures don't use UEFI firmware
  • Some machines can be flashed using the flashrom plugin instead of the uefi plugin

@hughsie

Looking over the latest SMBIOS spec, I notice that BIOS characteristics extension byte 2, bit 3 should represent this when using SMBIOS 2.3 and later.

@superm1
Copy link
Member

superm1 commented Jul 9, 2019

Given the UEFI spec can at least indicate it's UEFI capable my thought is:

  1. In UEFI plugin initialization routine determine whether or not it's UEFI capable by checking SMBIOS tables.
  2. If it's UEFI capable but /sys/firmware/efi is missing then create an internal non-updatable dummy device with:
  • Title built from DMI strings (like UEFI does today)
  • Version captured from DMI information bios_version sysfs attribute
  • Pre-filled update_error with something along the lines of System is not booted in UEFI mode, unable to perform firmware updates

Then at least fwupdmgr get-devices should show the situation and give this "warning" that's desired.

Something else that I think would be useful but off topic for this project is if the OS installer could do this same check and warn someone before installing the OS to avoid people getting into this situation in the first place.

superm1 pushed a commit that referenced this issue Jul 9, 2019
* In startup, check BIOS characteristics for UEFI supported instead of for /sys/firmware/uefi
* In coldplug check for /sys/firmware/uefi
* If /sys/firwmare/uefi missing, create a dummy device telling the user it is in legacy mode
superm1 pushed a commit that referenced this issue Jul 9, 2019
* In startup, check BIOS characteristics for UEFI supported instead of for /sys/firmware/uefi
* In coldplug check for /sys/firmware/uefi
* If /sys/firwmare/uefi missing, create a dummy device telling the user it is in legacy mode
superm1 pushed a commit that referenced this issue Jul 10, 2019
* In startup, check BIOS characteristics for UEFI supported instead of for /sys/firmware/uefi
* In coldplug check for /sys/firmware/uefi
* If /sys/firwmare/uefi missing, create a dummy device telling the user it is in legacy mode
@jankratochvil
Copy link
Contributor Author

Thanks for the fix although I have to admit on my machine I see no change.
killall fwupd;fwupdmgr update prints nothing.
killall fwupd;fwupdmgr get-devices prints no warning, just the 3 devices in non-EFI mode (with EFI it should be 5 devices).
There is some new assertion error in output from /usr/libexec/fwupd/fwupd -v but I do not find that as user-visible indication s/he should turn on EFI.

log.diff.txt

@hughsie
Copy link
Member

hughsie commented Jul 11, 2019

@superm1 looks like something dodgy with the GError handling...

@superm1
Copy link
Member

superm1 commented Jul 11, 2019

@jankratochvil Can you please try that ^

@jankratochvil
Copy link
Contributor Author

jankratochvil commented Jul 11, 2019

I do not see much change, fwupdmgr itself still prints nothing, just the fwupd daemon now has a bit different messages as attached. But still nothing clear I should enable EFI. Diff of current wip/superm1/legacy-mode against current master:
diff12.txt

@hughsie
Copy link
Member

hughsie commented Jul 11, 2019

ignoring device 4f0e158ef035547f055c65c2d1d611313c20c491 [uefi] existing device e2623122c99d58220498aacbfcfdb1baebbae3c5 [amt] already exists

Maybe "UEFI" as a device ID isn't so good?

@superm1
Copy link
Member

superm1 commented Jul 11, 2019

I just pushed a modification to use UEFI-dummy as device ID instead, can you try that?

@jankratochvil
Copy link
Contributor Author

Here is the new /usr/libexec/fwupd/fwupd -v output. I still do not see any attempt to make fwupdmgr print anything. Couldn't you boot some your UEFI test machine in non-UEFI mode for debugging? Sending patch + logs back and forth is not so efficient debugging method from my own experience.
diff23.txt

@superm1
Copy link
Member

superm1 commented Jul 11, 2019

According to your logs, it looks like there should have been a device created now with that error titled "20KGS23S08 System Firmware", are you sure you didn't see it in fwupdmgr get-devices output?

@superm1
Copy link
Member

superm1 commented Jul 11, 2019

And regarding my own testing, my development machine does not support legacy mode.

@jankratochvil
Copy link
Contributor Author

According to your logs, it looks like there should have been a device created now with that error titled "20KGS23S08 System Firmware", are you sure you didn't see it in fwupdmgr get-devices output?

Right, I still see only the 3 non-EFI devices and not the 2 additional EFI-only ones, the situation is the same as I originally reported in: #1220 (comment)

@superm1
Copy link
Member

superm1 commented Jul 11, 2019 via email

@jankratochvil
Copy link
Contributor Author

jankratochvil commented Jul 11, 2019

fwupdmgr get-devices --show-all-devices

OK, yes, with these patches and with --show-all-devices there is now newly visible:

20KGS23S08 System Firmware
  DeviceId:             123fd4143619569d8ddb6ea47d1d3911eb5ef07a
  Guid:                 230c8b18-8d9b-53ec-838b-6cfc0383493a <- main-system-firmware
  Plugin:               uefi
  Flags:                internal|require-ac|registered|needs-reboot
  Vendor:               LENOVO
  Version:              N23ET63W (1.38 )
  VersionFormat:        plain
  Icon:                 computer
  Created:              2019-07-11
  UpdateError:          Firmware can not be updated in legacy mode, switch to UEFI mode.

Still not sure if I would figure out to use --show-all-devices, maybe without --show-all-devices it could print to stderr there is a device with an UpdateError. Although I should be able to patch this user interface change myself.

fwupd-output.tar.gz

@superm1
Copy link
Member

superm1 commented Jul 11, 2019

Are you sure that device doesn't show up without show all devices and this branch?

I thought we had a special case we show devices with an UpdateError. If not we can change that.

@jankratochvil
Copy link
Contributor Author

Are you sure that device doesn't show up without show all devices and this branch?

Check that fwupd-output.tar.gz file I posted above.

I thought we had a special case we show devices with an UpdateError. If not we can change that.

I do not see such behavior.

@superm1
Copy link
Member

superm1 commented Jul 11, 2019

Ah, I see the problem. We're not using the right function in fu_util_get_devices. I've pushed one more fix to that branch, can you try again?

@jankratochvil
Copy link
Contributor Author

Yes, it looks good now, thanks! I will test it for real when Lenovo releases next docking station update, if any. I am a bit curious if it was really called 20KGS23S08 System Firmware and not that ThinkPad Thunderbolt 3 Dock but the dock does not seem to be updateable (even in UEFI mode) and I cannot get the original firmware state of my computer. Anyway it is definitely an improvement.
fwupd-1.2.10-0.1alpha.fc29.x86_64-get-devices.txt

@superm1
Copy link
Member

superm1 commented Jul 11, 2019

Okay great thanks, as soon as CI passes I'll merge it.

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

4 participants