Re: [PATCH v3 08/10] powerpc/vas: Return paste instruction failure if no active window

[Nicholas Piggin <npiggin@gmail.com>]

Excerpts from Haren Myneni's message of January 22, 2022 5:59 am:
> 
> The VAS window may not be active if the system looses credits and
> the NX generates page fault when it receives request on unmap
> paste address.
> 
> The kernel handles the fault by remap new paste address if the
> window is active again, Otherwise return the paste instruction
> failure if the executed instruction that caused the fault was
> a paste.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/ppc-opcode.h   |  2 ++
>  arch/powerpc/platforms/book3s/vas-api.c | 47 ++++++++++++++++++++++++-
>  2 files changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
> index efad07081cc0..fe2a69206588 100644
> --- a/arch/powerpc/include/asm/ppc-opcode.h
> +++ b/arch/powerpc/include/asm/ppc-opcode.h
> @@ -262,6 +262,8 @@
>  #define PPC_INST_MFSPR_PVR		0x7c1f42a6
>  #define PPC_INST_MFSPR_PVR_MASK		0xfc1ffffe
>  #define PPC_INST_MTMSRD			0x7c000164
> +#define PPC_INST_PASTE			0x7c20070d
> +#define PPC_INST_PASTE_MASK		0xfc2007ff
>  #define PPC_INST_POPCNTB		0x7c0000f4
>  #define PPC_INST_POPCNTB_MASK		0xfc0007fe
>  #define PPC_INST_RFEBB			0x4c000124
> diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c
> index 5ceba75c13eb..2ffd34bc4032 100644
> --- a/arch/powerpc/platforms/book3s/vas-api.c
> +++ b/arch/powerpc/platforms/book3s/vas-api.c
> @@ -351,6 +351,41 @@ static int coproc_release(struct inode *inode, struct file *fp)
>  	return 0;
>  }
>  
> +/*
> + * If the executed instruction that caused the fault was a paste, then
> + * clear regs CR0[EQ], advance NIP, and return 0. Else return error code.
> + */
> +static int do_fail_paste(void)
> +{
> +	struct pt_regs *regs = current->thread.regs;
> +	u32 instword;
> +
> +	if (WARN_ON_ONCE(!regs))
> +		return -EINVAL;
> +
> +	if (WARN_ON_ONCE(!user_mode(regs)))
> +		return -EINVAL;
> +
> +	/*
> +	 * If we couldn't translate the instruction, the driver should
> +	 * return success without handling the fault, it will be retried
> +	 * or the instruction fetch will fault.
> +	 */
> +	if (get_user(instword, (u32 __user *)(regs->nip)))
> +		return -EAGAIN;

We say this, and yet the caller seems to SIGBUS us on this return
value.

> +
> +	/*
> +	 * Not a paste instruction, driver may fail the fault.
> +	 */
> +	if ((instword & PPC_INST_PASTE_MASK) != PPC_INST_PASTE)
> +		return -ENOENT;
> +
> +	regs->ccr &= ~0xe0000000;	/* Clear CR0[0-2] to fail paste */
> +	regs_add_return_ip(regs, 4);	/* Skip the paste */

Maybe instead of 'Skip the paste' the comment should be 'Emulated the 
paste'.

> +
> +	return 0;
> +}
> +
>  /*
>   * This fault handler is invoked when the VAS/NX generates page fault on
>   * the paste address. Happens if the kernel closes window in hypervisor
> @@ -403,9 +438,19 @@ static vm_fault_t vas_mmap_fault(struct vm_fault *vmf)
>  	}
>  	mutex_unlock(&txwin->task_ref.mmap_mutex);
>  
> -	return VM_FAULT_SIGBUS;
> +	/*
> +	 * Received this fault due to closing the actual window.
> +	 * It can happen during migration or lost credits.
> +	 * Since no mapping, return the paste instruction failure
> +	 * to the user space.
> +	 */
> +	ret = do_fail_paste();
> +	if (!ret)
> +		return VM_FAULT_NOPAGE;

  if (!ret || ret == -EAGAIN)

  ?

Thanks,
Nick

>  
> +	return VM_FAULT_SIGBUS;
>  }
> +
>  static const struct vm_operations_struct vas_vm_ops = {
>  	.fault = vas_mmap_fault,
>  };
> -- 
> 2.27.0
> 
> 
>