diff mbox series

[V5,06/14] powerpc/vas: Setup thread IRQ handler per VAS instance

Message ID 1579680639.26081.31.camel@hbabu-laptop (mailing list archive)
State Superseded
Headers show
Series powerpc/vas: Page fault handling for user space NX requests | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch powerpc/merge (20862247a368dbb75d6e97d82345999adaacf3cc)
snowpatch_ozlabs/checkpatch warning total: 0 errors, 0 warnings, 12 checks, 215 lines checked
snowpatch_ozlabs/needsstable success Patch has no Fixes tags

Commit Message

Haren Myneni Jan. 22, 2020, 8:10 a.m. UTC
Setup thread IRQ handler per each VAS instance. When NX sees a fault
on CRB, kernel gets an interrupt and vas_fault_handler will be
executed to process fault CRBs. Read all valid CRBs from fault FIFO,
determine the corresponding send window from CRB and process fault
requests.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/vas-fault.c  | 85 +++++++++++++++++++++++++++++
 arch/powerpc/platforms/powernv/vas-window.c | 60 ++++++++++++++++++++
 arch/powerpc/platforms/powernv/vas.c        | 21 ++++++-
 arch/powerpc/platforms/powernv/vas.h        |  4 ++
 4 files changed, 169 insertions(+), 1 deletion(-)

Comments

