linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/10] powerpc/pseries/vas: NXGZIP support with DLPAR
@ 2022-01-21 19:52 Haren Myneni
  2022-01-21 19:54 ` [PATCH v3 01/10] powerpc/pseries/vas: Use common names in VAS capability structure Haren Myneni
                   ` (9 more replies)
  0 siblings, 10 replies; 27+ messages in thread
From: Haren Myneni @ 2022-01-21 19:52 UTC (permalink / raw)
  To: mpe, linuxppc-dev, npiggin


PowerPC provides HW compression with NX coprocessor. This feature
is available on both PowerNV and PowerVM and included in Linux.
Since each powerpc chip has one NX coprocessor, the VAS introduces
the concept of windows / credits to manage access to this hardware
resource. On powerVM, these limited resources should be available
across all LPARs. So the hypervisor assigns the specific credits
to each LPAR based on processor entitlement so that one LPAR does
not overload NX. The hypervisor can reject the window open request
to a partition if exceeds its credit limit (1 credit per window).

So the total number of target credits in a partition can be changed
if the core configuration is modified. The hypervisor expects the
partition to modify its window usage depends on new target
credits. For example, if the partition uses more credits than the
new target credits, it should close the excessive windows so that
the NX resource will be available to other partitions.

This patch series enables OS to support this dynamic credit
management with DLPAR core removal/add.

Core removal operation:
- Get new VAS capabilities from the hypervisor when the DLPAR
  notifier is received. This capabilities provides the new target
  credits based on new processor entitlement. In the case of QoS
  credit changes, the notification will be issued by updating
  the target_creds via sysfs.
- If the partition is already used more than the new target credits,
  the kernel selects windows, unmap the current paste address and
  close them in the hypervisor, It uses LIFO to identify these
  windows - last windows that are opened are the first ones to be
  closed.
- When the user space issue requests on these windows, NX generates
  page fault on the unmap paste address. The kernel handles the
  fault by returning the paste instruction failure if the window is
  not active (means unmap paste). Then up to the library / user
  space to fall back to SW compression or manage with the current
  windows.

Core add operation:
- The kernel can see increased target credits from the new VAS
  capabilities.
- Scans the window list for the closed windows in the hypervisor
  due to lost credit before and selects windows based on same LIFO.
- Make these corresponding windows active and create remap with
  the same VMA on the new paste address in the fault handler.
- Then the user space should expect paste successful later.

Patch 1: Define common names for sysfs target/used/avail_creds so
         that same sysfs entries can be used even on PowerNV later.
Patch 2: Add VAS notifier for DLPAR core add / removal
Patch 3: Save LPID in the vas window struct  during initial window
         open and use it when reopen later.
Patch 4: When credits are available, reopen windows that are closed
         before with core removal.
Patch 5: Close windows in the hypervisor when the partition exceeds
         its usage than the new target credits.
Patch 6: If the window is closed in the hypervisor before the user
         space issue the initial mmap(), return -EACCES failure.
Patch 7: Add new mmap fault handler which handles the page fault
         from NX on paste address.
Patch 8: Return the paste instruction failure if the window is not
         active.
Patch 9 & 10: The user space determines the credit usage with sysfs
         target/avail/used_creds interfaces. drmgr uses target_creds
        to notify OS for QoS credit changes.

Thanks to Nicholas Piggin and Aneesh Kumar for the valuable suggestions
on the NXGZIP design to support DLPAR operations.

Changes in v2:
- Rebase 5.16-rc5
- Use list safe functions to iterate windows list
- Changes to show the actual value in sysfs used_credits even though
  some windows are inactive with core removal. Reflects -ve value in
  sysfs avail_creds to let userspace know that it opened more windows
  than the current maximum LPAR credits.

Changes in v3:
- Rebase 5.16
- Reconfigure VAS windows only for CPU hotplug events.

Haren Myneni (10):
  powerpc/pseries/vas: Use common names in VAS capability structure
  powerpc/pseries/vas: Add notifier for DLPAR core removal/add
  powerpc/pseries/vas: Save partition PID in pseries_vas_window struct
  powerpc/pseries/vas: Reopen windows with DLPAR core add
  powerpc/pseries/vas: Close windows with DLPAR core removal
  powerpc/vas: Map paste address only if window is active
  powerpc/vas: Add paste address mmap fault handler
  powerpc/vas: Return paste instruction failure if window is not active
  powerpc/pseries/vas: sysfs interface to export capabilities
  powerpc/pseries/vas: Write 'target_creds' for QoS credits change

 arch/powerpc/include/asm/ppc-opcode.h      |   2 +
 arch/powerpc/include/asm/vas.h             |  17 ++
 arch/powerpc/platforms/book3s/vas-api.c    | 129 ++++++++-
 arch/powerpc/platforms/pseries/Makefile    |   2 +-
 arch/powerpc/platforms/pseries/vas-sysfs.c | 250 ++++++++++++++++
 arch/powerpc/platforms/pseries/vas.c       | 319 +++++++++++++++++++--
 arch/powerpc/platforms/pseries/vas.h       |  23 +-
 7 files changed, 715 insertions(+), 27 deletions(-)
 create mode 100644 arch/powerpc/platforms/pseries/vas-sysfs.c

-- 
2.27.0



^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v3 01/10] powerpc/pseries/vas: Use common names in VAS capability structure
  2022-01-21 19:52 [PATCH v3 00/10] powerpc/pseries/vas: NXGZIP support with DLPAR Haren Myneni
@ 2022-01-21 19:54 ` Haren Myneni
  2022-02-14  2:14   ` Nicholas Piggin
  2022-01-21 19:54 ` [PATCH v3 02/10] powerpc/pseries/vas: Add notifier for DLPAR core removal/add Haren Myneni
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 27+ messages in thread
From: Haren Myneni @ 2022-01-21 19:54 UTC (permalink / raw)
  To: mpe, linuxppc-dev, npiggin


target/used/avail_creds provides credits usage to user space via
sysfs and the same interface can be used on PowerNV in future.
Remove "lpar" from these names so that applicable on both PowerVM
and PowerNV.

Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/vas.c | 10 +++++-----
 arch/powerpc/platforms/pseries/vas.h |  6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
index d243ddc58827..c0737379cc7b 100644
--- a/arch/powerpc/platforms/pseries/vas.c
+++ b/arch/powerpc/platforms/pseries/vas.c
@@ -310,8 +310,8 @@ static struct vas_window *vas_allocate_window(int vas_id, u64 flags,
 
 	cop_feat_caps = &caps->caps;
 
-	if (atomic_inc_return(&cop_feat_caps->used_lpar_creds) >
-			atomic_read(&cop_feat_caps->target_lpar_creds)) {
+	if (atomic_inc_return(&cop_feat_caps->used_creds) >
+			atomic_read(&cop_feat_caps->target_creds)) {
 		pr_err("Credits are not available to allocate window\n");
 		rc = -EINVAL;
 		goto out;
@@ -385,7 +385,7 @@ static struct vas_window *vas_allocate_window(int vas_id, u64 flags,
 	free_irq_setup(txwin);
 	h_deallocate_vas_window(txwin->vas_win.winid);
 out:
-	atomic_dec(&cop_feat_caps->used_lpar_creds);
+	atomic_dec(&cop_feat_caps->used_creds);
 	kfree(txwin);
 	return ERR_PTR(rc);
 }
@@ -445,7 +445,7 @@ static int vas_deallocate_window(struct vas_window *vwin)
 	}
 
 	list_del(&win->win_list);
-	atomic_dec(&caps->used_lpar_creds);
+	atomic_dec(&caps->used_creds);
 	mutex_unlock(&vas_pseries_mutex);
 
 	put_vas_user_win_ref(&vwin->task_ref);
@@ -521,7 +521,7 @@ static int __init get_vas_capabilities(u8 feat, enum vas_cop_feat_type type,
 	}
 	caps->max_lpar_creds = be16_to_cpu(hv_caps->max_lpar_creds);
 	caps->max_win_creds = be16_to_cpu(hv_caps->max_win_creds);
-	atomic_set(&caps->target_lpar_creds,
+	atomic_set(&caps->target_creds,
 		   be16_to_cpu(hv_caps->target_lpar_creds));
 	if (feat == VAS_GZIP_DEF_FEAT) {
 		caps->def_lpar_creds = be16_to_cpu(hv_caps->def_lpar_creds);
diff --git a/arch/powerpc/platforms/pseries/vas.h b/arch/powerpc/platforms/pseries/vas.h
index 4ecb3fcabd10..fa7ce74f1e49 100644
--- a/arch/powerpc/platforms/pseries/vas.h
+++ b/arch/powerpc/platforms/pseries/vas.h
@@ -72,9 +72,9 @@ struct vas_cop_feat_caps {
 	};
 	/* Total LPAR available credits. Can be different from max LPAR */
 	/* credits due to DLPAR operation */
-	atomic_t	target_lpar_creds;
-	atomic_t	used_lpar_creds; /* Used credits so far */
-	u16		avail_lpar_creds; /* Remaining available credits */
+	atomic_t	target_creds;
+	atomic_t	used_creds;	/* Used credits so far */
+	u16		avail_creds;	/* Remaining available credits */
 };
 
 /*
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 02/10] powerpc/pseries/vas: Add notifier for DLPAR core removal/add
  2022-01-21 19:52 [PATCH v3 00/10] powerpc/pseries/vas: NXGZIP support with DLPAR Haren Myneni
  2022-01-21 19:54 ` [PATCH v3 01/10] powerpc/pseries/vas: Use common names in VAS capability structure Haren Myneni
@ 2022-01-21 19:54 ` Haren Myneni
  2022-02-14  2:27   ` Nicholas Piggin
  2022-01-21 19:55 ` [PATCH v3 03/10] powerpc/pseries/vas: Save LPID in pseries_vas_window struct Haren Myneni
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 27+ messages in thread
From: Haren Myneni @ 2022-01-21 19:54 UTC (permalink / raw)
  To: mpe, linuxppc-dev, npiggin


The hypervisor assigns credits for each LPAR based on number of
cores configured in that system. So expects to release credits
(means windows) when the core is removed. This patch adds notifier
for core removal/add so that the OS closes windows if the system
looses credits due to core removal and reopen windows when the
credits available later.

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

diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
index c0737379cc7b..d2c8292bfb33 100644
--- a/arch/powerpc/platforms/pseries/vas.c
+++ b/arch/powerpc/platforms/pseries/vas.c
@@ -538,6 +538,39 @@ static int __init get_vas_capabilities(u8 feat, enum vas_cop_feat_type type,
 	return 0;
 }
 
+/*
+ * Total number of default credits available (target_credits)
+ * in LPAR depends on number of cores configured. It varies based on
+ * whether processors are in shared mode or dedicated mode.
+ * Get the notifier when CPU configuration is changed with DLPAR
+ * operation so that get the new target_credits (vas default capabilities)
+ * and then update the existing windows usage if needed.
+ */
+static int pseries_vas_notifier(struct notifier_block *nb,
+				unsigned long action, void *data)
+{
+	struct of_reconfig_data *rd = data;
+	struct device_node *dn = rd->dn;
+	const __be32 *intserv = NULL;
+	int len, rc = 0;
+
+	if ((action == OF_RECONFIG_ATTACH_NODE) ||
+		(action == OF_RECONFIG_DETACH_NODE))
+		intserv = of_get_property(dn, "ibm,ppc-interrupt-server#s",
+					  &len);
+	/*
+	 * Processor config is not changed
+	 */
+	if (!intserv)
+		return NOTIFY_OK;
+
+	return rc;
+}
+
+static struct notifier_block pseries_vas_nb = {
+	.notifier_call = pseries_vas_notifier,
+};
+
 static int __init pseries_vas_init(void)
 {
 	struct hv_vas_cop_feat_caps *hv_cop_caps;
@@ -591,6 +624,10 @@ static int __init pseries_vas_init(void)
 			goto out_cop;
 	}
 
+	/* Processors can be added/removed only on LPAR */
+	if (copypaste_feat && firmware_has_feature(FW_FEATURE_LPAR))
+		of_reconfig_notifier_register(&pseries_vas_nb);
+
 	pr_info("GZIP feature is available\n");
 
 out_cop:
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 03/10] powerpc/pseries/vas: Save LPID in pseries_vas_window struct
  2022-01-21 19:52 [PATCH v3 00/10] powerpc/pseries/vas: NXGZIP support with DLPAR Haren Myneni
  2022-01-21 19:54 ` [PATCH v3 01/10] powerpc/pseries/vas: Use common names in VAS capability structure Haren Myneni
  2022-01-21 19:54 ` [PATCH v3 02/10] powerpc/pseries/vas: Add notifier for DLPAR core removal/add Haren Myneni
@ 2022-01-21 19:55 ` Haren Myneni
  2022-02-14  2:41   ` Nicholas Piggin
  2022-01-21 19:56 ` [PATCH v3 04/10] powerpc/pseries/vas: Reopen windows with DLPAR core add Haren Myneni
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 27+ messages in thread
From: Haren Myneni @ 2022-01-21 19:55 UTC (permalink / raw)
  To: mpe, linuxppc-dev, npiggin


The kernel sets the VAS window with partition PID when is opened in
the hypervisor. During DLPAR operation, windows can be closed and
reopened in the hypervisor when the credit is available. So saves
this PID in pseries_vas_window struct when the window is opened
initially and reuse it later during DLPAR operation.

Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/vas.c | 7 ++++---
 arch/powerpc/platforms/pseries/vas.h | 1 +
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/vas.c
b/arch/powerpc/platforms/pseries/vas.c
index d2c8292bfb33..2ef56157634f 100644
--- a/arch/powerpc/platforms/pseries/vas.c
+++ b/arch/powerpc/platforms/pseries/vas.c
@@ -107,7 +107,6 @@ static int h_deallocate_vas_window(u64 winid)
 static int h_modify_vas_window(struct pseries_vas_window *win)
 {
 	long rc;
-	u32 lpid = mfspr(SPRN_PID);
 
 	/*
 	 * AMR value is not supported in Linux VAS implementation.
@@ -115,7 +114,7 @@ static int h_modify_vas_window(struct
pseries_vas_window *win)
 	 */
 	do {
 		rc = plpar_hcall_norets(H_MODIFY_VAS_WINDOW,
-					win->vas_win.winid, lpid, 0,
+					win->vas_win.winid, win->lpid,
0,
 					VAS_MOD_WIN_FLAGS, 0);
 
 		rc = hcall_return_busy_check(rc);
@@ -125,7 +124,7 @@ static int h_modify_vas_window(struct
pseries_vas_window *win)
 		return 0;
 
 	pr_err("H_MODIFY_VAS_WINDOW error: %ld, winid %u lpid %u\n",
-			rc, win->vas_win.winid, lpid);
+			rc, win->vas_win.winid, win->lpid);
 	return -EIO;
 }
 
@@ -338,6 +337,8 @@ static struct vas_window *vas_allocate_window(int
vas_id, u64 flags,
 		}
 	}
 
+	txwin->lpid = mfspr(SPRN_PID);
+
 	/*
 	 * Allocate / Deallocate window hcalls and setup / free IRQs
 	 * have to be protected with mutex.
diff --git a/arch/powerpc/platforms/pseries/vas.h
b/arch/powerpc/platforms/pseries/vas.h
index fa7ce74f1e49..0538760d13be 100644
--- a/arch/powerpc/platforms/pseries/vas.h
+++ b/arch/powerpc/platforms/pseries/vas.h
@@ -115,6 +115,7 @@ struct pseries_vas_window {
 	u64 domain[6];		/* Associativity domain Ids */
 				/* this window is allocated */
 	u64 util;
+	u32 lpid;
 
 	/* List of windows opened which is used for LPM */
 	struct list_head win_list;
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 04/10] powerpc/pseries/vas: Reopen windows with DLPAR core add
  2022-01-21 19:52 [PATCH v3 00/10] powerpc/pseries/vas: NXGZIP support with DLPAR Haren Myneni
                   ` (2 preceding siblings ...)
  2022-01-21 19:55 ` [PATCH v3 03/10] powerpc/pseries/vas: Save LPID in pseries_vas_window struct Haren Myneni
