[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] x86/boot: Drop incorrect mapping at l2_xenmap[0]
On 29.11.2021 18:42, Andrew Cooper wrote: > On 29/11/2021 17:26, Andrew Cooper wrote: >> It has been 4 years since the default load address changed from 1M to 2M, and >> _stext ceased residing in l2_xenmap[0]. We should not be inserting an unused >> mapping. >> >> To ensure we don't create/remove mappings accidentally, loop from 0 and obey >> _PAGE_PRESENT on all entries. >> >> Fixes: 7ed93f3a0dff ("x86: change default load address from 1 MiB to 2 MiB") >> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> >> --- >> CC: Jan Beulich <JBeulich@xxxxxxxx> >> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx> >> CC: Wei Liu <wl@xxxxxxx> >> >> I ought to have spotted this in c/s 52975142d154 ("x86/boot: Create the >> l2_xenmap[] mappings dynamically") too. >> --- >> xen/arch/x86/setup.c | 10 +++------- >> 1 file changed, 3 insertions(+), 7 deletions(-) >> >> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c >> index da47cdea14a1..6f241048425c 100644 >> --- a/xen/arch/x86/setup.c >> +++ b/xen/arch/x86/setup.c >> @@ -1279,16 +1279,12 @@ void __init noreturn __start_xen(unsigned long mbi_p) >> >> /* The only data mappings to be relocated are in the Xen area. >> */ >> pl2e = __va(__pa(l2_xenmap)); >> - /* >> - * Undo the temporary-hooking of the l1_directmap. >> __2M_text_start >> - * is contained in this PTE. >> - */ >> + >> BUG_ON(using_2M_mapping() && >> l2_table_offset((unsigned long)_erodata) == >> l2_table_offset((unsigned long)_stext)); >> - *pl2e++ = l2e_from_pfn(xen_phys_start >> PAGE_SHIFT, >> - PAGE_HYPERVISOR_RX | _PAGE_PSE); >> - for ( i = 1; i < L2_PAGETABLE_ENTRIES; i++, pl2e++ ) >> + >> + for ( i = 0; i < L2_PAGETABLE_ENTRIES; i++, pl2e++ ) >> { >> unsigned int flags; >> > > Actually, on further consideration, this hunk should be included too, to > cross-check that we don't zap any of the dynamically created mappings. I agree in principle, but ... > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -1320,7 +1320,7 @@ void __init noreturn __start_xen(unsigned long mbi_p) > } > else > { > - *pl2e = l2e_empty(); > + ASSERT_UNREACHABLE(); > continue; > } ... this isn't going to help non-debug builds. Dropping the zapping would mean release builds then run with an unadjusted PTE. I can see two options: Extract the flags from the existing PTE as fallback (i.e. keeping the ASSERT_UNREACHABLE()) or use BUG() instead. Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |