On Wed, 2020-04-15 at 10:23 +0200, Jan Beulich wrote:
> We have a global variable where the necessary L2 table is recorded;
> no
> need to inspect L4 and L3 tables (and this way a few less places will
> eventually need adjustment when we want to support 5-level page
> tables).
> Also avoid setting up the L3 entry, as the address range never gets
> used
> anyway (it'll be dropped altogether in a subsequent patch).
>
> Signed-off-by: Jan Beulich
>
> --- a/xen/arch/x86/x86_64/mm.c
> +++ b/xen/arch/x86/x86_64/mm.c
> @@ -219,9 +219,7 @@ static void destroy_compat_m2p_mapping(s
> {
> unsigned long i, va, rwva, pt_pfn;
> unsigned long smap = info->spfn, emap = info->spfn;
> -
> -l3_pgentry_t *l3_ro_mpt;
> -l2_pgentry_t *l2_ro_mpt;
> +l2_pgentry_t *l2_ro_mpt = compat_idle_pg_table_l2;
>
> if ( smap > ((RDWR_COMPAT_MPT_VIRT_END -
> RDWR_COMPAT_MPT_VIRT_START) >> 2) )
> return;
> @@ -229,12 +227,6 @@ static void destroy_compat_m2p_mapping(s
> if ( emap > ((RDWR_COMPAT_MPT_VIRT_END -
> RDWR_COMPAT_MPT_VIRT_START) >> 2) )
> emap = (RDWR_COMPAT_MPT_VIRT_END -
> RDWR_COMPAT_MPT_VIRT_START) >> 2;
>
> -l3_ro_mpt =
> l4e_to_l3e(idle_pg_table[l4_table_offset(HIRO_COMPAT_MPT_VIRT_START)]
> );
> -
> -ASSERT(l3e_get_flags(l3_ro_mpt[l3_table_offset(HIRO_COMPAT_MPT_V
> IRT_START)]) & _PAGE_PRESENT);
> -
> -l2_ro_mpt =
> l3e_to_l2e(l3_ro_mpt[l3_table_offset(HIRO_COMPAT_MPT_VIRT_START)]);
> -
> for ( i = smap; i < emap; )
> {
> va = HIRO_COMPAT_MPT_VIRT_START +
> @@ -323,7 +315,6 @@ static int setup_compat_m2p_table(struct
> unsigned long i, va, smap, emap, rwva, epfn = info->epfn;
> mfn_t mfn;
> unsigned int n;
> -l3_pgentry_t *l3_ro_mpt = NULL;
> l2_pgentry_t *l2_ro_mpt = NULL;
> int err = 0;
>
> @@ -342,13 +333,7 @@ static int setup_compat_m2p_table(struct
> emap = ( (epfn + ((1UL << (L2_PAGETABLE_SHIFT - 2)) - 1 )) &
> ~((1UL << (L2_PAGETABLE_SHIFT - 2)) - 1) );
>
> -va = HIRO_COMPAT_MPT_VIRT_START +
> - smap * sizeof(*compat_machine_to_phys_mapping);
> -l3_ro_mpt = l4e_to_l3e(idle_pg_table[l4_table_offset(va)]);
> -
> -ASSERT(l3e_get_flags(l3_ro_mpt[l3_table_offset(va)]) &
> _PAGE_PRESENT);
> -
> -l2_ro_mpt = l3e_to_l2e(l3_ro_mpt[l3_table_offset(va)]);
> +l2_ro_mpt = compat_idle_pg_table_l2;
>
> #define MFN(x) (((x) << L2_PAGETABLE_SHIFT) / sizeof(unsigned int))
> #define CNT ((sizeof(*frame_table) & -sizeof(*frame_table)) / \
> @@ -627,16 +612,10 @@ void __init paging_init(void)
> #undef MFN
>
> /* Create user-accessible L2 directory to map the MPT for compat
> guests. */
> -BUILD_BUG_ON(l4_table_offset(RDWR_MPT_VIRT_START) !=
> - l4_table_offset(HIRO_COMPAT_MPT_VIRT_START));
> -l3_ro_mpt = l4e_to_l3e(idle_pg_table[l4_table_offset(
> -HIRO_COMPAT_MPT_VIRT_START)]);
> if ( (l2_ro_mpt = alloc_xen_pagetable()) == NULL )
> goto nomem;
> compat_idle_pg_table_l2 = l2_ro_mpt;
> clear_page(l2_ro_mpt);
> -l3e_write(&l3_ro_mpt[l3_table_offset(HIRO_COMPAT_MPT_VIRT_START)
> ],
> - l3e_from_paddr(__pa(l2_ro_mpt),
> __PAGE_HYPERVISOR_RO));
> l2_ro_mpt += l2_table_offset(HIRO_COMPAT_MPT_VIRT_START);
> /* Allocate and map the compatibility mode machine-to-phys
> table. */
> mpt_size = (mpt_size >> 1) + (1UL << (L2_PAGETABLE_SHIFT - 1));
The code around here, I am wondering if there is a reason to put it in
this patch. If we bisect, we can end up in a commit which says the
address range of compat is still there in config.h, but actually it's
gone, so this probably belongs to the 2nd patch.
Apart from that,
Reviewed-by: Hongyan Xia
I would like to drop relevant map/unmap patches and replace them with
the new clean-up ones (and place them at the beginning of the series),
if there is no objection with that.
Hongyan