@ 2022-01-21 19:56 ` Haren Myneni
  2022-02-14  3:08   ` Nicholas Piggin
  2022-01-21 19:57 ` [PATCH v3 05/10] powerpc/pseries/vas: Close windows with DLPAR core removal Haren Myneni
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 27+ messages in thread
From: Haren Myneni @ 2022-01-21 19:56 UTC (permalink / raw)
  To: mpe, linuxppc-dev, npiggin


VAS windows can be closed in the hypervisor due to lost credits
when the core is removed. If these credits are available later
for core add, reopen these windows and set them active. When the
kernel sees page fault on the paste address, it creates new mapping
on the new paste address. Then the user space can continue to use
these windows and send HW compression requests to NX successfully.

Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/include/asm/vas.h          |  16 +++
 arch/powerpc/platforms/book3s/vas-api.c |   1 +
 arch/powerpc/platforms/pseries/vas.c    | 144 ++++++++++++++++++++++++
 arch/powerpc/platforms/pseries/vas.h    |   8 +-
 4 files changed, 163 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/include/asm/vas.h b/arch/powerpc/include/asm/vas.h
index 57573d9c1e09..f1efe86563cc 100644
--- a/arch/powerpc/include/asm/vas.h
+++ b/arch/powerpc/include/asm/vas.h
@@ -29,6 +29,19 @@
 #define VAS_THRESH_FIFO_GT_QTR_FULL	2
 #define VAS_THRESH_FIFO_GT_EIGHTH_FULL	3
 
+/*
+ * VAS window status
+ */
+#define VAS_WIN_ACTIVE		0x0	/* Used in platform independent */
+					/* vas mmap() */
+/* The hypervisor returns these values */
+#define VAS_WIN_CLOSED		0x00000001
+#define VAS_WIN_INACTIVE	0x00000002 /* Inactive due to HW failure */
+#define VAS_WIN_MOD_IN_PROCESS	0x00000003 /* Process of being modified, */
+					   /* deallocated, or quiesced */
+/* Linux status bits */
+#define VAS_WIN_NO_CRED_CLOSE	0x00000004 /* Window is closed due to */
+					   /* lost credit */
 /*
  * Get/Set bit fields
  */
@@ -59,6 +72,8 @@ 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 */
+	struct mutex mmap_mutex;	/* protects paste address mmap() */
+					/* with DLPAR close/open windows */
 };
 
 /*
@@ -67,6 +82,7 @@ struct vas_user_win_ref {
 struct vas_window {
 	u32 winid;
 	u32 wcreds_max;	/* Window credits */
+	u32 status;
 	enum vas_cop_type cop;
 	struct vas_user_win_ref task_ref;
 	char *dbgname;
diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c
index 4d82c92ddd52..2b0ced611f32 100644
--- a/arch/powerpc/platforms/book3s/vas-api.c
+++ b/arch/powerpc/platforms/book3s/vas-api.c
@@ -316,6 +316,7 @@ static int coproc_ioc_tx_win_open(struct file *fp, unsigned long arg)
 		return PTR_ERR(txwin);
 	}
 
+	mutex_init(&txwin->task_ref.mmap_mutex);
 	cp_inst->txwin = txwin;
 
 	return 0;
diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
index 2ef56157634f..d9ff73d7704d 100644
--- a/arch/powerpc/platforms/pseries/vas.c
+++ b/arch/powerpc/platforms/pseries/vas.c
@@ -501,6 +501,7 @@ static int __init get_vas_capabilities(u8 feat, enum vas_cop_feat_type type,
 	memset(vcaps, 0, sizeof(*vcaps));
 	INIT_LIST_HEAD(&vcaps->list);
 
+	vcaps->feat = feat;
 	caps = &vcaps->caps;
 
 	rc = h_query_vas_capabilities(H_QUERY_VAS_CAPABILITIES, feat,
@@ -539,6 +540,145 @@ static int __init get_vas_capabilities(u8 feat, enum vas_cop_feat_type type,
 	return 0;
 }
 
+/*
+ * VAS windows can be closed due to lost credits when the core is
+ * removed. So reopen them if credits are available due to DLPAR
+ * core add and set the window active status. When NX sees the page
+ * fault on the unmapped paste address, the kernel handles the fault
+ * by setting the remapping to new paste address if the window is
+ * active.
+ */
+static int reconfig_open_windows(struct vas_caps *vcaps, int creds)
+{
+	long domain[PLPAR_HCALL9_BUFSIZE] = {VAS_DEFAULT_DOMAIN_ID};
+	struct vas_cop_feat_caps *caps = &vcaps->caps;
+	struct pseries_vas_window *win = NULL, *tmp;
+	int rc, mv_ents = 0;
+
+	/*
+	 * Nothing to do if there are no closed windows.
+	 */
+	if (!vcaps->close_wins)
+		return 0;
+
+	/*
+	 * For the core removal, the hypervisor reduces the credits
+	 * assigned to the LPAR and the kernel closes VAS windows
+	 * in the hypervisor depends on reduced credits. The kernel
+	 * uses LIFO (the last windows that are opened will be closed
+	 * first) and expects to open in the same order when credits
+	 * are available.
+	 * For example, 40 windows are closed when the LPAR lost 2 cores
+	 * (dedicated). If 1 core is added, this LPAR can have 20 more
+	 * credits. It means the kernel can reopen 20 windows. So move
+	 * 20 entries in the VAS windows lost and reopen next 20 windows.
+	 */
+	if (vcaps->close_wins > creds)
+		mv_ents = vcaps->close_wins - creds;
+
+	list_for_each_entry_safe(win, tmp, &vcaps->list, win_list) {
+		if (!mv_ents)
+			break;
+
+		mv_ents--;
+	}
+
+	list_for_each_entry_safe_from(win, tmp, &vcaps->list, win_list) {
+		/*
+		 * Nothing to do on this window if it is not closed
+		 * with VAS_WIN_NO_CRED_CLOSE
+		 */
+		if (!(win->vas_win.status & VAS_WIN_NO_CRED_CLOSE))
+			continue;
+
+		rc = allocate_setup_window(win, (u64 *)&domain[0],
+					   caps->win_type);
+		if (rc)
+			return rc;
+
+		rc = h_modify_vas_window(win);
+		if (rc)
+			goto out;
+
+		mutex_lock(&win->vas_win.task_ref.mmap_mutex);
+		/*
+		 * Set window status to active
+		 */
+		win->vas_win.status &= ~VAS_WIN_NO_CRED_CLOSE;
+		mutex_unlock(&win->vas_win.task_ref.mmap_mutex);
+		win->win_type = caps->win_type;
+		if (!--vcaps->close_wins)
+			break;
+	}
+
+	return 0;
+out:
+	/*
+	 * Window modify HCALL failed. So close the window to the
+	 * hypervisor and return.
+	 */
+	free_irq_setup(win);
+	h_deallocate_vas_window(win->vas_win.winid);
+	return rc;
+}
+
+/*
+ * Get new VAS capabilities when the core add/removal configuration
+ * changes. Reconfig window configurations based on the credits
+ * availability from this new capabilities.
+ */
+static int vas_reconfig_capabilties(u8 type)
+{
+	struct hv_vas_cop_feat_caps *hv_caps;
+	struct vas_cop_feat_caps *caps;
+	int lpar_creds, new_creds;
+	struct vas_caps *vcaps;
+	int rc = 0;
+
+	if (type >= VAS_MAX_FEAT_TYPE) {
+		pr_err("Invalid credit type %d\n", type);
+		return -EINVAL;
+	}
+
+	vcaps = &vascaps[type];
+	caps = &vcaps->caps;
+
+	hv_caps = kmalloc(sizeof(*hv_caps), GFP_KERNEL);
+	if (!hv_caps)
+		return -ENOMEM;
+
+	mutex_lock(&vas_pseries_mutex);
+	rc = h_query_vas_capabilities(H_QUERY_VAS_CAPABILITIES, vcaps->feat,
+				      (u64)virt_to_phys(hv_caps));
+	if (rc)
+		goto out;
+
+	new_creds = be16_to_cpu(hv_caps->target_lpar_creds);
+
+	lpar_creds = atomic_read(&caps->target_creds);
+
+	atomic_set(&caps->target_creds, new_creds);
+	/*
+	 * The total number of available credits may be decreased or
+	 * inceased with DLPAR operation. Means some windows have to be
+	 * closed / reopened. Hold the vas_pseries_mutex so that the
+	 * the user space can not open new windows.
+	 */
+	if (lpar_creds <  new_creds) {
+		/*
+		 * If the existing target credits is less than the new
+		 * target, reopen windows if they are closed due to
+		 * the previous DLPAR (core removal).
+		 */
+		rc = reconfig_open_windows(vcaps, new_creds - lpar_creds);
+	}
+
+out:
+	mutex_unlock(&vas_pseries_mutex);
+	kfree(hv_caps);
+	return rc;
+}
+
 /*
  * Total number of default credits available (target_credits)
  * in LPAR depends on number of cores configured. It varies based on
@@ -565,6 +705,10 @@ static int pseries_vas_notifier(struct notifier_block *nb,
 	if (!intserv)
 		return NOTIFY_OK;
 
+	rc = vas_reconfig_capabilties(VAS_GZIP_DEF_FEAT_TYPE);
+	if (rc)
+		pr_err("Failed reconfig VAS capabilities with DLPAR\n");
+
 	return rc;
 }
 
diff --git a/arch/powerpc/platforms/pseries/vas.h b/arch/powerpc/platforms/pseries/vas.h
index 0538760d13be..45b62565955b 100644
--- a/arch/powerpc/platforms/pseries/vas.h
+++ b/arch/powerpc/platforms/pseries/vas.h
@@ -21,12 +21,6 @@
 #define VAS_MOD_WIN_FLAGS	(VAS_MOD_WIN_JOBS_KILL | VAS_MOD_WIN_DR | \
 				VAS_MOD_WIN_PR | VAS_MOD_WIN_SF)
 
-#define VAS_WIN_ACTIVE		0x0
-#define VAS_WIN_CLOSED		0x1
-#define VAS_WIN_INACTIVE	0x2	/* Inactive due to HW failure */
-/* Process of being modified, deallocated, or quiesced */
-#define VAS_WIN_MOD_IN_PROCESS	0x3
-
 #define VAS_COPY_PASTE_USER_MODE	0x00000001
 #define VAS_COP_OP_USER_MODE		0x00000010
 
@@ -84,6 +78,8 @@ struct vas_cop_feat_caps {
 struct vas_caps {
 	struct vas_cop_feat_caps caps;
 	struct list_head list;	/* List of open windows */
+	int close_wins;		/* closed windows in the hypervisor for DLPAR */
+	u8 feat;		/* Feature type */
 };
 
 /*
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 05/10] powerpc/pseries/vas: Close windows with DLPAR core removal
  2022-01-21 19:52 [PATCH v3 00/10] powerpc/pseries/vas: NXGZIP support with DLPAR Haren Myneni
                   ` (3 preceding siblings ...)
  2022-01-21 19:56 ` [PATCH v3 04/10] powerpc/pseries/vas: Reopen windows with DLPAR core add Haren Myneni
@ 2022-01-21 19:57 ` Haren Myneni
  2022-02-14  3:17   ` Nicholas Piggin
  2022-01-21 19:58 ` [PATCH v3 06/10] powerpc/vas: Map paste address only if window is active Haren Myneni
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 27+ messages in thread
From: Haren Myneni @ 2022-01-21 19:57 UTC (permalink / raw)
  To: mpe, linuxppc-dev, npiggin


The hypervisor reduces the available credits if the core is removed
from the LPAR. So there is possibility of using excessive credits
(windows) in the LPAR and the hypervisor expects the system to close
the excessive windows. Even though the user space can continue to use
these windows to send compression requests to NX, the hypervisor expects
the LPAR to reduce these windows usage so that NX load can be equally
distributed across all LPARs in the system.

When the DLPAR notifier is received, get the new VAS capabilities from
the hypervisor and close the excessive windows in the hypervisor. Also
the kernel unmaps the paste address so that the user space receives paste
failure until these windows are active with the later DLPAR (core add).

Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/include/asm/vas.h          |   1 +
 arch/powerpc/platforms/book3s/vas-api.c |   2 +
 arch/powerpc/platforms/pseries/vas.c    | 117 ++++++++++++++++++++++--
 arch/powerpc/platforms/pseries/vas.h    |   1 +
 4 files changed, 112 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/include/asm/vas.h b/arch/powerpc/include/asm/vas.h
index f1efe86563cc..ddc05a8fc2e3 100644
--- a/arch/powerpc/include/asm/vas.h
+++ b/arch/powerpc/include/asm/vas.h
@@ -74,6 +74,7 @@ struct vas_user_win_ref {
 	struct mm_struct *mm;	/* Linux process mm_struct */
 	struct mutex mmap_mutex;	/* protects paste address mmap() */
 					/* with DLPAR close/open windows */
+	struct vm_area_struct *vma;	/* Save VMA and used in DLPAR ops */
 };
 
 /*
diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c
index 2b0ced611f32..a63fd48e34a7 100644
--- a/arch/powerpc/platforms/book3s/vas-api.c
+++ b/arch/powerpc/platforms/book3s/vas-api.c
@@ -399,6 +399,8 @@ static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
 	pr_devel("%s(): paste addr %llx at %lx, rc %d\n", __func__,
 			paste_addr, vma->vm_start, rc);
 
+	txwin->task_ref.vma = vma;
+
 	return rc;
 }
 
diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
index d9ff73d7704d..75ccd0a599ec 100644
--- a/arch/powerpc/platforms/pseries/vas.c
+++ b/arch/powerpc/platforms/pseries/vas.c
@@ -370,13 +370,28 @@ static struct vas_window *vas_allocate_window(int vas_id, u64 flags,
 	if (rc)
 		goto out_free;
 
-	vas_user_win_add_mm_context(&txwin->vas_win.task_ref);
 	txwin->win_type = cop_feat_caps->win_type;
 	mutex_lock(&vas_pseries_mutex);
-	list_add(&txwin->win_list, &caps->list);
+	/*
+	 * Possible to loose the acquired credit with DLPAR core
+	 * removal after the window is opened. So if there are any
+	 * closed windows (means with lost credits), do not give new
+	 * window to user space. New windows will be opened only
+	 * after the existing windows are reopened when credits are
+	 * available.
+	 */
+	if (!caps->close_wins) {
+		list_add(&txwin->win_list, &caps->list);
+		caps->num_wins++;
+		mutex_unlock(&vas_pseries_mutex);
+		vas_user_win_add_mm_context(&txwin->vas_win.task_ref);
+		return &txwin->vas_win;
+	}
 	mutex_unlock(&vas_pseries_mutex);
 
-	return &txwin->vas_win;
+	put_vas_user_win_ref(&txwin->vas_win.task_ref);
+	rc = -EBUSY;
+	pr_err("No credit is available to allocate window\n");
 
 out_free:
 	/*
@@ -439,14 +454,24 @@ static int vas_deallocate_window(struct vas_window *vwin)
 
 	caps = &vascaps[win->win_type].caps;
 	mutex_lock(&vas_pseries_mutex);
-	rc = deallocate_free_window(win);
-	if (rc) {
-		mutex_unlock(&vas_pseries_mutex);
-		return rc;
-	}
+	/*
+	 * VAS window is already closed in the hypervisor when
+	 * lost the credit. So just remove the entry from
+	 * the list, remove task references and free vas_window
+	 * struct.
+	 */
+	if (win->vas_win.status & VAS_WIN_NO_CRED_CLOSE) {
+		rc = deallocate_free_window(win);
+		if (rc) {
+			mutex_unlock(&vas_pseries_mutex);
+			return rc;
+		}
+	} else
+		vascaps[win->win_type].close_wins--;
 
 	list_del(&win->win_list);
 	atomic_dec(&caps->used_creds);
+	vascaps[win->win_type].num_wins--;
 	mutex_unlock(&vas_pseries_mutex);
 
 	put_vas_user_win_ref(&vwin->task_ref);
@@ -622,6 +647,72 @@ static int reconfig_open_windows(struct vas_caps *vcaps, int creds)
 	return rc;
 }
 
+/*
+ * The hypervisor reduces the available credits if the LPAR lost core. It
+ * means the excessive windows should not be active and the user space
+ * should not be using these windows to send compression requests to NX.
+ * So the kernel closes the excessive windows and unmap the paste address
+ * such that the user space receives paste instruction failure. Then up to
+ * the user space to fall back to SW compression and manage with the
+ * existing windows.
+ */
+static int reconfig_close_windows(struct vas_caps *vcap, int excess_creds)
+{
+	struct pseries_vas_window *win, *tmp;
+	struct vas_user_win_ref *task_ref;
+	struct vm_area_struct *vma;
+	int rc = 0;
+
+	list_for_each_entry_safe(win, tmp, &vcap->list, win_list) {
+		/*
+		 * This window is already closed due to lost credit
+		 * before. Go for next window.
+		 */
+		if (win->vas_win.status & VAS_WIN_NO_CRED_CLOSE)
+			continue;
+
+		task_ref = &win->vas_win.task_ref;
+		mutex_lock(&task_ref->mmap_mutex);
+		vma = task_ref->vma;
+		/*
+		 * Number of available credits are reduced, So select
+		 * and close windows.
+		 */
+		win->vas_win.status |= VAS_WIN_NO_CRED_CLOSE;
+
+		mmap_write_lock(task_ref->mm);
+		/*
+		 * vma is set in the original mapping. But this mapping
+		 * is done with mmap() after the window is opened with ioctl.
+		 * so we may not see the original mapping if the core remove
+		 * is done before the original mmap() and after the ioctl.
+		 */
+		if (vma)
+			zap_page_range(vma, vma->vm_start,
+					vma->vm_end - vma->vm_start);
+
+		mmap_write_unlock(task_ref->mm);
+		mutex_unlock(&task_ref->mmap_mutex);
+		/*
+		 * Close VAS window in the hypervisor, but do not
+		 * free vas_window struct since it may be reused
+		 * when the credit is available later (DLPAR with
+		 * adding cores). This struct will be used
+		 * later when the process issued with close(FD).
+		 */
+		rc = deallocate_free_window(win);
+		if (rc)
+			return rc;
+
+		vcap->close_wins++;
+
+		if (!--excess_creds)
+			break;
+	}
+
+	return 0;
+}
+
 /*
  * Get new VAS capabilities when the core add/removal configuration
  * changes. Reconfig window configurations based on the credits
@@ -633,7 +724,7 @@ static int vas_reconfig_capabilties(u8 type)
 	struct vas_cop_feat_caps *caps;
 	int lpar_creds, new_creds;
 	struct vas_caps *vcaps;
-	int rc = 0;
+	int rc = 0, active_wins;
 
 	if (type >= VAS_MAX_FEAT_TYPE) {
 		pr_err("Invalid credit type %d\n", type);
@@ -671,6 +762,14 @@ static int vas_reconfig_capabilties(u8 type)
 		 * the previous DLPAR (core removal).
 		 */
 		rc = reconfig_open_windows(vcaps, new_creds - lpar_creds);
+	} else {
+		/*
+		 * # active windows is more than new LPAR available
+		 * credits. So close the excessive windows.
+		 */
+		active_wins = vcaps->num_wins - vcaps->close_wins;
+		if (active_wins > new_creds)
+			rc = reconfig_close_windows(vcaps, active_wins - new_creds);
 	}
 
 out:
diff --git a/arch/powerpc/platforms/pseries/vas.h b/arch/powerpc/platforms/pseries/vas.h
index 45b62565955b..8ce9b84693e8 100644
--- a/arch/powerpc/platforms/pseries/vas.h
+++ b/arch/powerpc/platforms/pseries/vas.h
@@ -79,6 +79,7 @@ struct vas_caps {
 	struct vas_cop_feat_caps caps;
 	struct list_head list;	/* List of open windows */
 	int close_wins;		/* closed windows in the hypervisor for DLPAR */
+	int num_wins;		/* Number of windows opened */
 	u8 feat;		/* Feature type */
 };
 
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 06/10] powerpc/vas: Map paste address only if window is active
  2022-01-21 19:52 [PATCH v3 00/10] powerpc/pseries/vas: NXGZIP support with DLPAR Haren Myneni
                   ` (4 preceding siblings ...)
  2022-01-21 19:57 ` [PATCH v3 05/10] powerpc/pseries/vas: Close windows with DLPAR core removal Haren Myneni
@ 2022-01-21 19:58 ` Haren Myneni
  2022-02-14  3:20   ` Nicholas Piggin
  2022-01-21 19:59 ` [PATCH v3 07/10] powerpc/vas: Add paste address mmap fault handler Haren Myneni
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 27+ messages in thread
From: Haren Myneni @ 2022-01-21 19:58 UTC (permalink / raw)
  To: mpe, linuxppc-dev, npiggin


The paste address mapping is done with mmap() after the window is
opened with ioctl. But the window can be closed due to lost credit
due to core removal before mmap(). So if the window is not active,
return mmap() failure with -EACCES and expects the user space reissue
mmap() when the window is active or open new window when the credit
is available.

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

diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c
index a63fd48e34a7..2d06bd1b1935 100644
--- a/arch/powerpc/platforms/book3s/vas-api.c
+++ b/arch/powerpc/platforms/book3s/vas-api.c
@@ -379,10 +379,27 @@ static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
 		return -EACCES;
 	}
 
+	/*
+	 * The initial mapping is done after the window is opened
+	 * with ioctl. But this window might have been closed
+	 * due to lost credit (core removal on PowerVM) before mmap().
+	 * So if the window is not active, return mmap() failure
+	 * with -EACCES and expects the user space reconfigure (mmap)
+	 * window when it is active again or open new window when
+	 * the credit is available.
+	 */
+	mutex_lock(&txwin->task_ref.mmap_mutex);
+	if (txwin->status != VAS_WIN_ACTIVE) {
+		pr_err("%s(): Window is not active\n", __func__);
+		rc = -EACCES;
+		goto out;
+	}
+
 	paste_addr = cp_inst->coproc->vops->paste_addr(txwin);
 	if (!paste_addr) {
 		pr_err("%s(): Window paste address failed\n", __func__);
-		return -EINVAL;
+		rc = -EINVAL;
+		goto out;
 	}
 
 	pfn = paste_addr >> PAGE_SHIFT;
@@ -401,6 +418,8 @@ static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
 
 	txwin->task_ref.vma = vma;
 
+out:
+	mutex_unlock(&txwin->task_ref.mmap_mutex);
 	return rc;
 }
 
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 07/10] powerpc/vas: Add paste address mmap fault handler
  2022-01-21 19:52 [PATCH v3 00/10] powerpc/pseries/vas: NXGZIP support with DLPAR Haren Myneni
                   ` (5 preceding siblings ...)
  2022-01-21 19:58 ` [PATCH v3 06/10] powerpc/vas: Map paste address only if window is active Haren Myneni
@ 2022-01-21 19:59 ` Haren Myneni
  2022-02-14  3:37   ` Nicholas Piggin
  2022-01-21 19:59 ` [PATCH v3 08/10] powerpc/vas: Return paste instruction failure if no active window Haren Myneni
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 27+ messages in thread
From: Haren Myneni @ 2022-01-21 19:59 UTC (permalink / raw)
  To: mpe, linuxppc-dev, npiggin


The user space opens VAS windows and issues NX requests by pasting
CRB on the corresponding paste address mmap. When the system looses
credits due to core removal, the kernel has to close the window in
the hypervisor and make the window inactive by unmapping this paste
address. Also the OS has to handle NX request page faults if the user
space issue NX requests.

This handler remap the new paste address with the same VMA when the
window is active again (due to core add with DLPAR). Otherwise
returns paste failure.

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

diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c
index 2d06bd1b1935..5ceba75c13eb 100644
--- a/arch/powerpc/platforms/book3s/vas-api.c
+++ b/arch/powerpc/platforms/book3s/vas-api.c
@@ -351,6 +351,65 @@ static int coproc_release(struct inode *inode, struct file *fp)
 	return 0;
 }
 
+/*
+ * This fault handler is invoked when the VAS/NX generates page fault on
+ * the paste address. Happens if the kernel closes window in hypervisor
+ * (on PowerVM) due to lost credit or the paste address is not mapped.
+ */
+static vm_fault_t vas_mmap_fault(struct vm_fault *vmf)
+{
+	struct vm_area_struct *vma = vmf->vma;
+	struct file *fp = vma->vm_file;
+	struct coproc_instance *cp_inst = fp->private_data;
+	struct vas_window *txwin;
+	u64 paste_addr;
+	int ret;
+
+	/*
+	 * window is not opened. Shouldn't expect this error.
+	 */
+	if (!cp_inst || !cp_inst->txwin) {
+		pr_err("%s(): No send window open?\n", __func__);
+		return VM_FAULT_SIGBUS;
+	}
+
+	txwin = cp_inst->txwin;
+	/*
+	 * Fault is coming due to missing from the original mmap.
+	 * Can happen only when the window is closed due to lost
+	 * credit before mmap() or the user space issued NX request
+	 * without mapping.
+	 */
+	if (txwin->task_ref.vma != vmf->vma) {
+		pr_err("%s(): No previous mapping with paste address\n",
+			__func__);
+		return VM_FAULT_SIGBUS;
+	}
+
+	mutex_lock(&txwin->task_ref.mmap_mutex);
+	/*
+	 * The window may be inactive due to lost credit (Ex: core
+	 * removal with DLPAR). When the window is active again when
+	 * the credit is available, remap with the new paste address.
+	 */
+	if (txwin->status == VAS_WIN_ACTIVE) {
+		paste_addr = cp_inst->coproc->vops->paste_addr(txwin);
+		if (paste_addr) {
+			ret = vmf_insert_pfn(vma, vma->vm_start,
+					(paste_addr >> PAGE_SHIFT));
+			mutex_unlock(&txwin->task_ref.mmap_mutex);
+			return ret;
+		}
+	}
+	mutex_unlock(&txwin->task_ref.mmap_mutex);
+
+	return VM_FAULT_SIGBUS;
+
+}
+static const struct vm_operations_struct vas_vm_ops = {
+	.fault = vas_mmap_fault,
+};
+
 static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
 {
 	struct coproc_instance *cp_inst = fp->private_data;
@@ -417,6 +476,7 @@ static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
 			paste_addr, vma->vm_start, rc);
 
 	txwin->task_ref.vma = vma;
