[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/2] x86: drop unnecessary page table walking in compat r/o M2P handling



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 <jbeulich@xxxxxxxx>
> 
> --- 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 <hongyxia@xxxxxxxxxx>

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




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.