Re: [PATCH v2 6/8] x86: rename copy_{from,to}_user() to copy_{from,to}_guest_pv()

From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Andrew Cooper <andrew.cooper3@citrix.com>, Wei Liu <wl@xen.org>,
	George Dunlap <george.dunlap@citrix.com>,
	Ian Jackson <iwj@xenproject.org>
Subject: Re: [PATCH v2 6/8] x86: rename copy_{from,to}_user() to copy_{from,to}_guest_pv()
Date: Tue, 23 Feb 2021 12:04:00 +0100	[thread overview]
Message-ID: <YDThIFB7ox6qdfFE@Air-de-Roger> (raw)
In-Reply-To: <5104a32f-e2a1-06a5-a637-9702e4562b81@suse.com>

On Wed, Feb 17, 2021 at 09:22:32AM +0100, Jan Beulich wrote:
> Bring them (back) in line with __copy_{from,to}_guest_pv(). Since it
> falls in the same group, also convert clear_user(). Instead of adjusting
> __raw_clear_guest(), drop it - it's unused and would require a non-
> checking __clear_guest_pv() which we don't have.
> 
> Add previously missing __user at some call sites and in the function
> declarations.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

> 
> --- a/xen/arch/x86/pv/emul-inv-op.c
> +++ b/xen/arch/x86/pv/emul-inv-op.c
> @@ -33,7 +33,7 @@ static int emulate_forced_invalid_op(str
>      eip = regs->rip;
>  
>      /* Check for forced emulation signature: ud2 ; .ascii "xen". */
> -    if ( (rc = copy_from_user(sig, (char *)eip, sizeof(sig))) != 0 )
> +    if ( (rc = copy_from_guest_pv(sig, (char __user *)eip, sizeof(sig))) != 0 )
>      {
>          pv_inject_page_fault(0, eip + sizeof(sig) - rc);
>          return EXCRET_fault_fixed;
> @@ -43,7 +43,8 @@ static int emulate_forced_invalid_op(str
>      eip += sizeof(sig);
>  
>      /* We only emulate CPUID. */
> -    if ( ( rc = copy_from_user(instr, (char *)eip, sizeof(instr))) != 0 )
> +    if ( (rc = copy_from_guest_pv(instr, (char __user *)eip,
> +                                  sizeof(instr))) != 0 )
>      {
>          pv_inject_page_fault(0, eip + sizeof(instr) - rc);
>          return EXCRET_fault_fixed;
> --- a/xen/arch/x86/pv/iret.c
> +++ b/xen/arch/x86/pv/iret.c
> @@ -54,8 +54,8 @@ unsigned long do_iret(void)
>      struct iret_context iret_saved;
>      struct vcpu *v = current;
>  
> -    if ( unlikely(copy_from_user(&iret_saved, (void *)regs->rsp,
> -                                 sizeof(iret_saved))) )
> +    if ( unlikely(copy_from_guest_pv(&iret_saved, (void __user *)regs->rsp,
> +                                     sizeof(iret_saved))) )
>      {
>          gprintk(XENLOG_ERR,
>                  "Fault while reading IRET context from guest stack\n");
> --- a/xen/arch/x86/pv/ro-page-fault.c
> +++ b/xen/arch/x86/pv/ro-page-fault.c
> @@ -90,7 +90,8 @@ static int ptwr_emulated_update(unsigned
>  
>          /* Align address; read full word. */
>          addr &= ~(sizeof(full) - 1);
> -        if ( (rc = copy_from_user(&full, (void *)addr, sizeof(full))) != 0 )
> +        if ( (rc = copy_from_guest_pv(&full, (void __user *)addr,
> +                                      sizeof(full))) != 0 )
>          {
>              x86_emul_pagefault(0, /* Read fault. */
>                                 addr + sizeof(full) - rc,
> --- a/xen/arch/x86/usercopy.c
> +++ b/xen/arch/x86/usercopy.c
> @@ -109,19 +109,17 @@ unsigned int copy_from_guest_ll(void *to
>  #if GUARD(1) + 0
>  
>  /**
> - * copy_to_user: - Copy a block of data into user space.
> - * @to:   Destination address, in user space.
> - * @from: Source address, in kernel space.
> + * copy_to_guest_pv: - Copy a block of data into guest space.

I would expand to 'PV guest' here and below, FAOD.

Thanks, Roger.