* [PATCH v2] Add support for ESRT loading under Xen
@ 2022-08-28 2:51 Demi Marie Obenour
2022-09-05 11:46 ` Ard Biesheuvel
0 siblings, 1 reply; 3+ messages in thread
From: Demi Marie Obenour @ 2022-08-28 2:51 UTC (permalink / raw)
To: Ard Biesheuvel, Juergen Gross, Stefano Stabellini, Oleksandr Tyshchenko
Cc: Demi Marie Obenour, linux-efi, linux-kernel, xen-devel
This is needed for fwupd to work in Qubes OS.
Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
---
Changes since v1:
- Use a different type (struct xen_efi_mem_info) for memory information
provided by Xen, as Xen reports it in a different way than the
standard Linux functions do.
drivers/firmware/efi/esrt.c | 49 +++++++++++++++++++++++++++----------
drivers/xen/efi.c | 32 ++++++++++++++++++++++++++
include/linux/efi.h | 18 ++++++++++++++
3 files changed, 86 insertions(+), 13 deletions(-)
diff --git a/drivers/firmware/efi/esrt.c b/drivers/firmware/efi/esrt.c
index 2a2f52b017e736dd995c69e8aeb5fbd7761732e5..c0fc149a838044cc16bb08a374a0c8ea6b7dcbff 100644
--- a/drivers/firmware/efi/esrt.c
+++ b/drivers/firmware/efi/esrt.c
@@ -243,27 +243,50 @@ 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;
+ uint32_t 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);
+ } else if (IS_ENABLED(CONFIG_XEN_EFI) && efi_enabled(EFI_PARAVIRT)) {
+ struct xen_efi_mem_info info;
+
+ if (!xen_efi_mem_info_query(efi.esrt, &info)) {
+ pr_warn("Failed to lookup ESRT header in Xen memory map\n");
+ return;
+ }
+
+ type = info.type;
+ max = info.addr + info.size;
+
+ /* Recent Xen versions relocate the ESRT to memory of type
+ * EfiRuntimeServicesData, which Xen will not reuse. If the ESRT
+ * is not in EfiRuntimeServicesData memory, it has not been reserved
+ * by Xen and might be allocated to other guests, so it cannot
+ * safely be used. */
+ if (type != EFI_RUNTIME_SERVICES_DATA) {
+ pr_warn("Xen did not reserve ESRT, ignoring it\n");
+ return;
+ }
+ } 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 +356,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");
diff --git a/drivers/xen/efi.c b/drivers/xen/efi.c
index d1ff2186ebb48a7c0981ecb6d4afcbbb25ffcea0..b313f213822f0fd5ba6448f6f6f453cfda4c7e23 100644
--- a/drivers/xen/efi.c
+++ b/drivers/xen/efi.c
@@ -26,6 +26,7 @@
#include <xen/interface/xen.h>
#include <xen/interface/platform.h>
+#include <xen/page.h>
#include <xen/xen.h>
#include <xen/xen-ops.h>
@@ -40,6 +41,37 @@
#define efi_data(op) (op.u.efi_runtime_call)
+static_assert(XEN_PAGE_SHIFT == EFI_PAGE_SHIFT,
+ "Mismatch between EFI_PAGE_SHIFT and XEN_PAGE_SHIFT");
+
+bool xen_efi_mem_info_query(u64 phys_addr, struct xen_efi_mem_info *md)
+{
+ struct xen_platform_op op = {
+ .cmd = XENPF_firmware_info,
+ .u.firmware_info = {
+ .type = XEN_FW_EFI_INFO,
+ .index = XEN_FW_EFI_MEM_INFO,
+ .u.efi_info.mem.addr = phys_addr,
+ .u.efi_info.mem.size = ((u64)-1ULL) - phys_addr,
+ }
+ };
+ union xenpf_efi_info *info = &op.u.firmware_info.u.efi_info;
+ int rc;
+
+ memset(md, 0, sizeof(*md)); /* initialize md even on failure */
+ rc = HYPERVISOR_platform_op(&op);
+ if (rc) {
+ pr_warn("Could not obtain information on address %llu from Xen: "
+ "error %d\n", phys_addr, rc);
+ return false;
+ }
+ md->addr = info->mem.addr;
+ md->size = info->mem.size;
+ md->attr = info->mem.attr;
+ md->type = info->mem.type;
+ return true;
+}
+
static efi_status_t xen_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
{
struct xen_platform_op op = INIT_EFI_OP(get_time);
diff --git a/include/linux/efi.h b/include/linux/efi.h
index d2b84c2fec39f0268324d1a38a73ed67786973c9..0598869cdc924aef0e2b9cacc4450b728e1a98c7 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1327,1 +1327,19 @@ struct linux_efi_coco_secret_area {
+/* Result of a XEN_FW_EFI_MEM_INFO query */
+struct xen_efi_mem_info {
+ uint64_t addr; /* address queried */
+ uint64_t size; /* remaining bytes in memory region */
+ uint64_t attr; /* attributes */
+ uint32_t type; /* type */
+};
+
+#if IS_ENABLED(CONFIG_XEN_EFI)
+extern bool xen_efi_mem_info_query(u64 phys_addr, struct xen_efi_mem_info *out_md);
+#else
+static inline bool xen_efi_mem_info_query(u64 phys_addr, struct xen_efi_mem_info *out_md)
+{
+ BUILD_BUG();
+ return false;
+}
+#endif
+
#endif /* _LINUX_EFI_H */
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] Add support for ESRT loading under Xen
2022-08-28 2:51 [PATCH v2] Add support for ESRT loading under Xen Demi Marie Obenour
@ 2022-09-05 11:46 ` Ard Biesheuvel
2022-09-13 13:27 ` Demi Marie Obenour
0 siblings, 1 reply; 3+ messages in thread
From: Ard Biesheuvel @ 2022-09-05 11:46 UTC (permalink / raw)
To: Demi Marie Obenour
Cc: Juergen Gross, Stefano Stabellini, Oleksandr Tyshchenko,
linux-efi, linux-kernel, xen-devel
On Sun, 28 Aug 2022 at 04:52, Demi Marie Obenour
<demi@invisiblethingslab.com> wrote:
>
> This is needed for fwupd to work in Qubes OS.
>
Please elaborate on:
- the current situation
- why this is a problem
- why your approach is a reasonable solution.
> Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
> ---
> Changes since v1:
>
> - Use a different type (struct xen_efi_mem_info) for memory information
> provided by Xen, as Xen reports it in a different way than the
> standard Linux functions do.
>
> drivers/firmware/efi/esrt.c | 49 +++++++++++++++++++++++++++----------
> drivers/xen/efi.c | 32 ++++++++++++++++++++++++++
> include/linux/efi.h | 18 ++++++++++++++
> 3 files changed, 86 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/firmware/efi/esrt.c b/drivers/firmware/efi/esrt.c
> index 2a2f52b017e736dd995c69e8aeb5fbd7761732e5..c0fc149a838044cc16bb08a374a0c8ea6b7dcbff 100644
> --- a/drivers/firmware/efi/esrt.c
> +++ b/drivers/firmware/efi/esrt.c
> @@ -243,27 +243,50 @@ 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;
> + uint32_t 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);
> + } else if (IS_ENABLED(CONFIG_XEN_EFI) && efi_enabled(EFI_PARAVIRT)) {
> + struct xen_efi_mem_info info;
> +
> + if (!xen_efi_mem_info_query(efi.esrt, &info)) {
> + pr_warn("Failed to lookup ESRT header in Xen memory map\n");
> + return;
> + }
> +
> + type = info.type;
> + max = info.addr + info.size;
> +
> + /* Recent Xen versions relocate the ESRT to memory of type
> + * EfiRuntimeServicesData, which Xen will not reuse. If the ESRT
This violates the EFI spec, which spells out very clearly that the
ESRT must be in EfiBootServicesData memory. Why are you deviating from
this?
> + * is not in EfiRuntimeServicesData memory, it has not been reserved
> + * by Xen and might be allocated to other guests, so it cannot
> + * safely be used. */
> + if (type != EFI_RUNTIME_SERVICES_DATA) {
> + pr_warn("Xen did not reserve ESRT, ignoring it\n");
> + return;
> + }
> + } 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 +356,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");
> diff --git a/drivers/xen/efi.c b/drivers/xen/efi.c
> index d1ff2186ebb48a7c0981ecb6d4afcbbb25ffcea0..b313f213822f0fd5ba6448f6f6f453cfda4c7e23 100644
> --- a/drivers/xen/efi.c
> +++ b/drivers/xen/efi.c
> @@ -26,6 +26,7 @@
>
> #include <xen/interface/xen.h>
> #include <xen/interface/platform.h>
> +#include <xen/page.h>
> #include <xen/xen.h>
> #include <xen/xen-ops.h>
>
> @@ -40,6 +41,37 @@
>
> #define efi_data(op) (op.u.efi_runtime_call)
>
> +static_assert(XEN_PAGE_SHIFT == EFI_PAGE_SHIFT,
> + "Mismatch between EFI_PAGE_SHIFT and XEN_PAGE_SHIFT");
> +
> +bool xen_efi_mem_info_query(u64 phys_addr, struct xen_efi_mem_info *md)
> +{
> + struct xen_platform_op op = {
> + .cmd = XENPF_firmware_info,
> + .u.firmware_info = {
> + .type = XEN_FW_EFI_INFO,
> + .index = XEN_FW_EFI_MEM_INFO,
> + .u.efi_info.mem.addr = phys_addr,
> + .u.efi_info.mem.size = ((u64)-1ULL) - phys_addr,
> + }
> + };
> + union xenpf_efi_info *info = &op.u.firmware_info.u.efi_info;
> + int rc;
> +
> + memset(md, 0, sizeof(*md)); /* initialize md even on failure */
> + rc = HYPERVISOR_platform_op(&op);
> + if (rc) {
> + pr_warn("Could not obtain information on address %llu from Xen: "
> + "error %d\n", phys_addr, rc);
> + return false;
> + }
> + md->addr = info->mem.addr;
> + md->size = info->mem.size;
> + md->attr = info->mem.attr;
> + md->type = info->mem.type;
> + return true;
> +}
> +
> static efi_status_t xen_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
> {
> struct xen_platform_op op = INIT_EFI_OP(get_time);
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index d2b84c2fec39f0268324d1a38a73ed67786973c9..0598869cdc924aef0e2b9cacc4450b728e1a98c7 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -1327,1 +1327,19 @@ struct linux_efi_coco_secret_area {
> +/* Result of a XEN_FW_EFI_MEM_INFO query */
> +struct xen_efi_mem_info {
> + uint64_t addr; /* address queried */
> + uint64_t size; /* remaining bytes in memory region */
> + uint64_t attr; /* attributes */
> + uint32_t type; /* type */
> +};
> +
> +#if IS_ENABLED(CONFIG_XEN_EFI)
> +extern bool xen_efi_mem_info_query(u64 phys_addr, struct xen_efi_mem_info *out_md);
> +#else
> +static inline bool xen_efi_mem_info_query(u64 phys_addr, struct xen_efi_mem_info *out_md)
> +{
> + BUILD_BUG();
> + return false;
> +}
> +#endif
> +
> #endif /* _LINUX_EFI_H */
> --
> Sincerely,
> Demi Marie Obenour (she/her/hers)
> Invisible Things Lab
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] Add support for ESRT loading under Xen
2022-09-05 11:46 ` Ard Biesheuvel
@ 2022-09-13 13:27 ` Demi Marie Obenour
0 siblings, 0 replies; 3+ messages in thread
From: Demi Marie Obenour @ 2022-09-13 13:27 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Juergen Gross, Stefano Stabellini, Oleksandr Tyshchenko,
linux-efi, linux-kernel, xen-devel
[-- Attachment #1: Type: text/plain, Size: 4840 bytes --]
On Mon, Sep 05, 2022 at 01:46:54PM +0200, Ard Biesheuvel wrote:
> On Sun, 28 Aug 2022 at 04:52, Demi Marie Obenour
> <demi@invisiblethingslab.com> wrote:
> >
> > This is needed for fwupd to work in Qubes OS.
> >
>
> Please elaborate on:
Will do in v3.
> - the current situation
The ESRT is not available in dom0 under Xen.
> - why this is a problem
fwupd requires the ESRT to be available in dom0. Without it, users
cannot update their firmware.
> - why your approach is a reasonable solution.
It is the approach already chosen by Xen upstream. See below for
details.
> > Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
> > ---
> > Changes since v1:
> >
> > - Use a different type (struct xen_efi_mem_info) for memory information
> > provided by Xen, as Xen reports it in a different way than the
> > standard Linux functions do.
> >
> > drivers/firmware/efi/esrt.c | 49 +++++++++++++++++++++++++++----------
> > drivers/xen/efi.c | 32 ++++++++++++++++++++++++++
> > include/linux/efi.h | 18 ++++++++++++++
> > 3 files changed, 86 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/firmware/efi/esrt.c b/drivers/firmware/efi/esrt.c
> > index 2a2f52b017e736dd995c69e8aeb5fbd7761732e5..c0fc149a838044cc16bb08a374a0c8ea6b7dcbff 100644
> > --- a/drivers/firmware/efi/esrt.c
> > +++ b/drivers/firmware/efi/esrt.c
> > @@ -243,27 +243,50 @@ 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;
> > + uint32_t 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);
> > + } else if (IS_ENABLED(CONFIG_XEN_EFI) && efi_enabled(EFI_PARAVIRT)) {
> > + struct xen_efi_mem_info info;
> > +
> > + if (!xen_efi_mem_info_query(efi.esrt, &info)) {
> > + pr_warn("Failed to lookup ESRT header in Xen memory map\n");
> > + return;
> > + }
> > +
> > + type = info.type;
> > + max = info.addr + info.size;
> > +
> > + /* Recent Xen versions relocate the ESRT to memory of type
> > + * EfiRuntimeServicesData, which Xen will not reuse. If the ESRT
>
> This violates the EFI spec, which spells out very clearly that the
> ESRT must be in EfiBootServicesData memory. Why are you deviating from
> this?
Xen will freely use EfiBootServicesData memory for its own purposes
after calling ExitBootServices(). In particular, such memory may be
allocated to, and become writable by, other guests. Since the ESRT is
(of necessity) trusted, it cannot be used by Linux unless it is
guaranteed to not be writable by other guests.
Earlier patches to Xen just reserved the region containing the ESRT in
the EFI memory map. However, this was tricky to implement correctly and
required a new platform op to alert dom0 that the ESRT had been reserved
by Xen. The final patch accepted upstream instead checks if the ESRT is
in EfiBootServicesData memory, and if it is, relocates it to
EfiRuntimeServicesData memory. This allowes using existing hypercalls
to check if the ESRT has been reserved by Xen, which is exactly what
this patch does.
If I recall correctly, some firmware already allocates the ESRT from
EfiRuntimeServicesData memory, so operating systems already need to
support this case. Furthermore, the ESRT must not be clobbered by the
OS or hypervisor, so EfiRuntimeServicesData is a more logical choice
anyway.
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-13 13:28 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-28 2:51 [PATCH v2] Add support for ESRT loading under Xen Demi Marie Obenour
2022-09-05 11:46 ` Ard Biesheuvel
2022-09-13 13:27 ` Demi Marie Obenour
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.