fwupd support under Xen - firmware updates with the UEFI capsule

fwupd support under Xen - firmware updates with the UEFI capsule

[Norbert Kaminski]

[-- Attachment #1: Type: text/plain, Size: 3225 bytes --]

Hello all,

I'm trying to add support for the firmware updates with the UEFI capsule in
Qubes OS. I've got the troubles with reading ESRT (EFI System Resource 
Table)
in the dom0, which is based on the EFI memory map. The EFI_MEMMAP is not
enabled despite the loaded drivers (CONFIG_EFI, CONFIG_EFI_ESRT) and kernel
cmdline parameters (add_efi_memmap):

```
[    3.451249] efi: EFI_MEMMAP is not enabled.
```

The fwupd bases on the ESRT entries, which provide the system firmware 
GUID.
The GUID is checked using LVFS metadata, which contains information 
about updates.
When efi_memmap is not enabled, there are no ESRT entries in the sysfs, 
and fwupd
has no information about the system firmware GUID.  It is therefore not 
possible to
check whether updates are available for the BIOS.

This is how the ESRT entries looks in the Ubuntu:

```
ubuntu@ubuntu:/sys/firmware/efi/esrt$ ll
total 0
drwxr-xr-x 3 root root    0 Jul 27 13:14 ./
drwxr-xr-x 6 root root    0 Jul 27 13:13 ../
drwxr-xr-x 3 root root    0 Jul 27 13:17 entries/
-r-------- 1 root root 4096 Jul 27 13:17 fw_resource_count
-r-------- 1 root root 4096 Jul 27 13:17 fw_resource_count_max
-r-------- 1 root root 4096 Jul 27 13:17 fw_resource_version
ubuntu@ubuntu:/sys/firmware/efi/esrt/entries/entry0$ ll
total 0
drwxr-xr-x 2 root root    0 Jul 27 13:17 ./
drwxr-xr-x 3 root root    0 Jul 27 13:17 ../
-r-------- 1 root root 4096 Jul 27 13:17 capsule_flags
-r-------- 1 root root 4096 Jul 27 13:17 fw_class
-r-------- 1 root root 4096 Jul 27 13:17 fw_type
-r-------- 1 root root 4096 Jul 27 13:17 fw_version
-r-------- 1 root root 4096 Jul 27 13:17 last_attempt_status
-r-------- 1 root root 4096 Jul 27 13:17 last_attempt_version
-r-------- 1 root root 4096 Jul 27 13:17 lowest_supported_fw_version
ubuntu@ubuntu:/sys/firmware/efi/esrt/entries/entry0$ sudo cat fw_class
34578c72-11dc-4378-bc7f-b643866f598c
```

This is the source code of the ESRT driver, which provides those 
directories:

https://gitlab.com/cki-project/kernel-ark/-/blob/os-build/drivers/firmware/efi/esrt.c 


EFI_MEMMAP dependency is in the 248th line:

https://gitlab.com/cki-project/kernel-ark/-/blob/os-build/drivers/firmware/efi/esrt.c#L248

I need to pass ESRT to the dom0. What would be the best way to do that?

Ps. Marek Marczykowski-Górecki (Qubes /Project lead) /found some more 
information,
where the problem lays:

/EFI_MEMMAP is not enabled on EFI_PARAVIRT (which I believe is the case 
on Xen dom0):/

/https://github.com/torvalds/linux/blob/92ed301919932f777713b9172e525674157e983d/drivers/firmware/efi/memmap.c#L110/

/My reading the source code says the Xen side to extract this info 
exists, but
Linux doesn't use it specifically, EFI config table address is get here:/

/https://github.com/torvalds/linux/blob/master/arch/x86/xen/efi.c#L56-L63/

/But then nothing uses efi_systab_xen.tables.
efi_config_parse_tables() function should be called on those addresses:
/

/https://github.com/torvalds/linux/blob/master/drivers/firmware/efi/efi.c#L542
/

/But I don't think it is called in PV dom0 boot path (not fully sure 
about that yet)./


Best Regards,
Norbert Kamiński
Junior Embedded Systems Engineer
GPG key ID: 9E9F90AFE10F466A
3mdeb.com


