On 1/24/2026 12:03 AM, Gregory Price wrote:
On Fri, Jan 23, 2026 at 10:43:12AM +0800, fanhuang wrote:
+bool e820_update_entry_type(uint64_t start, uint64_t length, uint32_t new_type)
+{
+ uint64_t end = start + length;
+ assert(!e820_done);
+
+ /* For E820_SOFT_RESERVED, validate range is within E820_RAM */
+ if (new_type == E820_SOFT_RESERVED) {
+ bool range_in_ram = false;
+
+ for (size_t j = 0; j < e820_entries; j++) {
+ uint64_t ram_start = le64_to_cpu(e820_table[j].address);
+ uint64_t ram_end = ram_start + le64_to_cpu(e820_table[j].length);
+ uint32_t ram_type = le32_to_cpu(e820_table[j].type);
+
+ if (ram_type == E820_RAM && ram_start <= start && ram_end >= end) {
+ range_in_ram = true;
+ break;
+ }
+ }
+ if (!range_in_ram) {
+ return false;
+ }
+ }
------- nit --------
this can be shortened: (j == e820_entries) implies not found.
if (new_type == E820_SOFT_RESERVED) {
for (size_t j = 0; j < e820_entries; j++) {
uint64_t ram_start = le64_to_cpu(e820_table[j].address);
uint64_t ram_end = ram_start + le64_to_cpu(e820_table[j].length);
uint32_t ram_type = le32_to_cpu(e820_table[j].type);
if (ram_type == E820_RAM && ram_start <= start && ram_end >= end)
break;
}
if j == e820_entries: /* Not found */
return false;
}
Haven't looked at the surrounding code, this seems like you might define
static bool e820_has_ram_region(start, end) { /* above code */ }
then the 820_update_entry_type lines can just be
if (new_type == E820_SOFT_RESERVED &&
!e820_has_ram_region(start, end))
return false;
only worth it if e820_has_ram_region() deduplicates other stuff.
-------------------
I don't see anything else immediately concerning and the above is more
of a nit than anything.
I haven't tested this, you might at least add the OS-facing results to
the changelog so someone can replicate your setup.
For example the early-boot print showing E820 and ACPI SRAT data.
[reasonable memdev definitions]
-numa node,nodeid=0,memdev=m0
-numa node,nodeid=1,memdev=m1,memmap-type=spm
-numa node,nodeid=2,memdev=m2,memmap-type=reserved
resulting in:
[ 0.000000] BIOS-provided physical RAM map:
[ 0.000000] BIOS-e820: [mem 0x0000000000000000-0x00000000afffffff] usable
[ 0.000000] BIOS-e820: [mem 0x0000000100000000-0x000000c04fffffff] usable
[ 0.000000] BIOS-e820: [mem 0x000000c050000000-0x000001004fffffff] soft
reserved
[ 0.000000] BIOS-e820: [mem 0x0000010050000000-0x000001006fffffff] reserved
[ 0.003409] ACPI: SRAT: Node 0 PXM 0 [mem 0x00000000-0xafffffff]
[ 0.003410] ACPI: SRAT: Node 0 PXM 0 [mem 0x100000000-0xc04fffffff]
[ 0.003411] ACPI: SRAT: Node 1 PXM 1 [mem 0xc050000000-0x1004fffffff] hotplug
[ 0.003411] ACPI: SRAT: Node 2 PXM 2 [mem 0x10050000000-0x1006fffffff]
hotplug
(fake entries above)
(i don't actually know if RESERVED gets added to SRAT)
This would at least show the node-association and the types are correct.
Thanks,
~Gregory
Hi Gregory,
Thanks for the review and feedback on v5!
Regarding your suggestions:
1. Simplifying the range_in_ram check:
I've updated the code following your suggestion, with one adjustment -
since the loop variable 'j' is declared with block scope in C99/C11, it
needs to be declared outside the for loop to remain accessible after
the loop exits:
if (new_type == E820_SOFT_RESERVED) {
size_t j;
for (j = 0; j < e820_entries; j++) {
uint64_t ram_start = le64_to_cpu(e820_table[j].address);
uint64_t ram_end = ram_start + le64_to_cpu(e820_table[j].length);
uint32_t ram_type = le32_to_cpu(e820_table[j].type);
if (ram_type == E820_RAM && ram_start <= start && ram_end >= end)
break;
}
if (j == e820_entries) /* Not found */
return false;
}
This saves a variable, though it slightly trades off readability.
Would you prefer this approach, or do you think the original
'range_in_ram' boolean makes the intent clearer? Happy to go either way.
2. Extracting e820_has_ram_region() helper:
I searched the codebase and found that the other places using E820_RAM
(fw_cfg.c, tdx.c) are iterating to collect all RAM entries, rather than
checking if a range falls within RAM. Since there's no other code that
would benefit from this helper, I'll keep it inline for now. If similar
logic is needed elsewhere in the future, we can refactor then.
3. Adding OS-facing test results:
Great suggestion! I've replied the E820 and SRAT output examples to the
v5 cover letter, so reviewers can verify the expected behavior.
Thanks,
Jerry
