diff mbox series

[PATCHv6,03/15] efi/x86: Get full memory map in allocate_e820()

Message ID 20220517153444.11195-4-kirill.shutemov@linux.intel.com
State Superseded
Headers show
Series mm, x86/cc: Implement support for unaccepted memory | expand

Commit Message

Kirill A. Shutemov May 17, 2022, 3:34 p.m. UTC
Currently allocate_e820() only interested in the size of map and size of
memory descriptor to determine how many e820 entries the kernel needs.

UEFI Specification version 2.9 introduces a new memory type --
unaccepted memory. To track unaccepted memory kernel needs to allocate
a bitmap. The size of the bitmap is dependent on the maximum physical
address present in the system. A full memory map is required to find
the maximum address.

Modify allocate_e820() to get a full memory map.

This is preparation for the next patch that implements handling of
unaccepted memory in EFI stub.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
 drivers/firmware/efi/libstub/x86-stub.c | 28 +++++++++++--------------
 1 file changed, 12 insertions(+), 16 deletions(-)

Comments

David Hildenbrand June 1, 2022, 9 a.m. UTC | #1
On 17.05.22 17:34, Kirill A. Shutemov wrote:
> Currently allocate_e820() only interested in the size of map and size of
> memory descriptor to determine how many e820 entries the kernel needs.
> 
> UEFI Specification version 2.9 introduces a new memory type --
> unaccepted memory. To track unaccepted memory kernel needs to allocate
> a bitmap. The size of the bitmap is dependent on the maximum physical
> address present in the system. A full memory map is required to find
> the maximum address.
> 
> Modify allocate_e820() to get a full memory map.

Usually we use max_pfn, if we want to know the maximum pfn that's
present in the system (well, IIRC, excluding hotunplug).

How exactly will this (different?) maximum from UEFI for the bitmap
interact with

max_pfn = e820__end_of_ram_pfn();

from e820 in existing code

?
Kirill A. Shutemov June 1, 2022, 2:35 p.m. UTC | #2
On Wed, Jun 01, 2022 at 11:00:23AM +0200, David Hildenbrand wrote:
> On 17.05.22 17:34, Kirill A. Shutemov wrote:
> > Currently allocate_e820() only interested in the size of map and size of
> > memory descriptor to determine how many e820 entries the kernel needs.
> > 
> > UEFI Specification version 2.9 introduces a new memory type --
> > unaccepted memory. To track unaccepted memory kernel needs to allocate
> > a bitmap. The size of the bitmap is dependent on the maximum physical
> > address present in the system. A full memory map is required to find
> > the maximum address.
> > 
> > Modify allocate_e820() to get a full memory map.
> 
> Usually we use max_pfn, if we want to know the maximum pfn that's
> present in the system (well, IIRC, excluding hotunplug).
> 
> How exactly will this (different?) maximum from UEFI for the bitmap
> interact with
> 
> max_pfn = e820__end_of_ram_pfn();
> 
> from e820 in existing code
> 
> ?

I'm not sure I understand the question.

On EFI system, E820 is constructed based on EFI memory map and size of
bitmap calculated based of EFI memmap will always be enough to address all
memory. e820__end_of_ram_pfn() can be smaller than what what we calculate
as size of memory here, if kernel reserve very top of the memory, but it
will never be larger.

Later during the boot we use e820__end_of_ram_pfn() to infer size of
bitmap and it is safe.
David Hildenbrand June 1, 2022, 2:39 p.m. UTC | #3
On 01.06.22 16:35, Kirill A. Shutemov wrote:
> On Wed, Jun 01, 2022 at 11:00:23AM +0200, David Hildenbrand wrote:
>> On 17.05.22 17:34, Kirill A. Shutemov wrote:
>>> Currently allocate_e820() only interested in the size of map and size of
>>> memory descriptor to determine how many e820 entries the kernel needs.
>>>
>>> UEFI Specification version 2.9 introduces a new memory type --
>>> unaccepted memory. To track unaccepted memory kernel needs to allocate
>>> a bitmap. The size of the bitmap is dependent on the maximum physical
>>> address present in the system. A full memory map is required to find
>>> the maximum address.
>>>
>>> Modify allocate_e820() to get a full memory map.
>>
>> Usually we use max_pfn, if we want to know the maximum pfn that's
>> present in the system (well, IIRC, excluding hotunplug).
>>
>> How exactly will this (different?) maximum from UEFI for the bitmap
>> interact with
>>
>> max_pfn = e820__end_of_ram_pfn();
>>
>> from e820 in existing code
>>
>> ?
> 
> I'm not sure I understand the question.