+	vma->vm_ops = &vas_vm_ops;
 
 out:
 	mutex_unlock(&txwin->task_ref.mmap_mutex);
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 08/10] powerpc/vas: Return paste instruction failure if no active window
  2022-01-21 19:52 [PATCH v3 00/10] powerpc/pseries/vas: NXGZIP support with DLPAR Haren Myneni
                   ` (6 preceding siblings ...)
  2022-01-21 19:59 ` [PATCH v3 07/10] powerpc/vas: Add paste address mmap fault handler Haren Myneni
@ 2022-01-21 19:59 ` Haren Myneni
  2022-02-14  3:41   ` Nicholas Piggin
  2022-01-21 20:00 ` [PATCH v3 09/10] powerpc/pseries/vas: sysfs interface to export capabilities Haren Myneni
  2022-01-21 20:01 ` [PATCH v3 10/10] powerpc/pseries/vas: Write 'target_creds' for QoS credits change Haren Myneni
  9 siblings, 1 reply; 27+ messages in thread
From: Haren Myneni @ 2022-01-21 19:59 UTC (permalink / raw)
  To: mpe, linuxppc-dev, npiggin


The VAS window may not be active if the system looses credits and
the NX generates page fault when it receives request on unmap
paste address.

The kernel handles the fault by remap new paste address if the
window is active again, Otherwise return the paste instruction
failure if the executed instruction that caused the fault was
a paste.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/include/asm/ppc-opcode.h   |  2 ++
 arch/powerpc/platforms/book3s/vas-api.c | 47 ++++++++++++++++++++++++-
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index efad07081cc0..fe2a69206588 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -262,6 +262,8 @@
 #define PPC_INST_MFSPR_PVR		0x7c1f42a6
 #define PPC_INST_MFSPR_PVR_MASK		0xfc1ffffe
 #define PPC_INST_MTMSRD			0x7c000164
+#define PPC_INST_PASTE			0x7c20070d
+#define PPC_INST_PASTE_MASK		0xfc2007ff
 #define PPC_INST_POPCNTB		0x7c0000f4
 #define PPC_INST_POPCNTB_MASK		0xfc0007fe
 #define PPC_INST_RFEBB			0x4c000124
diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c
index 5ceba75c13eb..2ffd34bc4032 100644
--- a/arch/powerpc/platforms/book3s/vas-api.c
+++ b/arch/powerpc/platforms/book3s/vas-api.c
@@ -351,6 +351,41 @@ static int coproc_release(struct inode *inode, struct file *fp)
 	return 0;
 }
 
+/*
+ * If the executed instruction that caused the fault was a paste, then
+ * clear regs CR0[EQ], advance NIP, and return 0. Else return error code.
+ */
+static int do_fail_paste(void)
+{
+	struct pt_regs *regs = current->thread.regs;
+	u32 instword;
+
+	if (WARN_ON_ONCE(!regs))
+		return -EINVAL;
+
+	if (WARN_ON_ONCE(!user_mode(regs)))
+		return -EINVAL;
+
+	/*
+	 * If we couldn't translate the instruction, the driver should
+	 * return success without handling the fault, it will be retried
+	 * or the instruction fetch will fault.
+	 */
+	if (get_user(instword, (u32 __user *)(regs->nip)))
+		return -EAGAIN;
+
+	/*
+	 * Not a paste instruction, driver may fail the fault.
+	 */
+	if ((instword & PPC_INST_PASTE_MASK) != PPC_INST_PASTE)
+		return -ENOENT;
+
+	regs->ccr &= ~0xe0000000;	/* Clear CR0[0-2] to fail paste */
+	regs_add_return_ip(regs, 4);	/* Skip the paste */
+
+	return 0;
+}
+
 /*
  * This fault handler is invoked when the VAS/NX generates page fault on
  * the paste address. Happens if the kernel closes window in hypervisor
@@ -403,9 +438,19 @@ static vm_fault_t vas_mmap_fault(struct vm_fault *vmf)
 	}
 	mutex_unlock(&txwin->task_ref.mmap_mutex);
 
-	return VM_FAULT_SIGBUS;
+	/*
+	 * Received this fault due to closing the actual window.
+	 * It can happen during migration or lost credits.
+	 * Since no mapping, return the paste instruction failure
+	 * to the user space.
+	 */
+	ret = do_fail_paste();
+	if (!ret)
+		return VM_FAULT_NOPAGE;
 
+	return VM_FAULT_SIGBUS;
 }
+
 static const struct vm_operations_struct vas_vm_ops = {
 	.fault = vas_mmap_fault,
 };
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 09/10] powerpc/pseries/vas: sysfs interface to export capabilities
  2022-01-21 19:52 [PATCH v3 00/10] powerpc/pseries/vas: NXGZIP support with DLPAR Haren Myneni
                   ` (7 preceding siblings ...)
  2022-01-21 19:59 ` [PATCH v3 08/10] powerpc/vas: Return paste instruction failure if no active window Haren Myneni
@ 2022-01-21 20:00 ` Haren Myneni
  2022-02-14  3:49   ` Nicholas Piggin
  2022-01-21 20:01 ` [PATCH v3 10/10] powerpc/pseries/vas: Write 'target_creds' for QoS credits change Haren Myneni
  9 siblings, 1 reply; 27+ messages in thread
From: Haren Myneni @ 2022-01-21 20:00 UTC (permalink / raw)
  To: mpe, linuxppc-dev, npiggin


The hypervisor provides the available VAS GZIP capabilities such
as default or QoS window type and the target available credits in
each type. This patch creates sysfs entries and exports the target,
used and the available credits for each feature.

This interface can be used by the user space to determine the credits
usage or to set the target credits in the case of QoS type (for DLPAR).

/sys/devices/vas/vas0/gzip/def_caps: (default GZIP capabilities)
	avail_creds /* Available credits to use */
	target_creds /* Total credits available. Can be
			 /* changed with DLPAR operation */
	used_creds  /* Used credits */

/sys/devices/vas/vas0/gzip/qos_caps (QoS GZIP capabilities)
	avail_creds
	target_creds
	used_creds

Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/Makefile    |   2 +-
 arch/powerpc/platforms/pseries/vas-sysfs.c | 218 +++++++++++++++++++++
 arch/powerpc/platforms/pseries/vas.c       |   6 +
 arch/powerpc/platforms/pseries/vas.h       |   6 +
 4 files changed, 231 insertions(+), 1 deletion(-)
 create mode 100644 arch/powerpc/platforms/pseries/vas-sysfs.c

diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
index ee60b59024b4..29b522d2c755 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -29,6 +29,6 @@ obj-$(CONFIG_PPC_SVM)		+= svm.o
 obj-$(CONFIG_FA_DUMP)		+= rtas-fadump.o
 
 obj-$(CONFIG_SUSPEND)		+= suspend.o
-obj-$(CONFIG_PPC_VAS)		+= vas.o
+obj-$(CONFIG_PPC_VAS)		+= vas.o vas-sysfs.o
 
 obj-$(CONFIG_ARCH_HAS_CC_PLATFORM)	+= cc_platform.o
diff --git a/arch/powerpc/platforms/pseries/vas-sysfs.c b/arch/powerpc/platforms/pseries/vas-sysfs.c
new file mode 100644
index 000000000000..f7609cdef8f8
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/vas-sysfs.c
@@ -0,0 +1,218 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright 2016-17 IBM Corp.
+ */
+
+#define pr_fmt(fmt) "vas: " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/miscdevice.h>
+#include <linux/kobject.h>
+#include <linux/slab.h>
+#include <linux/mm.h>
+
+#include "vas.h"
+
+#ifdef CONFIG_SYSFS
+static struct kobject *pseries_vas_kobj;
+static struct kobject *gzip_caps_kobj;
+
+struct vas_caps_entry {
+	struct kobject kobj;
+	struct vas_cop_feat_caps *caps;
+};
+
+#define to_caps_entry(entry) container_of(entry, struct vas_caps_entry, kobj)
+
+static ssize_t avail_creds_show(struct vas_cop_feat_caps *caps, char *buf)
+{
+	/*
+	 * avail_creds may be -ve if used_creds is oversubscribed,
+	 * can happen if target_creds is reduced with DLPAR core removal.
+	 */
+	int avail_creds = atomic_read(&caps->target_creds) -
+			atomic_read(&caps->used_creds);
+	return sprintf(buf, "%d\n", avail_creds);
+}
+
+#define sysfs_capbs_entry_read(_name)					\
+static ssize_t _name##_show(struct vas_cop_feat_caps *caps, char *buf) 	\
+{									\
+	return sprintf(buf, "%d\n", atomic_read(&caps->_name));	\
+}
+
+struct vas_sysfs_entry {
+	struct attribute attr;
+	ssize_t (*show)(struct vas_cop_feat_caps *, char *);
+	ssize_t (*store)(struct vas_cop_feat_caps *, const char *, size_t);
+};
+
+#define VAS_ATTR_RO(_name)	\
+	sysfs_capbs_entry_read(_name);		\
+	static struct vas_sysfs_entry _name##_attribute = __ATTR(_name,	\
+				0444, _name##_show, NULL);
+
+VAS_ATTR_RO(target_creds);
+VAS_ATTR_RO(used_creds);
+
+static struct vas_sysfs_entry avail_creds_attribute =
+	__ATTR(avail_creds, 0444, avail_creds_show, NULL);
+
+static struct attribute *vas_capab_attrs[] = {
+	&target_creds_attribute.attr,
+	&used_creds_attribute.attr,
+	&avail_creds_attribute.attr,
+	NULL,
+};
+
+static ssize_t vas_type_show(struct kobject *kobj, struct attribute *attr,
+			     char *buf)
+{
+	struct vas_caps_entry *centry;
+	struct vas_cop_feat_caps *caps;
+	struct vas_sysfs_entry *entry;
+
+	centry = to_caps_entry(kobj);
+	caps = centry->caps;
+	entry = container_of(attr, struct vas_sysfs_entry, attr);
+
+	if (!entry->show)
+		return -EIO;
+
+	return entry->show(caps, buf);
+}
+
+static ssize_t vas_type_store(struct kobject *kobj, struct attribute *attr,
+			      const char *buf, size_t count)
+{
+	struct vas_caps_entry *centry;
+	struct vas_cop_feat_caps *caps;
+	struct vas_sysfs_entry *entry;
+
+	centry = to_caps_entry(kobj);
+	caps = centry->caps;
+	entry = container_of(attr, struct vas_sysfs_entry, attr);
+	if (!entry->store)
+		return -EIO;
+
+	return entry->store(caps, buf, count);
+}
+
+static void vas_type_release(struct kobject *kobj)
+{
+	struct vas_caps_entry *centry = to_caps_entry(kobj);
+	kfree(centry);
+}
+
+static const struct sysfs_ops vas_sysfs_ops = {
+	.show	=	vas_type_show,
+	.store	=	vas_type_store,
+};
+
+static struct kobj_type vas_attr_type = {
+		.release	=	vas_type_release,
+		.sysfs_ops      =       &vas_sysfs_ops,
+		.default_attrs  =       vas_capab_attrs,
+};
+
+static char *vas_caps_kobj_name(struct vas_cop_feat_caps *caps,
+				struct kobject **kobj)
+{
+	if (caps->descriptor == VAS_GZIP_QOS_CAPABILITIES) {
+		*kobj = gzip_caps_kobj;
+		return "qos_caps";
+	} else if (caps->descriptor == VAS_GZIP_DEFAULT_CAPABILITIES) {
+		*kobj = gzip_caps_kobj;
+		return "def_caps";
+	} else
+		return "Unknown";
+}
+
+/*
+ * Add feature specific capability dir entry.
+ * Ex: VDefGzip or VQosGzip
+ */
+int sysfs_add_vas_caps(struct vas_cop_feat_caps *caps)
+{
+	struct vas_caps_entry *centry;
+	struct kobject *kobj = NULL;
+	int ret = 0;
+	char *name;
+
+	centry = kzalloc(sizeof(*centry), GFP_KERNEL);
+	if (!centry)
+		return -ENOMEM;
+
+	kobject_init(&centry->kobj, &vas_attr_type);
+	centry->caps = caps;
+	name  = vas_caps_kobj_name(caps, &kobj);
+
+	if (kobj) {
+		ret = kobject_add(&centry->kobj, kobj, "%s", name);
+
+		if (ret) {
+			pr_err("VAS: sysfs kobject add / event failed %d\n",
+					ret);
+			kobject_put(&centry->kobj);
+		}
+	}
+
+	return ret;
+}
+
+static struct miscdevice vas_miscdev = {
+	.minor = MISC_DYNAMIC_MINOR,
+	.name = "vas",
+};
+
+/*
+ * Add VAS and VasCaps (overall capabilities) dir entries.
+ */
+int __init sysfs_pseries_vas_init(struct vas_all_caps *vas_caps)
+{
+	int ret;
+
+	ret = misc_register(&vas_miscdev);
+	if (ret < 0) {
+		pr_err("%s: register vas misc device failed\n", __func__);
+		return ret;
+	}
+
+	/*
+	 * The hypervisor does not expose multiple VAS instances, but can
+	 * see multiple VAS instances on PowerNV. So create 'vas0' directory
+	 * on PowerVM.
+	 */
+	pseries_vas_kobj = kobject_create_and_add("vas0",
+					&vas_miscdev.this_device->kobj);
+	if (!pseries_vas_kobj) {
+		pr_err("Failed to create VAS sysfs entry\n");
+		return -ENOMEM;
+	}
+
+	if ((vas_caps->feat_type & VAS_GZIP_QOS_FEAT_BIT) ||
+		(vas_caps->feat_type & VAS_GZIP_DEF_FEAT_BIT)) {
+		gzip_caps_kobj = kobject_create_and_add("gzip",
+						       pseries_vas_kobj);
+		if (!gzip_caps_kobj) {
+			pr_err("Failed to create VAS GZIP capability entry\n");
+			kobject_put(pseries_vas_kobj);
+			return -ENOMEM;
+		}
+	}
+
+	return 0;
+}
+
+#else
+int sysfs_add_vas_caps(struct vas_cop_feat_caps *caps)
+{
+	return 0;
+}
+
+int __init sysfs_pseries_vas_init(struct vas_all_caps *vas_caps)
+{
+	return 0;
+}
+#endif
diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
index 75ccd0a599ec..32098e4a2786 100644
--- a/arch/powerpc/platforms/pseries/vas.c
+++ b/arch/powerpc/platforms/pseries/vas.c
@@ -560,6 +560,10 @@ static int __init get_vas_capabilities(u8 feat, enum vas_cop_feat_type type,
 		}
 	}
 
+	rc = sysfs_add_vas_caps(caps);
+	if (rc)
+		return rc;
+
 	copypaste_feat = true;
 
 	return 0;
@@ -843,6 +847,8 @@ static int __init pseries_vas_init(void)
 	caps_all.descriptor = be64_to_cpu(hv_caps->descriptor);
 	caps_all.feat_type = be64_to_cpu(hv_caps->feat_type);
 
+	sysfs_pseries_vas_init(&caps_all);
+
 	hv_cop_caps = kmalloc(sizeof(*hv_cop_caps), GFP_KERNEL);
 	if (!hv_cop_caps) {
 		rc = -ENOMEM;
diff --git a/arch/powerpc/platforms/pseries/vas.h b/arch/powerpc/platforms/pseries/vas.h
index 8ce9b84693e8..bc393bd74030 100644
--- a/arch/powerpc/platforms/pseries/vas.h
+++ b/arch/powerpc/platforms/pseries/vas.h
@@ -24,6 +24,9 @@
 #define VAS_COPY_PASTE_USER_MODE	0x00000001
 #define VAS_COP_OP_USER_MODE		0x00000010
 
+#define VAS_GZIP_QOS_CAPABILITIES	0x56516F73477A6970
+#define VAS_GZIP_DEFAULT_CAPABILITIES	0x56446566477A6970
+
 /*
  * Co-processor feature - GZIP QoS windows or GZIP default windows
  */
@@ -120,4 +123,7 @@ struct pseries_vas_window {
 	char *name;
 	int fault_virq;
 };
+
+int sysfs_add_vas_caps(struct vas_cop_feat_caps *caps);
+int __init sysfs_pseries_vas_init(struct vas_all_caps *vas_caps);
 #endif /* _VAS_H */
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 10/10] powerpc/pseries/vas: Write 'target_creds' for QoS credits change
  2022-01-21 19:52 [PATCH v3 00/10] powerpc/pseries/vas: NXGZIP support with DLPAR Haren Myneni
                   ` (8 preceding siblings ...)
  2022-01-21 20:00 ` [PATCH v3 09/10] powerpc/pseries/vas: sysfs interface to export capabilities Haren Myneni
@ 2022-01-21 20:01 ` Haren Myneni
  9 siblings, 0 replies; 27+ messages in thread
From: Haren Myneni @ 2022-01-21 20:01 UTC (permalink / raw)
  To: mpe, linuxppc-dev, npiggin


PowerVM support two types of credits - Default (uses normal priority
FIFO) and Qality of service (QoS uses high priproty FIFO). The user
decides the number of QoS credits and sets this value with HMC
interface. With the core add/removal, this value can be changed in HMC
which invokes drmgr to communicate to the kernel.

This patch adds an interface so that drmgr command can write the new
target QoS credits in sysfs. But the kernel gets the new QoS
capabilities from the hypervisor whenever target_creds is updated
to make sure sync with the values in the hypervisor.

Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/vas-sysfs.c | 34 +++++++++++++++++++++-
 arch/powerpc/platforms/pseries/vas.c       |  2 +-
 arch/powerpc/platforms/pseries/vas.h       |  1 +
 3 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/vas-sysfs.c b/arch/powerpc/platforms/pseries/vas-sysfs.c
index f7609cdef8f8..66f1a0224811 100644
--- a/arch/powerpc/platforms/pseries/vas-sysfs.c
+++ b/arch/powerpc/platforms/pseries/vas-sysfs.c
@@ -36,6 +36,34 @@ static ssize_t avail_creds_show(struct vas_cop_feat_caps *caps, char *buf)
 	return sprintf(buf, "%d\n", avail_creds);
 }
 
