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 <ha...@linux.ibm.com>
> ---
>  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 <npig...@gmail.com>

Thanks,
Nick

Reply via email to