Skip to content

Fix Xen PVH boot and init_physical_heap low-memory trimming bug #2137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
niklasfemerstrand wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix Xen PVH boot and init_physical_heap low-memory trimming bug #2137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 24 additions & 15 deletions platform/pc/service.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ range kern_get_elf(void)

BSS_RO_AFTER_INIT static boolean have_rdseed;
BSS_RO_AFTER_INIT static boolean have_rdrand;
BSS_RO_AFTER_INIT boolean pvh_boot;

static boolean hw_seed(u64 * seed, boolean rdseed)
{
Expand Down Expand Up @@ -205,8 +206,9 @@ void start_secondary_cores(kernel_heaps kh)

void count_cpus_present(void)
{
/* Read ACPI tables for MADT access */
init_acpi_tables(get_kernel_heaps());
/* Read ACPI tables for MADT access (not available on PVH) */
if (!pvh_boot)
init_acpi_tables(get_kernel_heaps());

@francescolavra francescolavra Mar 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be possible to call both init_acpi_tables() and init_acpi() on a guest without ACPI, in fact Nanos runs just fine on ARM guests without ACPI. I also tested a modified kernel where the AcpiOsGetRootPointer() function in src/x86_64/acpi.c always returns 0 (to simulate an x86 guest without ACPI) and it boots just fine, both with and without KVM.
If the kernel crashes on a Xen PVH guest when trying to initialize ACPI, I would rather debug and fix that, and avoid the pvh_boot global variable.


#ifdef SMP_ENABLE
if (acpi_walk_madt(stack_closure_func(madt_handler, count_processors_handler))) {
Expand Down Expand Up @@ -265,21 +267,24 @@ static void find_initial_pages(void)

void init_physical_heap(void)
{
/* Carve the bootstrap heap out of a physical memory region. */
/* Remove low memory area from physical regions so it can be used for things
like starting secondary CPUs. This must be a separate pass because the
search below may break early, skipping remaining regions. */
for_regions(e) {
if (e->type == REGION_PHYSICAL) {
/* Remove low memory area from physical memory regions, so that it can be used for
* things like starting secondary CPUs. */
if (e->base < MB) {
u64 end = e->base + e->length;
if (end > MB) {
e->base = MB;
e->length = end - MB;
} else {
e->length = 0;
}
if (e->type == REGION_PHYSICAL && e->base < MB) {
u64 end = e->base + e->length;
if (end > MB) {
e->base = MB;
e->length = end - MB;
} else {
e->length = 0;
}
}
}

/* Carve the bootstrap heap out of a physical memory region. */
for_regions(e) {
if (e->type == REGION_PHYSICAL) {
u64 base = pad(e->base, PAGESIZE);
u64 end = e->base + e->length;
u64 length = (end & ~MASK(PAGELOG)) - base;
Expand Down Expand Up @@ -437,12 +442,15 @@ void pvh_start(hvm_start_info start_info)
{
if (start_info->magic != HVM_START_MAGIC_VALUE)
return;
pvh_boot = true;
regions->type = 0;
hvm_memmap_entry mem_table = pointer_from_u64(start_info->memmap_paddr);
for (int i = 0; i < start_info->memmap_entries; i++) {
if (mem_table[i].type == HVM_MEMMAP_TYPE_RAM)
create_region(mem_table[i].addr, mem_table[i].size, REGION_PHYSICAL);
}
if (start_info->rsdp_paddr)
create_region(start_info->rsdp_paddr, sizeof(u64), REGION_RSDP);
init_service(0, 0, start_info);
}

Expand Down Expand Up @@ -528,7 +536,8 @@ void detect_devices(kernel_heaps kh, storage_attach sa)
}

/* misc / platform */
init_acpi(kh);
if (!pvh_boot)
init_acpi(kh);

init_virtio_balloon(kh);
init_virtio_rng(kh);
Expand Down
2 changes: 2 additions & 0 deletions src/x86_64/init.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ extern pvh_start

bits 32
pvh_start32:
; ESP is undefined at PVH entry, we need a temporary stack before calls
mov esp, 0xa000

@francescolavra francescolavra Mar 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, but since there already is a stack pointer initialization a few instructions below this, I would remove that in favor of this one (and update/move the relevant comment).

PREPARE_LONG_MODE eax
; set up minimal mapping to be able to run in 64-bit mode, carving page
; tables from the top of the first 1MB of memory (which will not be
Expand Down
9 changes: 6 additions & 3 deletions src/x86_64/interrupt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#include <symtab.h>
#include <drivers/acpi.h>

extern boolean pvh_boot;

//#define INT_DEBUG
#ifdef INT_DEBUG
#define int_debug(x, ...) do {tprintf(sym(int), 0, ss(x), ##__VA_ARGS__);} while(0)
Expand Down Expand Up @@ -212,7 +214,7 @@ void common_handler()
if (handlers[i]) {
ci->state = cpu_interrupt;
apply(handlers[i]);
if (i >= INTERRUPT_VECTOR_START)
if (i >= INTERRUPT_VECTOR_START && !pvh_boot)
lapic_eoi();

/* enqueue interrupted user thread */
Expand Down Expand Up @@ -383,8 +385,9 @@ void init_interrupts(kernel_heaps kh)
assert(v != INVALID_PHYSICAL);
spurious_int_vector = v;

/* APIC initialization */
init_apic(kh);
/* APIC initialization, skip for PVH (Xen uses event channels) */
if (!pvh_boot)
init_apic(kh);

@francescolavra francescolavra Mar 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked at the Xen source, and as far as I can tell, Xen PVH guests do have a local APIC (see https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/x86/domain.c;h=9ba2774762ccfc2b79d6892c55eaa06e5e66ac29;hb=HEAD#l784, where the LAPIC emulation flag is marked as required for PVH domU). So if there are issues getting it to work, I prefer to sort them out instead of disabling the APIC code.

}

void triple_fault(void)
Expand Down