Hi Marek,
On Sun, 16 Aug 2020 at 02:20, Marek Marczykowski-Górecki
<marmarek@xxxxxxxxxxxxxxxxxxxxxx> wrote:
>
> In case of Xen PV dom0, Xen passes along info about system tables (see
> arch/x86/xen/efi.c), but not the memory map from EFI. This makes sense
> as it is Xen responsible for managing physical memory address space.
> In this case, it doesn't make sense to condition using ESRT table on
> availability of EFI memory map, as it isn't Linux kernel responsible for
> it. Skip this part on Xen PV (let Xen do the right thing if it deems
> necessary) and use ESRT table normally.
>
> This is a requirement for using fwupd in PV dom0 to update UEFI using
> capsules.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> ---
> drivers/firmware/efi/esrt.c | 47 ++++++++++++++++++++-----------------
> 1 file changed, 25 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/firmware/efi/esrt.c b/drivers/firmware/efi/esrt.c
> index d5915272141f..5c49f2aaa4b1 100644
> --- a/drivers/firmware/efi/esrt.c
> +++ b/drivers/firmware/efi/esrt.c
> @@ -245,36 +245,38 @@ 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;
>
> - 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");
> - return;
> - }
> + if (efi_enabled(EFI_MEMMAP)) {
> + 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");
> + 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);
> - 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);
> + return;
> + }
>
> - size = sizeof(*esrt);
> - max -= efi.esrt;
> + size = sizeof(*esrt);
> + max -= efi.esrt;
>
> - if (max < size) {
> - pr_err("ESRT header doesn't fit on single memory map entry. (size: %zu max: %zu)\n",
> - size, max);
> - return;
> + if (max < size) {
> + pr_err("ESRT header doesn't fit on single memory map entry. (size: %zu max: %zu)\n",
> + size, max);
> + return;
> + }
> }
>
> va = early_memremap(efi.esrt, size);
> @@ -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_MEMMAP) && md.type == EFI_BOOT_SERVICES_DATA)
> efi_mem_reserve(esrt_data, esrt_data_size);
>
This does not look correct to me. Why doesn't the region need to be
reserved on a Xen boot? The OS may overwrite it otherwise.
> pr_debug("esrt-init: loaded.\n");
> --
> 2.25.4
>
