On Fri, 30 Sept 2022 at 01:02, Demi Marie Obenour
<demi@xxxxxxxxxxxxxxxxxxxxxx> wrote:
>
> fwupd requires access to the EFI System Resource Table (ESRT) to
> discover which firmware can be updated by the OS. Currently, Linux does
> not expose the ESRT when running as a Xen dom0. Therefore, it is not
> possible to use fwupd in a Xen dom0, which is a serious problem for e.g.
> Qubes OS.
>
> Before Xen 4.17, this was not fixable due to hypervisor limitations.
> The UEFI specification requires the ESRT to be in EfiBootServicesData
> memory, which Xen will use for whatever purposes it likes. Therefore,
> Linux cannot safely access the ESRT, as Xen may have overwritten it.
>
> Starting with Xen 4.17, Xen checks if the ESRT is in EfiBootServicesData
> or EfiRuntimeServicesData memory. If the ESRT is in EfiBootServicesData
> memory, Xen replaces the ESRT with a copy in memory that it has
> reserved. Such memory is currently of type EFI_RUNTIME_SERVICES_DATA,
> but in the future it will be of type EFI_ACPI_RECLAIM_MEMORY. This
> ensures that the ESRT can safely be accessed by the OS.
>
> When running as a Xen dom0, use the new
> xen_config_table_memory_region_max() function to determine if Xen has
> reserved the ESRT and, if so, find the end of the memory region
> containing it. This allows programs such as fwupd which require the
> ESRT to run under Xen, and so makes fwupd support in Qubes OS possible.
>
> Signed-off-by: Demi Marie Obenour <demi@xxxxxxxxxxxxxxxxxxxxxx>
Why do we need this patch? I'd expect esrt_table_exists() to return
false when patch 1/2 is applied.
> ---
> drivers/firmware/efi/esrt.c | 43 ++++++++++++++++++++++++++-----------
> 1 file changed, 30 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/firmware/efi/esrt.c b/drivers/firmware/efi/esrt.c
> index 2a2f52b017e736dd995c69e8aeb5fbd7761732e5..a0642bc161b4b1f94f818b8c9f46511fe2424bb2 100644
> --- a/drivers/firmware/efi/esrt.c
> +++ b/drivers/firmware/efi/esrt.c
> @@ -243,27 +243,44 @@ void __init efi_esrt_init(void)
> void *va;
> struct efi_system_resource_table tmpesrt;
> size_t size, max, entry_size, entries_size;
> - efi_memory_desc_t md;
> - int rc;
> phys_addr_t end;
> -
> - if (!efi_enabled(EFI_MEMMAP))
> - return;
> + u32 type;
>
> pr_debug("esrt-init: loading.\n");
> if (!esrt_table_exists())
> return;
>
> - rc = efi_mem_desc_lookup(efi.esrt, &md);
> - if (rc < 0 ||
> - (!(md.attribute & EFI_MEMORY_RUNTIME) &&
> - md.type != EFI_BOOT_SERVICES_DATA &&
> - md.type != EFI_RUNTIME_SERVICES_DATA)) {
> - pr_warn("ESRT header is not in the memory map.\n");
> + if (efi_enabled(EFI_MEMMAP)) {
> + efi_memory_desc_t md;
> +
> + if (efi_mem_desc_lookup(efi.esrt, &md) < 0 ||
> + (!(md.attribute & EFI_MEMORY_RUNTIME) &&
> + md.type != EFI_BOOT_SERVICES_DATA &&
> + md.type != EFI_RUNTIME_SERVICES_DATA)) {
> + pr_warn("ESRT header is not in the memory map.\n");
> + return;
> + }
> +
> + type = md.type;
> + max = efi_mem_desc_end(&md);
> +#ifdef CONFIG_XEN_EFI
> + } else if (efi_enabled(EFI_PARAVIRT)) {
> + max = xen_config_table_memory_region_max(efi.esrt);
> + /*
> + * This might be wrong, but it doesn't matter.
> + * xen_config_table_memory_region_max() checks the type
> + * of the memory region, and if it returns 0, the code
> + * below will fail without looking at the type. Choose
> + * a value that will not cause * subsequent code to try
> + * to reserve the memory containing the ESRT, as either
> + * Xen or the firmware has done so already.
> + */
> + type = EFI_RUNTIME_SERVICES_DATA;
> +#endif
> + } else {
> return;
> }
>
> - max = efi_mem_desc_end(&md);
> if (max < efi.esrt) {
> pr_err("EFI memory descriptor is invalid. (esrt: %p max: %p)\n",
> (void *)efi.esrt, (void *)max);
> @@ -333,7 +350,7 @@ 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 (type == EFI_BOOT_SERVICES_DATA)
> efi_mem_reserve(esrt_data, esrt_data_size);
>
> pr_debug("esrt-init: loaded.\n");
> --
> Sincerely,
> Demi Marie Obenour (she/her/hers)
> Invisible Things Lab
>
