drivers/firmware/efi/esrt.c | 47 ++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 22 deletions(-)
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@invisiblethingslab.com>
---
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);
pr_debug("esrt-init: loaded.\n");
--
2.25.4
Hi Marek,
On Sun, 16 Aug 2020 at 02:20, Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> 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@invisiblethingslab.com>
> ---
> 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
>
On Mon, Aug 17, 2020 at 10:16:07AM +0200, Ard Biesheuvel wrote:
> > @@ -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.
In case of Xen, it is Xen responsibility to do that. Otherwise even if dom0
would not use it, Xen could allocate that physical memory to another
guest.
--
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?
© 2016 - 2024 Red Hat, Inc.