Re: [Xen-devel] [PATCH RFC 18/55] x86/mm: switch to new APIs in modify_xen_mappings

["Nuernberger, Stefan" <snu@xxxxxxxxx>]

On Thu, 2019-02-07 at 16:44 +0000, Wei Liu wrote:
> Page tables allocated in that function should be mapped and unmapped
> now.
> 
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
>  xen/arch/x86/mm.c | 31 ++++++++++++++++++++++---------
>  1 file changed, 22 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index 1ea2974c1f..18c7b43705 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -5436,6 +5436,7 @@ int modify_xen_mappings(unsigned long s,
> unsigned long e, unsigned int nf)
>          if ( l3e_get_flags(*pl3e) & _PAGE_PSE )
>          {
>              l2_pgentry_t *l2t;
> +            mfn_t mfn;

nit: I wouldn't mind if these scoped mfns were caled l2t_mfn / l1t_mfn
in this patch, too.

>  
>              if ( l2_table_offset(v) == 0 &&
>                   l1_table_offset(v) == 0 &&
> @@ -5452,13 +5453,15 @@ int modify_xen_mappings(unsigned long s,
> unsigned long e, unsigned int nf)
>              }
>  
>              /* PAGE1GB: shatter the superpage and fall through. */
> -            l2t = alloc_xen_pagetable();
> -            if ( !l2t )
> +            mfn = alloc_xen_pagetable_new();
> +            if ( mfn_eq(mfn, INVALID_MFN) )
>              {
>                  ASSERT(rc == -ENOMEM);
>                  goto out;
>              }
>  
> +            l2t = map_xen_pagetable_new(mfn);

Is map_xen_pagetable always guaranteed to succeed on a valid mfn (also
in the future)? Otherwise the validity check should be done on l2t as
before instead of mfn. But it looks like map_xen_pagetable{_new} does
not deal with invalid mfns.

> +
>              for ( i = 0; i < L2_PAGETABLE_ENTRIES; i++ )
>                  l2e_write(l2t + i,
>                            l2e_from_pfn(l3e_get_pfn(*pl3e) +
> @@ -5469,14 +5472,17 @@ int modify_xen_mappings(unsigned long s,
> unsigned long e, unsigned int nf)
>              if ( (l3e_get_flags(*pl3e) & _PAGE_PRESENT) &&
>                   (l3e_get_flags(*pl3e) & _PAGE_PSE) )
>              {
> -                l3e_write_atomic(pl3e,
> l3e_from_mfn(virt_to_mfn(l2t),
> -                                                    __PAGE_HYPERVISO
> R));
> +                l3e_write_atomic(pl3e, l3e_from_mfn(mfn,
> __PAGE_HYPERVISOR));
> +                UNMAP_XEN_PAGETABLE_NEW(l2t);
>                  l2t = NULL;

That NULL assignment is redundant now. It's done by the UNMAP macro.

>              }
>              if ( locking )
>                  spin_unlock(&map_pgdir_lock);
>              if ( l2t )
> -                free_xen_pagetable(l2t);
> +            {
> +                UNMAP_XEN_PAGETABLE_NEW(l2t);
> +                free_xen_pagetable_new(mfn);
> +            }
>          }
>  
>          /*
> @@ -5511,15 +5517,18 @@ int modify_xen_mappings(unsigned long s,
> unsigned long e, unsigned int nf)
>              else
>              {
>                  l1_pgentry_t *l1t;
> +                mfn_t mfn;
>  
>                  /* PSE: shatter the superpage and try again. */
> -                l1t = alloc_xen_pagetable();
> -                if ( !l1t )
> +                mfn = alloc_xen_pagetable_new();
> +                if ( mfn_eq(mfn, INVALID_MFN) )
>                  {
>                      ASSERT(rc == -ENOMEM);
>                      goto out;
>                  }
>  
> +                l1t = map_xen_pagetable_new(mfn);
> +
>                  for ( i = 0; i < L1_PAGETABLE_ENTRIES; i++ )
>                      l1e_write(&l1t[i],
>                                l1e_from_pfn(l2e_get_pfn(*pl2e) + i,
> @@ -5529,14 +5538,18 @@ int modify_xen_mappings(unsigned long s,
> unsigned long e, unsigned int nf)
>                  if ( (l2e_get_flags(*pl2e) & _PAGE_PRESENT) &&
>                       (l2e_get_flags(*pl2e) & _PAGE_PSE) )
>                  {
> -                    l2e_write_atomic(pl2e,
> l2e_from_mfn(virt_to_mfn(l1t),
> +                    l2e_write_atomic(pl2e, l2e_from_mfn(mfn,
>                                                          __PAGE_HYPER
> VISOR));
> +                    UNMAP_XEN_PAGETABLE_NEW(l1t);
>                      l1t = NULL;
>                  }
>                  if ( locking )
>                      spin_unlock(&map_pgdir_lock);
>                  if ( l1t )
> -                    free_xen_pagetable(l1t);
> +                {
> +                    UNMAP_XEN_PAGETABLE_NEW(l1t);
> +                    free_xen_pagetable_new(mfn);
> +                }
>              }
>          }
>          else



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel