Excerpts from Haren Myneni's message of June 18, 2021 6:32 am: > > 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> > Reviewed-by: Nicholas Piggin <npiggin@xxxxxxxxx> > --- > arch/powerpc/include/asm/vas.h | 3 + > arch/powerpc/platforms/book3s/vas-api.c | 167 +++++++++++++++++++++ > arch/powerpc/platforms/powernv/vas-fault.c | 155 ++----------------- > 3 files changed, 179 insertions(+), 146 deletions(-) > > diff --git a/arch/powerpc/include/asm/vas.h b/arch/powerpc/include/asm/vas.h > index 71cff6d6bf3a..6b41c0818958 100644 > --- a/arch/powerpc/include/asm/vas.h > +++ b/arch/powerpc/include/asm/vas.h > @@ -230,4 +230,7 @@ int vas_register_coproc_api(struct module *mod, enum vas_cop_type cop_type, > void vas_unregister_coproc_api(void); > > int get_vas_user_win_ref(struct vas_user_win_ref *task_ref); > +void vas_update_csb(struct coprocessor_request_block *crb, > + struct vas_user_win_ref *task_ref); > +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 4ce82500f4c5..30172e52e16b 100644 > --- a/arch/powerpc/platforms/book3s/vas-api.c > +++ b/arch/powerpc/platforms/book3s/vas-api.c > @@ -10,6 +10,9 @@ > #include <linux/fs.h> > #include <linux/slab.h> > #include <linux/uaccess.h> > +#include <linux/kthread.h> > +#include <linux/sched/signal.h> > +#include <linux/mmu_context.h> > #include <linux/io.h> > #include <asm/vas.h> > #include <uapi/asm/vas-api.h> > @@ -94,6 +97,170 @@ int get_vas_user_win_ref(struct vas_user_win_ref *task_ref) > return 0; > } > > +/* > + * Successful return must release the task reference with > + * put_task_struct > + */ > +static bool ref_get_pid_and_task(struct vas_user_win_ref *task_ref, > + struct task_struct **tskp, struct pid **pidp) > +{ > + struct task_struct *tsk; > + struct pid *pid; > + > + pid = task_ref->pid; > + tsk = get_pid_task(pid, PIDTYPE_PID); > + if (!tsk) { > + pid = task_ref->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 false; > + } > + > + /* Return if the task is exiting. */ > + if (tsk->flags & PF_EXITING) { > + put_task_struct(tsk); > + return false; > + } > + > + *tskp = tsk; > + *pidp = pid; > + > + return true; > +} Thanks for making this change. I think that's good to factor all these out and put them together. Reviewed-by: Nicholas Piggin <npiggin@xxxxxxxxx> > + > +/* > + * 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_user_win_ref *task_ref) > +{ > + 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(!task_ref->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; > + > + /* > + * 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 (!ref_get_pid_and_task(task_ref, &tsk, &pid)) > + return; > + > + kthread_use_mm(task_ref->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(task_ref->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; > diff --git a/arch/powerpc/platforms/powernv/vas-fault.c b/arch/powerpc/platforms/powernv/vas-fault.c > index ac3a71ec3bd5..2729ac541fb3 100644 > --- a/arch/powerpc/platforms/powernv/vas-fault.c > +++ b/arch/powerpc/platforms/powernv/vas-fault.c > @@ -26,150 +26,6 @@ > */ > #define VAS_FAULT_WIN_FIFO_SIZE (4 << 20) > > -static void 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); > -} > - > -/* > - * 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. > - */ > -static void update_csb(struct vas_window *window, > - struct coprocessor_request_block *crb) > -{ > - 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(!window->task_ref.mm || !window->user_win)) > - 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 = window->task_ref.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 = window->task_ref.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(window->task_ref.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(window->task_ref.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); > -} > - > static void dump_fifo(struct vas_instance *vinst, void *entry) > { > unsigned long *end = vinst->fault_fifo + vinst->fault_fifo_size; > @@ -272,7 +128,7 @@ irqreturn_t vas_fault_thread_fn(int irq, void *data) > vinst->vas_id, vinst->fault_fifo, fifo, > vinst->fault_crbs); > > - dump_crb(crb); > + vas_dump_crb(crb); > window = vas_pswid_to_window(vinst, > be32_to_cpu(crb->stamp.nx.pswid)); > > @@ -293,7 +149,14 @@ irqreturn_t vas_fault_thread_fn(int irq, void *data) > > WARN_ON_ONCE(1); > } else { > - update_csb(window, crb); > + /* > + * NX sees faults only with user space windows. > + */ > + if (window->user_win) > + vas_update_csb(crb, &window->task_ref); > + else > + WARN_ON_ONCE(!window->user_win); > + > /* > * Return credit for send window after processing > * fault CRB. > -- > 2.18.2 > > >