| Message ID | 20210425201318.15447-14-julien@xen.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: mm: Remove open-coding mappings | expand |
On Sun, 25 Apr 2021, Julien Grall wrote: > From: Julien Grall <jgrall@amazon.com> > > During early boot, it is not possible to use xen_{,un}map_table() > if the page tables are not residing the Xen binary. > > This is a blocker to switch some of the helpers to use xen_pt_update() > as we may need to allocate extra page tables and access them before > the domheap has been initialized (see setup_xenheap_mappings()). > > xen_{,un}map_table() are now updated to use the PMAP helpers for early > boot map/unmap. Note that the special case for page-tables residing > in Xen binary has been dropped because it is "complex" and was > only added as a workaround in 8d4f1b8878e0 ("xen/arm: mm: Allow > generic xen page-tables helpers to be called early"). > > Signed-off-by: Julien Grall <jgrall@amazon.com> In terms of boot stages: - SYS_STATE_early_boot --> use pmap_map - greater than SYS_STATE_early_boot --> map_domain_page While actually pmap would be able to work as far as SYS_STATE_boot, but we don't need it. Is it worth simplifying it by changing the checks in the previous patch to be against SYS_STATE_early_boot? In any case: Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > Changes in v2: > - New patch > --- > xen/arch/arm/mm.c | 33 +++++++++------------------------ > 1 file changed, 9 insertions(+), 24 deletions(-) > > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > index 5e713b599611..f5768f2d4a81 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -41,6 +41,7 @@ > #include <xen/sizes.h> > #include <xen/libfdt/libfdt.h> > > +#include <asm/pmap.h> > #include <asm/setup.h> > > /* Override macros from asm/page.h to make them work with mfn_t */ > @@ -967,27 +968,11 @@ void *ioremap(paddr_t pa, size_t len) > static lpae_t *xen_map_table(mfn_t mfn) > { > /* > - * We may require to map the page table before map_domain_page() is > - * useable. The requirements here is it must be useable as soon as > - * page-tables are allocated dynamically via alloc_boot_pages(). > - * > - * We need to do the check on physical address rather than virtual > - * address to avoid truncation on Arm32. Therefore is_kernel() cannot > - * be used. > + * During early boot, map_domain_page() may be unusable. Use the > + * PMAP to map temporarily a page-table. > */ > if ( system_state == SYS_STATE_early_boot ) > - { > - if ( is_xen_fixed_mfn(mfn) ) > - { > - /* > - * It is fine to demote the type because the size of Xen > - * will always fit in vaddr_t. > - */ > - vaddr_t offset = mfn_to_maddr(mfn) - virt_to_maddr(&_start); > - > - return (lpae_t *)(XEN_VIRT_START + offset); > - } > - } > + return pmap_map(mfn); > > return map_domain_page(mfn); > } > @@ -996,12 +981,12 @@ static void xen_unmap_table(const lpae_t *table) > { > /* > * During early boot, xen_map_table() will not use map_domain_page() > - * for page-tables residing in Xen binary. So skip the unmap part. > + * but the PMAP. > */ > - if ( system_state == SYS_STATE_early_boot && is_kernel(table) ) > - return; > - > - unmap_domain_page(table); > + if ( system_state == SYS_STATE_early_boot ) > + pmap_unmap(table); > + else > + unmap_domain_page(table);
Hi Stefano, On 15/05/2021 00:35, Stefano Stabellini wrote: > On Sun, 25 Apr 2021, Julien Grall wrote: >> From: Julien Grall <jgrall@amazon.com> >> >> During early boot, it is not possible to use xen_{,un}map_table() >> if the page tables are not residing the Xen binary. >> >> This is a blocker to switch some of the helpers to use xen_pt_update() >> as we may need to allocate extra page tables and access them before >> the domheap has been initialized (see setup_xenheap_mappings()). >> >> xen_{,un}map_table() are now updated to use the PMAP helpers for early >> boot map/unmap. Note that the special case for page-tables residing >> in Xen binary has been dropped because it is "complex" and was >> only added as a workaround in 8d4f1b8878e0 ("xen/arm: mm: Allow >> generic xen page-tables helpers to be called early"). >> >> Signed-off-by: Julien Grall <jgrall@amazon.com> > > In terms of boot stages: > > - SYS_STATE_early_boot --> use pmap_map > - greater than SYS_STATE_early_boot --> map_domain_page > > While actually pmap would be able to work as far as SYS_STATE_boot, but > we don't need it. Is it worth simplifying it by changing the checks in > the previous patch to be against SYS_STATE_early_boot? We need to differentiate between when this is used and when PMAP can be used. The ASSERT() is here to check that the PMAP code is not used outside of its limit. In the current implementation, PMAP can be used at any time until we start bring-up secondary CPUs. So the ASSERT() is correct because doesn't restrict unnecessarily the use of it. Note, the code is going to be moved to common code in the next revision. > > In any case: > > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> Thank you! Cheers,
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 5e713b599611..f5768f2d4a81 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -41,6 +41,7 @@ #include <xen/sizes.h> #include <xen/libfdt/libfdt.h> +#include <asm/pmap.h> #include <asm/setup.h> /* Override macros from asm/page.h to make them work with mfn_t */ @@ -967,27 +968,11 @@ void *ioremap(paddr_t pa, size_t len) static lpae_t *xen_map_table(mfn_t mfn) { /* - * We may require to map the page table before map_domain_page() is - * useable. The requirements here is it must be useable as soon as - * page-tables are allocated dynamically via alloc_boot_pages(). - * - * We need to do the check on physical address rather than virtual - * address to avoid truncation on Arm32. Therefore is_kernel() cannot - * be used. + * During early boot, map_domain_page() may be unusable. Use the + * PMAP to map temporarily a page-table. */ if ( system_state == SYS_STATE_early_boot ) - { - if ( is_xen_fixed_mfn(mfn) ) - { - /* - * It is fine to demote the type because the size of Xen - * will always fit in vaddr_t. - */ - vaddr_t offset = mfn_to_maddr(mfn) - virt_to_maddr(&_start); - - return (lpae_t *)(XEN_VIRT_START + offset); - } - } + return pmap_map(mfn); return map_domain_page(mfn); } @@ -996,12 +981,12 @@ static void xen_unmap_table(const lpae_t *table) { /* * During early boot, xen_map_table() will not use map_domain_page() - * for page-tables residing in Xen binary. So skip the unmap part. + * but the PMAP. */ - if ( system_state == SYS_STATE_early_boot && is_kernel(table) ) - return; - - unmap_domain_page(table); + if ( system_state == SYS_STATE_early_boot ) + pmap_unmap(table); + else + unmap_domain_page(table); } static int create_xen_table(lpae_t *entry)