| Message ID | 20240820082012.31316-3-jgross@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Commit | ba88829706e2c5b7238638fc2b0713edf596495e |
| Headers | show |
| Series | xen: fix dom0 PV boot on some AMD machines | expand |
On 20.08.2024 10:20, Juergen Gross wrote: > When booting as a Xen PV dom0 the memory layout of the dom0 is > modified to match that of the host, as this requires less changes in > the kernel for supporting Xen. > > There are some cases, though, which are problematic, as it is the Xen > hypervisor selecting the kernel's load address plus some other data, > which might conflict with the host's memory map. > > These conflicts are detected at boot time and result in a boot error. > In order to support handling at least some of these conflicts in > future, introduce a generic helper function which will later gain the > ability to adapt the memory layout when possible. > > Add the missing check for the xen_start_info area. > > Signed-off-by: Juergen Gross <jgross@suse.com> > Tested-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> However, since you mention the start_info area it may be worth adding half a sentence to the description also mentioning why the hypervisor allocated stack page doesn't need checking. In fact this may want to extend to initrd and phys-mach map as well, to cover everything Xen sets up on behalf of the kernel. Jan
On 20.08.24 10:35, Jan Beulich wrote: > On 20.08.2024 10:20, Juergen Gross wrote: >> When booting as a Xen PV dom0 the memory layout of the dom0 is >> modified to match that of the host, as this requires less changes in >> the kernel for supporting Xen. >> >> There are some cases, though, which are problematic, as it is the Xen >> hypervisor selecting the kernel's load address plus some other data, >> which might conflict with the host's memory map. >> >> These conflicts are detected at boot time and result in a boot error. >> In order to support handling at least some of these conflicts in >> future, introduce a generic helper function which will later gain the >> ability to adapt the memory layout when possible. >> >> Add the missing check for the xen_start_info area. >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> Tested-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > > Reviewed-by: Jan Beulich <jbeulich@suse.com> > > However, since you mention the start_info area it may be worth adding half > a sentence to the description also mentioning why the hypervisor allocated > stack page doesn't need checking. In fact this may want to extend to > initrd and phys-mach map as well, to cover everything Xen sets up on behalf > of the kernel. Okay. Juergen
diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c index f1ce39d6d32c..839e6613753d 100644 --- a/arch/x86/xen/mmu_pv.c +++ b/arch/x86/xen/mmu_pv.c @@ -2018,10 +2018,7 @@ void __init xen_reserve_special_pages(void) void __init xen_pt_check_e820(void) { - if (xen_is_e820_reserved(xen_pt_base, xen_pt_size)) { - xen_raw_console_write("Xen hypervisor allocated page table memory conflicts with E820 map\n"); - BUG(); - } + xen_chk_is_e820_usable(xen_pt_base, xen_pt_size, "page table"); } static unsigned char dummy_mapping[PAGE_SIZE] __page_aligned_bss; diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c index 4bcc70a71b7d..96765180514b 100644 --- a/arch/x86/xen/setup.c +++ b/arch/x86/xen/setup.c @@ -567,7 +567,7 @@ static void __init xen_ignore_unusable(void) } } -bool __init xen_is_e820_reserved(phys_addr_t start, phys_addr_t size) +static bool __init xen_is_e820_reserved(phys_addr_t start, phys_addr_t size) { struct e820_entry *entry; unsigned mapcnt; @@ -624,6 +624,23 @@ phys_addr_t __init xen_find_free_area(phys_addr_t size) return 0; } +/* + * Check for an area in physical memory to be usable for non-movable purposes. + * An area is considered to usable if the used E820 map lists it to be RAM. + * In case the area is not usable, crash the system with an error message. + */ +void __init xen_chk_is_e820_usable(phys_addr_t start, phys_addr_t size, + const char *component) +{ + if (!xen_is_e820_reserved(start, size)) + return; + + xen_raw_console_write("Xen hypervisor allocated "); + xen_raw_console_write(component); + xen_raw_console_write(" memory conflicts with E820 map\n"); + BUG(); +} + /* * Like memcpy, but with physical addresses for dest and src. */ @@ -824,11 +841,16 @@ char * __init xen_memory_setup(void) * Failing now is better than running into weird problems later due * to relocating (and even reusing) pages with kernel text or data. */ - if (xen_is_e820_reserved(__pa_symbol(_text), - __pa_symbol(_end) - __pa_symbol(_text))) { - xen_raw_console_write("Xen hypervisor allocated kernel memory conflicts with E820 map\n"); - BUG(); - } + xen_chk_is_e820_usable(__pa_symbol(_text), + __pa_symbol(_end) - __pa_symbol(_text), + "kernel"); + + /* + * Check for a conflict of the xen_start_info memory with the target + * E820 map. + */ + xen_chk_is_e820_usable(__pa(xen_start_info), sizeof(*xen_start_info), + "xen_start_info"); /* * Check for a conflict of the hypervisor supplied page tables with diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h index 0cf16fc79e0b..9a27d1d653d3 100644 --- a/arch/x86/xen/xen-ops.h +++ b/arch/x86/xen/xen-ops.h @@ -48,7 +48,8 @@ void xen_mm_unpin_all(void); void __init xen_relocate_p2m(void); #endif -bool __init xen_is_e820_reserved(phys_addr_t start, phys_addr_t size); +void __init xen_chk_is_e820_usable(phys_addr_t start, phys_addr_t size, + const char *component); unsigned long __ref xen_chk_extra_mem(unsigned long pfn); void __init xen_inv_extra_mem(void); void __init xen_remap_memory(void);