Michael Neuling Feb. 7, 2020, 5:57 a.m. UTC | #1
>  /*
> + * Process CRBs that we receive on the fault window.
> + */
> +irqreturn_t vas_fault_handler(int irq, void *data)
> +{
> +	struct vas_instance *vinst = data;
> +	struct coprocessor_request_block buf, *crb;
> +	struct vas_window *window;
> +	void *fifo;
> +
> +	/*
> +	 * VAS can interrupt with multiple page faults. So process all
> +	 * valid CRBs within fault FIFO until reaches invalid CRB.
> +	 * NX updates nx_fault_stamp in CRB and pastes in fault FIFO.
> +	 * kernel retrives send window from parition send window ID
> +	 * (pswid) in nx_fault_stamp. So pswid should be non-zero and
> +	 * use this to check whether CRB is valid.
> +	 * After reading CRB entry, it is reset with 0's in fault FIFO.
> +	 *
> +	 * In case kernel receives another interrupt with different page
> +	 * fault and CRBs are processed by the previous handling, will be
> +	 * returned from this function when it sees invalid CRB (means 0's).
> +	 */
> +	do {
> +		mutex_lock(&vinst->mutex);

This isn't going to work.

From Documentation/locking/mutex-design.rst

    - Mutexes may not be used in hardware or software interrupt
      contexts such as tasklets and timers.

Mikey
Haren Myneni Feb. 10, 2020, 5:17 a.m. UTC | #2
On Fri, 2020-02-07 at 16:57 +1100, Michael Neuling wrote:
> >  /*
> > + * Process CRBs that we receive on the fault window.
> > + */
> > +irqreturn_t vas_fault_handler(int irq, void *data)
> > +{
> > +	struct vas_instance *vinst = data;
> > +	struct coprocessor_request_block buf, *crb;
> > +	struct vas_window *window;
> > +	void *fifo;
> > +
> > +	/*
> > +	 * VAS can interrupt with multiple page faults. So process all
> > +	 * valid CRBs within fault FIFO until reaches invalid CRB.
> > +	 * NX updates nx_fault_stamp in CRB and pastes in fault FIFO.
> > +	 * kernel retrives send window from parition send window ID
> > +	 * (pswid) in nx_fault_stamp. So pswid should be non-zero and
> > +	 * use this to check whether CRB is valid.
> > +	 * After reading CRB entry, it is reset with 0's in fault FIFO.
> > +	 *
> > +	 * In case kernel receives another interrupt with different page
> > +	 * fault and CRBs are processed by the previous handling, will be
> > +	 * returned from this function when it sees invalid CRB (means 0's).
> > +	 */
> > +	do {
> > +		mutex_lock(&vinst->mutex);
> 
> This isn't going to work.
> 
> From Documentation/locking/mutex-design.rst
> 
>     - Mutexes may not be used in hardware or software interrupt
>       contexts such as tasklets and timers.

Initially used kernel thread per VAS instance and later using IRQ
thread. 

vas_fault_handler() is IRQ thread function, not IRQ handler. I thought
we can use mutex_lock() in thread function.

> 
> Mikey
>
Michael Neuling Feb. 11, 2020, 4:08 a.m. UTC | #3
On Sun, 2020-02-09 at 21:17 -0800, Haren Myneni wrote:
> On Fri, 2020-02-07 at 16:57 +1100, Michael Neuling wrote:
> > >  /*
> > > + * Process CRBs that we receive on the fault window.
> > > + */
> > > +irqreturn_t vas_fault_handler(int irq, void *data)
> > > +{
> > > +	struct vas_instance *vinst = data;
> > > +	struct coprocessor_request_block buf, *crb;
> > > +	struct vas_window *window;
> > > +	void *fifo;
> > > +
> > > +	/*
> > > +	 * VAS can interrupt with multiple page faults. So process all
> > > +	 * valid CRBs within fault FIFO until reaches invalid CRB.
> > > +	 * NX updates nx_fault_stamp in CRB and pastes in fault FIFO.
> > > +	 * kernel retrives send window from parition send window ID
> > > +	 * (pswid) in nx_fault_stamp. So pswid should be non-zero and
> > > +	 * use this to check whether CRB is valid.
> > > +	 * After reading CRB entry, it is reset with 0's in fault FIFO.
> > > +	 *
> > > +	 * In case kernel receives another interrupt with different page
> > > +	 * fault and CRBs are processed by the previous handling, will be
> > > +	 * returned from this function when it sees invalid CRB (means 0's).
> > > +	 */
> > > +	do {
> > > +		mutex_lock(&vinst->mutex);
> > 
> > This isn't going to work.
> > 
> > From Documentation/locking/mutex-design.rst
> > 
> >     - Mutexes may not be used in hardware or software interrupt
> >       contexts such as tasklets and timers.
> 
> Initially used kernel thread per VAS instance and later using IRQ
> thread. 
> 
> vas_fault_handler() is IRQ thread function, not IRQ handler. I thought
> we can use mutex_lock() in thread function.

Sorry, I missed it was a threaded IRQ handler, so I think is ok to use a
mutex_lock() in there.

You should run with CONFIG DEBUG_MUTEXES and CONFIG_LOCKDEP enabled to give you
some more confidence.

It would be good to document how this mutex is used and document the start of
the function so it doesn't get changed later to a non-threaded handler. 

Mikey
diff mbox series

Patch

diff --git a/arch/powerpc/platforms/powernv/vas-fault.c b/arch/powerpc/platforms/powernv/vas-fault.c
index b0258ed..5c2cada 100644
--- a/arch/powerpc/platforms/powernv/vas-fault.c
+++ b/arch/powerpc/platforms/powernv/vas-fault.c
@@ -11,6 +11,7 @@ 
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 #include <linux/kthread.h>
+#include <linux/mmu_context.h>
 #include <asm/icswx.h>
 
 #include "vas.h"
@@ -25,6 +26,90 @@ 
 #define VAS_FAULT_WIN_FIFO_SIZE	(4 << 20)
 
 /*
+ * Process CRBs that we receive on the fault window.
+ */
+irqreturn_t vas_fault_handler(int irq, void *data)
+{
+	struct vas_instance *vinst = data;
+	struct coprocessor_request_block buf, *crb;
+	struct vas_window *window;
+	void *fifo;
+
+	/*
+	 * VAS can interrupt with multiple page faults. So process all
+	 * valid CRBs within fault FIFO until reaches invalid CRB.
+	 * NX updates nx_fault_stamp in CRB and pastes in fault FIFO.
+	 * kernel retrives send window from parition send window ID
+	 * (pswid) in nx_fault_stamp. So pswid should be non-zero and
+	 * use this to check whether CRB is valid.
+	 * After reading CRB entry, it is reset with 0's in fault FIFO.
+	 *
+	 * In case kernel receives another interrupt with different page
+	 * fault and CRBs are processed by the previous handling, will be
+	 * returned from this function when it sees invalid CRB (means 0's).
+	 */
+	do {
+		mutex_lock(&vinst->mutex);
+
+		/*
+		 * Advance the fault fifo pointer to next CRB.
+		 * Use CRB_SIZE rather than sizeof(*crb) since the latter is
+		 * aligned to CRB_ALIGN (256) but the CRB written to by VAS is
+		 * only CRB_SIZE in len.
+		 */
+		fifo = vinst->fault_fifo + (vinst->fault_crbs * CRB_SIZE);
+		crb = fifo;
+
+		/*
+		 * NX pastes nx_fault_stamp in fault FIFO for each fault.
+		 * So use pswid to check whether fault CRB is valid.
+		 * pswid returned from NX will be in _be32, but just
+		 * checking non-zero value to make sure the CRB is valid.
+		 * Return if reached invalid CRB.
+		 */
+		if (!crb->stamp.nx.pswid) {
+			mutex_unlock(&vinst->mutex);
+			return IRQ_HANDLED;
+		}
+
+		vinst->fault_crbs++;
+		if (vinst->fault_crbs == (vinst->fault_fifo_size / CRB_SIZE))
+			vinst->fault_crbs = 0;
+
+		crb = &buf;
+		memcpy(crb, fifo, CRB_SIZE);
+		memset(fifo, 0, CRB_SIZE);
+		mutex_unlock(&vinst->mutex);
+
+		pr_devel("VAS[%d] fault_fifo %p, fifo %p, fault_crbs %d\n",
+				vinst->vas_id, vinst->fault_fifo, fifo,
+				vinst->fault_crbs);
+
+		window = vas_pswid_to_window(vinst,
+				be32_to_cpu(crb->stamp.nx.pswid));
+
+		if (IS_ERR(window)) {
+			/*
+			 * We got an interrupt about a specific send
+			 * window but we can't find that window and we can't
+			 * even clean it up (return credit).
+			 * But we should not get here.
+			 */
+			pr_err("VAS[%d] fault_fifo %p, fifo %p, pswid 0x%x, fault_crbs %d bad CRB?\n",
+				vinst->vas_id, vinst->fault_fifo, fifo,
+				be32_to_cpu(crb->stamp.nx.pswid),
+				vinst->fault_crbs);
+
+			WARN_ON_ONCE(1);
+			return IRQ_HANDLED;
+		}
+
+	} while (true);
+
+	return IRQ_HANDLED;
+}
+
+/*
  * Fault window is opened per VAS instance. NX pastes fault CRB in fault
  * FIFO upon page faults.
  */
diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index 1783fa9..7c6f55f 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -1040,6 +1040,15 @@  struct vas_window *vas_tx_win_open(int vasid, enum vas_cop_type cop,
 		}
 	} else {
 		/*
+		 * Interrupt hanlder or fault window setup failed. Means
+		 * NX can not generate fault for page fault. So not
+		 * opening for user space tx window.
+		 */
+		if (!vinst->virq) {
+			rc = -ENODEV;
+			goto free_window;
+		}
+		/*
 		 * A user mapping must ensure that context switch issues
 		 * CP_ABORT for this thread.
 		 */
@@ -1254,3 +1263,54 @@  int vas_win_close(struct vas_window *window)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(vas_win_close);
+
+struct vas_window *vas_pswid_to_window(struct vas_instance *vinst,
+		uint32_t pswid)
+{
+	int winid;
+	struct vas_window *window;
+
+	if (!pswid) {
+		pr_devel("%s: called for pswid 0!\n", __func__);
+		return ERR_PTR(-ESRCH);
+	}
+
+	decode_pswid(pswid, NULL, &winid);
+
+	if (winid >= VAS_WINDOWS_PER_CHIP)
+		return ERR_PTR(-ESRCH);
+
+	/*
+	 * If application closes the window before the hardware
+	 * returns the fault CRB, we should wait in vas_win_close()
+	 * for the pending requests. so the window must be active
+	 * and the process alive.
+	 *
+	 * If its a kernel process, we should not get any faults and
+	 * should not get here.
+	 */
+	window = vinst->windows[winid];
+
+	if (!window) {
+		pr_err("PSWID decode: Could not find window for winid %d pswid %d vinst 0x%p\n",
+			winid, pswid, vinst);
+		return NULL;
+	}
+
+	/*
+	 * Do some sanity checks on the decoded window.  Window should be
+	 * NX GZIP user send window. FTW windows should not incur faults
+	 * since their CRBs are ignored (not queued on FIFO or processed
+	 * by NX).
+	 */
+	if (!window->tx_win || !window->user_win || !window->nx_win ||
+			window->cop == VAS_COP_TYPE_FAULT ||
+			window->cop == VAS_COP_TYPE_FTW) {
+		pr_err("PSWID decode: id %d, tx %d, user %d, nx %d, cop %d\n",
+			winid, window->tx_win, window->user_win,
+			window->nx_win, window->cop);
+		WARN_ON(1);
+	}
+
+	return window;
+}
diff --git a/arch/powerpc/platforms/powernv/vas.c b/arch/powerpc/platforms/powernv/vas.c
index 557c8e4..46ea57d 100644
--- a/arch/powerpc/platforms/powernv/vas.c
+++ b/arch/powerpc/platforms/powernv/vas.c
@@ -14,6 +14,8 @@ 
 #include <linux/of_platform.h>
 #include <linux/of_address.h>
 #include <linux/of.h>
+#include <linux/irqdomain.h>
+#include <linux/interrupt.h>
 #include <asm/prom.h>
 #include <asm/xive.h>
 
@@ -26,7 +28,24 @@ 
 
 static int vas_irq_fault_window_setup(struct vas_instance *vinst)
 {
-	return vas_setup_fault_window(vinst);
+	char devname[64];
+	int rc = 0;
+
+	snprintf(devname, sizeof(devname), "vas-%d", vinst->vas_id);
+	rc = request_threaded_irq(vinst->virq, NULL, vas_fault_handler,
+					IRQF_ONESHOT, devname, vinst);
+	if (rc) {
+		pr_err("VAS[%d]: Request IRQ(%d) failed with %d\n",
+				vinst->vas_id, vinst->virq, rc);
+		goto out;
+	}
+
+	rc = vas_setup_fault_window(vinst);
+	if (rc)
+		free_irq(vinst->virq, vinst);
+
+out:
+	return rc;
 }
 
 static int init_vas_instance(struct platform_device *pdev)
diff --git a/arch/powerpc/platforms/powernv/vas.h b/arch/powerpc/platforms/powernv/vas.h
index 9f08daa..879f5b4 100644
--- a/arch/powerpc/platforms/powernv/vas.h
+++ b/arch/powerpc/platforms/powernv/vas.h
@@ -315,6 +315,7 @@  struct vas_instance {
 
 	u64 irq_port;
 	int virq;
+	int fault_crbs;
 	int fault_fifo_size;
 	void *fault_fifo;
 	struct vas_window *fault_win; /* Fault window */
@@ -413,6 +414,9 @@  struct vas_winctx {
 extern void vas_window_init_dbgdir(struct vas_window *win);
 extern void vas_window_free_dbgdir(struct vas_window *win);
 extern int vas_setup_fault_window(struct vas_instance *vinst);
+extern irqreturn_t vas_fault_handler(int irq, void *data);
+extern struct vas_window *vas_pswid_to_window(struct vas_instance *vinst,
+						uint32_t pswid);
 
 static inline void vas_log_write(struct vas_window *win, char *name,
 			void *regptr, u64 val)