[-- Attachment #2: Type: text/html, Size: 5509 bytes --]

Re: fwupd support under Xen - firmware updates with the UEFI capsule

[Jan Beulich]

On 28.07.2020 09:41, Norbert Kaminski wrote:
> I'm trying to add support for the firmware updates with the UEFI capsule in
> Qubes OS. I've got the troubles with reading ESRT (EFI System Resource Table)
> in the dom0, which is based on the EFI memory map. The EFI_MEMMAP is not
> enabled despite the loaded drivers (CONFIG_EFI, CONFIG_EFI_ESRT) and kernel
> cmdline parameters (add_efi_memmap):
> 
> ```
> [    3.451249] efi: EFI_MEMMAP is not enabled.
> ```

It is, according to my understanding, a layering violation to expose
the EFI memory map to Dom0. It's not supposed to make use of this
information in any way. Hence any functionality depending on its use
also needs to be implemented in the hypervisor, with Dom0 making a
suitable hypercall to access this functionality. (And I find it
quite natural to expect that Xen gets involved in an update of the
firmware of a system.)

Jan

Re: fwupd support under Xen - firmware updates with the UEFI capsule

[Andrew Cooper]

On 28/07/2020 21:00, Jan Beulich wrote:
> On 28.07.2020 09:41, Norbert Kaminski wrote:
>> I'm trying to add support for the firmware updates with the UEFI
>> capsule in
>> Qubes OS. I've got the troubles with reading ESRT (EFI System
>> Resource Table)
>> in the dom0, which is based on the EFI memory map. The EFI_MEMMAP is not
>> enabled despite the loaded drivers (CONFIG_EFI, CONFIG_EFI_ESRT) and
>> kernel
>> cmdline parameters (add_efi_memmap):
>>
>> ```
>> [    3.451249] efi: EFI_MEMMAP is not enabled.
>> ```
>
> It is, according to my understanding, a layering violation to expose
> the EFI memory map to Dom0. It's not supposed to make use of this
> information in any way. Hence any functionality depending on its use
> also needs to be implemented in the hypervisor, with Dom0 making a
> suitable hypercall to access this functionality. (And I find it
> quite natural to expect that Xen gets involved in an update of the
> firmware of a system.)

ERST is a table (read only by the looks of things) which is a catalogue
of various bits of firmware in the system, including GUIDs for
identification, and version information.

It is the kind of data which the hardware domain should have access to,
and AFAICT, behaves just like a static ACPI table.

Presumably it wants to an E820 reserved region so dom0 gets indent
access, and something in the EFI subsystem needs extending to pass the
ERST address to dom0.

~Andrew

Re: fwupd support under Xen - firmware updates with the UEFI capsule

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 3673 bytes --]

On Tue, Jul 28, 2020 at 10:01:33PM +0100, Andrew Cooper wrote:
> On 28/07/2020 21:00, Jan Beulich wrote:
> > On 28.07.2020 09:41, Norbert Kaminski wrote:
> >> I'm trying to add support for the firmware updates with the UEFI
> >> capsule in
> >> Qubes OS. I've got the troubles with reading ESRT (EFI System
> >> Resource Table)
> >> in the dom0, which is based on the EFI memory map. The EFI_MEMMAP is not
> >> enabled despite the loaded drivers (CONFIG_EFI, CONFIG_EFI_ESRT) and
> >> kernel
> >> cmdline parameters (add_efi_memmap):
> >>
> >> ```
> >> [    3.451249] efi: EFI_MEMMAP is not enabled.
> >> ```
> >
> > It is, according to my understanding, a layering violation to expose
> > the EFI memory map to Dom0. It's not supposed to make use of this
> > information in any way. Hence any functionality depending on its use
> > also needs to be implemented in the hypervisor, with Dom0 making a
> > suitable hypercall to access this functionality. (And I find it
> > quite natural to expect that Xen gets involved in an update of the
> > firmware of a system.)
> 
> ERST is a table (read only by the looks of things) which is a catalogue
> of various bits of firmware in the system, including GUIDs for
> identification, and version information.
> 
> It is the kind of data which the hardware domain should have access to,
> and AFAICT, behaves just like a static ACPI table.
> 
> Presumably it wants to an E820 reserved region so dom0 gets indent
> access, and something in the EFI subsystem needs extending to pass the
> ERST address to dom0.