+/*
+ * This function is used to get the notification from the drmgr when
+ * QoS credits are changed as part of DLPAR core add/removal. Though
+ * receiving the total QoS credits here, get the official QoS
+ * capabilities from the hypervisor.
+ */
+static ssize_t target_creds_store(struct vas_cop_feat_caps *caps,
+				       const char *buf, size_t count)
+{
+	int err;
+	u16 creds;
+
+	/*
+	 * Nothing to do for default credit type.
+	 */
+	if (caps->win_type == VAS_GZIP_DEF_FEAT_TYPE)
+		return -EOPNOTSUPP;
+
+	err = kstrtou16(buf, 0, &creds);
+	if (!err)
+		err = vas_reconfig_capabilties(caps->win_type);
+
+	if (err)
+		return -EINVAL;
+
+	return count;
+}
+
 #define sysfs_capbs_entry_read(_name)					\
 static ssize_t _name##_show(struct vas_cop_feat_caps *caps, char *buf) 	\
 {									\
@@ -52,8 +80,12 @@ struct vas_sysfs_entry {
 	sysfs_capbs_entry_read(_name);		\
 	static struct vas_sysfs_entry _name##_attribute = __ATTR(_name,	\
 				0444, _name##_show, NULL);
+#define VAS_ATTR(_name)							\
+	sysfs_capbs_entry_read(_name);					\
+	static struct vas_sysfs_entry _name##_attribute = __ATTR(_name, \
+				0644, _name##_show, _name##_store)
 
-VAS_ATTR_RO(target_creds);
+VAS_ATTR(target_creds);
 VAS_ATTR_RO(used_creds);
 
 static struct vas_sysfs_entry avail_creds_attribute =
diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
index 32098e4a2786..3400f4fc6609 100644
--- a/arch/powerpc/platforms/pseries/vas.c
+++ b/arch/powerpc/platforms/pseries/vas.c
@@ -722,7 +722,7 @@ static int reconfig_close_windows(struct vas_caps *vcap, int excess_creds)
  * changes. Reconfig window configurations based on the credits
  * availability from this new capabilities.
  */
-static int vas_reconfig_capabilties(u8 type)
+int vas_reconfig_capabilties(u8 type)
 {
 	struct hv_vas_cop_feat_caps *hv_caps;
 	struct vas_cop_feat_caps *caps;
diff --git a/arch/powerpc/platforms/pseries/vas.h b/arch/powerpc/platforms/pseries/vas.h
index bc393bd74030..4cf1d0ef66a5 100644
--- a/arch/powerpc/platforms/pseries/vas.h
+++ b/arch/powerpc/platforms/pseries/vas.h
@@ -125,5 +125,6 @@ struct pseries_vas_window {
 };
 
 int sysfs_add_vas_caps(struct vas_cop_feat_caps *caps);
+int vas_reconfig_capabilties(u8 type);
 int __init sysfs_pseries_vas_init(struct vas_all_caps *vas_caps);
 #endif /* _VAS_H */
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 01/10] powerpc/pseries/vas: Use common names in VAS capability structure
  2022-01-21 19:54 ` [PATCH v3 01/10] powerpc/pseries/vas: Use common names in VAS capability structure Haren Myneni
@ 2022-02-14  2:14   ` Nicholas Piggin
  2022-02-15 18:32     ` Haren Myneni
  0 siblings, 1 reply; 27+ messages in thread
From: Nicholas Piggin @ 2022-02-14  2:14 UTC (permalink / raw)
  To: Haren Myneni, linuxppc-dev, mpe

Excerpts from Haren Myneni's message of January 22, 2022 5:54 am:
> 
> target/used/avail_creds provides credits usage to user space via
> sysfs and the same interface can be used on PowerNV in future.
> Remove "lpar" from these names so that applicable on both PowerVM
> and PowerNV.

But not in this series? This is just to save you having to do more
renaming later?

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

> 
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/vas.c | 10 +++++-----
>  arch/powerpc/platforms/pseries/vas.h |  6 +++---
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
> index d243ddc58827..c0737379cc7b 100644
> --- a/arch/powerpc/platforms/pseries/vas.c
> +++ b/arch/powerpc/platforms/pseries/vas.c
> @@ -310,8 +310,8 @@ static struct vas_window *vas_allocate_window(int vas_id, u64 flags,
>  
>  	cop_feat_caps = &caps->caps;
>  
> -	if (atomic_inc_return(&cop_feat_caps->used_lpar_creds) >
> -			atomic_read(&cop_feat_caps->target_lpar_creds)) {
> +	if (atomic_inc_return(&cop_feat_caps->used_creds) >
> +			atomic_read(&cop_feat_caps->target_creds)) {
>  		pr_err("Credits are not available to allocate window\n");
>  		rc = -EINVAL;
>  		goto out;
> @@ -385,7 +385,7 @@ static struct vas_window *vas_allocate_window(int vas_id, u64 flags,
>  	free_irq_setup(txwin);
>  	h_deallocate_vas_window(txwin->vas_win.winid);
>  out:
> -	atomic_dec(&cop_feat_caps->used_lpar_creds);
> +	atomic_dec(&cop_feat_caps->used_creds);
>  	kfree(txwin);
>  	return ERR_PTR(rc);
>  }
> @@ -445,7 +445,7 @@ static int vas_deallocate_window(struct vas_window *vwin)
>  	}
>  
>  	list_del(&win->win_list);
> -	atomic_dec(&caps->used_lpar_creds);
> +	atomic_dec(&caps->used_creds);
>  	mutex_unlock(&vas_pseries_mutex);
>  
>  	put_vas_user_win_ref(&vwin->task_ref);
> @@ -521,7 +521,7 @@ static int __init get_vas_capabilities(u8 feat, enum vas_cop_feat_type type,
>  	}
>  	caps->max_lpar_creds = be16_to_cpu(hv_caps->max_lpar_creds);
>  	caps->max_win_creds = be16_to_cpu(hv_caps->max_win_creds);
> -	atomic_set(&caps->target_lpar_creds,
> +	atomic_set(&caps->target_creds,
>  		   be16_to_cpu(hv_caps->target_lpar_creds));
>  	if (feat == VAS_GZIP_DEF_FEAT) {
>  		caps->def_lpar_creds = be16_to_cpu(hv_caps->def_lpar_creds);
> diff --git a/arch/powerpc/platforms/pseries/vas.h b/arch/powerpc/platforms/pseries/vas.h
> index 4ecb3fcabd10..fa7ce74f1e49 100644
> --- a/arch/powerpc/platforms/pseries/vas.h
> +++ b/arch/powerpc/platforms/pseries/vas.h
> @@ -72,9 +72,9 @@ struct vas_cop_feat_caps {
>  	};
>  	/* Total LPAR available credits. Can be different from max LPAR */
>  	/* credits due to DLPAR operation */
> -	atomic_t	target_lpar_creds;
> -	atomic_t	used_lpar_creds; /* Used credits so far */
> -	u16		avail_lpar_creds; /* Remaining available credits */
> +	atomic_t	target_creds;
> +	atomic_t	used_creds;	/* Used credits so far */
> +	u16		avail_creds;	/* Remaining available credits */
>  };
>  
>  /*
> -- 
> 2.27.0
> 
> 
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 02/10] powerpc/pseries/vas: Add notifier for DLPAR core removal/add
  2022-01-21 19:54 ` [PATCH v3 02/10] powerpc/pseries/vas: Add notifier for DLPAR core removal/add Haren Myneni
@ 2022-02-14  2:27   ` Nicholas Piggin
  2022-02-16  1:07     ` Haren Myneni
  0 siblings, 1 reply; 27+ messages in thread
From: Nicholas Piggin @ 2022-02-14  2:27 UTC (permalink / raw)
  To: Haren Myneni, linuxppc-dev, mpe; +Cc: Nathan Lynch

Excerpts from Haren Myneni's message of January 22, 2022 5:54 am:
> 
> The hypervisor assigns credits for each LPAR based on number of
> cores configured in that system. So expects to release credits
> (means windows) when the core is removed. This patch adds notifier
> for core removal/add so that the OS closes windows if the system
> looses credits due to core removal and reopen windows when the
> credits available later.

This could be improved. As far as I can tell,

 The hypervisor assigns vas credits (windows) for each LPAR based on the 
 number of cores configured in that system. The OS is expected to 
 release credits when cores are removed, and may allocate more when 
 cores are added.

Or can you only re-use credits that you previously lost?

> 
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/vas.c | 37 ++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
> index c0737379cc7b..d2c8292bfb33 100644
> --- a/arch/powerpc/platforms/pseries/vas.c
> +++ b/arch/powerpc/platforms/pseries/vas.c
> @@ -538,6 +538,39 @@ static int __init get_vas_capabilities(u8 feat, enum vas_cop_feat_type type,
>  	return 0;
>  }
>  
> +/*
> + * Total number of default credits available (target_credits)
> + * in LPAR depends on number of cores configured. It varies based on
> + * whether processors are in shared mode or dedicated mode.
> + * Get the notifier when CPU configuration is changed with DLPAR
> + * operation so that get the new target_credits (vas default capabilities)
> + * and then update the existing windows usage if needed.
> + */
> +static int pseries_vas_notifier(struct notifier_block *nb,
> +				unsigned long action, void *data)
> +{
> +	struct of_reconfig_data *rd = data;
> +	struct device_node *dn = rd->dn;
> +	const __be32 *intserv = NULL;
> +	int len, rc = 0;
> +
> +	if ((action == OF_RECONFIG_ATTACH_NODE) ||
> +		(action == OF_RECONFIG_DETACH_NODE))

I suppose the OF notifier is the way to do it (cc Nathan).

Could this patch be folded in with where it acually does something? It 
makes it easier to review and understand how the notifier is used.


> +		intserv = of_get_property(dn, "ibm,ppc-interrupt-server#s",
> +					  &len);
> +	/*
> +	 * Processor config is not changed
> +	 */
> +	if (!intserv)
> +		return NOTIFY_OK;
> +
> +	return rc;
> +}
> +
> +static struct notifier_block pseries_vas_nb = {
> +	.notifier_call = pseries_vas_notifier,
> +};
> +
>  static int __init pseries_vas_init(void)
>  {
>  	struct hv_vas_cop_feat_caps *hv_cop_caps;
> @@ -591,6 +624,10 @@ static int __init pseries_vas_init(void)
>  			goto out_cop;
>  	}
>  
> +	/* Processors can be added/removed only on LPAR */

What does this comment mean? DLPAR?

Thanks,
Nick

> +	if (copypaste_feat && firmware_has_feature(FW_FEATURE_LPAR))
> +		of_reconfig_notifier_register(&pseries_vas_nb);
> +
>  	pr_info("GZIP feature is available\n");
>  
>  out_cop:
> -- 
> 2.27.0
> 
> 
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 03/10] powerpc/pseries/vas: Save LPID in pseries_vas_window struct
  2022-01-21 19:55 ` [PATCH v3 03/10] powerpc/pseries/vas: Save LPID in pseries_vas_window struct Haren Myneni
@ 2022-02-14  2:41   ` Nicholas Piggin
  2022-02-16  1:09     ` Haren Myneni
  0 siblings, 1 reply; 27+ messages in thread
From: Nicholas Piggin @ 2022-02-14  2:41 UTC (permalink / raw)
  To: Haren Myneni, linuxppc-dev, mpe

Excerpts from Haren Myneni's message of January 22, 2022 5:55 am:
> 
> The kernel sets the VAS window with partition PID when is opened in
> the hypervisor. During DLPAR operation, windows can be closed and
> reopened in the hypervisor when the credit is available. So saves
> this PID in pseries_vas_window struct when the window is opened
> initially and reuse it later during DLPAR operation.

This probably shouldn't be called lpid, while you're changing it.
"partition PID" and "LPAR PID" is also confusing. I know the name
somewhat comes from the specifiction, but pid/PID would be fine,
it's clear we are talking about "this LPAR" when in pseries code.

> 
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/vas.c | 7 ++++---
>  arch/powerpc/platforms/pseries/vas.h | 1 +
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/vas.c
> b/arch/powerpc/platforms/pseries/vas.c
> index d2c8292bfb33..2ef56157634f 100644
> --- a/arch/powerpc/platforms/pseries/vas.c
> +++ b/arch/powerpc/platforms/pseries/vas.c
> @@ -107,7 +107,6 @@ static int h_deallocate_vas_window(u64 winid)
>  static int h_modify_vas_window(struct pseries_vas_window *win)
>  {
>  	long rc;
> -	u32 lpid = mfspr(SPRN_PID);
>  
>  	/*
>  	 * AMR value is not supported in Linux VAS implementation.
> @@ -115,7 +114,7 @@ static int h_modify_vas_window(struct
> pseries_vas_window *win)
>  	 */
>  	do {
>  		rc = plpar_hcall_norets(H_MODIFY_VAS_WINDOW,
> -					win->vas_win.winid, lpid, 0,
> +					win->vas_win.winid, win->lpid,
> 0,
>  					VAS_MOD_WIN_FLAGS, 0);
>  
>  		rc = hcall_return_busy_check(rc);
> @@ -125,7 +124,7 @@ static int h_modify_vas_window(struct
> pseries_vas_window *win)
>  		return 0;
>  
>  	pr_err("H_MODIFY_VAS_WINDOW error: %ld, winid %u lpid %u\n",
> -			rc, win->vas_win.winid, lpid);
> +			rc, win->vas_win.winid, win->lpid);
>  	return -EIO;
>  }
>  
> @@ -338,6 +337,8 @@ static struct vas_window *vas_allocate_window(int
> vas_id, u64 flags,
>  		}
>  	}
>  
> +	txwin->lpid = mfspr(SPRN_PID);
> +
>  	/*
>  	 * Allocate / Deallocate window hcalls and setup / free IRQs
>  	 * have to be protected with mutex.
> diff --git a/arch/powerpc/platforms/pseries/vas.h
> b/arch/powerpc/platforms/pseries/vas.h
> index fa7ce74f1e49..0538760d13be 100644
> --- a/arch/powerpc/platforms/pseries/vas.h
> +++ b/arch/powerpc/platforms/pseries/vas.h
> @@ -115,6 +115,7 @@ struct pseries_vas_window {
>  	u64 domain[6];		/* Associativity domain Ids */
>  				/* this window is allocated */
>  	u64 util;
> +	u32 lpid;

Comment could be "PID associated with this window".

BTW, is the TID parameter deprecated? Doesn't seem that we use that.

Thanks,
Nick

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 04/10] powerpc/pseries/vas: Reopen windows with DLPAR core add
  2022-01-21 19:56 ` [PATCH v3 04/10] powerpc/pseries/vas: Reopen windows with DLPAR core add Haren Myneni
@ 2022-02-14  3:08   ` Nicholas Piggin
  2022-02-16  1:38     ` Haren Myneni
  0 siblings, 1 reply; 27+ messages in thread
From: Nicholas Piggin @ 2022-02-14  3:08 UTC (permalink / raw)
  To: Haren Myneni, linuxppc-dev, mpe

Excerpts from Haren Myneni's message of January 22, 2022 5:56 am:
> 
> VAS windows can be closed in the hypervisor due to lost credits
> when the core is removed. If these credits are available later
> for core add, reopen these windows and set them active. When the
> kernel sees page fault on the paste address, it creates new mapping
> on the new paste address. Then the user space can continue to use
> these windows and send HW compression requests to NX successfully.

Any reason to put this before the close windows patch? It would be
more logical to put it afterwards AFAIKS.

> 
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/vas.h          |  16 +++
>  arch/powerpc/platforms/book3s/vas-api.c |   1 +
>  arch/powerpc/platforms/pseries/vas.c    | 144 ++++++++++++++++++++++++
>  arch/powerpc/platforms/pseries/vas.h    |   8 +-
>  4 files changed, 163 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/vas.h b/arch/powerpc/include/asm/vas.h
> index 57573d9c1e09..f1efe86563cc 100644
> --- a/arch/powerpc/include/asm/vas.h
> +++ b/arch/powerpc/include/asm/vas.h
> @@ -29,6 +29,19 @@
>  #define VAS_THRESH_FIFO_GT_QTR_FULL	2
>  #define VAS_THRESH_FIFO_GT_EIGHTH_FULL	3
>  
> +/*
> + * VAS window status
> + */
> +#define VAS_WIN_ACTIVE		0x0	/* Used in platform independent */
> +					/* vas mmap() */
> +/* The hypervisor returns these values */
> +#define VAS_WIN_CLOSED		0x00000001
> +#define VAS_WIN_INACTIVE	0x00000002 /* Inactive due to HW failure */
> +#define VAS_WIN_MOD_IN_PROCESS	0x00000003 /* Process of being modified, */

While you're moving these and adding a comment, it would be good to 
list what hcalls they are relevant to. H_QUERY_VAS_WINDOW (which is not
used anywhere yet?) These are also a 1-byte field, so '0x00', '0x01' etc
would be more appropriate.

> +					   /* deallocated, or quiesced */
> +/* Linux status bits */
> +#define VAS_WIN_NO_CRED_CLOSE	0x00000004 /* Window is closed due to */
> +					   /* lost credit */

This is mixing a user defined bit field with hcall API value field. You
also AFAIKS as yet don't fill in the hypervisor status anywhere.

I would make this it's own field entirely. A boolean would be nice, if
possible.

>  /*
>   * Get/Set bit fields
>   */
> @@ -59,6 +72,8 @@ 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 */
> +	struct mutex mmap_mutex;	/* protects paste address mmap() */
> +					/* with DLPAR close/open windows */
>  };
>  
>  /*
> @@ -67,6 +82,7 @@ struct vas_user_win_ref {
>  struct vas_window {
>  	u32 winid;
>  	u32 wcreds_max;	/* Window credits */
> +	u32 status;
>  	enum vas_cop_type cop;
>  	struct vas_user_win_ref task_ref;
>  	char *dbgname;
> diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c
> index 4d82c92ddd52..2b0ced611f32 100644
> --- a/arch/powerpc/platforms/book3s/vas-api.c
> +++ b/arch/powerpc/platforms/book3s/vas-api.c
> @@ -316,6 +316,7 @@ static int coproc_ioc_tx_win_open(struct file *fp, unsigned long arg)
>  		return PTR_ERR(txwin);
>  	}
>  
> +	mutex_init(&txwin->task_ref.mmap_mutex);
>  	cp_inst->txwin = txwin;
>  
>  	return 0;
> diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
> index 2ef56157634f..d9ff73d7704d 100644
> --- a/arch/powerpc/platforms/pseries/vas.c
> +++ b/arch/powerpc/platforms/pseries/vas.c
> @@ -501,6 +501,7 @@ static int __init get_vas_capabilities(u8 feat, enum vas_cop_feat_type type,
>  	memset(vcaps, 0, sizeof(*vcaps));
>  	INIT_LIST_HEAD(&vcaps->list);
>  
> +	vcaps->feat = feat;
>  	caps = &vcaps->caps;
>  
>  	rc = h_query_vas_capabilities(H_QUERY_VAS_CAPABILITIES, feat,
> @@ -539,6 +540,145 @@ static int __init get_vas_capabilities(u8 feat, enum vas_cop_feat_type type,
>  	return 0;
>  }
>  
> +/*
> + * VAS windows can be closed due to lost credits when the core is
> + * removed. So reopen them if credits are available due to DLPAR
> + * core add and set the window active status. When NX sees the page
> + * fault on the unmapped paste address, the kernel handles the fault
> + * by setting the remapping to new paste address if the window is
> + * active.
> + */
> +static int reconfig_open_windows(struct vas_caps *vcaps, int creds)
> +{
> +	long domain[PLPAR_HCALL9_BUFSIZE] = {VAS_DEFAULT_DOMAIN_ID};
> +	struct vas_cop_feat_caps *caps = &vcaps->caps;
> +	struct pseries_vas_window *win = NULL, *tmp;
> +	int rc, mv_ents = 0;
> +
> +	/*
> +	 * Nothing to do if there are no closed windows.
> +	 */
> +	if (!vcaps->close_wins)
> +		return 0;
> +
> +	/*
> +	 * For the core removal, the hypervisor reduces the credits
> +	 * assigned to the LPAR and the kernel closes VAS windows
> +	 * in the hypervisor depends on reduced credits. The kernel
> +	 * uses LIFO (the last windows that are opened will be closed
> +	 * first) and expects to open in the same order when credits
> +	 * are available.
> +	 * For example, 40 windows are closed when the LPAR lost 2 cores
> +	 * (dedicated). If 1 core is added, this LPAR can have 20 more
> +	 * credits. It means the kernel can reopen 20 windows. So move
> +	 * 20 entries in the VAS windows lost and reopen next 20 windows.
> +	 */
> +	if (vcaps->close_wins > creds)
> +		mv_ents = vcaps->close_wins - creds;
> +
> +	list_for_each_entry_safe(win, tmp, &vcaps->list, win_list) {
> +		if (!mv_ents)
> +			break;
> +
> +		mv_ents--;
> +	}
> +
> +	list_for_each_entry_safe_from(win, tmp, &vcaps->list, win_list) {
> +		/*
> +		 * Nothing to do on this window if it is not closed
> +		 * with VAS_WIN_NO_CRED_CLOSE
> +		 */
> +		if (!(win->vas_win.status & VAS_WIN_NO_CRED_CLOSE))
> +			continue;
> +
> +		rc = allocate_setup_window(win, (u64 *)&domain[0],
> +					   caps->win_type);
> +		if (rc)
> +			return rc;
> +
> +		rc = h_modify_vas_window(win);
> +		if (rc)
> +			goto out;
> +
> +		mutex_lock(&win->vas_win.task_ref.mmap_mutex);
> +		/*
> +		 * Set window status to active
> +		 */
> +		win->vas_win.status &= ~VAS_WIN_NO_CRED_CLOSE;
> +		mutex_unlock(&win->vas_win.task_ref.mmap_mutex);

What's the mutex protecting? If status can change, what happens if it 
changed between checking it above and this point?

> +		win->win_type = caps->win_type;
> +		if (!--vcaps->close_wins)
> +			break;
> +	}
> +
> +	return 0;
> +out:
> +	/*
> +	 * Window modify HCALL failed. So close the window to the
> +	 * hypervisor and return.
> +	 */
> +	free_irq_setup(win);
> +	h_deallocate_vas_window(win->vas_win.winid);
> +	return rc;
> +}
> +
> +/*
> + * Get new VAS capabilities when the core add/removal configuration
> + * changes. Reconfig window configurations based on the credits
> + * availability from this new capabilities.
> + */
> +static int vas_reconfig_capabilties(u8 type)
> +{
> +	struct hv_vas_cop_feat_caps *hv_caps;
> +	struct vas_cop_feat_caps *caps;
> +	int lpar_creds, new_creds;
> +	struct vas_caps *vcaps;
> +	int rc = 0;
> +
> +	if (type >= VAS_MAX_FEAT_TYPE) {
> +		pr_err("Invalid credit type %d\n", type);
> +		return -EINVAL;
> +	}
> +
> +	vcaps = &vascaps[type];
> +	caps = &vcaps->caps;
> +
> +	hv_caps = kmalloc(sizeof(*hv_caps), GFP_KERNEL);
> +	if (!hv_caps)
> +		return -ENOMEM;
> +
> +	mutex_lock(&vas_pseries_mutex);
> +	rc = h_query_vas_capabilities(H_QUERY_VAS_CAPABILITIES, vcaps->feat,
> +				      (u64)virt_to_phys(hv_caps));
> +	if (rc)
> +		goto out;
> +
> +	new_creds = be16_to_cpu(hv_caps->target_lpar_creds);
> +
> +	lpar_creds = atomic_read(&caps->target_creds);

NBD but it would be slightly clearer that new_creds is the absolute 
value not the number of new credits if you call these two
new_nr_creds and old_nr_creds or prev_nr_creds.

> +
> +	atomic_set(&caps->target_creds, new_creds);
> +	/*
> +	 * The total number of available credits may be decreased or
> +	 * inceased with DLPAR operation. Means some windows have to be
> +	 * closed / reopened. Hold the vas_pseries_mutex so that the
> +	 * the user space can not open new windows.
> +	 */
> +	if (lpar_creds <  new_creds) {
> +		/*
> +		 * If the existing target credits is less than the new
> +		 * target, reopen windows if they are closed due to
> +		 * the previous DLPAR (core removal).
> +		 */
> +		rc = reconfig_open_windows(vcaps, new_creds - lpar_creds);
> +	}
> +
> +out:
> +	mutex_unlock(&vas_pseries_mutex);
> +	kfree(hv_caps);
> +	return rc;
> +}
> +
>  /*
>   * Total number of default credits available (target_credits)
>   * in LPAR depends on number of cores configured. It varies based on
> @@ -565,6 +705,10 @@ static int pseries_vas_notifier(struct notifier_block *nb,
>  	if (!intserv)
>  		return NOTIFY_OK;
>  
> +	rc = vas_reconfig_capabilties(VAS_GZIP_DEF_FEAT_TYPE);
> +	if (rc)
> +		pr_err("Failed reconfig VAS capabilities with DLPAR\n");
> +
>  	return rc;
>  }
>  
> diff --git a/arch/powerpc/platforms/pseries/vas.h b/arch/powerpc/platforms/pseries/vas.h
> index 0538760d13be..45b62565955b 100644
> --- a/arch/powerpc/platforms/pseries/vas.h
> +++ b/arch/powerpc/platforms/pseries/vas.h
> @@ -21,12 +21,6 @@
>  #define VAS_MOD_WIN_FLAGS	(VAS_MOD_WIN_JOBS_KILL | VAS_MOD_WIN_DR | \
>  				VAS_MOD_WIN_PR | VAS_MOD_WIN_SF)
>  
> -#define VAS_WIN_ACTIVE		0x0
> -#define VAS_WIN_CLOSED		0x1
> -#define VAS_WIN_INACTIVE	0x2	/* Inactive due to HW failure */
> -/* Process of being modified, deallocated, or quiesced */
> -#define VAS_WIN_MOD_IN_PROCESS	0x3
> -
>  #define VAS_COPY_PASTE_USER_MODE	0x00000001
>  #define VAS_COP_OP_USER_MODE		0x00000010
>  
> @@ -84,6 +78,8 @@ struct vas_cop_feat_caps {
>  struct vas_caps {
>  	struct vas_cop_feat_caps caps;
>  	struct list_head list;	/* List of open windows */
> +	int close_wins;		/* closed windows in the hypervisor for DLPAR */

'nr_closed_wins'

This does not look like a capability. Although I guess neither do some
of the fields (used_creds) in vas_cop_feat_caps. Why not at least put
this there with the rest of those credits fields?

Thanks,
Nick

> +	u8 feat;		/* Feature type */
>  };
>  
>  /*
> -- 
> 2.27.0
> 
> 
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 05/10] powerpc/pseries/vas: Close windows with DLPAR core removal
  2022-01-21 19:57 ` [PATCH v3 05/10] powerpc/pseries/vas: Close windows with DLPAR core removal Haren Myneni
@ 2022-02-14  3:17   ` Nicholas Piggin
  2022-02-16  1:48     ` Haren Myneni
  0 siblings, 1 reply; 27+ messages in thread
From: Nicholas Piggin @ 2022-02-14  3:17 UTC (permalink / raw)
  To: Haren Myneni, linuxppc-dev, mpe

Excerpts from Haren Myneni's message of January 22, 2022 5:57 am:
> 
> The hypervisor reduces the available credits if the core is removed
> from the LPAR. So there is possibility of using excessive credits
> (windows) in the LPAR and the hypervisor expects the system to close
> the excessive windows. Even though the user space can continue to use
> these windows to send compression requests to NX, the hypervisor expects
> the LPAR to reduce these windows usage so that NX load can be equally
> distributed across all LPARs in the system.
> 
> When the DLPAR notifier is received, get the new VAS capabilities from
> the hypervisor and close the excessive windows in the hypervisor. Also
> the kernel unmaps the paste address so that the user space receives paste
> failure until these windows are active with the later DLPAR (core add).

The changelog needs work. Unmapping the window and the ramifications of
that needs more description here or in comments.

> 
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/vas.h          |   1 +
>  arch/powerpc/platforms/book3s/vas-api.c |   2 +
>  arch/powerpc/platforms/pseries/vas.c    | 117 ++++++++++++++++++++++--
>  arch/powerpc/platforms/pseries/vas.h    |   1 +
>  4 files changed, 112 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/vas.h b/arch/powerpc/include/asm/vas.h
> index f1efe86563cc..ddc05a8fc2e3 100644
> --- a/arch/powerpc/include/asm/vas.h
> +++ b/arch/powerpc/include/asm/vas.h
> @@ -74,6 +74,7 @@ struct vas_user_win_ref {
>  	struct mm_struct *mm;	/* Linux process mm_struct */
>  	struct mutex mmap_mutex;	/* protects paste address mmap() */
>  					/* with DLPAR close/open windows */
> +	struct vm_area_struct *vma;	/* Save VMA and used in DLPAR ops */
>  };
>  
>  /*
> diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c
> index 2b0ced611f32..a63fd48e34a7 100644
> --- a/arch/powerpc/platforms/book3s/vas-api.c
> +++ b/arch/powerpc/platforms/book3s/vas-api.c
> @@ -399,6 +399,8 @@ static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
>  	pr_devel("%s(): paste addr %llx at %lx, rc %d\n", __func__,
>  			paste_addr, vma->vm_start, rc);
>  
> +	txwin->task_ref.vma = vma;
> +
>  	return rc;
>  }
>  
> diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
> index d9ff73d7704d..75ccd0a599ec 100644
> --- a/arch/powerpc/platforms/pseries/vas.c
> +++ b/arch/powerpc/platforms/pseries/vas.c
> @@ -370,13 +370,28 @@ static struct vas_window *vas_allocate_window(int vas_id, u64 flags,
>  	if (rc)
>  		goto out_free;
>  
> -	vas_user_win_add_mm_context(&txwin->vas_win.task_ref);
>  	txwin->win_type = cop_feat_caps->win_type;
>  	mutex_lock(&vas_pseries_mutex);
> -	list_add(&txwin->win_list, &caps->list);
> +	/*
> +	 * Possible to loose the acquired credit with DLPAR core

s/loose/lose/g

> +	 * removal after the window is opened. So if there are any
> +	 * closed windows (means with lost credits), do not give new
> +	 * window to user space. New windows will be opened only
> +	 * after the existing windows are reopened when credits are
> +	 * available.
> +	 */
> +	if (!caps->close_wins) {
> +		list_add(&txwin->win_list, &caps->list);
> +		caps->num_wins++;
> +		mutex_unlock(&vas_pseries_mutex);
> +		vas_user_win_add_mm_context(&txwin->vas_win.task_ref);
> +		return &txwin->vas_win;
> +	}
>  	mutex_unlock(&vas_pseries_mutex);
>  
> -	return &txwin->vas_win;
> +	put_vas_user_win_ref(&txwin->vas_win.task_ref);
> +	rc = -EBUSY;
> +	pr_err("No credit is available to allocate window\n");
>  
>  out_free:
>  	/*
> @@ -439,14 +454,24 @@ static int vas_deallocate_window(struct vas_window *vwin)
>  
>  	caps = &vascaps[win->win_type].caps;
>  	mutex_lock(&vas_pseries_mutex);
> -	rc = deallocate_free_window(win);
> -	if (rc) {
> -		mutex_unlock(&vas_pseries_mutex);
> -		return rc;
> -	}
> +	/*
> +	 * VAS window is already closed in the hypervisor when
> +	 * lost the credit. So just remove the entry from
> +	 * the list, remove task references and free vas_window
> +	 * struct.
> +	 */
> +	if (win->vas_win.status & VAS_WIN_NO_CRED_CLOSE) {
> +		rc = deallocate_free_window(win);
> +		if (rc) {
> +			mutex_unlock(&vas_pseries_mutex);
> +			return rc;
> +		}
> +	} else
> +		vascaps[win->win_type].close_wins--;
>  
>  	list_del(&win->win_list);
>  	atomic_dec(&caps->used_creds);
> +	vascaps[win->win_type].num_wins--;
>  	mutex_unlock(&vas_pseries_mutex);
>  
>  	put_vas_user_win_ref(&vwin->task_ref);
> @@ -622,6 +647,72 @@ static int reconfig_open_windows(struct vas_caps *vcaps, int creds)
>  	return rc;
>  }
>  
> +/*
> + * The hypervisor reduces the available credits if the LPAR lost core. It
> + * means the excessive windows should not be active and the user space
> + * should not be using these windows to send compression requests to NX.
> + * So the kernel closes the excessive windows and unmap the paste address
> + * such that the user space receives paste instruction failure. Then up to
> + * the user space to fall back to SW compression and manage with the
> + * existing windows.
> + */
> +static int reconfig_close_windows(struct vas_caps *vcap, int excess_creds)
> +{
> +	struct pseries_vas_window *win, *tmp;
> +	struct vas_user_win_ref *task_ref;
> +	struct vm_area_struct *vma;
> +	int rc = 0;
> +
> +	list_for_each_entry_safe(win, tmp, &vcap->list, win_list) {
> +		/*
> +		 * This window is already closed due to lost credit
> +		 * before. Go for next window.
> +		 */
> +		if (win->vas_win.status & VAS_WIN_NO_CRED_CLOSE)
> +			continue;
> +
> +		task_ref = &win->vas_win.task_ref;
> +		mutex_lock(&task_ref->mmap_mutex);
> +		vma = task_ref->vma;
> +		/*
> +		 * Number of available credits are reduced, So select
> +		 * and close windows.
> +		 */
> +		win->vas_win.status |= VAS_WIN_NO_CRED_CLOSE;
> +
> +		mmap_write_lock(task_ref->mm);
> +		/*
> +		 * vma is set in the original mapping. But this mapping
> +		 * is done with mmap() after the window is opened with ioctl.
> +		 * so we may not see the original mapping if the core remove
> +		 * is done before the original mmap() and after the ioctl.
> +		 */
> +		if (vma)
> +			zap_page_range(vma, vma->vm_start,
> +					vma->vm_end - vma->vm_start);
> +
> +		mmap_write_unlock(task_ref->mm);
> +		mutex_unlock(&task_ref->mmap_mutex);
> +		/*
> +		 * Close VAS window in the hypervisor, but do not
> +		 * free vas_window struct since it may be reused
> +		 * when the credit is available later (DLPAR with
> +		 * adding cores). This struct will be used
> +		 * later when the process issued with close(FD).
> +		 */
> +		rc = deallocate_free_window(win);
> +		if (rc)
> +			return rc;
> +
> +		vcap->close_wins++;
> +
> +		if (!--excess_creds)
> +			break;
> +	}
> +
> +	return 0;
> +}
> +
>  /*
>   * Get new VAS capabilities when the core add/removal configuration
>   * changes. Reconfig window configurations based on the credits
> @@ -633,7 +724,7 @@ static int vas_reconfig_capabilties(u8 type)
>  	struct vas_cop_feat_caps *caps;
>  	int lpar_creds, new_creds;
>  	struct vas_caps *vcaps;
> -	int rc = 0;
> +	int rc = 0, active_wins;
>  
>  	if (type >= VAS_MAX_FEAT_TYPE) {
>  		pr_err("Invalid credit type %d\n", type);
> @@ -671,6 +762,14 @@ static int vas_reconfig_capabilties(u8 type)
>  		 * the previous DLPAR (core removal).
>  		 */
>  		rc = reconfig_open_windows(vcaps, new_creds - lpar_creds);
> +	} else {
> +		/*
> +		 * # active windows is more than new LPAR available
> +		 * credits. So close the excessive windows.
> +		 */
> +		active_wins = vcaps->num_wins - vcaps->close_wins;
> +		if (active_wins > new_creds)
> +			rc = reconfig_close_windows(vcaps, active_wins - new_creds);
>  	}
>  
>  out:
> diff --git a/arch/powerpc/platforms/pseries/vas.h b/arch/powerpc/platforms/pseries/vas.h
> index 45b62565955b..8ce9b84693e8 100644
> --- a/arch/powerpc/platforms/pseries/vas.h
> +++ b/arch/powerpc/platforms/pseries/vas.h
> @@ -79,6 +79,7 @@ struct vas_caps {
>  	struct vas_cop_feat_caps caps;
>  	struct list_head list;	/* List of open windows */
>  	int close_wins;		/* closed windows in the hypervisor for DLPAR */
> +	int num_wins;		/* Number of windows opened */

num -> nr is usually best.

And I know you like to abbreviate names, but it helps unfamiliar readers 
to use a few more letters. num_wins -> nr_windows or even better 
nr_open_windows.

Same comment about location of the field. Also it's not quite clear to 
me what the relationship is between your open windows here and used 
credits in the vas_cop_feat_caps structure. In other comments and 
changelogs you use credit and window interchangeably.


>  	u8 feat;		/* Feature type */
>  };
>  
> -- 
> 2.27.0
> 
> 
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 06/10] powerpc/vas: Map paste address only if window is active
  2022-01-21 19:58 ` [PATCH v3 06/10] powerpc/vas: Map paste address only if window is active Haren Myneni
@ 2022-02-14  3:20   ` Nicholas Piggin
  2022-02-16  1:58     ` Haren Myneni
  0 siblings, 1 reply; 27+ messages in thread
From: Nicholas Piggin @ 2022-02-14  3:20 UTC (permalink / raw)
  To: Haren Myneni, linuxppc-dev, mpe

Excerpts from Haren Myneni's message of January 22, 2022 5:58 am:
> 
> The paste address mapping is done with mmap() after the window is
> opened with ioctl. But the window can be closed due to lost credit
> due to core removal before mmap(). So if the window is not active,
> return mmap() failure with -EACCES and expects the user space reissue
> mmap() when the window is active or open new window when the credit
> is available.
> 
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/platforms/book3s/vas-api.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c
> index a63fd48e34a7..2d06bd1b1935 100644
> --- a/arch/powerpc/platforms/book3s/vas-api.c
> +++ b/arch/powerpc/platforms/book3s/vas-api.c
> @@ -379,10 +379,27 @@ static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
>  		return -EACCES;
>  	}
>  
> +	/*
> +	 * The initial mapping is done after the window is opened
> +	 * with ioctl. But this window might have been closed
> +	 * due to lost credit (core removal on PowerVM) before mmap().

What does "initial mapping" mean?

mapping ~= mmap, in kernel speak.

You will have to differentiate the concepts.

> +	 * So if the window is not active, return mmap() failure
> +	 * with -EACCES and expects the user space reconfigure (mmap)
> +	 * window when it is active again or open new window when
> +	 * the credit is available.
> +	 */
> +	mutex_lock(&txwin->task_ref.mmap_mutex);
> +	if (txwin->status != VAS_WIN_ACTIVE) {
> +		pr_err("%s(): Window is not active\n", __func__);
> +		rc = -EACCES;
> +		goto out;
> +	}
> +
>  	paste_addr = cp_inst->coproc->vops->paste_addr(txwin);
>  	if (!paste_addr) {
>  		pr_err("%s(): Window paste address failed\n", __func__);
> -		return -EINVAL;
> +		rc = -EINVAL;
> +		goto out;
>  	}
>  
>  	pfn = paste_addr >> PAGE_SHIFT;
> @@ -401,6 +418,8 @@ static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
>  
>  	txwin->task_ref.vma = vma;
>  
> +out:
> +	mutex_unlock(&txwin->task_ref.mmap_mutex);

If the hypervisor can revoke a window at any point with DLPAR, it's not 
clear *why* this is needed. The hypervisor could cause your window to 
close right after this mmap() returns, right? So an explanation for 
exactly what this patch is needed for beyond that would help.

Thanks,
Nick

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 07/10] powerpc/vas: Add paste address mmap fault handler
  2022-01-21 19:59 ` [PATCH v3 07/10] powerpc/vas: Add paste address mmap fault handler Haren Myneni
@ 2022-02-14  3:37   ` Nicholas Piggin
  2022-02-16  2:21     ` Haren Myneni
  0 siblings, 1 reply; 27+ messages in thread
From: Nicholas Piggin @ 2022-02-14  3:37 UTC (permalink / raw)
  To: Haren Myneni, linuxppc-dev, mpe

Excerpts from Haren Myneni's message of January 22, 2022 5:59 am:
> 
> The user space opens VAS windows and issues NX requests by pasting
> CRB on the corresponding paste address mmap. When the system looses

s/loose/lose/g throughout the series.

> credits due to core removal, the kernel has to close the window in
> the hypervisor

By the way what if the kernel does not close the window and we try
to access the memory? The hypervisor will inject faults?

> and make the window inactive by unmapping this paste
> address. Also the OS has to handle NX request page faults if the user
> space issue NX requests.
> 
> This handler remap the new paste address with the same VMA when the
> window is active again (due to core add with DLPAR). Otherwise
> returns paste failure.

This patch should come before (or combined with) the patch that zaps 
PTEs. Putting it afterwards is logically backward. Even if you don't
really expect the series to half work in a half bisected state, it
just makes the changes easier to follow.

Thanks,
Nick

> 
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/platforms/book3s/vas-api.c | 60 +++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c
> index 2d06bd1b1935..5ceba75c13eb 100644
> --- a/arch/powerpc/platforms/book3s/vas-api.c
> +++ b/arch/powerpc/platforms/book3s/vas-api.c
> @@ -351,6 +351,65 @@ static int coproc_release(struct inode *inode, struct file *fp)
>  	return 0;
>  }
>  
> +/*
> + * This fault handler is invoked when the VAS/NX generates page fault on
> + * the paste address.

The core generates the page fault here, right? paste destination is 
translated by the core MMU (the instruction is executed in the core,
afterall).

> Happens if the kernel closes window in hypervisor
> + * (on PowerVM) due to lost credit or the paste address is not mapped.

Call it pseries everywhere if you're talking about the API and Linux
code, rather than some specific quirk or issue of of the PowerVM
implementation.

> + */
> +static vm_fault_t vas_mmap_fault(struct vm_fault *vmf)
> +{
> +	struct vm_area_struct *vma = vmf->vma;
> +	struct file *fp = vma->vm_file;
> +	struct coproc_instance *cp_inst = fp->private_data;
> +	struct vas_window *txwin;
> +	u64 paste_addr;
> +	int ret;
> +
> +	/*
> +	 * window is not opened. Shouldn't expect this error.
> +	 */
> +	if (!cp_inst || !cp_inst->txwin) {
> +		pr_err("%s(): No send window open?\n", __func__);

Probably don't put PR_ERROR logs with question marks in them. The
administrator knows less than you to answer the question.

"Unexpected fault on paste address with TX window closed" etc.

Then you don't need the comment either because the message explains it.

> +		return VM_FAULT_SIGBUS;
> +	}
> +
> +	txwin = cp_inst->txwin;
> +	/*
> +	 * Fault is coming due to missing from the original mmap.

Rather than a vague comment like this (which we already know a fault 
comes from a missing or insufficient PTE), you could point to exactly
the code which zaps the PTEs.

> +	 * Can happen only when the window is closed due to lost
> +	 * credit before mmap() or the user space issued NX request
> +	 * without mapping.
> +	 */
> +	if (txwin->task_ref.vma != vmf->vma) {
> +		pr_err("%s(): No previous mapping with paste address\n",
> +			__func__);
> +		return VM_FAULT_SIGBUS;
> +	}
> +
> +	mutex_lock(&txwin->task_ref.mmap_mutex);
> +	/*
> +	 * The window may be inactive due to lost credit (Ex: core
> +	 * removal with DLPAR). When the window is active again when
> +	 * the credit is available, remap with the new paste address.

Remap also typically means mapping the same physical memory at a 
different virtual address. So when you say remap with the new paste
address, in Linux mm terms that means you're mapping the same window
at a different virtual address.

But you're faulting a different physical address into the same
virtual.

> +	 */
> +	if (txwin->status == VAS_WIN_ACTIVE) {
> +		paste_addr = cp_inst->coproc->vops->paste_addr(txwin);
> +		if (paste_addr) {
> +			ret = vmf_insert_pfn(vma, vma->vm_start,
> +					(paste_addr >> PAGE_SHIFT));
> +			mutex_unlock(&txwin->task_ref.mmap_mutex);
> +			return ret;
> +		}
> +	}
> +	mutex_unlock(&txwin->task_ref.mmap_mutex);

Here a comment about how userspace is supposed to handle the
window-closed condition might be appropriate.

Thanks,
Nick

> +
> +	return VM_FAULT_SIGBUS;
> +
> +}
> +static const struct vm_operations_struct vas_vm_ops = {
> +	.fault = vas_mmap_fault,
> +};
> +
>  static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
>  {
>  	struct coproc_instance *cp_inst = fp->private_data;
> @@ -417,6 +476,7 @@ static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
>  			paste_addr, vma->vm_start, rc);
>  
>  	txwin->task_ref.vma = vma;
> +	vma->vm_ops = &vas_vm_ops;
>  
>  out:
>  	mutex_unlock(&txwin->task_ref.mmap_mutex);
> -- 
> 2.27.0
> 
> 
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 08/10] powerpc/vas: Return paste instruction failure if no active window
  2022-01-21 19:59 ` [PATCH v3 08/10] powerpc/vas: Return paste instruction failure if no active window Haren Myneni
@ 2022-02-14  3:41   ` Nicholas Piggin
  0 siblings, 0 replies; 27+ messages in thread
