Re: [PATCH v9 10/13] x86/smpboot: switch clone_mapping() to new APIs

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.04.2021 13:05, Hongyan Xia wrote:
> @@ -742,51 +742,58 @@ static int clone_mapping(const void *ptr, 
> root_pgentry_t *rpt)
>          }
>      }
>  
> +    UNMAP_DOMAIN_PAGE(pl1e);
> +    UNMAP_DOMAIN_PAGE(pl2e);
> +    UNMAP_DOMAIN_PAGE(pl3e);

Just one minor remark: A pedantic(?) compiler might warn about the
setting to NULL of pl3e here, when

>      if ( !(root_get_flags(rpt[root_table_offset(linear)]) & _PAGE_PRESENT) )
>      {
> -        pl3e = alloc_xen_pagetable();
> +        mfn_t l3mfn;
> +
> +        pl3e = alloc_map_clear_xen_pt(&l3mfn);
>          rc = -ENOMEM;
>          if ( !pl3e )
>              goto out;
> -        clear_page(pl3e);
>          l4e_write(&rpt[root_table_offset(linear)],
> -                  l4e_from_paddr(__pa(pl3e), __PAGE_HYPERVISOR));
> +                  l4e_from_mfn(l3mfn, __PAGE_HYPERVISOR));
>      }
>      else
> -        pl3e = l4e_to_l3e(rpt[root_table_offset(linear)]);
> +        pl3e = map_l3t_from_l4e(rpt[root_table_offset(linear)]);

... it is guaranteed to get initialized again before any further
consumption. IOW strictly speaking the last of those three would
want to be unmap_domain_page(), just like you have ...

> @@ -802,6 +809,9 @@ static int clone_mapping(const void *ptr, root_pgentry_t 
> *rpt)
>  
>      rc = 0;
>   out:
> +    unmap_domain_page(pl1e);
> +    unmap_domain_page(pl2e);
> +    unmap_domain_page(pl3e);
>      return rc;
>  }

... here.

Jan