Excerpts from Haren Myneni's message of May 21, 2021 7:31 pm:
>
> Take pid and mm references when each window opens and drops during
> close. This functionality is needed for powerNV and pseries. So
> this patch defines the existing code as functions in common book3s
> platform vas-api.c
>
> Signed-off-by: Haren Myneni <ha...@linux.ibm.com>
Seems like a good idea to put these into their own helper functions.
> ---
> arch/powerpc/include/asm/vas.h | 25 +++++++++
> arch/powerpc/platforms/book3s/vas-api.c | 51 ++++++++++++++++++
> arch/powerpc/platforms/powernv/vas-fault.c | 10 ++--
> arch/powerpc/platforms/powernv/vas-window.c | 57 ++-------------------
> arch/powerpc/platforms/powernv/vas.h | 6 +--
> 5 files changed, 88 insertions(+), 61 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/vas.h b/arch/powerpc/include/asm/vas.h
> index 668303198772..3f2b02461a76 100644
> --- a/arch/powerpc/include/asm/vas.h
> +++ b/arch/powerpc/include/asm/vas.h
> @@ -5,6 +5,9 @@
>
> #ifndef _ASM_POWERPC_VAS_H
> #define _ASM_POWERPC_VAS_H
> +#include <linux/sched/mm.h>
> +#include <linux/mmu_context.h>
> +#include <asm/icswx.h>
> #include <uapi/asm/vas-api.h>
>
> struct vas_window;
> @@ -49,6 +52,17 @@ enum vas_cop_type {
> VAS_COP_TYPE_MAX,
> };
>
> +/*
> + * User space VAS windows are opened by tasks and take references
> + * to pid and mm until windows are closed.
> + * Stores pid, mm, and tgid for each window.
> + */
> +struct vas_user_win_ref {
> + struct pid *pid; /* PID of owner */
> + struct pid *tgid; /* Thread group ID of owner */
> + struct mm_struct *mm; /* Linux process mm_struct */
> +};
> +
> /*
> * User space window operations used for powernv and powerVM
> */
> @@ -59,6 +73,16 @@ struct vas_user_win_ops {
> int (*close_win)(void *);
> };
>
> +static inline void vas_drop_reference_pid_mm(struct vas_user_win_ref *ref)
> +{
> + /* Drop references to pid and mm */
> + put_pid(ref->pid);
> + if (ref->mm) {
> + mm_context_remove_vas_window(ref->mm);
> + mmdrop(ref->mm);
> + }
> +}
You don't have to make up a new name for such a thing because you
already have one
put_vas_user_win_ref(struct vas_user_win_ref *ref)
> +
> /*
> * Receive window attributes specified by the (in-kernel) owner of window.
> */
> @@ -192,4 +216,5 @@ int vas_register_coproc_api(struct module *mod, enum
> vas_cop_type cop_type,
> struct vas_user_win_ops *vops);
> void vas_unregister_coproc_api(void);
>
> +int vas_reference_pid_mm(struct vas_user_win_ref *task_ref);
> #endif /* __ASM_POWERPC_VAS_H */
> diff --git a/arch/powerpc/platforms/book3s/vas-api.c
> b/arch/powerpc/platforms/book3s/vas-api.c
> index 6c39320bfb9b..a0141bfb2e4b 100644
> --- a/arch/powerpc/platforms/book3s/vas-api.c
> +++ b/arch/powerpc/platforms/book3s/vas-api.c
> @@ -55,6 +55,57 @@ static char *coproc_devnode(struct device *dev, umode_t
> *mode)
> return kasprintf(GFP_KERNEL, "crypto/%s", dev_name(dev));
> }
>
> +/*
> + * Take reference to pid and mm
> + */
> +int vas_reference_pid_mm(struct vas_user_win_ref *task_ref)
> +{
So this is quite different from a typical refcount object in that it's
opening it for access as well. I would split it in two functions, one
matching put_vas_user_win_ref() and appearing in the same place in code,
which is up to about mmput and another function that adds the window and
does the CP_ABORT etc... hmm, where do you release tgid?
Thanks,
Nick
> + /*
> + * Window opened by a child thread may not be closed when
> + * it exits. So take reference to its pid and release it
> + * when the window is free by parent thread.
> + * Acquire a reference to the task's pid to make sure
> + * pid will not be re-used - needed only for multithread
> + * applications.
> + */
> + task_ref->pid = get_task_pid(current, PIDTYPE_PID);
> + /*
> + * Acquire a reference to the task's mm.
> + */
> + task_ref->mm = get_task_mm(current);
> + if (!task_ref->mm) {
> + put_pid(task_ref->pid);
> + pr_err("VAS: pid(%d): mm_struct is not found\n",
> + current->pid);
> + return -EPERM;
> + }
> +
> + mmgrab(task_ref->mm);
> + mmput(task_ref->mm);
> + mm_context_add_vas_window(task_ref->mm);
> + /*
> + * Process closes window during exit. In the case of
> + * multithread application, the child thread can open
> + * window and can exit without closing it. Expects parent
> + * thread to use and close the window. So do not need
> + * to take pid reference for parent thread.
> + */
> + task_ref->tgid = find_get_pid(task_tgid_vnr(current));
> + /*
> + * Even a process that has no foreign real address mapping can
> + * use an unpaired COPY instruction (to no real effect). Issue
> + * CP_ABORT to clear any pending COPY and prevent a covert
> + * channel.
> + *
> + * __switch_to() will issue CP_ABORT on future context switches
> + * if process / thread has any open VAS window (Use
> + * current->mm->context.vas_windows).
> + */
> + asm volatile(PPC_CP_ABORT);
> +
> + return 0;
> +}
> +
> 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 3d21fce254b7..ac3a71ec3bd5 100644
> --- a/arch/powerpc/platforms/powernv/vas-fault.c
> +++ b/arch/powerpc/platforms/powernv/vas-fault.c
> @@ -73,7 +73,7 @@ static void update_csb(struct vas_window *window,
> * 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->mm || !window->user_win))
> + if (WARN_ON_ONCE(!window->task_ref.mm || !window->user_win))
> return;
>
> csb_addr = (void __user *)be64_to_cpu(crb->csb_addr);
> @@ -92,7 +92,7 @@ static void update_csb(struct vas_window *window,
> csb.address = crb->stamp.nx.fault_storage_addr;
> csb.flags = 0;
>
> - pid = window->pid;
> + pid = window->task_ref.pid;
> tsk = get_pid_task(pid, PIDTYPE_PID);
> /*
> * Process closes send window after all pending NX requests are
> @@ -111,7 +111,7 @@ static void update_csb(struct vas_window *window,
> * a window and exits without closing it.
> */
> if (!tsk) {
> - pid = window->tgid;
> + pid = window->task_ref.tgid;
> tsk = get_pid_task(pid, PIDTYPE_PID);
> /*
> * Parent thread (tgid) will be closing window when it
> @@ -127,7 +127,7 @@ static void update_csb(struct vas_window *window,
> return;
> }
>
> - kthread_use_mm(window->mm);
> + 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
> @@ -139,7 +139,7 @@ static void update_csb(struct vas_window *window,
> smp_mb();
> rc = copy_to_user(csb_addr, &csb, sizeof(u8));
> }
> - kthread_unuse_mm(window->mm);
> + kthread_unuse_mm(window->task_ref.mm);
> put_task_struct(tsk);
>
> /* Success */
> diff --git a/arch/powerpc/platforms/powernv/vas-window.c
> b/arch/powerpc/platforms/powernv/vas-window.c
> index 3ccd3edcaf1a..ffd619e5a218 100644
> --- a/arch/powerpc/platforms/powernv/vas-window.c
> +++ b/arch/powerpc/platforms/powernv/vas-window.c
> @@ -1065,51 +1065,9 @@ struct vas_window *vas_tx_win_open(int vasid, enum
> vas_cop_type cop,
> rc = -ENODEV;
> goto free_window;
> }
> -
> - /*
> - * Window opened by a child thread may not be closed when
> - * it exits. So take reference to its pid and release it
> - * when the window is free by parent thread.
> - * Acquire a reference to the task's pid to make sure
> - * pid will not be re-used - needed only for multithread
> - * applications.
> - */
> - txwin->pid = get_task_pid(current, PIDTYPE_PID);
> - /*
> - * Acquire a reference to the task's mm.
> - */
> - txwin->mm = get_task_mm(current);
> -
> - if (!txwin->mm) {
> - put_pid(txwin->pid);
> - pr_err("VAS: pid(%d): mm_struct is not found\n",
> - current->pid);
> - rc = -EPERM;
> + rc = vas_reference_pid_mm(&txwin->task_ref);
> + if (rc)
> goto free_window;
> - }
> -
> - mmgrab(txwin->mm);
> - mmput(txwin->mm);
> - mm_context_add_vas_window(txwin->mm);
> - /*
> - * Process closes window during exit. In the case of
> - * multithread application, the child thread can open
> - * window and can exit without closing it. Expects parent
> - * thread to use and close the window. So do not need
> - * to take pid reference for parent thread.
> - */
> - txwin->tgid = find_get_pid(task_tgid_vnr(current));
> - /*
> - * Even a process that has no foreign real address mapping can
> - * use an unpaired COPY instruction (to no real effect). Issue
> - * CP_ABORT to clear any pending COPY and prevent a covert
> - * channel.
> - *
> - * __switch_to() will issue CP_ABORT on future context switches
> - * if process / thread has any open VAS window (Use
> - * current->mm->context.vas_windows).
> - */
> - asm volatile(PPC_CP_ABORT);
> }
>
> set_vinst_win(vinst, txwin);
> @@ -1339,14 +1297,9 @@ int vas_win_close(struct vas_window *window)
>
> /* if send window, drop reference to matching receive window */
> if (window->tx_win) {
> - if (window->user_win) {
> - /* Drop references to pid and mm */
> - put_pid(window->pid);
> - if (window->mm) {
> - mm_context_remove_vas_window(window->mm);
> - mmdrop(window->mm);
> - }
> - }
> + if (window->user_win)
> + vas_drop_reference_pid_mm(&window->task_ref);
> +
> put_rx_win(window->rxwin);
> }
>
> diff --git a/arch/powerpc/platforms/powernv/vas.h
> b/arch/powerpc/platforms/powernv/vas.h
> index c7db3190baca..f354dd5c51bd 100644
> --- a/arch/powerpc/platforms/powernv/vas.h
> +++ b/arch/powerpc/platforms/powernv/vas.h
> @@ -357,11 +357,9 @@ struct vas_window {
> bool user_win; /* True if user space window */
> void *hvwc_map; /* HV window context */
> void *uwc_map; /* OS/User window context */
> - struct pid *pid; /* Linux process id of owner */
> - struct pid *tgid; /* Thread group ID of owner */
> - struct mm_struct *mm; /* Linux process mm_struct */
> int wcreds_max; /* Window credits */
>
> + struct vas_user_win_ref task_ref;
> char *dbgname;
> struct dentry *dbgdir;
>
> @@ -443,7 +441,7 @@ extern void vas_win_paste_addr(struct vas_window *window,
> u64 *addr,
>
> static inline int vas_window_pid(struct vas_window *window)
> {
> - return pid_vnr(window->pid);
> + return pid_vnr(window->task_ref.pid);
> }
>
> static inline void vas_log_write(struct vas_window *win, char *name,
> --
> 2.18.2
>
>
>