From: Nicholas Piggin @ 2022-02-14  3:41 UTC (permalink / raw)
  To: Haren Myneni, linuxppc-dev, mpe

Excerpts from Haren Myneni's message of January 22, 2022 5:59 am:
> 
> The VAS window may not be active if the system looses credits and
> the NX generates page fault when it receives request on unmap
> paste address.
> 
> The kernel handles the fault by remap new paste address if the
> window is active again, Otherwise return the paste instruction
> failure if the executed instruction that caused the fault was
> a paste.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/ppc-opcode.h   |  2 ++
>  arch/powerpc/platforms/book3s/vas-api.c | 47 ++++++++++++++++++++++++-
>  2 files changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
> index efad07081cc0..fe2a69206588 100644
> --- a/arch/powerpc/include/asm/ppc-opcode.h
> +++ b/arch/powerpc/include/asm/ppc-opcode.h
> @@ -262,6 +262,8 @@
>  #define PPC_INST_MFSPR_PVR		0x7c1f42a6
>  #define PPC_INST_MFSPR_PVR_MASK		0xfc1ffffe
>  #define PPC_INST_MTMSRD			0x7c000164
> +#define PPC_INST_PASTE			0x7c20070d
> +#define PPC_INST_PASTE_MASK		0xfc2007ff
>  #define PPC_INST_POPCNTB		0x7c0000f4
>  #define PPC_INST_POPCNTB_MASK		0xfc0007fe
>  #define PPC_INST_RFEBB			0x4c000124
> diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c
> index 5ceba75c13eb..2ffd34bc4032 100644
> --- a/arch/powerpc/platforms/book3s/vas-api.c
> +++ b/arch/powerpc/platforms/book3s/vas-api.c
> @@ -351,6 +351,41 @@ static int coproc_release(struct inode *inode, struct file *fp)
>  	return 0;
>  }
>  
> +/*
> + * If the executed instruction that caused the fault was a paste, then
> + * clear regs CR0[EQ], advance NIP, and return 0. Else return error code.
> + */
> +static int do_fail_paste(void)
> +{
> +	struct pt_regs *regs = current->thread.regs;
> +	u32 instword;
> +
> +	if (WARN_ON_ONCE(!regs))
> +		return -EINVAL;
> +
> +	if (WARN_ON_ONCE(!user_mode(regs)))
> +		return -EINVAL;
> +
> +	/*
> +	 * If we couldn't translate the instruction, the driver should
> +	 * return success without handling the fault, it will be retried
> +	 * or the instruction fetch will fault.
> +	 */
> +	if (get_user(instword, (u32 __user *)(regs->nip)))
> +		return -EAGAIN;

We say this, and yet the caller seems to SIGBUS us on this return
value.

> +
> +	/*
> +	 * Not a paste instruction, driver may fail the fault.
> +	 */
> +	if ((instword & PPC_INST_PASTE_MASK) != PPC_INST_PASTE)
> +		return -ENOENT;
> +
> +	regs->ccr &= ~0xe0000000;	/* Clear CR0[0-2] to fail paste */
> +	regs_add_return_ip(regs, 4);	/* Skip the paste */

Maybe instead of 'Skip the paste' the comment should be 'Emulated the 
paste'.

> +
> +	return 0;
> +}
> +
>  /*
>   * This fault handler is invoked when the VAS/NX generates page fault on
>   * the paste address. Happens if the kernel closes window in hypervisor
> @@ -403,9 +438,19 @@ static vm_fault_t vas_mmap_fault(struct vm_fault *vmf)
>  	}
>  	mutex_unlock(&txwin->task_ref.mmap_mutex);
>  
> -	return VM_FAULT_SIGBUS;
> +	/*
> +	 * Received this fault due to closing the actual window.
> +	 * It can happen during migration or lost credits.
> +	 * Since no mapping, return the paste instruction failure
> +	 * to the user space.
> +	 */
> +	ret = do_fail_paste();
> +	if (!ret)
> +		return VM_FAULT_NOPAGE;

  if (!ret || ret == -EAGAIN)

  ?

Thanks,
Nick

>  
> +	return VM_FAULT_SIGBUS;
>  }
> +
>  static const struct vm_operations_struct vas_vm_ops = {
>  	.fault = vas_mmap_fault,
>  };
> -- 
> 2.27.0
> 
> 
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 09/10] powerpc/pseries/vas: sysfs interface to export capabilities
  2022-01-21 20:00 ` [PATCH v3 09/10] powerpc/pseries/vas: sysfs interface to export capabilities Haren Myneni
@ 2022-02-14  3:49   ` Nicholas Piggin
  0 siblings, 0 replies; 27+ messages in thread
From: Nicholas Piggin @ 2022-02-14  3:49 UTC (permalink / raw)
  To: Haren Myneni, linuxppc-dev, mpe

Excerpts from Haren Myneni's message of January 22, 2022 6:00 am:
> 
> The hypervisor provides the available VAS GZIP capabilities such
> as default or QoS window type and the target available credits in
> each type. This patch creates sysfs entries and exports the target,
> used and the available credits for each feature.
> 
> This interface can be used by the user space to determine the credits
> usage or to set the target credits in the case of QoS type (for DLPAR).
> 
> /sys/devices/vas/vas0/gzip/def_caps: (default GZIP capabilities)
> 	avail_creds /* Available credits to use */
> 	target_creds /* Total credits available. Can be
> 			 /* changed with DLPAR operation */
> 	used_creds  /* Used credits */
> 
> /sys/devices/vas/vas0/gzip/qos_caps (QoS GZIP capabilities)
> 	avail_creds
> 	target_creds
> 	used_creds

Can we not use these abbreviations in sysfs?

In the code it's one thing (I still don't prefer them but that's up to 
you), but we should be a bit better in external ABIs. Can we also get a 
better description of the difference between available credits and 
target credits? They both appear to be credits available.

default_capabilities
  nr_available_credits
  nr_target_credits
  nr_used_credits

qos_capabilities
  ...

Keeping this description in the tree rather than just in the changelog
would be nice too.

> 
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/Makefile    |   2 +-
>  arch/powerpc/platforms/pseries/vas-sysfs.c | 218 +++++++++++++++++++++
>  arch/powerpc/platforms/pseries/vas.c       |   6 +
>  arch/powerpc/platforms/pseries/vas.h       |   6 +
>  4 files changed, 231 insertions(+), 1 deletion(-)
>  create mode 100644 arch/powerpc/platforms/pseries/vas-sysfs.c
> 
> diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
> index ee60b59024b4..29b522d2c755 100644
> --- a/arch/powerpc/platforms/pseries/Makefile
> +++ b/arch/powerpc/platforms/pseries/Makefile
> @@ -29,6 +29,6 @@ obj-$(CONFIG_PPC_SVM)		+= svm.o
>  obj-$(CONFIG_FA_DUMP)		+= rtas-fadump.o
>  
>  obj-$(CONFIG_SUSPEND)		+= suspend.o
> -obj-$(CONFIG_PPC_VAS)		+= vas.o
> +obj-$(CONFIG_PPC_VAS)		+= vas.o vas-sysfs.o
>  
>  obj-$(CONFIG_ARCH_HAS_CC_PLATFORM)	+= cc_platform.o
> diff --git a/arch/powerpc/platforms/pseries/vas-sysfs.c b/arch/powerpc/platforms/pseries/vas-sysfs.c
> new file mode 100644
> index 000000000000..f7609cdef8f8
> --- /dev/null
> +++ b/arch/powerpc/platforms/pseries/vas-sysfs.c
> @@ -0,0 +1,218 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright 2016-17 IBM Corp.

Seems to have suffered a copy-paste problem.

