Re: [PATCH v8 12/25] efi: Move exit_boot_services into a function
On 1/4/22 11:52, Simon Glass wrote:
Hi Heinrich,
On Thu, 30 Dec 2021 at 22:41, Heinrich Schuchardt wrote:
On 12/29/21 19:57, Simon Glass wrote:
At present this code is inline in the app and stub. But they do the same
thing. The difference is that the stub does it immediately and the app
doesn't want to do it until the end (when it boots a kernel) or not at
all, if returning to UEFI.
Move it into a function so it can be called as needed.
Also store the memory map so that it can be accessed within the app if
needed.
The memory map is *not* a static object. It may change with any API call
that you make. You must read the memory map immediately before calling
ExitBootServices(). The valid value of MapKey typically will change with
any change of the memory map. Calling ExitBootServices() with the wrong
value of MapKey will lead to a failure. Storing these values except for
immediate use makes no sense.
Signed-off-by: Simon Glass
---
(no changes since v6)
Changes in v6:
- Fix typo in function comment
Changes in v2:
- Add a sentence about what the patch does
include/efi.h | 32 ++
lib/efi/efi.c | 68 ++
lib/efi/efi_app.c | 3 ++
lib/efi/efi_stub.c | 66
4 files changed, 114 insertions(+), 55 deletions(-)
diff --git a/include/efi.h b/include/efi.h
index d4785478585..2a84223d235 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -407,6 +407,12 @@ static inline struct efi_mem_desc *efi_get_next_mem_desc(
* @sys_table: Pointer to system table
* @boot: Pointer to boot-services table
* @run: Pointer to runtime-services table
+ * @memmap_key: Key returned from get_memory_map()
+ * @memmap_desc: List of memory-map records
+ * @memmap_alloc: Amount of memory allocated for memory map list
+ * @memmap_size Size of memory-map list in bytes
+ * @memmap_desc_size: Size of an individual memory-map record, in bytes
+ * @memmap_version: Memory-map version
*
* @use_pool_for_malloc: true if all allocation should go through the EFI
'pool'
* methods allocate_pool() and free_pool(); false to use 'pages' methods
@@ -424,6 +430,12 @@ struct efi_priv {
struct efi_system_table *sys_table;
struct efi_boot_services *boot;
struct efi_runtime_services *run;
+ efi_uintn_t memmap_key;
+ struct efi_mem_desc *memmap_desc;
+ efi_uintn_t memmap_alloc;
+ efi_uintn_t memmap_size;
+ efi_uintn_t memmap_desc_size;
+ u32 memmap_version;
/* app: */
bool use_pool_for_malloc;
@@ -574,4 +586,24 @@ void efi_putc(struct efi_priv *priv, const char ch);
*/
int efi_info_get(enum efi_entry_t type, void **datap, int *sizep);
+/**
+ * efi_store_memory_map() - Collect the memory-map info from EFI
+ *
+ * Collect the memory info and store it for later use, e.g. in calling
+ * exit_boot_services()
+ *
+ * @priv:Pointer to private EFI structure
+ * @return 0 if OK, non-zero on error
+ */
+int efi_store_memory_map(struct efi_priv *priv);
+
+/**
+ * efi_call_exit_boot_services() - Handle the exit-boot-service procedure
+ *
+ * Tell EFI we don't want their boot services anymore
+ *
+ * Return: 0 if OK, non-zero on error
+ */
+int efi_call_exit_boot_services(void);
+
#endif /* _LINUX_EFI_H */
diff --git a/lib/efi/efi.c b/lib/efi/efi.c
index cd6bf47b180..20da88c9151 100644
--- a/lib/efi/efi.c
+++ b/lib/efi/efi.c
@@ -135,3 +135,71 @@ void efi_free(struct efi_priv *priv, void *ptr)
boot->free_pool(ptr);
}
+
+int efi_store_memory_map(struct efi_priv *priv)
+{
+ struct efi_boot_services *boot = priv->sys_table->boottime;
+ efi_uintn_t size, desc_size;
+ efi_status_t ret;
+
+ /* Get the memory map so we can switch off EFI */
+ size = 0;
+ ret = boot->get_memory_map(&size, NULL, &priv->memmap_key,
+&priv->memmap_desc_size,
+&priv->memmap_version);
+ if (ret != EFI_BUFFER_TOO_SMALL) {
+ printhex2(EFI_BITS_PER_LONG);
+ putc(' ');
+ printhex2(ret);
+ puts(" No memory map\n");
+ return ret;
+ }
+ /*
+ * Since doing a malloc() may change the memory map and also we want to
+ * be able to read the memory map in efi_call_exit_boot_services()
+ * below, after more changes have happened
+ */
+ priv->memmap_alloc = size + 1024;
GetMemoryMap() must be called immediately before calling ExitBootServices().
Yes that's right. I remember reading that in the spec.
If efi_malloc() invokes AllocatePages() or AllocatePohl(), you should
add DescriptorSize to MemoryMapSize and not some random value (e.g. 1024).
This is copying existing code. If you want to change this, we can do
it in a follow-on patch.
For that discussion, How do you know it will be increased by
sizeof(DescriptorSize) ? Is that in the spec somewhere?
In one run of the stub with qemu it
Re: [PATCH v8 12/25] efi: Move exit_boot_services into a function
Hi Heinrich,
On Thu, 30 Dec 2021 at 22:41, Heinrich Schuchardt wrote:
>
> On 12/29/21 19:57, Simon Glass wrote:
> > At present this code is inline in the app and stub. But they do the same
> > thing. The difference is that the stub does it immediately and the app
> > doesn't want to do it until the end (when it boots a kernel) or not at
> > all, if returning to UEFI.
> >
> > Move it into a function so it can be called as needed.
> >
> > Also store the memory map so that it can be accessed within the app if
> > needed.
>
> The memory map is *not* a static object. It may change with any API call
> that you make. You must read the memory map immediately before calling
> ExitBootServices(). The valid value of MapKey typically will change with
> any change of the memory map. Calling ExitBootServices() with the wrong
> value of MapKey will lead to a failure. Storing these values except for
> immediate use makes no sense.
>
> >
> > Signed-off-by: Simon Glass
> > ---
> >
> > (no changes since v6)
> >
> > Changes in v6:
> > - Fix typo in function comment
> >
> > Changes in v2:
> > - Add a sentence about what the patch does
> >
> > include/efi.h | 32 ++
> > lib/efi/efi.c | 68 ++
> > lib/efi/efi_app.c | 3 ++
> > lib/efi/efi_stub.c | 66
> > 4 files changed, 114 insertions(+), 55 deletions(-)
> >
> > diff --git a/include/efi.h b/include/efi.h
> > index d4785478585..2a84223d235 100644
> > --- a/include/efi.h
> > +++ b/include/efi.h
> > @@ -407,6 +407,12 @@ static inline struct efi_mem_desc
> > *efi_get_next_mem_desc(
> >* @sys_table: Pointer to system table
> >* @boot: Pointer to boot-services table
> >* @run: Pointer to runtime-services table
> > + * @memmap_key: Key returned from get_memory_map()
> > + * @memmap_desc: List of memory-map records
> > + * @memmap_alloc: Amount of memory allocated for memory map list
> > + * @memmap_size Size of memory-map list in bytes
> > + * @memmap_desc_size: Size of an individual memory-map record, in bytes
> > + * @memmap_version: Memory-map version
> >*
> >* @use_pool_for_malloc: true if all allocation should go through the EFI
> > 'pool'
> >* methods allocate_pool() and free_pool(); false to use 'pages' methods
> > @@ -424,6 +430,12 @@ struct efi_priv {
> > struct efi_system_table *sys_table;
> > struct efi_boot_services *boot;
> > struct efi_runtime_services *run;
> > + efi_uintn_t memmap_key;
> > + struct efi_mem_desc *memmap_desc;
> > + efi_uintn_t memmap_alloc;
> > + efi_uintn_t memmap_size;
> > + efi_uintn_t memmap_desc_size;
> > + u32 memmap_version;
> >
> > /* app: */
> > bool use_pool_for_malloc;
> > @@ -574,4 +586,24 @@ void efi_putc(struct efi_priv *priv, const char ch);
> >*/
> > int efi_info_get(enum efi_entry_t type, void **datap, int *sizep);
> >
> > +/**
> > + * efi_store_memory_map() - Collect the memory-map info from EFI
> > + *
> > + * Collect the memory info and store it for later use, e.g. in calling
> > + * exit_boot_services()
> > + *
> > + * @priv:Pointer to private EFI structure
> > + * @return 0 if OK, non-zero on error
> > + */
> > +int efi_store_memory_map(struct efi_priv *priv);
> > +
> > +/**
> > + * efi_call_exit_boot_services() - Handle the exit-boot-service procedure
> > + *
> > + * Tell EFI we don't want their boot services anymore
> > + *
> > + * Return: 0 if OK, non-zero on error
> > + */
> > +int efi_call_exit_boot_services(void);
> > +
> > #endif /* _LINUX_EFI_H */
> > diff --git a/lib/efi/efi.c b/lib/efi/efi.c
> > index cd6bf47b180..20da88c9151 100644
> > --- a/lib/efi/efi.c
> > +++ b/lib/efi/efi.c
> > @@ -135,3 +135,71 @@ void efi_free(struct efi_priv *priv, void *ptr)
> >
> > boot->free_pool(ptr);
> > }
> > +
> > +int efi_store_memory_map(struct efi_priv *priv)
> > +{
> > + struct efi_boot_services *boot = priv->sys_table->boottime;
> > + efi_uintn_t size, desc_size;
> > + efi_status_t ret;
> > +
> > + /* Get the memory map so we can switch off EFI */
> > + size = 0;
> > + ret = boot->get_memory_map(&size, NULL, &priv->memmap_key,
> > +&priv->memmap_desc_size,
> > +&priv->memmap_version);
> > + if (ret != EFI_BUFFER_TOO_SMALL) {
> > + printhex2(EFI_BITS_PER_LONG);
> > + putc(' ');
> > + printhex2(ret);
> > + puts(" No memory map\n");
> > + return ret;
> > + }
> > + /*
> > + * Since doing a malloc() may change the memory map and also we want
> > to
> > + * be able to read the memory map in efi_call_exit_boot_services()
> > + * below, after more changes have happened
> > + */
> > + priv->memmap_alloc = size + 1024;
>
> GetMemoryMap() must be called immediately before calling ExitBootServices().
Yes that's right. I remember reading
Re: [PATCH v8 12/25] efi: Move exit_boot_services into a function
On 12/29/21 19:57, Simon Glass wrote:
At present this code is inline in the app and stub. But they do the same
thing. The difference is that the stub does it immediately and the app
doesn't want to do it until the end (when it boots a kernel) or not at
all, if returning to UEFI.
Move it into a function so it can be called as needed.
Also store the memory map so that it can be accessed within the app if
needed.
The memory map is *not* a static object. It may change with any API call
that you make. You must read the memory map immediately before calling
ExitBootServices(). The valid value of MapKey typically will change with
any change of the memory map. Calling ExitBootServices() with the wrong
value of MapKey will lead to a failure. Storing these values except for
immediate use makes no sense.
Signed-off-by: Simon Glass
---
(no changes since v6)
Changes in v6:
- Fix typo in function comment
Changes in v2:
- Add a sentence about what the patch does
include/efi.h | 32 ++
lib/efi/efi.c | 68 ++
lib/efi/efi_app.c | 3 ++
lib/efi/efi_stub.c | 66
4 files changed, 114 insertions(+), 55 deletions(-)
diff --git a/include/efi.h b/include/efi.h
index d4785478585..2a84223d235 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -407,6 +407,12 @@ static inline struct efi_mem_desc *efi_get_next_mem_desc(
* @sys_table: Pointer to system table
* @boot: Pointer to boot-services table
* @run: Pointer to runtime-services table
+ * @memmap_key: Key returned from get_memory_map()
+ * @memmap_desc: List of memory-map records
+ * @memmap_alloc: Amount of memory allocated for memory map list
+ * @memmap_size Size of memory-map list in bytes
+ * @memmap_desc_size: Size of an individual memory-map record, in bytes
+ * @memmap_version: Memory-map version
*
* @use_pool_for_malloc: true if all allocation should go through the EFI
'pool'
*methods allocate_pool() and free_pool(); false to use 'pages' methods
@@ -424,6 +430,12 @@ struct efi_priv {
struct efi_system_table *sys_table;
struct efi_boot_services *boot;
struct efi_runtime_services *run;
+ efi_uintn_t memmap_key;
+ struct efi_mem_desc *memmap_desc;
+ efi_uintn_t memmap_alloc;
+ efi_uintn_t memmap_size;
+ efi_uintn_t memmap_desc_size;
+ u32 memmap_version;
/* app: */
bool use_pool_for_malloc;
@@ -574,4 +586,24 @@ void efi_putc(struct efi_priv *priv, const char ch);
*/
int efi_info_get(enum efi_entry_t type, void **datap, int *sizep);
+/**
+ * efi_store_memory_map() - Collect the memory-map info from EFI
+ *
+ * Collect the memory info and store it for later use, e.g. in calling
+ * exit_boot_services()
+ *
+ * @priv: Pointer to private EFI structure
+ * @return 0 if OK, non-zero on error
+ */
+int efi_store_memory_map(struct efi_priv *priv);
+
+/**
+ * efi_call_exit_boot_services() - Handle the exit-boot-service procedure
+ *
+ * Tell EFI we don't want their boot services anymore
+ *
+ * Return: 0 if OK, non-zero on error
+ */
+int efi_call_exit_boot_services(void);
+
#endif /* _LINUX_EFI_H */
diff --git a/lib/efi/efi.c b/lib/efi/efi.c
index cd6bf47b180..20da88c9151 100644
--- a/lib/efi/efi.c
+++ b/lib/efi/efi.c
@@ -135,3 +135,71 @@ void efi_free(struct efi_priv *priv, void *ptr)
boot->free_pool(ptr);
}
+
+int efi_store_memory_map(struct efi_priv *priv)
+{
+ struct efi_boot_services *boot = priv->sys_table->boottime;
+ efi_uintn_t size, desc_size;
+ efi_status_t ret;
+
+ /* Get the memory map so we can switch off EFI */
+ size = 0;
+ ret = boot->get_memory_map(&size, NULL, &priv->memmap_key,
+ &priv->memmap_desc_size,
+ &priv->memmap_version);
+ if (ret != EFI_BUFFER_TOO_SMALL) {
+ printhex2(EFI_BITS_PER_LONG);
+ putc(' ');
+ printhex2(ret);
+ puts(" No memory map\n");
+ return ret;
+ }
+ /*
+* Since doing a malloc() may change the memory map and also we want to
+* be able to read the memory map in efi_call_exit_boot_services()
+* below, after more changes have happened
+*/
+ priv->memmap_alloc = size + 1024;
GetMemoryMap() must be called immediately before calling ExitBootServices().
If efi_malloc() invokes AllocatePages() or AllocatePool(), you should
add DescriptorSize to MemoryMapSize and not some random value (e.g. 1024).
+ priv->memmap_size = priv->memmap_alloc;
+ priv->memmap_desc = efi_malloc(priv, size, &ret);
+ if (!priv->memmap_desc) {
+ printhex2(ret);
+ puts(" No memory for memory descriptor\n");
+ return ret;
+ }
+
+ ret = boot->get_memory_map(&priv->memmap_size, priv->memmap_desc,
+
[PATCH v8 12/25] efi: Move exit_boot_services into a function
At present this code is inline in the app and stub. But they do the same
thing. The difference is that the stub does it immediately and the app
doesn't want to do it until the end (when it boots a kernel) or not at
all, if returning to UEFI.
Move it into a function so it can be called as needed.
Also store the memory map so that it can be accessed within the app if
needed.
Signed-off-by: Simon Glass
---
(no changes since v6)
Changes in v6:
- Fix typo in function comment
Changes in v2:
- Add a sentence about what the patch does
include/efi.h | 32 ++
lib/efi/efi.c | 68 ++
lib/efi/efi_app.c | 3 ++
lib/efi/efi_stub.c | 66
4 files changed, 114 insertions(+), 55 deletions(-)
diff --git a/include/efi.h b/include/efi.h
index d4785478585..2a84223d235 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -407,6 +407,12 @@ static inline struct efi_mem_desc *efi_get_next_mem_desc(
* @sys_table: Pointer to system table
* @boot: Pointer to boot-services table
* @run: Pointer to runtime-services table
+ * @memmap_key: Key returned from get_memory_map()
+ * @memmap_desc: List of memory-map records
+ * @memmap_alloc: Amount of memory allocated for memory map list
+ * @memmap_size Size of memory-map list in bytes
+ * @memmap_desc_size: Size of an individual memory-map record, in bytes
+ * @memmap_version: Memory-map version
*
* @use_pool_for_malloc: true if all allocation should go through the EFI
'pool'
* methods allocate_pool() and free_pool(); false to use 'pages' methods
@@ -424,6 +430,12 @@ struct efi_priv {
struct efi_system_table *sys_table;
struct efi_boot_services *boot;
struct efi_runtime_services *run;
+ efi_uintn_t memmap_key;
+ struct efi_mem_desc *memmap_desc;
+ efi_uintn_t memmap_alloc;
+ efi_uintn_t memmap_size;
+ efi_uintn_t memmap_desc_size;
+ u32 memmap_version;
/* app: */
bool use_pool_for_malloc;
@@ -574,4 +586,24 @@ void efi_putc(struct efi_priv *priv, const char ch);
*/
int efi_info_get(enum efi_entry_t type, void **datap, int *sizep);
+/**
+ * efi_store_memory_map() - Collect the memory-map info from EFI
+ *
+ * Collect the memory info and store it for later use, e.g. in calling
+ * exit_boot_services()
+ *
+ * @priv: Pointer to private EFI structure
+ * @return 0 if OK, non-zero on error
+ */
+int efi_store_memory_map(struct efi_priv *priv);
+
+/**
+ * efi_call_exit_boot_services() - Handle the exit-boot-service procedure
+ *
+ * Tell EFI we don't want their boot services anymore
+ *
+ * Return: 0 if OK, non-zero on error
+ */
+int efi_call_exit_boot_services(void);
+
#endif /* _LINUX_EFI_H */
diff --git a/lib/efi/efi.c b/lib/efi/efi.c
index cd6bf47b180..20da88c9151 100644
--- a/lib/efi/efi.c
+++ b/lib/efi/efi.c
@@ -135,3 +135,71 @@ void efi_free(struct efi_priv *priv, void *ptr)
boot->free_pool(ptr);
}
+
+int efi_store_memory_map(struct efi_priv *priv)
+{
+ struct efi_boot_services *boot = priv->sys_table->boottime;
+ efi_uintn_t size, desc_size;
+ efi_status_t ret;
+
+ /* Get the memory map so we can switch off EFI */
+ size = 0;
+ ret = boot->get_memory_map(&size, NULL, &priv->memmap_key,
+ &priv->memmap_desc_size,
+ &priv->memmap_version);
+ if (ret != EFI_BUFFER_TOO_SMALL) {
+ printhex2(EFI_BITS_PER_LONG);
+ putc(' ');
+ printhex2(ret);
+ puts(" No memory map\n");
+ return ret;
+ }
+ /*
+* Since doing a malloc() may change the memory map and also we want to
+* be able to read the memory map in efi_call_exit_boot_services()
+* below, after more changes have happened
+*/
+ priv->memmap_alloc = size + 1024;
+ priv->memmap_size = priv->memmap_alloc;
+ priv->memmap_desc = efi_malloc(priv, size, &ret);
+ if (!priv->memmap_desc) {
+ printhex2(ret);
+ puts(" No memory for memory descriptor\n");
+ return ret;
+ }
+
+ ret = boot->get_memory_map(&priv->memmap_size, priv->memmap_desc,
+ &priv->memmap_key, &desc_size,
+ &priv->memmap_version);
+ if (ret) {
+ printhex2(ret);
+ puts(" Can't get memory map\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+int efi_call_exit_boot_services(void)
+{
+ struct efi_priv *priv = efi_get_priv();
+ const struct efi_boot_services *boot = priv->boot;
+ efi_uintn_t size;
+ u32 version;
+ efi_status_t ret;
+
+ size = priv->memmap_alloc;
+ ret = boot->get_memory_map(&size, priv->memmap_desc,
+ &priv->memmap_key,
+
