Excerpts from Haren Myneni's message of April 18, 2021 7:03 am:
>
> NX issues an interrupt when sees fault on user space buffer.
If a coprocessor encounters an error translating an address, the VAS
will cause an interrupt in the host.
> The
> kernel processes the fault by updating CSB. This functionality is
> same for both powerNV and pseries. So this patch moves these
> functions to common vas-api.c and the actual functionality is not
> changed.
>
> Signed-off-by: Haren Myneni <haren@xxxxxxxxxxxxx>
> ---
> arch/powerpc/include/asm/vas.h | 3 +
> arch/powerpc/platforms/book3s/vas-api.c | 146 ++++++++++++++++++-
> arch/powerpc/platforms/powernv/vas-fault.c | 155 ++-------------------
> 3 files changed, 157 insertions(+), 147 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/vas.h b/arch/powerpc/include/asm/vas.h
> index 2daaa1a2a9a9..66bf8fb1a1be 100644
> --- a/arch/powerpc/include/asm/vas.h
> +++ b/arch/powerpc/include/asm/vas.h
> @@ -210,4 +210,7 @@ int vas_register_coproc_api(struct module *mod, enum vas_cop_type cop_type,
> void vas_unregister_coproc_api(void);
>
> int vas_reference_task(struct vas_win_task *vtask);
> +void vas_update_csb(struct coprocessor_request_block *crb,
> + struct vas_win_task *vtask);
> +void vas_dump_crb(struct coprocessor_request_block *crb);
> #endif /* __ASM_POWERPC_VAS_H */
> diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c
> index d98caa734154..dc131b2e4acd 100644
> --- a/arch/powerpc/platforms/book3s/vas-api.c
> +++ b/arch/powerpc/platforms/book3s/vas-api.c
> @@ -111,6 +111,150 @@ int vas_reference_task(struct vas_win_task *vtask)
> return 0;
> }
>
> +/*
> + * Update the CSB to indicate a translation error.
> + *
> + * User space will be polling on CSB after the request is issued.
> + * If NX can handle the request without any issues, it updates CSB.
> + * Whereas if NX encounters page fault, the kernel will handle the
> + * fault and update CSB with translation error.
> + *
> + * If we are unable to update the CSB means copy_to_user failed due to
> + * invalid csb_addr, send a signal to the process.
> + */
> +void vas_update_csb(struct coprocessor_request_block *crb,
> + struct vas_win_task *vtask)
> +{
> + struct coprocessor_status_block csb;
> + struct kernel_siginfo info;
> + struct task_struct *tsk;
> + void __user *csb_addr;
> + struct pid *pid;
> + int rc;
> +
> + /*
> + * NX user space windows can not be opened for task->mm=NULL
> + * and faults will not be generated for kernel requests.
> + */
> + if (WARN_ON_ONCE(!vtask->mm))
> + return;
> +
> + csb_addr = (void __user *)be64_to_cpu(crb->csb_addr);
> +
> + memset(&csb, 0, sizeof(csb));
> + csb.cc = CSB_CC_FAULT_ADDRESS;
> + csb.ce = CSB_CE_TERMINATION;
> + csb.cs = 0;
> + csb.count = 0;
> +
> + /*
> + * NX operates and returns in BE format as defined CRB struct.
> + * So saves fault_storage_addr in BE as NX pastes in FIFO and
> + * expects user space to convert to CPU format.
> + */
> + csb.address = crb->stamp.nx.fault_storage_addr;
> + csb.flags = 0;
> +
> + pid = vtask->pid;
> + tsk = get_pid_task(pid, PIDTYPE_PID);
> + /*
> + * Process closes send window after all pending NX requests are
> + * completed. In multi-thread applications, a child thread can
> + * open a window and can exit without closing it. May be some
> + * requests are pending or this window can be used by other
> + * threads later. We should handle faults if NX encounters
> + * pages faults on these requests. Update CSB with translation
> + * error and fault address. If csb_addr passed by user space is
> + * invalid, send SEGV signal to pid saved in window. If the
> + * child thread is not running, send the signal to tgid.
> + * Parent thread (tgid) will close this window upon its exit.
> + *
> + * pid and mm references are taken when window is opened by
> + * process (pid). So tgid is used only when child thread opens
> + * a window and exits without closing it.
> + */
> + if (!tsk) {
> + pid = vtask->tgid;
> + tsk = get_pid_task(pid, PIDTYPE_PID);
> + /*
> + * Parent thread (tgid) will be closing window when it
> + * exits. So should not get here.
> + */
> + if (WARN_ON_ONCE(!tsk))
> + return;
> + }
> +
> + /* Return if the task is exiting. */
> + if (tsk->flags & PF_EXITING) {
> + put_task_struct(tsk);
> + return;
> + }
> +
> + kthread_use_mm(vtask->mm);
> + rc = copy_to_user(csb_addr, &csb, sizeof(csb));
> + /*
> + * User space polls on csb.flags (first byte). So add barrier
> + * then copy first byte with csb flags update.
> + */
> + if (!rc) {
> + csb.flags = CSB_V;
> + /* Make sure update to csb.flags is visible now */
> + smp_mb();
> + rc = copy_to_user(csb_addr, &csb, sizeof(u8));
> + }
> + kthread_unuse_mm(vtask->mm);
> + put_task_struct(tsk);
> +
> + /* Success */
> + if (!rc)
> + return;
> +
> +
> + pr_debug("Invalid CSB address 0x%p signalling pid(%d)\n",
> + csb_addr, pid_vnr(pid));
> +
> + clear_siginfo(&info);
> + info.si_signo = SIGSEGV;
> + info.si_errno = EFAULT;
> + info.si_code = SEGV_MAPERR;
> + info.si_addr = csb_addr;
> + /*
> + * process will be polling on csb.flags after request is sent to
> + * NX. So generally CSB update should not fail except when an
> + * application passes invalid csb_addr. So an error message will
> + * be displayed and leave it to user space whether to ignore or
> + * handle this signal.
> + */
> + rcu_read_lock();
> + rc = kill_pid_info(SIGSEGV, &info, pid);
> + rcu_read_unlock();
> +
> + pr_devel("%s(): pid %d kill_proc_info() rc %d\n", __func__,
> + pid_vnr(pid), rc);
> +}
> +
> +void vas_dump_crb(struct coprocessor_request_block *crb)
> +{
> + struct data_descriptor_entry *dde;
> + struct nx_fault_stamp *nx;
> +
> + dde = &crb->source;
> + pr_devel("SrcDDE: addr 0x%llx, len %d, count %d, idx %d, flags %d\n",
> + be64_to_cpu(dde->address), be32_to_cpu(dde->length),
> + dde->count, dde->index, dde->flags);
> +
> + dde = &crb->target;
> + pr_devel("TgtDDE: addr 0x%llx, len %d, count %d, idx %d, flags %d\n",
> + be64_to_cpu(dde->address), be32_to_cpu(dde->length),
> + dde->count, dde->index, dde->flags);
> +
> + nx = &crb->stamp.nx;
> + pr_devel("NX Stamp: PSWID 0x%x, FSA 0x%llx, flags 0x%x, FS 0x%x\n",
> + be32_to_cpu(nx->pswid),
> + be64_to_cpu(crb->stamp.nx.fault_storage_addr),
> + nx->flags, nx->fault_status);
> +}
> +
> static int coproc_open(struct inode *inode, struct file *fp)
> {
> struct coproc_instance *cp_inst;
> @@ -272,7 +416,7 @@ static struct file_operations coproc_fops = {
> * extended to other coprocessor types later.
> */
> int vas_register_coproc_api(struct module *mod, enum vas_cop_type cop_type,
> - const char *name, struct vas_user_win_ops *vops)
> + const char *name, struct vas_user_win_ops *vops)
> {
> int rc = -EINVAL;
> dev_t devno;
This change should go back where you added the code. But you've
brought back vas_register_corpoc_api? In... patch 2, by the looks
which just renames them back. Perhaps think about keeping the
vas_register_coproc_api in patch 1 in that case, and just adding
the new powernv specific wrapper over it.
But for this patch,
Reviewed-by: Nicholas Piggin <npiggin@xxxxxxxxx>
Thanks,
Nick