> +	/*
> +	 * The hypervisor does not expose multiple VAS instances, but can
> +	 * see multiple VAS instances on PowerNV. So create 'vas0' directory
> +	 * on PowerVM.

powernv / pseries when you're talking about the APIs or Linux 
implementation (or you can use OPAL / PAPR for APIs specifically too).

Thanks,
Nick

> +	 */
> +	pseries_vas_kobj = kobject_create_and_add("vas0",
> +					&vas_miscdev.this_device->kobj);
> +	if (!pseries_vas_kobj) {
> +		pr_err("Failed to create VAS sysfs entry\n");
> +		return -ENOMEM;
> +	}
> +
> +	if ((vas_caps->feat_type & VAS_GZIP_QOS_FEAT_BIT) ||
> +		(vas_caps->feat_type & VAS_GZIP_DEF_FEAT_BIT)) {
> +		gzip_caps_kobj = kobject_create_and_add("gzip",
> +						       pseries_vas_kobj);
> +		if (!gzip_caps_kobj) {
> +			pr_err("Failed to create VAS GZIP capability entry\n");
> +			kobject_put(pseries_vas_kobj);
> +			return -ENOMEM;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +#else
> +int sysfs_add_vas_caps(struct vas_cop_feat_caps *caps)
> +{
> +	return 0;
> +}
> +
> +int __init sysfs_pseries_vas_init(struct vas_all_caps *vas_caps)
> +{
> +	return 0;
> +}
> +#endif
> diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
> index 75ccd0a599ec..32098e4a2786 100644
> --- a/arch/powerpc/platforms/pseries/vas.c
> +++ b/arch/powerpc/platforms/pseries/vas.c
> @@ -560,6 +560,10 @@ static int __init get_vas_capabilities(u8 feat, enum vas_cop_feat_type type,
>  		}
>  	}
>  
> +	rc = sysfs_add_vas_caps(caps);
> +	if (rc)
> +		return rc;
> +
>  	copypaste_feat = true;
>  
>  	return 0;
> @@ -843,6 +847,8 @@ static int __init pseries_vas_init(void)
>  	caps_all.descriptor = be64_to_cpu(hv_caps->descriptor);
>  	caps_all.feat_type = be64_to_cpu(hv_caps->feat_type);
>  
> +	sysfs_pseries_vas_init(&caps_all);
> +
>  	hv_cop_caps = kmalloc(sizeof(*hv_cop_caps), GFP_KERNEL);
>  	if (!hv_cop_caps) {
>  		rc = -ENOMEM;
> diff --git a/arch/powerpc/platforms/pseries/vas.h b/arch/powerpc/platforms/pseries/vas.h
> index 8ce9b84693e8..bc393bd74030 100644
> --- a/arch/powerpc/platforms/pseries/vas.h
> +++ b/arch/powerpc/platforms/pseries/vas.h
> @@ -24,6 +24,9 @@
>  #define VAS_COPY_PASTE_USER_MODE	0x00000001
>  #define VAS_COP_OP_USER_MODE		0x00000010
>  
> +#define VAS_GZIP_QOS_CAPABILITIES	0x56516F73477A6970
> +#define VAS_GZIP_DEFAULT_CAPABILITIES	0x56446566477A6970
> +
>  /*
>   * Co-processor feature - GZIP QoS windows or GZIP default windows
>   */
> @@ -120,4 +123,7 @@ struct pseries_vas_window {
>  	char *name;
>  	int fault_virq;
>  };
> +
> +int sysfs_add_vas_caps(struct vas_cop_feat_caps *caps);
> +int __init sysfs_pseries_vas_init(struct vas_all_caps *vas_caps);
>  #endif /* _VAS_H */
> -- 
> 2.27.0
> 
> 
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 01/10] powerpc/pseries/vas: Use common names in VAS capability structure
  2022-02-14  2:14   ` Nicholas Piggin
@ 2022-02-15 18:32     ` Haren Myneni
  0 siblings, 0 replies; 27+ messages in thread
From: Haren Myneni @ 2022-02-15 18:32 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev, mpe

On Mon, 2022-02-14 at 12:14 +1000, Nicholas Piggin wrote:
> Excerpts from Haren Myneni's message of January 22, 2022 5:54 am:
> > target/used/avail_creds provides credits usage to user space via
> > sysfs and the same interface can be used on PowerNV in future.
> > Remove "lpar" from these names so that applicable on both PowerVM
> > and PowerNV.
> 
> But not in this series? This is just to save you having to do more
> renaming later?

Thanks for your review. 
Yes, Removing _lpar_ in struct elements to make it clear so that can
easily add in sysfs patch.  

> 
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> 
> > Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> > ---
> >  arch/powerpc/platforms/pseries/vas.c | 10 +++++-----
> >  arch/powerpc/platforms/pseries/vas.h |  6 +++---
> >  2 files changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/powerpc/platforms/pseries/vas.c
> > b/arch/powerpc/platforms/pseries/vas.c
> > index d243ddc58827..c0737379cc7b 100644
> > --- a/arch/powerpc/platforms/pseries/vas.c
> > +++ b/arch/powerpc/platforms/pseries/vas.c
> > @@ -310,8 +310,8 @@ static struct vas_window
> > *vas_allocate_window(int vas_id, u64 flags,
> >  
> >  	cop_feat_caps = &caps->caps;
> >  
> > -	if (atomic_inc_return(&cop_feat_caps->used_lpar_creds) >
> > -			atomic_read(&cop_feat_caps->target_lpar_creds)) 
> > {
> > +	if (atomic_inc_return(&cop_feat_caps->used_creds) >
> > +			atomic_read(&cop_feat_caps->target_creds)) {
> >  		pr_err("Credits are not available to allocate
> > window\n");
> >  		rc = -EINVAL;
> >  		goto out;
> > @@ -385,7 +385,7 @@ static struct vas_window
> > *vas_allocate_window(int vas_id, u64 flags,
> >  	free_irq_setup(txwin);
> >  	h_deallocate_vas_window(txwin->vas_win.winid);
> >  out:
> > -	atomic_dec(&cop_feat_caps->used_lpar_creds);
> > +	atomic_dec(&cop_feat_caps->used_creds);
> >  	kfree(txwin);
> >  	return ERR_PTR(rc);
> >  }
> > @@ -445,7 +445,7 @@ static int vas_deallocate_window(struct
> > vas_window *vwin)
> >  	}
> >  
> >  	list_del(&win->win_list);
> > -	atomic_dec(&caps->used_lpar_creds);
> > +	atomic_dec(&caps->used_creds);
> >  	mutex_unlock(&vas_pseries_mutex);
> >  
> >  	put_vas_user_win_ref(&vwin->task_ref);
> > @@ -521,7 +521,7 @@ static int __init get_vas_capabilities(u8 feat,
> > enum vas_cop_feat_type type,
> >  	}
> >  	caps->max_lpar_creds = be16_to_cpu(hv_caps->max_lpar_creds);
> >  	caps->max_win_creds = be16_to_cpu(hv_caps->max_win_creds);
> > -	atomic_set(&caps->target_lpar_creds,
> > +	atomic_set(&caps->target_creds,
> >  		   be16_to_cpu(hv_caps->target_lpar_creds));
> >  	if (feat == VAS_GZIP_DEF_FEAT) {
> >  		caps->def_lpar_creds = be16_to_cpu(hv_caps-
> > >def_lpar_creds);
> > diff --git a/arch/powerpc/platforms/pseries/vas.h
> > b/arch/powerpc/platforms/pseries/vas.h
> > index 4ecb3fcabd10..fa7ce74f1e49 100644
> > --- a/arch/powerpc/platforms/pseries/vas.h
> > +++ b/arch/powerpc/platforms/pseries/vas.h
> > @@ -72,9 +72,9 @@ struct vas_cop_feat_caps {
> >  	};
> >  	/* Total LPAR available credits. Can be different from max LPAR
> > */
> >  	/* credits due to DLPAR operation */
> > -	atomic_t	target_lpar_creds;
> > -	atomic_t	used_lpar_creds; /* Used credits so far */
> > -	u16		avail_lpar_creds; /* Remaining available credits */
> > +	atomic_t	target_creds;
> > +	atomic_t	used_creds;	/* Used credits so far */
> > +	u16		avail_creds;	/* Remaining available credits */
> >  };
> >  
> >  /*
> > -- 
> > 2.27.0
> > 
> > 
> > 


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 02/10] powerpc/pseries/vas: Add notifier for DLPAR core removal/add
  2022-02-14  2:27   ` Nicholas Piggin
@ 2022-02-16  1:07     ` Haren Myneni
  0 siblings, 0 replies; 27+ messages in thread
From: Haren Myneni @ 2022-02-16  1:07 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev, mpe; +Cc: Nathan Lynch

On Mon, 2022-02-14 at 12:27 +1000, Nicholas Piggin wrote:
> Excerpts from Haren Myneni's message of January 22, 2022 5:54 am:
> > The hypervisor assigns credits for each LPAR based on number of
> > cores configured in that system. So expects to release credits
> > (means windows) when the core is removed. This patch adds notifier
> > for core removal/add so that the OS closes windows if the system
> > looses credits due to core removal and reopen windows when the
> > credits available later.
> 
> This could be improved. As far as I can tell,
> 
>  The hypervisor assigns vas credits (windows) for each LPAR based on
> the 
>  number of cores configured in that system. The OS is expected to 
>  release credits when cores are removed, and may allocate more when 
>  cores are added.
> 
> Or can you only re-use credits that you previously lost?

yes, reopen windows / re-use credits when the previously lost credits
are available. 
> 
> > Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> > ---
> >  arch/powerpc/platforms/pseries/vas.c | 37
> > ++++++++++++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> > 
> > diff --git a/arch/powerpc/platforms/pseries/vas.c
> > b/arch/powerpc/platforms/pseries/vas.c
> > index c0737379cc7b..d2c8292bfb33 100644
> > --- a/arch/powerpc/platforms/pseries/vas.c
> > +++ b/arch/powerpc/platforms/pseries/vas.c
> > @@ -538,6 +538,39 @@ static int __init get_vas_capabilities(u8
> > feat, enum vas_cop_feat_type type,
> >  	return 0;
> >  }
> >  
> > +/*
> > + * Total number of default credits available (target_credits)
> > + * in LPAR depends on number of cores configured. It varies based
> > on
> > + * whether processors are in shared mode or dedicated mode.
> > + * Get the notifier when CPU configuration is changed with DLPAR
> > + * operation so that get the new target_credits (vas default
> > capabilities)
> > + * and then update the existing windows usage if needed.
> > + */
> > +static int pseries_vas_notifier(struct notifier_block *nb,
> > +				unsigned long action, void *data)
> > +{
> > +	struct of_reconfig_data *rd = data;
> > +	struct device_node *dn = rd->dn;
> > +	const __be32 *intserv = NULL;
> > +	int len, rc = 0;
> > +
> > +	if ((action == OF_RECONFIG_ATTACH_NODE) ||
> > +		(action == OF_RECONFIG_DETACH_NODE))
> 
> I suppose the OF notifier is the way to do it (cc Nathan).

Using notifier here. registering notifier
with of_reconfig_notifier_register() as in other places (hotplug-
cpu.c pseries_smp_notifier())

> 
> Could this patch be folded in with where it acually does something?
> It 
> makes it easier to review and understand how the notifier is used.

Added this notifier as a seperate patch to make it smaller. Sure, I can
include this patch in 'Add reconfig_close/open_windows() patch'. 
> 
> 
> > +		intserv = of_get_property(dn, "ibm,ppc-interrupt-
> > server#s",
> > +					  &len);
> > +	/*
> > +	 * Processor config is not changed
> > +	 */
> > +	if (!intserv)
> > +		return NOTIFY_OK;
> > +
> > +	return rc;
> > +}
> > +
> > +static struct notifier_block pseries_vas_nb = {
> > +	.notifier_call = pseries_vas_notifier,
> > +};
> > +
> >  static int __init pseries_vas_init(void)
> >  {
> >  	struct hv_vas_cop_feat_caps *hv_cop_caps;
> > @@ -591,6 +624,10 @@ static int __init pseries_vas_init(void)
> >  			goto out_cop;
> >  	}
> >  
> > +	/* Processors can be added/removed only on LPAR */
> 
> What does this comment mean? DLPAR?

I will remve it, basically trying to say that this notifier is called
when core is removed / added. 

Thanks
haren

> 
> Thanks,
> Nick
> 
> > +	if (copypaste_feat && firmware_has_feature(FW_FEATURE_LPAR))
> > +		of_reconfig_notifier_register(&pseries_vas_nb);
> > +
> >  	pr_info("GZIP feature is available\n");
> >  
> >  out_cop:
> > -- 
> > 2.27.0
> > 
> > 
> > 


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 03/10] powerpc/pseries/vas: Save LPID in pseries_vas_window struct
  2022-02-14  2:41   ` Nicholas Piggin
@ 2022-02-16  1:09     ` Haren Myneni
  0 siblings, 0 replies; 27+ messages in thread
From: Haren Myneni @ 2022-02-16  1:09 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev, mpe

On Mon, 2022-02-14 at 12:41 +1000, Nicholas Piggin wrote:
> Excerpts from Haren Myneni's message of January 22, 2022 5:55 am:
> > The kernel sets the VAS window with partition PID when is opened in
> > the hypervisor. During DLPAR operation, windows can be closed and
> > reopened in the hypervisor when the credit is available. So saves
> > this PID in pseries_vas_window struct when the window is opened
> > initially and reuse it later during DLPAR operation.
> 
> This probably shouldn't be called lpid, while you're changing it.
> "partition PID" and "LPAR PID" is also confusing. I know the name
> somewhat comes from the specifiction, but pid/PID would be fine,
> it's clear we are talking about "this LPAR" when in pseries code.
> 
> > Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> > ---
> >  arch/powerpc/platforms/pseries/vas.c | 7 ++++---
> >  arch/powerpc/platforms/pseries/vas.h | 1 +
> >  2 files changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/powerpc/platforms/pseries/vas.c
> > b/arch/powerpc/platforms/pseries/vas.c
> > index d2c8292bfb33..2ef56157634f 100644
> > --- a/arch/powerpc/platforms/pseries/vas.c
> > +++ b/arch/powerpc/platforms/pseries/vas.c
> > @@ -107,7 +107,6 @@ static int h_deallocate_vas_window(u64 winid)
> >  static int h_modify_vas_window(struct pseries_vas_window *win)
> >  {
> >  	long rc;
> > -	u32 lpid = mfspr(SPRN_PID);
> >  
> >  	/*
> >  	 * AMR value is not supported in Linux VAS implementation.
> > @@ -115,7 +114,7 @@ static int h_modify_vas_window(struct
> > pseries_vas_window *win)
> >  	 */
> >  	do {
> >  		rc = plpar_hcall_norets(H_MODIFY_VAS_WINDOW,
> > -					win->vas_win.winid, lpid, 0,
> > +					win->vas_win.winid, win->lpid,
> > 0,
> >  					VAS_MOD_WIN_FLAGS, 0);
> >  
> >  		rc = hcall_return_busy_check(rc);
> > @@ -125,7 +124,7 @@ static int h_modify_vas_window(struct
> > pseries_vas_window *win)
> >  		return 0;
> >  
> >  	pr_err("H_MODIFY_VAS_WINDOW error: %ld, winid %u lpid %u\n",
> > -			rc, win->vas_win.winid, lpid);
> > +			rc, win->vas_win.winid, win->lpid);
> >  	return -EIO;
> >  }
> >  
> > @@ -338,6 +337,8 @@ static struct vas_window
> > *vas_allocate_window(int
> > vas_id, u64 flags,
> >  		}
> >  	}
> >  
> > +	txwin->lpid = mfspr(SPRN_PID);
> > +
> >  	/*
> >  	 * Allocate / Deallocate window hcalls and setup / free IRQs
> >  	 * have to be protected with mutex.
> > diff --git a/arch/powerpc/platforms/pseries/vas.h
> > b/arch/powerpc/platforms/pseries/vas.h
> > index fa7ce74f1e49..0538760d13be 100644
> > --- a/arch/powerpc/platforms/pseries/vas.h
> > +++ b/arch/powerpc/platforms/pseries/vas.h
> > @@ -115,6 +115,7 @@ struct pseries_vas_window {
> >  	u64 domain[6];		/* Associativity domain Ids */
> >  				/* this window is allocated */
> >  	u64 util;
> > +	u32 lpid;
> 
> Comment could be "PID associated with this window".   

yes, will add this comment.

> 
> BTW, is the TID parameter deprecated? Doesn't seem that we use that.

Right, tpid is deprecated on p10 and we are not using it.

Thanks
Haren

> 
> Thanks,
> Nick


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 04/10] powerpc/pseries/vas: Reopen windows with DLPAR core add
  2022-02-14  3:08   ` Nicholas Piggin
@ 2022-02-16  1:38     ` Haren Myneni
  0 siblings, 0 replies; 27+ messages in thread
From: Haren Myneni @ 2022-02-16  1:38 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev, mpe

On Mon, 2022-02-14 at 13:08 +1000, Nicholas Piggin wrote:
> Excerpts from Haren Myneni's message of January 22, 2022 5:56 am:
> > VAS windows can be closed in the hypervisor due to lost credits
> > when the core is removed. If these credits are available later
> > for core add, reopen these windows and set them active. When the
> > kernel sees page fault on the paste address, it creates new mapping
> > on the new paste address. Then the user space can continue to use
> > these windows and send HW compression requests to NX successfully.
> 
> Any reason to put this before the close windows patch? It would be
> more logical to put it afterwards AFAIKS.

reconfig_open_windows() is just to reopen and set the status flag when
windows are closed. I thought adding handler first before closing /
unmap helps during git bisect. 

I can change. 

> 
> > Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> > ---
> >  arch/powerpc/include/asm/vas.h          |  16 +++
> >  arch/powerpc/platforms/book3s/vas-api.c |   1 +
> >  arch/powerpc/platforms/pseries/vas.c    | 144
> > ++++++++++++++++++++++++
> >  arch/powerpc/platforms/pseries/vas.h    |   8 +-
> >  4 files changed, 163 insertions(+), 6 deletions(-)
> > 
> > diff --git a/arch/powerpc/include/asm/vas.h
> > b/arch/powerpc/include/asm/vas.h
> > index 57573d9c1e09..f1efe86563cc 100644
> > --- a/arch/powerpc/include/asm/vas.h
> > +++ b/arch/powerpc/include/asm/vas.h
> > @@ -29,6 +29,19 @@
> >  #define VAS_THRESH_FIFO_GT_QTR_FULL	2
> >  #define VAS_THRESH_FIFO_GT_EIGHTH_FULL	3
> >  
> > +/*
> > + * VAS window status
> > + */
> > +#define VAS_WIN_ACTIVE		0x0	/* Used in platform
> > independent */
> > +					/* vas mmap() */
> > +/* The hypervisor returns these values */
> > +#define VAS_WIN_CLOSED		0x00000001
> > +#define VAS_WIN_INACTIVE	0x00000002 /* Inactive due to HW
> > failure */
> > +#define VAS_WIN_MOD_IN_PROCESS	0x00000003 /* Process of being
> > modified, */
> 
> While you're moving these and adding a comment, it would be good to 
> list what hcalls they are relevant to. H_QUERY_VAS_WINDOW (which is
> not
> used anywhere yet?) These are also a 1-byte field, so '0x00', '0x01'
> etc
> would be more appropriate.

Yes, these status bits are assigned by the hypervisor and we are not
using / supporting them right now. I just list them as defined in PAPR
NX HCALLs.

For example: OS can modify the existing window with modify HCALL when
the window is used. During this time, the hypervisor return this status
with window query HCALL. 
 
> 
> > +					   /* deallocated, or quiesced
> > */
> > +/* Linux status bits */
> > +#define VAS_WIN_NO_CRED_CLOSE	0x00000004 /* Window is closed
> > due to */
> > +					   /* lost credit */
> 
> This is mixing a user defined bit field with hcall API value field.
> You
> also AFAIKS as yet don't fill in the hypervisor status anywhere.
> 
> I would make this it's own field entirely. A boolean would be nice,
> if
> possible.

Yes, HV status bits are not used here. 

In case if the window status is reported thorugh sysfs in future,
thought that it will be simpler to have one status flag. 

I can add 'hv_status' for the hypervisor status flag in pseries_vas-
window struct and 'status' for linux in vas_window struct. 

We also need one more status for migration. So boolean may not be used.

> 
> >  /*
> >   * Get/Set bit fields
> >   */
> > @@ -59,6 +72,8 @@ 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 */
> > +	struct mutex mmap_mutex;	/* protects paste address mmap() */
> > +					/* with DLPAR close/open
> > windows */
> >  };
> >  
> >  /*
> > @@ -67,6 +82,7 @@ struct vas_user_win_ref {
> >  struct vas_window {
> >  	u32 winid;
> >  	u32 wcreds_max;	/* Window credits */
> > +	u32 status;
> >  	enum vas_cop_type cop;
> >  	struct vas_user_win_ref task_ref;
> >  	char *dbgname;
> > diff --git a/arch/powerpc/platforms/book3s/vas-api.c
> > b/arch/powerpc/platforms/book3s/vas-api.c
> > index 4d82c92ddd52..2b0ced611f32 100644
> > --- a/arch/powerpc/platforms/book3s/vas-api.c
> > +++ b/arch/powerpc/platforms/book3s/vas-api.c
> > @@ -316,6 +316,7 @@ static int coproc_ioc_tx_win_open(struct file
> > *fp, unsigned long arg)
> >  		return PTR_ERR(txwin);
> >  	}
> >  
> > +	mutex_init(&txwin->task_ref.mmap_mutex);
> >  	cp_inst->txwin = txwin;
> >  
> >  	return 0;
> > diff --git a/arch/powerpc/platforms/pseries/vas.c
> > b/arch/powerpc/platforms/pseries/vas.c
> > index 2ef56157634f..d9ff73d7704d 100644
> > --- a/arch/powerpc/platforms/pseries/vas.c
> > +++ b/arch/powerpc/platforms/pseries/vas.c
> > @@ -501,6 +501,7 @@ static int __init get_vas_capabilities(u8 feat,
> > enum vas_cop_feat_type type,
> >  	memset(vcaps, 0, sizeof(*vcaps));
> >  	INIT_LIST_HEAD(&vcaps->list);
> >  
> > +	vcaps->feat = feat;
> >  	caps = &vcaps->caps;
> >  
> >  	rc = h_query_vas_capabilities(H_QUERY_VAS_CAPABILITIES, feat,
> > @@ -539,6 +540,145 @@ static int __init get_vas_capabilities(u8
> > feat, enum vas_cop_feat_type type,
> >  	return 0;
> >  }
> >  
> > +/*
> > + * VAS windows can be closed due to lost credits when the core is
> > + * removed. So reopen them if credits are available due to DLPAR
> > + * core add and set the window active status. When NX sees the
> > page
> > + * fault on the unmapped paste address, the kernel handles the
> > fault
> > + * by setting the remapping to new paste address if the window is
> > + * active.
> > + */
> > +static int reconfig_open_windows(struct vas_caps *vcaps, int
> > creds)
> > +{
> > +	long domain[PLPAR_HCALL9_BUFSIZE] = {VAS_DEFAULT_DOMAIN_ID};
> > +	struct vas_cop_feat_caps *caps = &vcaps->caps;
> > +	struct pseries_vas_window *win = NULL, *tmp;
> > +	int rc, mv_ents = 0;
> > +
> > +	/*
> > +	 * Nothing to do if there are no closed windows.
> > +	 */
> > +	if (!vcaps->close_wins)
> > +		return 0;
> > +
> > +	/*
> > +	 * For the core removal, the hypervisor reduces the credits
> > +	 * assigned to the LPAR and the kernel closes VAS windows
> > +	 * in the hypervisor depends on reduced credits. The kernel
> > +	 * uses LIFO (the last windows that are opened will be closed
> > +	 * first) and expects to open in the same order when credits
> > +	 * are available.
> > +	 * For example, 40 windows are closed when the LPAR lost 2
> > cores
> > +	 * (dedicated). If 1 core is added, this LPAR can have 20 more
> > +	 * credits. It means the kernel can reopen 20 windows. So move
> > +	 * 20 entries in the VAS windows lost and reopen next 20
> > windows.
> > +	 */
> > +	if (vcaps->close_wins > creds)
> > +		mv_ents = vcaps->close_wins - creds;
> > +
> > +	list_for_each_entry_safe(win, tmp, &vcaps->list, win_list) {
> > +		if (!mv_ents)
> > +			break;
> > +
> > +		mv_ents--;
> > +	}
> > +
> > +	list_for_each_entry_safe_from(win, tmp, &vcaps->list, win_list)
> > {
> > +		/*
> > +		 * Nothing to do on this window if it is not closed
> > +		 * with VAS_WIN_NO_CRED_CLOSE
> > +		 */
> > +		if (!(win->vas_win.status & VAS_WIN_NO_CRED_CLOSE))
> > +			continue;
> > +
> > +		rc = allocate_setup_window(win, (u64 *)&domain[0],
> > +					   caps->win_type);
> > +		if (rc)
> > +			return rc;
> > +
> > +		rc = h_modify_vas_window(win);
> > +		if (rc)
> > +			goto out;
> > +
> > +		mutex_lock(&win->vas_win.task_ref.mmap_mutex);
> > +		/*
> > +		 * Set window status to active
> > +		 */
> > +		win->vas_win.status &= ~VAS_WIN_NO_CRED_CLOSE;
> > +		mutex_unlock(&win->vas_win.task_ref.mmap_mutex);
> 
> What's the mutex protecting? If status can change, what happens if
> it 
> changed between checking it above and this point?

We need this mutex to protect status flag between
reconfig_close/open_windows() and VAS mmap handler. 
 We hold pseries_mutex() before calling recnfig_close/popen_windows(),
so not an issue here.

> 
> > +		win->win_type = caps->win_type;
> > +		if (!--vcaps->close_wins)
> > +			break;
> > +	}
> > +
> > +	return 0;
> > +out:
> > +	/*
> > +	 * Window modify HCALL failed. So close the window to the
> > +	 * hypervisor and return.
> > +	 */
> > +	free_irq_setup(win);
> > +	h_deallocate_vas_window(win->vas_win.winid);
> > +	return rc;
> > +}
> > +
> > +/*
> > + * Get new VAS capabilities when the core add/removal
> > configuration
> > + * changes. Reconfig window configurations based on the credits
> > + * availability from this new capabilities.
> > + */
> > +static int vas_reconfig_capabilties(u8 type)
> > +{
> > +	struct hv_vas_cop_feat_caps *hv_caps;
> > +	struct vas_cop_feat_caps *caps;
> > +	int lpar_creds, new_creds;
> > +	struct vas_caps *vcaps;
> > +	int rc = 0;
> > +
> > +	if (type >= VAS_MAX_FEAT_TYPE) {
> > +		pr_err("Invalid credit type %d\n", type);
> > +		return -EINVAL;
> > +	}
> > +
> > +	vcaps = &vascaps[type];
> > +	caps = &vcaps->caps;
> > +
> > +	hv_caps = kmalloc(sizeof(*hv_caps), GFP_KERNEL);
> > +	if (!hv_caps)
> > +		return -ENOMEM;
> > +
> > +	mutex_lock(&vas_pseries_mutex);
> > +	rc = h_query_vas_capabilities(H_QUERY_VAS_CAPABILITIES, vcaps-
> > >feat,
> > +				      (u64)virt_to_phys(hv_caps));
> > +	if (rc)
> > +		goto out;
> > +
> > +	new_creds = be16_to_cpu(hv_caps->target_lpar_creds);
> > +
> > +	lpar_creds = atomic_read(&caps->target_creds);
> 
> NBD but it would be slightly clearer that new_creds is the absolute 
> value not the number of new credits if you call these two
> new_nr_creds and old_nr_creds or prev_nr_creds.

Sure will change.
> 
> > +
> > +	atomic_set(&caps->target_creds, new_creds);
> > +	/*
> > +	 * The total number of available credits may be decreased or
> > +	 * inceased with DLPAR operation. Means some windows have to be
> > +	 * closed / reopened. Hold the vas_pseries_mutex so that the
> > +	 * the user space can not open new windows.
> > +	 */
> > +	if (lpar_creds <  new_creds) {
> > +		/*
> > +		 * If the existing target credits is less than the new
> > +		 * target, reopen windows if they are closed due to
> > +		 * the previous DLPAR (core removal).
> > +		 */
> > +		rc = reconfig_open_windows(vcaps, new_creds -
> > lpar_creds);
> > +	}
> > +
> > +out:
> > +	mutex_unlock(&vas_pseries_mutex);
> > +	kfree(hv_caps);
> > +	return rc;
> > +}
> > +
> >  /*
> >   * Total number of default credits available (target_credits)
> >   * in LPAR depends on number of cores configured. It varies based
> > on
> > @@ -565,6 +705,10 @@ static int pseries_vas_notifier(struct
> > notifier_block *nb,
> >  	if (!intserv)
> >  		return NOTIFY_OK;
> >  
> > +	rc = vas_reconfig_capabilties(VAS_GZIP_DEF_FEAT_TYPE);
> > +	if (rc)
> > +		pr_err("Failed reconfig VAS capabilities with
> > DLPAR\n");
> > +
> >  	return rc;
> >  }
> >  
> > diff --git a/arch/powerpc/platforms/pseries/vas.h
> > b/arch/powerpc/platforms/pseries/vas.h
> > index 0538760d13be..45b62565955b 100644
> > --- a/arch/powerpc/platforms/pseries/vas.h
> > +++ b/arch/powerpc/platforms/pseries/vas.h
> > @@ -21,12 +21,6 @@
> >  #define VAS_MOD_WIN_FLAGS	(VAS_MOD_WIN_JOBS_KILL | VAS_MOD_WIN_DR
> > | \
> >  				VAS_MOD_WIN_PR | VAS_MOD_WIN_SF)
> >  
> > -#define VAS_WIN_ACTIVE		0x0
> > -#define VAS_WIN_CLOSED		0x1
> > -#define VAS_WIN_INACTIVE	0x2	/* Inactive due to HW failure */
> > -/* Process of being modified, deallocated, or quiesced */
> > -#define VAS_WIN_MOD_IN_PROCESS	0x3
> > -
> >  #define VAS_COPY_PASTE_USER_MODE	0x00000001
> >  #define VAS_COP_OP_USER_MODE		0x00000010
> >  
> > @@ -84,6 +78,8 @@ struct vas_cop_feat_caps {
> >  struct vas_caps {
> >  	struct vas_cop_feat_caps caps;
> >  	struct list_head list;	/* List of open windows */
> > +	int close_wins;		/* closed windows in the hypervisor
> > for DLPAR */
> 
> 'nr_closed_wins'
> 
> This does not look like a capability. Although I guess neither do
> some
> of the fields (used_creds) in vas_cop_feat_caps. Why not at least put
> this there with the rest of those credits fields?

We can add used_credits() here since it is not part of HV capability.
It was included in previous pathes, So I did not change, 

Thanks
Haren

> 
> Thanks,
> Nick
> 
> > +	u8 feat;		/* Feature type */
> >  };
> >  
> >  /*
> > -- 
> > 2.27.0
> > 
> > 
> > 


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 05/10] powerpc/pseries/vas: Close windows with DLPAR core removal
  2022-02-14  3:17   ` Nicholas Piggin
@ 2022-02-16  1:48     ` Haren Myneni
  0 siblings, 0 replies; 27+ messages in thread
From: Haren Myneni @ 2022-02-16  1:48 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev, mpe

On Mon, 2022-02-14 at 13:17 +1000, Nicholas Piggin wrote:
> Excerpts from Haren Myneni's message of January 22, 2022 5:57 am:
> > The hypervisor reduces the available credits if the core is removed
> > from the LPAR. So there is possibility of using excessive credits
> > (windows) in the LPAR and the hypervisor expects the system to
> > close
> > the excessive windows. Even though the user space can continue to
> > use
> > these windows to send compression requests to NX, the hypervisor
> > expects
> > the LPAR to reduce these windows usage so that NX load can be
> > equally
> > distributed across all LPARs in the system.
> > 
> > When the DLPAR notifier is received, get the new VAS capabilities
> > from
> > the hypervisor and close the excessive windows in the hypervisor.
> > Also
> > the kernel unmaps the paste address so that the user space receives
> > paste
> > failure until these windows are active with the later DLPAR (core
> > add).
> 
> The changelog needs work. Unmapping the window and the ramifications
> of
> that needs more description here or in comments.

Thanks will change. 
> 
> > Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> > ---
> >  arch/powerpc/include/asm/vas.h          |   1 +
> >  arch/powerpc/platforms/book3s/vas-api.c |   2 +
> >  arch/powerpc/platforms/pseries/vas.c    | 117
> > ++++++++++++++++++++++--
> >  arch/powerpc/platforms/pseries/vas.h    |   1 +
> >  4 files changed, 112 insertions(+), 9 deletions(-)
> > 
> > diff --git a/arch/powerpc/include/asm/vas.h
> > b/arch/powerpc/include/asm/vas.h
> > index f1efe86563cc..ddc05a8fc2e3 100644
> > --- a/arch/powerpc/include/asm/vas.h
> > +++ b/arch/powerpc/include/asm/vas.h
> > @@ -74,6 +74,7 @@ struct vas_user_win_ref {
> >  	struct mm_struct *mm;	/* Linux process mm_struct */
> >  	struct mutex mmap_mutex;	/* protects paste address mmap() */
> >  					/* with DLPAR close/open
> > windows */
> > +	struct vm_area_struct *vma;	/* Save VMA and used in DLPAR ops
> > */
> >  };
> >  
> >  /*
> > diff --git a/arch/powerpc/platforms/book3s/vas-api.c
> > b/arch/powerpc/platforms/book3s/vas-api.c
> > index 2b0ced611f32..a63fd48e34a7 100644
> > --- a/arch/powerpc/platforms/book3s/vas-api.c
> > +++ b/arch/powerpc/platforms/book3s/vas-api.c
> > @@ -399,6 +399,8 @@ static int coproc_mmap(struct file *fp, struct
> > vm_area_struct *vma)
> >  	pr_devel("%s(): paste addr %llx at %lx, rc %d\n", __func__,
> >  			paste_addr, vma->vm_start, rc);
> >  
> > +	txwin->task_ref.vma = vma;
> > +
> >  	return rc;
> >  }
> >  
> > diff --git a/arch/powerpc/platforms/pseries/vas.c
> > b/arch/powerpc/platforms/pseries/vas.c
> > index d9ff73d7704d..75ccd0a599ec 100644
> > --- a/arch/powerpc/platforms/pseries/vas.c
> > +++ b/arch/powerpc/platforms/pseries/vas.c
> > @@ -370,13 +370,28 @@ static struct vas_window
> > *vas_allocate_window(int vas_id, u64 flags,
> >  	if (rc)
> >  		goto out_free;
> >  
> > -	vas_user_win_add_mm_context(&txwin->vas_win.task_ref);
> >  	txwin->win_type = cop_feat_caps->win_type;
> >  	mutex_lock(&vas_pseries_mutex);
> > -	list_add(&txwin->win_list, &caps->list);
> > +	/*
> > +	 * Possible to loose the acquired credit with DLPAR core
> 
> s/loose/lose/g
> 
> > +	 * removal after the window is opened. So if there are any
> > +	 * closed windows (means with lost credits), do not give new
> > +	 * window to user space. New windows will be opened only
> > +	 * after the existing windows are reopened when credits are
> > +	 * available.
> > +	 */
> > +	if (!caps->close_wins) {
> > +		list_add(&txwin->win_list, &caps->list);
> > +		caps->num_wins++;
> > +		mutex_unlock(&vas_pseries_mutex);
> > +		vas_user_win_add_mm_context(&txwin->vas_win.task_ref);
> > +		return &txwin->vas_win;
> > +	}
> >  	mutex_unlock(&vas_pseries_mutex);
> >  
> > -	return &txwin->vas_win;
> > +	put_vas_user_win_ref(&txwin->vas_win.task_ref);
> > +	rc = -EBUSY;
> > +	pr_err("No credit is available to allocate window\n");
> >  
> >  out_free:
> >  	/*
> > @@ -439,14 +454,24 @@ static int vas_deallocate_window(struct
> > vas_window *vwin)
> >  
> >  	caps = &vascaps[win->win_type].caps;
> >  	mutex_lock(&vas_pseries_mutex);
> > -	rc = deallocate_free_window(win);
> > -	if (rc) {
> > -		mutex_unlock(&vas_pseries_mutex);
> > -		return rc;
> > -	}
> > +	/*
> > +	 * VAS window is already closed in the hypervisor when
> > +	 * lost the credit. So just remove the entry from
> > +	 * the list, remove task references and free vas_window
> > +	 * struct.
> > +	 */
> > +	if (win->vas_win.status & VAS_WIN_NO_CRED_CLOSE) {
> > +		rc = deallocate_free_window(win);
> > +		if (rc) {
> > +			mutex_unlock(&vas_pseries_mutex);
> > +			return rc;
> > +		}
> > +	} else
> > +		vascaps[win->win_type].close_wins--;
> >  
> >  	list_del(&win->win_list);
> >  	atomic_dec(&caps->used_creds);
> > +	vascaps[win->win_type].num_wins--;
> >  	mutex_unlock(&vas_pseries_mutex);
> >  
> >  	put_vas_user_win_ref(&vwin->task_ref);
> > @@ -622,6 +647,72 @@ static int reconfig_open_windows(struct
> > vas_caps *vcaps, int creds)
> >  	return rc;
> >  }
> >  
> > +/*
> > + * The hypervisor reduces the available credits if the LPAR lost
> > core. It
> > + * means the excessive windows should not be active and the user
> > space
> > + * should not be using these windows to send compression requests
> > to NX.
> > + * So the kernel closes the excessive windows and unmap the paste
> > address
> > + * such that the user space receives paste instruction failure.
> > Then up to
> > + * the user space to fall back to SW compression and manage with
> > the
> > + * existing windows.
> > + */
> > +static int reconfig_close_windows(struct vas_caps *vcap, int
> > excess_creds)
> > +{
> > +	struct pseries_vas_window *win, *tmp;
> > +	struct vas_user_win_ref *task_ref;
> > +	struct vm_area_struct *vma;
> > +	int rc = 0;
> > +
> > +	list_for_each_entry_safe(win, tmp, &vcap->list, win_list) {
> > +		/*
> > +		 * This window is already closed due to lost credit
> > +		 * before. Go for next window.
> > +		 */
> > +		if (win->vas_win.status & VAS_WIN_NO_CRED_CLOSE)
> > +			continue;
> > +
> > +		task_ref = &win->vas_win.task_ref;
> > +		mutex_lock(&task_ref->mmap_mutex);
> > +		vma = task_ref->vma;
> > +		/*
> > +		 * Number of available credits are reduced, So select
> > +		 * and close windows.
> > +		 */
> > +		win->vas_win.status |= VAS_WIN_NO_CRED_CLOSE;
> > +
> > +		mmap_write_lock(task_ref->mm);
> > +		/*
> > +		 * vma is set in the original mapping. But this mapping
> > +		 * is done with mmap() after the window is opened with
> > ioctl.
> > +		 * so we may not see the original mapping if the core
> > remove
> > +		 * is done before the original mmap() and after the
> > ioctl.
> > +		 */
> > +		if (vma)
> > +			zap_page_range(vma, vma->vm_start,
> > +					vma->vm_end - vma->vm_start);
> > +
> > +		mmap_write_unlock(task_ref->mm);
> > +		mutex_unlock(&task_ref->mmap_mutex);
> > +		/*
> > +		 * Close VAS window in the hypervisor, but do not
> > +		 * free vas_window struct since it may be reused
> > +		 * when the credit is available later (DLPAR with
> > +		 * adding cores). This struct will be used
> > +		 * later when the process issued with close(FD).
> > +		 */
> > +		rc = deallocate_free_window(win);
> > +		if (rc)
> > +			return rc;
> > +
> > +		vcap->close_wins++;
> > +
> > +		if (!--excess_creds)
> > +			break;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  /*
> >   * Get new VAS capabilities when the core add/removal
> > configuration
> >   * changes. Reconfig window configurations based on the credits
> > @@ -633,7 +724,7 @@ static int vas_reconfig_capabilties(u8 type)
> >  	struct vas_cop_feat_caps *caps;
> >  	int lpar_creds, new_creds;
> >  	struct vas_caps *vcaps;
> > -	int rc = 0;
> > +	int rc = 0, active_wins;
> >  
> >  	if (type >= VAS_MAX_FEAT_TYPE) {
> >  		pr_err("Invalid credit type %d\n", type);
> > @@ -671,6 +762,14 @@ static int vas_reconfig_capabilties(u8 type)
> >  		 * the previous DLPAR (core removal).
> >  		 */
> >  		rc = reconfig_open_windows(vcaps, new_creds -
> > lpar_creds);
> > +	} else {
> > +		/*
> > +		 * # active windows is more than new LPAR available
> > +		 * credits. So close the excessive windows.
> > +		 */
> > +		active_wins = vcaps->num_wins - vcaps->close_wins;
> > +		if (active_wins > new_creds)
> > +			rc = reconfig_close_windows(vcaps, active_wins
> > - new_creds);
> >  	}
> >  
> >  out:
> > diff --git a/arch/powerpc/platforms/pseries/vas.h
> > b/arch/powerpc/platforms/pseries/vas.h
> > index 45b62565955b..8ce9b84693e8 100644
> > --- a/arch/powerpc/platforms/pseries/vas.h
> > +++ b/arch/powerpc/platforms/pseries/vas.h
> > @@ -79,6 +79,7 @@ struct vas_caps {
> >  	struct vas_cop_feat_caps caps;
> >  	struct list_head list;	/* List of open windows */
> >  	int close_wins;		/* closed windows in the hypervisor
> > for DLPAR */
> > +	int num_wins;		/* Number of windows opened */
> 
> num -> nr is usually best.
> 
> And I know you like to abbreviate names, but it helps unfamiliar
> readers 
> to use a few more letters. num_wins -> nr_windows or even better 
> nr_open_windows.
> 
> Same comment about location of the field. Also it's not quite clear
> to 
> me what the relationship is between your open windows here and used 
> credits in the vas_cop_feat_caps structure. In other comments and 
> changelogs you use credit and window interchangeably.

will change to nr_close_windows and nr_windows. 

used_credits is used for how many windows / credits are used by user
space. But when OS lost the credit, this window will be used in the
hypervisor, but not from the user space point of view. It should expect
this window is not active and should be available later. 

But OS will be closing this window in the hypervisor. So used
close_wins / num_wins are used to track the actual number of windows
opened/closed in the hypervisor. 

Thanks
Haren
> 
> 
> >  	u8 feat;		/* Feature type */
> >  };
> >  
> > -- 
> > 2.27.0
> > 
> > 
> > 


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 06/10] powerpc/vas: Map paste address only if window is active
  2022-02-14  3:20   ` Nicholas Piggin
@ 2022-02-16  1:58     ` Haren Myneni
  0 siblings, 0 replies; 27+ messages in thread
From: Haren Myneni @ 2022-02-16  1:58 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev, mpe

On Mon, 2022-02-14 at 13:20 +1000, Nicholas Piggin wrote:
> Excerpts from Haren Myneni's message of January 22, 2022 5:58 am:
> > The paste address mapping is done with mmap() after the window is
> > opened with ioctl. But the window can be closed due to lost credit
> > due to core removal before mmap(). So if the window is not active,
> > return mmap() failure with -EACCES and expects the user space
> > reissue
> > mmap() when the window is active or open new window when the credit
> > is available.
> > 
> > Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> > ---
> >  arch/powerpc/platforms/book3s/vas-api.c | 21 ++++++++++++++++++++-
> >  1 file changed, 20 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/powerpc/platforms/book3s/vas-api.c
> > b/arch/powerpc/platforms/book3s/vas-api.c
> > index a63fd48e34a7..2d06bd1b1935 100644
> > --- a/arch/powerpc/platforms/book3s/vas-api.c
> > +++ b/arch/powerpc/platforms/book3s/vas-api.c
> > @@ -379,10 +379,27 @@ static int coproc_mmap(struct file *fp,
> > struct vm_area_struct *vma)
> >  		return -EACCES;
> >  	}
> >  
> > +	/*
> > +	 * The initial mapping is done after the window is opened
> > +	 * with ioctl. But this window might have been closed
> > +	 * due to lost credit (core removal on PowerVM) before mmap().
> 
> What does "initial mapping" mean?
> 
> mapping ~= mmap, in kernel speak.

yes, the initial mapping is done with the actual mmap() call. 
> 
> You will have to differentiate the concepts.
> 
> > +	 * So if the window is not active, return mmap() failure
> > +	 * with -EACCES and expects the user space reconfigure (mmap)
> > +	 * window when it is active again or open new window when
> > +	 * the credit is available.
> > +	 */
> > +	mutex_lock(&txwin->task_ref.mmap_mutex);
> > +	if (txwin->status != VAS_WIN_ACTIVE) {
> > +		pr_err("%s(): Window is not active\n", __func__);
> > +		rc = -EACCES;
> > +		goto out;
> > +	}
> > +
> >  	paste_addr = cp_inst->coproc->vops->paste_addr(txwin);
> >  	if (!paste_addr) {
> >  		pr_err("%s(): Window paste address failed\n",
> > __func__);
> > -		return -EINVAL;
> > +		rc = -EINVAL;
> > +		goto out;
> >  	}
> >  
> >  	pfn = paste_addr >> PAGE_SHIFT;
> > @@ -401,6 +418,8 @@ static int coproc_mmap(struct file *fp, struct
> > vm_area_struct *vma)
> >  
> >  	txwin->task_ref.vma = vma;
> >  
> > +out:
> > +	mutex_unlock(&txwin->task_ref.mmap_mutex);
> 
> If the hypervisor can revoke a window at any point with DLPAR, it's
> not 
> clear *why* this is needed. The hypervisor could cause your window
> to 
> close right after this mmap() returns, right? So an explanation for 
> exactly what this patch is needed for beyond that would help.

