Re: [PATCH RFC 06/55] x86/mm: map_pages_to_xen should have one exit path

["Nuernberger, Stefan" <snu@amazon.de>]

On Thu, 2019-02-07 at 16:44 +0000, Wei Liu wrote:
> We will soon rewrite the function to handle dynamically mapping and
> unmapping of page tables.
> 
> No functional change.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  xen/arch/x86/mm.c | 34 +++++++++++++++++++++++++++-------
>  1 file changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index 4147a71c5d..3ab222c8ea 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -4887,9 +4887,11 @@ int map_pages_to_xen(
>      unsigned int flags)
>  {
>      bool locking = system_state > SYS_STATE_boot;
> +    l3_pgentry_t *pl3e, ol3e;

After limiting the scope of other variables in the previous patches,
you now widen the scope for this one. Is this so you can handle unmap
in a common exit/error path later?

>      l2_pgentry_t *pl2e, ol2e;
>      l1_pgentry_t *pl1e, ol1e;
>      unsigned int  i;
> +    int rc = -ENOMEM;
>  
>  #define flush_flags(oldf) do {                 \
>      unsigned int o_ = (oldf);                  \
> @@ -4907,10 +4909,13 @@ int map_pages_to_xen(
>  
>      while ( nr_mfns != 0 )
>      {
> -        l3_pgentry_t ol3e, *pl3e = virt_to_xen_l3e(virt);
> +        pl3e = virt_to_xen_l3e(virt);
>  
>          if ( !pl3e )
> -            return -ENOMEM;
> +        {
> +            ASSERT(rc == -ENOMEM);
> +            goto out;
> +        }
>          ol3e = *pl3e;
>  
>          if ( cpu_has_page1gb &&
> @@ -5002,7 +5007,10 @@ int map_pages_to_xen(
>  
>              l2t = alloc_xen_pagetable();
>              if ( l2t == NULL )
> -                return -ENOMEM;
> +            {
> +                ASSERT(rc == -ENOMEM);
> +                goto out;
> +            }
>  
>              for ( i = 0; i < L2_PAGETABLE_ENTRIES; i++ )
>                  l2e_write(l2t + i,
> @@ -5031,7 +5039,10 @@ int map_pages_to_xen(
>  
>          pl2e = virt_to_xen_l2e(virt);
>          if ( !pl2e )
> -            return -ENOMEM;
> +        {
> +            ASSERT(rc == -ENOMEM);
> +            goto out;
> +        }
>  
>          if ( ((((virt >> PAGE_SHIFT) | mfn_x(mfn)) &
>                 ((1u << PAGETABLE_ORDER) - 1)) == 0) &&
> @@ -5076,7 +5087,10 @@ int map_pages_to_xen(
>              {
>                  pl1e = virt_to_xen_l1e(virt);
>                  if ( pl1e == NULL )
> -                    return -ENOMEM;
> +                {
> +                    ASSERT(rc == -ENOMEM);
> +                    goto out;
> +                }
>              }
>              else if ( l2e_get_flags(*pl2e) & _PAGE_PSE )
>              {
> @@ -5104,7 +5118,10 @@ int map_pages_to_xen(
>  
>                  l1t = alloc_xen_pagetable();
>                  if ( l1t == NULL )
> -                    return -ENOMEM;
> +                {
> +                    ASSERT(rc == -ENOMEM);
> +                    goto out;
> +                }
>  
>                  for ( i = 0; i < L1_PAGETABLE_ENTRIES; i++ )
>                      l1e_write(&l1t[i],
> @@ -5250,7 +5267,10 @@ int map_pages_to_xen(
>  
>  #undef flush_flags
>  
> -    return 0;
> +    rc = 0;
> +
> + out:
> +    return rc;
>  }
>  
>  int populate_pt_range(unsigned long virt, unsigned long nr_mfns)



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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel