Hi Julien, > -----Original Message----- > Subject: [PATCH v5 3/5] xen/arm64: mm: Introduce helpers to > prepare/enable/disable the identity mapping > > From: Julien Grall <[email protected]> > > In follow-up patches we will need to have part of Xen identity mapped in > order to safely switch the TTBR. > > On some platform, the identity mapping may have to start at 0. If we always > keep the identity region mapped, NULL pointer dereference would lead to > access to valid mapping. > > It would be possible to relocate Xen to avoid clashing with address 0. > However the identity mapping is only meant to be used in very limited > places. Therefore it would be better to keep the identity region invalid > for most of the time. > > Two new external helpers are introduced: > - arch_setup_page_tables() will setup the page-tables so it is > easy to create the mapping afterwards. > - update_identity_mapping() will create/remove the identity mapping > > Signed-off-by: Julien Grall <[email protected]>
Reviewed-by: Henry Wang <[email protected]> With some nits below that can definitely be fixed on commit. Tested on FVP in Arm64 mode, so: Tested-by: Henry Wang <[email protected]> > +static void __init prepare_boot_identity_mapping(void) > +{ > + paddr_t id_addr = virt_to_maddr(_start); > + lpae_t pte; > + DECLARE_OFFSETS(id_offsets, id_addr); > + > + /* > + * We will be re-using the boot ID tables. They may not have been > + * zeroed but they should be unlinked. So it is fine to use > + * clear_page(). > + */ > + clear_page(boot_first_id); > + clear_page(boot_second_id); > + clear_page(boot_third_id); > + > + if ( id_offsets[0] != 0 ) > + panic("Cannot handled ID mapping above 512GB\n"); Nit: s/handled/handle/ > + > +static void __init prepare_runtime_identity_mapping(void) > +{ > + paddr_t id_addr = virt_to_maddr(_start); > + lpae_t pte; > + DECLARE_OFFSETS(id_offsets, id_addr); > + > + if ( id_offsets[0] >= IDENTITY_MAPPING_AREA_NR_L0 ) > + panic("Cannot handled ID mapping above 512GB\n"); Same here. > +void arch_setup_page_tables(void); > + > +/* > + * Enable/disable the identity mapping in the live page-tables (i.e. > + * the one pointer by TTBR_EL2). Nit: I might be wrong but I think s/pointer/pointed/. > + * > + * Note that nested a call (e.g. enable=true, enable=true) is not Nit: s/nested/nesting/ or "a nested call"? > + * supported. > + */ > +void update_identity_mapping(bool enable); Kind regards, Henry