Yes, the window can be closed by OS due to DLPAR after the mmap()
returns successfully which is a normal case - paste instruction failure
 until the window is reopened again.

But ths patch is mainly for window open by user space and dlpar happens
before the user space issue mmap().    

I will add more description in the commit log. 

Thanks
Haren

> 
> Thanks,
> Nick


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 07/10] powerpc/vas: Add paste address mmap fault handler
  2022-02-14  3:37   ` Nicholas Piggin
@ 2022-02-16  2:21     ` Haren Myneni
  0 siblings, 0 replies; 27+ messages in thread
From: Haren Myneni @ 2022-02-16  2:21 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev, mpe

On Mon, 2022-02-14 at 13:37 +1000, Nicholas Piggin wrote:
> Excerpts from Haren Myneni's message of January 22, 2022 5:59 am:
> > The user space opens VAS windows and issues NX requests by pasting
> > CRB on the corresponding paste address mmap. When the system looses
> 
> s/loose/lose/g throughout the series.
> 
> > credits due to core removal, the kernel has to close the window in
> > the hypervisor
> 
> By the way what if the kernel does not close the window and we try
> to access the memory? The hypervisor will inject faults?

The requests on the already opened windows will be successful even the
LPAR lost credits (due to core removal). But the hypervisor expects the
LPAR to behave like good citizen and give up resources with core
removal. So we do not see any issue with current upstream code for
DLPAR removal.

But we will have an issue with the migration. The hypervisor knows the
actulal number of credits assigned to the source LPAR before migration.
So assigns the same number on the destination. 

> 
> > and make the window inactive by unmapping this paste
> > address. Also the OS has to handle NX request page faults if the
> > user
> > space issue NX requests.
> > 
> > This handler remap the new paste address with the same VMA when the
> > window is active again (due to core add with DLPAR). Otherwise
> > returns paste failure.
> 
> This patch should come before (or combined with) the patch that zaps 
> PTEs. Putting it afterwards is logically backward. Even if you don't
> really expect the series to half work in a half bisected state, it
> just makes the changes easier to follow.
> 
> Thanks,
> Nick
> 
> > Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> > ---
> >  arch/powerpc/platforms/book3s/vas-api.c | 60
> > +++++++++++++++++++++++++
> >  1 file changed, 60 insertions(+)
> > 
> > diff --git a/arch/powerpc/platforms/book3s/vas-api.c
> > b/arch/powerpc/platforms/book3s/vas-api.c
> > index 2d06bd1b1935..5ceba75c13eb 100644
> > --- a/arch/powerpc/platforms/book3s/vas-api.c
> > +++ b/arch/powerpc/platforms/book3s/vas-api.c
> > @@ -351,6 +351,65 @@ static int coproc_release(struct inode *inode,
> > struct file *fp)
> >  	return 0;
> >  }
> >  
> > +/*
> > + * This fault handler is invoked when the VAS/NX generates page
> > fault on
> > + * the paste address.
> 
> The core generates the page fault here, right? paste destination is 
> translated by the core MMU (the instruction is executed in the core,
> afterall).

correct. Will update. 
> 
> > Happens if the kernel closes window in hypervisor
> > + * (on PowerVM) due to lost credit or the paste address is not
> > mapped.
> 
> Call it pseries everywhere if you're talking about the API and Linux
> code, rather than some specific quirk or issue of of the PowerVM
> implementation.
> 
> > + */
> > +static vm_fault_t vas_mmap_fault(struct vm_fault *vmf)
> > +{
> > +	struct vm_area_struct *vma = vmf->vma;
> > +	struct file *fp = vma->vm_file;
> > +	struct coproc_instance *cp_inst = fp->private_data;
> > +	struct vas_window *txwin;
> > +	u64 paste_addr;
> > +	int ret;
> > +
> > +	/*
> > +	 * window is not opened. Shouldn't expect this error.
> > +	 */
> > +	if (!cp_inst || !cp_inst->txwin) {
> > +		pr_err("%s(): No send window open?\n", __func__);
> 
> Probably don't put PR_ERROR logs with question marks in them. The
> administrator knows less than you to answer the question.
> 
> "Unexpected fault on paste address with TX window closed" etc.
> 
> Then you don't need the comment either because the message explains
> it.
> 
> > +		return VM_FAULT_SIGBUS;
> > +	}
> > +
> > +	txwin = cp_inst->txwin;
> > +	/*
> > +	 * Fault is coming due to missing from the original mmap.
> 
> Rather than a vague comment like this (which we already know a fault 
> comes from a missing or insufficient PTE), you could point to exactly
> the code which zaps the PTEs.
> 
> > +	 * Can happen only when the window is closed due to lost
> > +	 * credit before mmap() or the user space issued NX request
> > +	 * without mapping.
> > +	 */
> > +	if (txwin->task_ref.vma != vmf->vma) {
> > +		pr_err("%s(): No previous mapping with paste
> > address\n",
> > +			__func__);
> > +		return VM_FAULT_SIGBUS;
> > +	}
> > +
> > +	mutex_lock(&txwin->task_ref.mmap_mutex);
> > +	/*
> > +	 * The window may be inactive due to lost credit (Ex: core
> > +	 * removal with DLPAR). When the window is active again when
> > +	 * the credit is available, remap with the new paste address.
> 
> Remap also typically means mapping the same physical memory at a 
> different virtual address. So when you say remap with the new paste
> address, in Linux mm terms that means you're mapping the same window
> at a different virtual address.
> 
> But you're faulting a different physical address into the same
> virtual.

Yes, we get different physical address and this handler maps the this
paste address at the existing virtual address. will update this
comment.
> 
> > +	 */
> > +	if (txwin->status == VAS_WIN_ACTIVE) {
> > +		paste_addr = cp_inst->coproc->vops->paste_addr(txwin);
> > +		if (paste_addr) {
> > +			ret = vmf_insert_pfn(vma, vma->vm_start,
> > +					(paste_addr >> PAGE_SHIFT));
> > +			mutex_unlock(&txwin->task_ref.mmap_mutex);
> > +			return ret;
> > +		}
> > +	}
> > +	mutex_unlock(&txwin->task_ref.mmap_mutex);
> 
> Here a comment about how userspace is supposed to handle the
> window-closed condition might be appropriate.
> 
> Thanks,
> Nick
> 
> > +
> > +	return VM_FAULT_SIGBUS;
> > +
> > +}
> > +static const struct vm_operations_struct vas_vm_ops = {
> > +	.fault = vas_mmap_fault,
> > +};
> > +
> >  static int coproc_mmap(struct file *fp, struct vm_area_struct
> > *vma)
> >  {
> >  	struct coproc_instance *cp_inst = fp->private_data;
> > @@ -417,6 +476,7 @@ static int coproc_mmap(struct file *fp, struct
> > vm_area_struct *vma)
> >  			paste_addr, vma->vm_start, rc);
> >  
> >  	txwin->task_ref.vma = vma;
> > +	vma->vm_ops = &vas_vm_ops;
> >  
> >  out:
> >  	mutex_unlock(&txwin->task_ref.mmap_mutex);
> > -- 
> > 2.27.0
> > 
> > 
> > 


^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2022-02-16  2:22 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-21 19:52 [PATCH v3 00/10] powerpc/pseries/vas: NXGZIP support with DLPAR Haren Myneni
2022-01-21 19:54 ` [PATCH v3 01/10] powerpc/pseries/vas: Use common names in VAS capability structure Haren Myneni
2022-02-14  2:14   ` Nicholas Piggin
2022-02-15 18:32     ` Haren Myneni
2022-01-21 19:54 ` [PATCH v3 02/10] powerpc/pseries/vas: Add notifier for DLPAR core removal/add Haren Myneni
2022-02-14  2:27   ` Nicholas Piggin
2022-02-16  1:07     ` Haren Myneni
2022-01-21 19:55 ` [PATCH v3 03/10] powerpc/pseries/vas: Save LPID in pseries_vas_window struct Haren Myneni
2022-02-14  2:41   ` Nicholas Piggin
2022-02-16  1:09     ` Haren Myneni
2022-01-21 19:56 ` [PATCH v3 04/10] powerpc/pseries/vas: Reopen windows with DLPAR core add Haren Myneni
2022-02-14  3:08   ` Nicholas Piggin
2022-02-16  1:38     ` Haren Myneni
2022-01-21 19:57 ` [PATCH v3 05/10] powerpc/pseries/vas: Close windows with DLPAR core removal Haren Myneni
2022-02-14  3:17   ` Nicholas Piggin
2022-02-16  1:48     ` Haren Myneni
2022-01-21 19:58 ` [PATCH v3 06/10] powerpc/vas: Map paste address only if window is active Haren Myneni
2022-02-14  3:20   ` Nicholas Piggin
2022-02-16  1:58     ` Haren Myneni
2022-01-21 19:59 ` [PATCH v3 07/10] powerpc/vas: Add paste address mmap fault handler Haren Myneni
2022-02-14  3:37   ` Nicholas Piggin
2022-02-16  2:21     ` Haren Myneni
2022-01-21 19:59 ` [PATCH v3 08/10] powerpc/vas: Return paste instruction failure if no active window Haren Myneni
2022-02-14  3:41   ` Nicholas Piggin
2022-01-21 20:00 ` [PATCH v3 09/10] powerpc/pseries/vas: sysfs interface to export capabilities Haren Myneni
2022-02-14  3:49   ` Nicholas Piggin
2022-01-21 20:01 ` [PATCH v3 10/10] powerpc/pseries/vas: Write 'target_creds' for QoS credits change Haren Myneni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).