Essentially, if the PFN you calculate here for the bitmap size will
essentially match later max_pfn.

> 
> On EFI system, E820 is constructed based on EFI memory map and size of
> bitmap calculated based of EFI memmap will always be enough to address all
> memory. e820__end_of_ram_pfn() can be smaller than what what we calculate
> as size of memory here, if kernel reserve very top of the memory, but it
> will never be larger.
> 
> Later during the boot we use e820__end_of_ram_pfn() to infer size of
> bitmap and it is safe.
>
Kirill A. Shutemov June 1, 2022, 2:46 p.m. UTC | #4
On Wed, Jun 01, 2022 at 04:39:16PM +0200, David Hildenbrand wrote:
> On 01.06.22 16:35, Kirill A. Shutemov wrote:
> > On Wed, Jun 01, 2022 at 11:00:23AM +0200, David Hildenbrand wrote:
> >> On 17.05.22 17:34, Kirill A. Shutemov wrote:
> >>> Currently allocate_e820() only interested in the size of map and size of
> >>> memory descriptor to determine how many e820 entries the kernel needs.
> >>>
> >>> UEFI Specification version 2.9 introduces a new memory type --
> >>> unaccepted memory. To track unaccepted memory kernel needs to allocate
> >>> a bitmap. The size of the bitmap is dependent on the maximum physical
> >>> address present in the system. A full memory map is required to find
> >>> the maximum address.
> >>>
> >>> Modify allocate_e820() to get a full memory map.
> >>
> >> Usually we use max_pfn, if we want to know the maximum pfn that's
> >> present in the system (well, IIRC, excluding hotunplug).
> >>
> >> How exactly will this (different?) maximum from UEFI for the bitmap
> >> interact with
> >>
> >> max_pfn = e820__end_of_ram_pfn();
> >>
> >> from e820 in existing code
> >>
> >> ?
> > 
> > I'm not sure I understand the question.
> 
> Essentially, if the PFN you calculate here for the bitmap size will
> essentially match later max_pfn.

Yes, generally. But is can decrease if kernel transit memory from TYPE_RAM
to TYPE_RESERVE. In any case we will not step out of the allocated bitmap.
diff mbox series

Patch

diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 01ddd4502e28..252ee222ca1e 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -569,31 +569,27 @@  static efi_status_t alloc_e820ext(u32 nr_desc, struct setup_data **e820ext,
 }
 
 static efi_status_t allocate_e820(struct boot_params *params,
+				  struct efi_boot_memmap *map,
 				  struct setup_data **e820ext,
 				  u32 *e820ext_size)
 {
-	unsigned long map_size, desc_size, map_key;
 	efi_status_t status;
-	__u32 nr_desc, desc_version;
+	__u32 nr_desc;
 
-	/* Only need the size of the mem map and size of each mem descriptor */
-	map_size = 0;
-	status = efi_bs_call(get_memory_map, &map_size, NULL, &map_key,
-			     &desc_size, &desc_version);
-	if (status != EFI_BUFFER_TOO_SMALL)
-		return (status != EFI_SUCCESS) ? status : EFI_UNSUPPORTED;
-
-	nr_desc = map_size / desc_size + EFI_MMAP_NR_SLACK_SLOTS;
+	status = efi_get_memory_map(map);
+	if (status != EFI_SUCCESS)
+		return status;
 
-	if (nr_desc > ARRAY_SIZE(params->e820_table)) {
-		u32 nr_e820ext = nr_desc - ARRAY_SIZE(params->e820_table);
+	nr_desc = *map->map_size / *map->desc_size;
+	if (nr_desc > ARRAY_SIZE(params->e820_table) - EFI_MMAP_NR_SLACK_SLOTS) {
+		u32 nr_e820ext = nr_desc - ARRAY_SIZE(params->e820_table) +
+			EFI_MMAP_NR_SLACK_SLOTS;
 
 		status = alloc_e820ext(nr_e820ext, e820ext, e820ext_size);
-		if (status != EFI_SUCCESS)
-			return status;
 	}
 
-	return EFI_SUCCESS;
+	efi_bs_call(free_pool, *map->map);
+	return status;
 }
 
 struct exit_boot_struct {
@@ -642,7 +638,7 @@  static efi_status_t exit_boot(struct boot_params *boot_params, void *handle)
 	priv.boot_params	= boot_params;
 	priv.efi		= &boot_params->efi_info;
 
-	status = allocate_e820(boot_params, &e820ext, &e820ext_size);
+	status = allocate_e820(boot_params, &map, &e820ext, &e820ext_size);
 	if (status != EFI_SUCCESS)
 		return status;