I think most (if not all) pieces in Xen are already there - there is
XENPF_firmware_info with XEN_EFW_EFI_INFO + XEN_FW_EFI_CONFIG_TABLE
that gives address of the EFI config table. Linux saves it in
efi_systab_xen.tables (arch/x86/xen/efi.c:xen_efi_probe().
I haven't figured out yet if it does anything with that information, but
the content of /sys/firmware/efi/systab suggests it does.

It seems ESRT driver in Linux uses memmap just for some sanity checks
(if the ESRT points at memory with EFI_MEMORY_RUNTIME and appropriate
type). Perhaps the check (if really necessary) can be added to Xen and
in case of dom0 simply skipped in Linux.

Norbert, if you're brave enough ;) I would suggests trying the (Linux)
patch below:

-----8<-----
diff --git a/drivers/firmware/efi/esrt.c b/drivers/firmware/efi/esrt.c
index e3d692696583..a2a5ccbb00a8 100644
--- a/drivers/firmware/efi/esrt.c
+++ b/drivers/firmware/efi/esrt.c
@@ -245,13 +245,14 @@ void __init efi_esrt_init(void)
 	int rc;
 	phys_addr_t end;
 
-	if (!efi_enabled(EFI_MEMMAP))
+	if (!efi_enabled(EFI_MEMMAP) && !efi_enabled(EFI_PARAVIRT))
 		return;
 
 	pr_debug("esrt-init: loading.\n");
 	if (!esrt_table_exists())
 		return;
 
+	if (!efi_enabled(EFI_PARAVIRT)) {
 	rc = efi_mem_desc_lookup(efi.esrt, &md);
 	if (rc < 0 ||
 	    (!(md.attribute & EFI_MEMORY_RUNTIME) &&
@@ -276,6 +277,7 @@ void __init efi_esrt_init(void)
 		       size, max);
 		return;
 	}
+	}
 
 	va = early_memremap(efi.esrt, size);
 	if (!va) {
@@ -331,7 +333,8 @@ void __init efi_esrt_init(void)
 
 	end = esrt_data + size;
 	pr_info("Reserving ESRT space from %pa to %pa.\n", &esrt_data, &end);
-	if (md.type == EFI_BOOT_SERVICES_DATA)
+
+	if (!efi_enabled(EFI_PARAVIRT) && md.type == EFI_BOOT_SERVICES_DATA)
 		efi_mem_reserve(esrt_data, esrt_data_size);
 
 	pr_debug("esrt-init: loaded.\n");
----8<-----


-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: fwupd support under Xen - firmware updates with the UEFI capsule

[Jan Beulich]

On 28.07.2020 23:01, Andrew Cooper wrote:
> On 28/07/2020 21:00, Jan Beulich wrote:
>> On 28.07.2020 09:41, Norbert Kaminski wrote:
>>> I'm trying to add support for the firmware updates with the UEFI
>>> capsule in
>>> Qubes OS. I've got the troubles with reading ESRT (EFI System
>>> Resource Table)
>>> in the dom0, which is based on the EFI memory map. The EFI_MEMMAP is not
>>> enabled despite the loaded drivers (CONFIG_EFI, CONFIG_EFI_ESRT) and
>>> kernel
>>> cmdline parameters (add_efi_memmap):
>>>
>>> ```
>>> [    3.451249] efi: EFI_MEMMAP is not enabled.
>>> ```
>>
>> It is, according to my understanding, a layering violation to expose
>> the EFI memory map to Dom0. It's not supposed to make use of this
>> information in any way. Hence any functionality depending on its use
>> also needs to be implemented in the hypervisor, with Dom0 making a
>> suitable hypercall to access this functionality. (And I find it
>> quite natural to expect that Xen gets involved in an update of the
>> firmware of a system.)
> 
> ERST is a table (read only by the looks of things) which is a catalogue
> of various bits of firmware in the system, including GUIDs for
> identification, and version information.
> 
> It is the kind of data which the hardware domain should have access to,
> and AFAICT, behaves just like a static ACPI table.

I'm unaware of us hiding this table, so Dom0 has access.

> Presumably it wants to an E820 reserved region so dom0 gets indent
> access, and something in the EFI subsystem needs extending to pass the
> ERST address to dom0.

I'm afraid the beginning of this sentence is such that I can't guess
what exactly you mean. As per above - I don't see why Dom0 wouldn't
have access to ERST. What it doesn't (and shouldn't) have access to is
the raw EFI memory map (there's no E820 map there). There is a way
for Dom0 to get at some "cooked" memory map (XENMEM_machine_memory_map),
but of course in a r/o fashion only.

Jan

Re: fwupd support under Xen - firmware updates with the UEFI capsule

[norbert.kaminski]

On 29.07.2020 00:16, Marek Marczykowski-Górecki wrote:
> On Tue, Jul 28, 2020 at 10:01:33PM +0100, Andrew Cooper wrote:
>> On 28/07/2020 21:00, Jan Beulich wrote:
>> > On 28.07.2020 09:41, Norbert Kaminski wrote:
>> >> I'm trying to add support for the firmware updates with the UEFI
>> >> capsule in
>> >> Qubes OS. I've got the troubles with reading ESRT (EFI System
>> >> Resource Table)
>> >> in the dom0, which is based on the EFI memory map. The EFI_MEMMAP is not
>> >> enabled despite the loaded drivers (CONFIG_EFI, CONFIG_EFI_ESRT) and
>> >> kernel
>> >> cmdline parameters (add_efi_memmap):
>> >>
>> >> ```
>> >> [    3.451249] efi: EFI_MEMMAP is not enabled.
>> >> ```
>> >
>> > It is, according to my understanding, a layering violation to expose
>> > the EFI memory map to Dom0. It's not supposed to make use of this
>> > information in any way. Hence any functionality depending on its use
>> > also needs to be implemented in the hypervisor, with Dom0 making a
>> > suitable hypercall to access this functionality. (And I find it
>> > quite natural to expect that Xen gets involved in an update of the
>> > firmware of a system.)
>> 
>> ERST is a table (read only by the looks of things) which is a 
>> catalogue
>> of various bits of firmware in the system, including GUIDs for
>> identification, and version information.
>> 
>> It is the kind of data which the hardware domain should have access 
>> to,
>> and AFAICT, behaves just like a static ACPI table.
>> 
>> Presumably it wants to an E820 reserved region so dom0 gets indent
>> access, and something in the EFI subsystem needs extending to pass the
>> ERST address to dom0.
> 
> I think most (if not all) pieces in Xen are already there - there is
> XENPF_firmware_info with XEN_EFW_EFI_INFO + XEN_FW_EFI_CONFIG_TABLE
> that gives address of the EFI config table. Linux saves it in
> efi_systab_xen.tables (arch/x86/xen/efi.c:xen_efi_probe().
> I haven't figured out yet if it does anything with that information, 
> but
> the content of /sys/firmware/efi/systab suggests it does.
> 
> It seems ESRT driver in Linux uses memmap just for some sanity checks
> (if the ESRT points at memory with EFI_MEMORY_RUNTIME and appropriate
> type). Perhaps the check (if really necessary) can be added to Xen and
> in case of dom0 simply skipped in Linux.
> 
> Norbert, if you're brave enough ;) I would suggests trying the (Linux)
> patch below:
> 
> -----8<-----
> diff --git a/drivers/firmware/efi/esrt.c b/drivers/firmware/efi/esrt.c
> index e3d692696583..a2a5ccbb00a8 100644
> --- a/drivers/firmware/efi/esrt.c
> +++ b/drivers/firmware/efi/esrt.c
> @@ -245,13 +245,14 @@ void __init efi_esrt_init(void)
>  	int rc;
>  	phys_addr_t end;
> 
> -	if (!efi_enabled(EFI_MEMMAP))
> +	if (!efi_enabled(EFI_MEMMAP) && !efi_enabled(EFI_PARAVIRT))
>  		return;
> 
>  	pr_debug("esrt-init: loading.\n");
>  	if (!esrt_table_exists())
>  		return;
> 
> +	if (!efi_enabled(EFI_PARAVIRT)) {
>  	rc = efi_mem_desc_lookup(efi.esrt, &md);
>  	if (rc < 0 ||
>  	    (!(md.attribute & EFI_MEMORY_RUNTIME) &&
> @@ -276,6 +277,7 @@ void __init efi_esrt_init(void)
>  		       size, max);
>  		return;
>  	}
> +	}
> 
>  	va = early_memremap(efi.esrt, size);
>  	if (!va) {
> @@ -331,7 +333,8 @@ void __init efi_esrt_init(void)
> 
>  	end = esrt_data + size;
>  	pr_info("Reserving ESRT space from %pa to %pa.\n", &esrt_data, &end);
> -	if (md.type == EFI_BOOT_SERVICES_DATA)
> +
> +	if (!efi_enabled(EFI_PARAVIRT) && md.type == EFI_BOOT_SERVICES_DATA)
>  		efi_mem_reserve(esrt_data, esrt_data_size);
> 
>  	pr_debug("esrt-init: loaded.\n");
> ----8<-----
I've built the kernel with your patch. Unfortunately it doesn't bring 
expected
sysfs directories. We still need some changes here.

---
Best Regards,
Norbert Kamiński
Embedded Systems Engineer
GPG key ID: 9E9F90AFE10F466A
3mdeb.com