Re: [PATCH v2 1/6] x86/mem-paging: fold p2m_mem_paging_prep()'s main if()-s

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 23/04/2020 09:37, Jan Beulich wrote:
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1819,22 +1821,16 @@ int p2m_mem_paging_prep(struct domain *d
>              goto out;
>          mfn = page_to_mfn(page);
>          page_extant = 0;
> -    }
> -
> -    /* If we were given a buffer, now is the time to use it */
> -    if ( !page_extant && user_ptr )
> -    {
> -        void *guest_map;
> -        int rc;
>  
>          ASSERT( mfn_valid(mfn) );
>          guest_map = map_domain_page(mfn);
> -        rc = copy_from_user(guest_map, user_ptr, PAGE_SIZE);
> +        ret = copy_from_user(guest_map, user_ptr, PAGE_SIZE);
>          unmap_domain_page(guest_map);
> -        if ( rc )
> +        if ( ret )
>          {
> -            gdprintk(XENLOG_ERR, "Failed to load paging-in gfn %lx domain %u 
> "
> -                                 "bytes left %d\n", gfn_l, d->domain_id, rc);
> +            gdprintk(XENLOG_ERR,
> +                     "Failed to load paging-in gfn %lx Dom%d bytes left 
> %d\n",
> +                     gfn_l, d->domain_id, ret);

%pd, and "%pd gfn %lx" would be a more natural way to phrase it.

That said - I'm not sure how useful the information is.  We don't
normally print any diagnostics on -EFAULT and I don't see why this case
is special.

With at least %pd fixed, but preferably with the printk() dropped,
Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>