Re: [PATCH v5 04/17] powerpc/vas: Add platform specific user window operations

Excerpts from Haren Myneni's message of June 15, 2021 4:37 pm:

On Mon, 2021-06-14 at 12:24 +1000, Nicholas Piggin wrote:

quoted

Excerpts from Haren Myneni's message of June 13, 2021 8:57 pm:

quoted

PowerNV uses registers to open/close VAS windows, and getting the
paste address. Whereas the hypervisor calls are used on PowerVM.

This patch adds the platform specific user space window operations
and register with the common VAS user space interface.

Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/include/asm/vas.h              | 14 +++++-
 arch/powerpc/platforms/book3s/vas-api.c     | 53 +++++++++++++--
------
 arch/powerpc/platforms/powernv/vas-window.c | 45 ++++++++++++++++-
 3 files changed, 89 insertions(+), 23 deletions(-)

diff --git a/arch/powerpc/include/asm/vas.h

b/arch/powerpc/include/asm/vas.h
index bab7891d43f5..85318d7446c7 100644

--- a/arch/powerpc/include/asm/vas.h
+++ b/arch/powerpc/include/asm/vas.h

@@ -5,6 +5,7 @@
 
 #ifndef _ASM_POWERPC_VAS_H
 #define _ASM_POWERPC_VAS_H
+#include <uapi/asm/vas-api.h>
 
 struct vas_window;
 

@@ -48,6 +49,16 @@ enum vas_cop_type {
 	VAS_COP_TYPE_MAX,
 };
 
+/*
+ * User space window operations used for powernv and powerVM
+ */
+struct vas_user_win_ops {
+	struct vas_window * (*open_win)(struct vas_tx_win_open_attr *,
+				enum vas_cop_type);
+	u64 (*paste_addr)(struct vas_window *);
+	int (*close_win)(struct vas_window *);
+};

This looks better, but rather than pull in uapi and the user API 
structure here, could you just pass in vas_id and flags after the
common 
code does the user copy and verifies the version and other details?

I think it's generally good practice to limit the data that the usre
can influence as much as possible. Sorry for not picking up on that
earlier.

The user space pass vas_tx_win_open_attr struct - use only vas_id and
flags right now but it can be extended in future with reserve elements.
So passing the same struct to platform specific API.

do you prefer "struct vas_window * (*open_win)(vas_id, flags, cop)" and
extend later when more elments are used?

Yes I think so. The reason being so you don't sending data under the
control of user very far into the kernel. Better safe than sorry.

Thanks,
Nick