Excerpts from Haren Myneni's message of June 15, 2021 7:01 pm:

On Mon, 2021-06-14 at 13:07 +1000, Nicholas Piggin wrote:

quoted

Excerpts from Haren Myneni's message of June 13, 2021 9:02 pm:

quoted

NX generates an interrupt when sees a fault on the user space
buffer and the hypervisor forwards that interrupt to OS. Then
the kernel handles the interrupt by issuing H_GET_NX_FAULT hcall
to retrieve the fault CRB information.

This patch also adds changes to setup and free IRQ per each
window and also handles the fault by updating the CSB.

Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/vas.c | 108
+++++++++++++++++++++++++++
 1 file changed, 108 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/vas.c

b/arch/powerpc/platforms/pseries/vas.c
index fe375f7a7029..55185bdd3776 100644

--- a/arch/powerpc/platforms/pseries/vas.c
+++ b/arch/powerpc/platforms/pseries/vas.c

@@ -11,6 +11,7 @@
 #include <linux/types.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
+#include <linux/interrupt.h>
 #include <asm/machdep.h>
 #include <asm/hvcall.h>
 #include <asm/plpar_wrappers.h>

@@ -190,6 +191,58 @@ int h_query_vas_capabilities(const u64 hcall,

u8 query_type, u64 result)
 }
 EXPORT_SYMBOL_GPL(h_query_vas_capabilities);
 
+/*
+ * hcall to get fault CRB from pHyp.
+ */
+static int h_get_nx_fault(u32 winid, u64 buffer)
+{
+	long rc;
+
+	rc = plpar_hcall_norets(H_GET_NX_FAULT, winid, buffer);
+
+	switch (rc) {
+	case H_SUCCESS:
+		return 0;
+	case H_PARAMETER:
+		pr_err("HCALL(%x): Invalid window ID %u\n",
H_GET_NX_FAULT,
+		       winid);
+		return -EINVAL;
+	case H_PRIVILEGE:
+		pr_err("HCALL(%x): Window(%u): Invalid fault buffer
0x%llx\n",
+		       H_GET_NX_FAULT, winid, buffer);
+		return -EACCES;
+	default:
+		pr_err("HCALL(%x): Failed with error %ld for
window(%u)\n",
+		       H_GET_NX_FAULT, rc, winid);
+		return -EIO;

3 error messages have 3 different formats for window ID.

I agree with Michael you could just have one error message that
reports 
the return value. Also "H_GET_NX_FAULT: " would be nicer than
"HCALL(380): "

yes, Added just one printk for all error codes except for errors which
depend on arguments to HCALL (Ex: WinID).

Sure, I will add just one error message and print all arguments passed
to HCALL. 

pr_err("H_GET_NX_FAULT: window(%u), fault buffer(0x%llx) Failed with
error %ld\n", rc, winid, buffer);

Thanks.

quoted

Check how some other hcall failures are reported, "hcall failed: 
H_CALL_NAME" seems to have a few takers.

quoted

+	}
+}
+
+/*
+ * Handle the fault interrupt.
+ * When the fault interrupt is received for each window, query
pHyp to get
+ * the fault CRB on the specific fault. Then process the CRB by
updating
+ * CSB or send signal if the user space CSB is invalid.
+ * Note: pHyp forwards an interrupt for each fault request. So one
fault
+ *	CRB to process for each H_GET_NX_FAULT HCALL.
+ */
+irqreturn_t pseries_vas_fault_thread_fn(int irq, void *data)
+{
+	struct pseries_vas_window *txwin = data;
+	struct coprocessor_request_block crb;
+	struct vas_user_win_ref *tsk_ref;
+	int rc;
+
+	rc = h_get_nx_fault(txwin->vas_win.winid,
(u64)virt_to_phys(&crb));
+	if (!rc) {
+		tsk_ref = &txwin->vas_win.task_ref;
+		vas_dump_crb(&crb);
+		vas_update_csb(&crb, tsk_ref);
+	}
+
+	return IRQ_HANDLED;
+}
+
 /*
  * Allocate window and setup IRQ mapping.
  */

@@ -201,10 +254,51 @@ static int allocate_setup_window(struct

pseries_vas_window *txwin,
 	rc = h_allocate_vas_window(txwin, domain, wintype,
DEF_WIN_CREDS);
 	if (rc)
 		return rc;
+	/*
+	 * On powerVM, pHyp setup and forwards the fault interrupt per

           The hypervisor forwards the fault interrupt per-window...

quoted

+	 * window. So the IRQ setup and fault handling will be done for
+	 * each open window separately.
+	 */
+	txwin->fault_virq = irq_create_mapping(NULL, txwin->fault_irq);
+	if (!txwin->fault_virq) {
+		pr_err("Failed irq mapping %d\n", txwin->fault_irq);
+		rc = -EINVAL;
+		goto out_win;
+	}
+
+	txwin->name = kasprintf(GFP_KERNEL, "vas-win-%d",
+				txwin->vas_win.winid);
+	if (!txwin->name) {
+		rc = -ENOMEM;
+		goto out_irq;
+	}
+
+	rc = request_threaded_irq(txwin->fault_virq, NULL,
+				  pseries_vas_fault_thread_fn,
IRQF_ONESHOT,
+				  txwin->name, txwin);
+	if (rc) {
+		pr_err("VAS-Window[%d]: Request IRQ(%u) failed with
%d\n",
+		       txwin->vas_win.winid, txwin->fault_virq, rc);
+		goto out_free;
+	}
 
 	txwin->vas_win.wcreds_max = DEF_WIN_CREDS;
 
 	return 0;
+out_free:
+	kfree(txwin->name);
+out_irq:
+	irq_dispose_mapping(txwin->fault_virq);
+out_win:
+	h_deallocate_vas_window(txwin->vas_win.winid);
+	return rc;
+}
+
+static inline void free_irq_setup(struct pseries_vas_window
*txwin)
+{
+	free_irq(txwin->fault_virq, txwin);
+	irq_dispose_mapping(txwin->fault_virq);
+	kfree(txwin->name);

Nit, but this freeing is in a different order than the error handling
in 
the function does. I'd just keep it the same unless there is a reason
to 
be different, in which case it could use a comment.

shouldn't matter, I can change it to:
static inline void free_irq_setup(struct pseries_vas_window *txwin)
{
        free_irq(txwin->fault_virq, txwin);
        kfree(txwin->name);
        irq_dispose_mapping(txwin->fault_virq);
}

Okay, it wasn't a big deal I was just wondering. Given the changes,

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

Thanks,
Nick