[PATCH v2 0/6] Add character devices for indices and platform-dump RTAS

[PATCH v2 0/6] Add character devices for indices and platform-dump RTAS

[Haren Myneni]

Several APIs such as rtas_get_indices(), rtas_get_dynamic_sensor(),
rtas_set_dynamic_indicator() and rtas_platform_dump() provided by
librtas library are implemented in user space using rtas syscall
in combination with writable mappings of /dev/mem. But this
implementation is not compatible with system lockdown which
prohibits /dev/mem access. The current kernel already provides
char based driver interfaces for several RTAS calls such as VPD
and system parameters to support lockdown feature.

This patch series adds new char based drivers, /dev/papr-indices
for ibm,get-indices, ibm,get-dynamic-sensor-state and
ibm,set-dynamic-indicator RTAS Calls. and /dev/papr-platform-dump
for ibm,platform-dump. Providing the similar open/ioctl/read
interfaces to the user space as in the case of VPD and system
parameters.

I have made changes to librtas library to use the new kernel
interfaces if the corresponding device entry is available.

This patch series has the following patches:
powerpc/pseries: Define common functions for RTAS sequence calls
- For some of sequence based RTAS calls, the OS should not start
  another sequence with different input until the previous sequence
  is completed. So the sequence should be completed during ioctl()
  and expose the entire buffer during read(). ibm,get-indices is
  sequence based RTAS function similar to ibm,get-vpd and we already
  have the corresponding implementation for VPD driver. So update
  papr_rtas_sequence struct for RTAS call specific functions and move
  the top level sequence functions in to a separate file.

powerpc/pseries: Define papr_indices_io_block for papr-indices ioctls
- /dev/papr-indices driver supports ibm,get-indices,
  ibm,get-dynamic-sensor-state and ibm,set-dynamic-indicator RTAS Calls.
  papr-indices.h introduces 3 different ioctls for these RTAS calls and
  the corresponding ioctl input buffer.

powerpc/pseries: Add papr-indices char driver for ibm,get-indices
- Introduce /dev/papr-indices char based driver and add support for
  get-indices RTAS function

powerpc/pseries: Add ibm,set-dynamic-indicator RTAS call support
- Update /dev/papr-indices for set-dynamic-indicator RTAS function

powerpc/pseries: Add ibm,get-dynamic-sensor-state RTAS call support
-  Update /dev/papr-indices for  get-dynamic-sensor-state RTAS function

powerpc/pseries: Add papr-platform-dump character driver for dump
   retrieval
- Introduce /dev/papr-platform-dump char driver and adds support for
  ibm,platform-dump. Received suggestions from the previous post as a
  separate patch - Updated the patch with invalidating the dump using
  a separate ioctl.

Changelog:

v2:
- Added unlock rtas_ibm_set_dynamic_indicator_lock and
  rtas_ibm_get_dynamic_sensor_state_lock mutex for failure cases
  as reported by Dan Carpenter
- Fixed build warnings for the proper function parameter descriptions
  as reported by kernel test robot <lkp@intel.com> 

Haren Myneni (6):
  powerpc/pseries: Define common functions for RTAS sequence calls
  powerpc/pseries: Define papr_indices_io_block for papr-indices ioctls
  powerpc/pseries: Add papr-indices char driver for ibm,get-indices
  powerpc/pseries: Add ibm,set-dynamic-indicator RTAS call support
  powerpc/pseries: Add ibm,get-dynamic-sensor-state RTAS call support
  powerpc/pseries: Add papr-platform-dump character driver for dump
    retrieval

 arch/powerpc/include/asm/rtas.h               |   3 +
 arch/powerpc/include/uapi/asm/papr-indices.h  |  41 ++
 .../include/uapi/asm/papr-platform-dump.h     |  15 +
 arch/powerpc/kernel/rtas.c                    |   6 +-
 arch/powerpc/platforms/pseries/Makefile       |   3 +-
 arch/powerpc/platforms/pseries/papr-indices.c | 558 ++++++++++++++++++
 .../platforms/pseries/papr-platform-dump.c    | 408 +++++++++++++
 .../platforms/pseries/papr-rtas-common.c      | 243 ++++++++
 .../platforms/pseries/papr-rtas-common.h      |  45 ++
 arch/powerpc/platforms/pseries/papr-vpd.c     | 270 ++-------
 10 files changed, 1364 insertions(+), 228 deletions(-)
 create mode 100644 arch/powerpc/include/uapi/asm/papr-indices.h
 create mode 100644 arch/powerpc/include/uapi/asm/papr-platform-dump.h
 create mode 100644 arch/powerpc/platforms/pseries/papr-indices.c
 create mode 100644 arch/powerpc/platforms/pseries/papr-platform-dump.c
 create mode 100644 arch/powerpc/platforms/pseries/papr-rtas-common.c
 create mode 100644 arch/powerpc/platforms/pseries/papr-rtas-common.h

-- 
2.43.5

[PATCH v2 1/6] powerpc/pseries: Define common functions for RTAS sequence calls

[Haren Myneni]

The RTAS call can be normal where retrieves the data form the
hypervisor once or sequence based RTAS call which has to
issue multiple times until the complete data is obtained. For
some of these sequence RTAS calls, the OS should not interleave
calls with different input until the sequence is completed.
The data is collected for each call and copy to the buffer
for the entire sequence during ioctl() handle and then expose
this buffer to the user space with read() handle.

One such sequence RTAS call is ibm,get-vpd and its support is
already included in the current code. To add the similar support
for other sequence based calls, move the common functions in to
separate file and update papr_rtas_sequence struct with the
following callbacks so that RTAS call specific code will be
defined and executed to complete the sequence.

struct papr_rtas_sequence {
        int error;
        void params;
        void (*begin) (struct papr_rtas_sequence *, void *);
        void (*end) (struct papr_rtas_sequence *);
        const char * (*work) (struct papr_rtas_sequence *, size_t *);
};

params: Input parameters used to pass for RTAS call.
Begin:	RTAS call specific function to initialize data
	including work area allocation.
End:	RTAS call specific function to free up resources
	(free work area) after the sequence is completed.
Work:	The actual RTAS call specific function which collects
	the data from the hypervisor.

Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/Makefile       |   2 +-
 .../platforms/pseries/papr-rtas-common.c      | 243 ++++++++++++++++
 .../platforms/pseries/papr-rtas-common.h      |  45 +++
 arch/powerpc/platforms/pseries/papr-vpd.c     | 270 +++---------------
 4 files changed, 335 insertions(+), 225 deletions(-)
 create mode 100644 arch/powerpc/platforms/pseries/papr-rtas-common.c
 create mode 100644 arch/powerpc/platforms/pseries/papr-rtas-common.h

diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
index 7bf506f6b8c8..697c216b70dc 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -3,7 +3,7 @@ ccflags-$(CONFIG_PPC_PSERIES_DEBUG)	+= -DDEBUG
 
 obj-y			:= lpar.o hvCall.o nvram.o reconfig.o \
 			   of_helpers.o rtas-work-area.o papr-sysparm.o \
-			   papr-vpd.o \
+			   papr-rtas-common.o papr-vpd.o \
 			   setup.o iommu.o event_sources.o ras.o \
 			   firmware.o power.o dlpar.o mobility.o rng.o \
 			   pci.o pci_dlpar.o eeh_pseries.o msi.o \
diff --git a/arch/powerpc/platforms/pseries/papr-rtas-common.c b/arch/powerpc/platforms/pseries/papr-rtas-common.c
new file mode 100644
index 000000000000..0209ec09d1fd
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/papr-rtas-common.c
@@ -0,0 +1,243 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#define pr_fmt(fmt) "papr-common: " fmt
+
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/signal.h>
+#include <linux/slab.h>
+#include <linux/file.h>
+#include <linux/fs.h>
+#include <linux/anon_inodes.h>
+#include <linux/sched/signal.h>
+#include "papr-rtas-common.h"
+
+/*
+ * Sequence based RTAS HCALL has to issue multiple times to retrieve
+ * complete data from the hypervisor. For some of these RTAS calls,
+ * the OS should not interleave calls with different input until the
+ * sequence is completed. So data is collected for these calls during
+ * ioctl handle and export to user space with read() handle.
+ * This file provides common functions needed for such sequence based
+ * RTAS calls Ex: ibm,get-vpd and ibm,get-indices.
+ */
+
+bool papr_rtas_blob_has_data(const struct papr_rtas_blob *blob)
+{
+	return blob->data && blob->len;
+}
+
+void papr_rtas_blob_free(const struct papr_rtas_blob *blob)
+{
+	if (blob) {
+		kvfree(blob->data);
+		kfree(blob);
+	}
+}
+
+/**
+ * papr_rtas_blob_extend() - Append data to a &struct papr_rtas_blob.
+ * @blob: The blob to extend.
+ * @data: The new data to append to @blob.
+ * @len:  The length of @data.
+ *
+ * Context: May sleep.
+ * Return: -ENOMEM on allocation failure, 0 otherwise.
+ */
+static int papr_rtas_blob_extend(struct papr_rtas_blob *blob,
+				const char *data, size_t len)
+{
+	const size_t new_len = blob->len + len;
+	const size_t old_len = blob->len;
+	const char *old_ptr = blob->data;
+	char *new_ptr;
+
+	new_ptr = kvrealloc(old_ptr, new_len, GFP_KERNEL_ACCOUNT);
+	if (!new_ptr)
+		return -ENOMEM;
+
+	memcpy(&new_ptr[old_len], data, len);
+	blob->data = new_ptr;
+	blob->len = new_len;
+	return 0;
+}
+
+/**
+ * papr_rtas_blob_generate() - Construct a new &struct papr_rtas_blob.
+ * @seq: work function of the caller that is called to obtain
+ *       data with the caller RTAS call.
+ *
+ * The @work callback is invoked until it returns NULL. @seq is
+ * passed to @work in its first argument on each call. When
+ * @work returns data, it should store the data length in its
+ * second argument.
+ *
+ * Context: May sleep.
+ * Return: A completely populated &struct papr_rtas_blob, or NULL on error.
+ */
+static const struct papr_rtas_blob *
+papr_rtas_blob_generate(struct papr_rtas_sequence *seq)
+{
+	struct papr_rtas_blob *blob;
+	const char *buf;
+	size_t len;
+	int err = 0;
+
+	blob  = kzalloc(sizeof(*blob), GFP_KERNEL_ACCOUNT);
+	if (!blob)
+		return NULL;
+
+	if (!seq->work)
+		return ERR_PTR(-EINVAL);
+
+
+	while (err == 0 && (buf = seq->work(seq, &len)))
+		err = papr_rtas_blob_extend(blob, buf, len);
+
+	if (err != 0 || !papr_rtas_blob_has_data(blob))
+		goto free_blob;
+
+	return blob;
+free_blob:
+	papr_rtas_blob_free(blob);
+	return NULL;
+}
+
+int papr_rtas_sequence_set_err(struct papr_rtas_sequence *seq, int err)
+{
+	/* Preserve the first error recorded. */
+	if (seq->error == 0)
+		seq->error = err;
+
+	return seq->error;
+}
+
+/*
+ * Higher-level retrieval code below. These functions use the
+ * papr_rtas_blob_* and sequence_* APIs defined above to create fd-based
+ * handles for consumption by user space.
+ */
+
+/**
+ * papr_rtas_run_sequence() - Run a single retrieval sequence.
+ * @seq:	Functions of the caller to complete the sequence
+ * @param:	Inputs to the caller's RTAS call
+ *
+ * Context: May sleep. Holds a mutex and an RTAS work area for its
+ *          duration. Typically performs multiple sleepable slab
+ *          allocations.
+ *
+ * Return: A populated &struct papr_rtas_blob on success. Encoded error
+ * pointer otherwise.
+ */
+static const struct papr_rtas_blob *papr_rtas_run_sequence(struct papr_rtas_sequence *seq,
+							void *param)
+{
+	const struct papr_rtas_blob *blob;
+
+	if (seq->begin)
+		seq->begin(seq, param);
+
+	blob = papr_rtas_blob_generate(seq);
+	if (!blob)
+		papr_rtas_sequence_set_err(seq, -ENOMEM);
+
+	if (seq->end)
+		seq->end(seq);
+
+
+	if (seq->error) {
+		papr_rtas_blob_free(blob);
+		return ERR_PTR(seq->error);
+	}
+
+	return blob;
+}
+
+/**
+ * papr_rtas_retrieve() - Return the data blob that is exposed to
+ * user space.
+ * @seq: RTAS call specific functions to be invoked until the
+ *       sequence is completed.
+ * @param: RTAS calls specific parameters.
+ *
+ * Run sequences against @param until a blob is successfully
+ * instantiated, or a hard error is encountered, or a fatal signal is
+ * pending.
+ *
+ * Context: May sleep.
+ * Return: A fully populated data blob when successful. Encoded error
+ * pointer otherwise.
+ */
+const struct papr_rtas_blob *papr_rtas_retrieve(struct papr_rtas_sequence *seq,
+						void *param)
+{
+	const struct papr_rtas_blob *blob;
+
+	/*
+	 * EAGAIN means the sequence returns error with a -4 (data
+	 * changed and need to start the sequence) status from RTAS calls
+	 * and we should attempt a new sequence. PAPR+ (v2.13 R1–7.3.20–5
+	 * - ibm,get-vpd, R1–7.3.17–6 - ibm,get-indices) indicates that
+	 * this should be a transient condition, not something that
+	 * happens continuously. But we'll stop trying on a fatal signal.
+	 */
+	do {
+		blob = papr_rtas_run_sequence(seq, param);
+		if (!IS_ERR(blob)) /* Success. */
+			break;
+		if (PTR_ERR(blob) != -EAGAIN) /* Hard error. */
+			break;
+		cond_resched();
+	} while (!fatal_signal_pending(current));
+
+	return blob;
+}
+
+/**
+ * papr_rtas_setup_file_interface - Complete the sequence and obtain
+ * the data and export to user space with fd-based handles. Then the
+ * user spave gets the data with read() handle.
+ * @seq: RTAS call specific functions to get the data.
+ * @param: RTAS call specific input parameters.
+ * @fops: RTAS call specific file operations such as read().
+ * @name: RTAS call specific char device node.
+ *
+ * Return: FD handle for consumption by user space
+ */
+long papr_rtas_setup_file_interface(struct papr_rtas_sequence *seq,
+				void *param,
+				const struct file_operations *fops,
+				char *name)
+{
+	const struct papr_rtas_blob *blob;
+	struct file *file;
+	long ret;
+	int fd;
+
+	blob = papr_rtas_retrieve(seq, param);
+	if (IS_ERR(blob))
+		return PTR_ERR(blob);
+
+	fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC);
+	if (fd < 0) {
+		ret = fd;
+		goto free_blob;
+	}
+
+	file = anon_inode_getfile(name, fops, (void *)blob, O_RDONLY);
+	if (IS_ERR(file)) {
+		ret = PTR_ERR(file);
+		goto put_fd;
+	}
+
+	file->f_mode |= FMODE_LSEEK | FMODE_PREAD;
+	fd_install(fd, file);
+	return fd;
+
+put_fd:
+	put_unused_fd(fd);
+free_blob:
+	papr_rtas_blob_free(blob);
+	return ret;
+}
diff --git a/arch/powerpc/platforms/pseries/papr-rtas-common.h b/arch/powerpc/platforms/pseries/papr-rtas-common.h
new file mode 100644
index 000000000000..54e9686da04b
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/papr-rtas-common.h
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _ASM_POWERPC_PAPR_RTAS_COMMON_H
+#define _ASM_POWERPC_PAPR_RTAS_COMMON_H
+
+#include <linux/types.h>
+
+/*
+ * Internal "blob" APIs for accumulating RTAS call results into
+ * an immutable buffer to be attached to a file descriptor.
+ */
+struct papr_rtas_blob {
+	const char *data;
+	size_t len;
+};
+
+/**
+ * struct papr_sequence - State for managing a sequence of RTAS calls.
+ * @error:  Shall be zero as long as the sequence has not encountered an error,
+ *          -ve errno otherwise. Use papr_rtas_sequence_set_err() to update.
+ * @params: Parameter block to pass to rtas_*() calls.
+ * @begin: Work area allocation and initialize the needed parameter
+ *         values passed to RTAS call
+ * @end: Free the allocated work area
+ * @work: Obtain data with RTAS call and invoke it until the sequence is
+ *        completed.
+ *
+ */
+struct papr_rtas_sequence {
+	int error;
+	void *params;
+	void (*begin)(struct papr_rtas_sequence *seq, void *param);
+	void (*end)(struct papr_rtas_sequence *seq);
+	const char *(*work)(struct papr_rtas_sequence *seq, size_t *len);
+};
+
+extern bool papr_rtas_blob_has_data(const struct papr_rtas_blob *blob);
+extern void papr_rtas_blob_free(const struct papr_rtas_blob *blob);
+extern int papr_rtas_sequence_set_err(struct papr_rtas_sequence *seq,
+		int err);
+extern const struct papr_rtas_blob *papr_rtas_retrieve(struct papr_rtas_sequence *seq, void *param);
+extern long papr_rtas_setup_file_interface(struct papr_rtas_sequence *seq,
+		void *param, const struct file_operations *fops, char *name);
+
+#endif /* _ASM_POWERPC_PAPR_RTAS_COMMON_H */
+
diff --git a/arch/powerpc/platforms/pseries/papr-vpd.c b/arch/powerpc/platforms/pseries/papr-vpd.c
index 1574176e3ffc..89b9ab64bc2f 100644
--- a/arch/powerpc/platforms/pseries/papr-vpd.c
+++ b/arch/powerpc/platforms/pseries/papr-vpd.c
@@ -2,7 +2,6 @@
 
 #define pr_fmt(fmt) "papr-vpd: " fmt
 
-#include <linux/anon_inodes.h>
 #include <linux/build_bug.h>
 #include <linux/file.h>
 #include <linux/fs.h>
@@ -20,6 +19,7 @@
 #include <asm/rtas-work-area.h>
 #include <asm/rtas.h>
 #include <uapi/asm/papr-vpd.h>
+#include "papr-rtas-common.h"
 
 /*
  * Function-specific return values for ibm,get-vpd, derived from PAPR+
@@ -93,6 +93,7 @@ static int rtas_ibm_get_vpd(struct rtas_ibm_get_vpd_params *params)
 		break;
 	case RTAS_IBM_GET_VPD_START_OVER:
 		ret = -EAGAIN;
+		pr_info_ratelimited("VPD changed during retrieval, retrying\n");
 		break;
 	case RTAS_IBM_GET_VPD_MORE_DATA:
 		params->sequence = rets[0];
@@ -118,91 +119,6 @@ static int rtas_ibm_get_vpd(struct rtas_ibm_get_vpd_params *params)
 	return ret;
 }
 
-/*
- * Internal VPD "blob" APIs for accumulating ibm,get-vpd results into
- * an immutable buffer to be attached to a file descriptor.
- */
-struct vpd_blob {
-	const char *data;
-	size_t len;
-};
-
-static bool vpd_blob_has_data(const struct vpd_blob *blob)
-{
-	return blob->data && blob->len;
-}
-
-static void vpd_blob_free(const struct vpd_blob *blob)
-{
-	if (blob) {
-		kvfree(blob->data);
-		kfree(blob);
-	}
-}
-
-/**
- * vpd_blob_extend() - Append data to a &struct vpd_blob.
- * @blob: The blob to extend.
- * @data: The new data to append to @blob.
- * @len:  The length of @data.
- *
- * Context: May sleep.
- * Return: -ENOMEM on allocation failure, 0 otherwise.
- */
-static int vpd_blob_extend(struct vpd_blob *blob, const char *data, size_t len)
-{
-	const size_t new_len = blob->len + len;
-	const size_t old_len = blob->len;
-	const char *old_ptr = blob->data;
-	char *new_ptr;
-
-	new_ptr = kvrealloc(old_ptr, new_len, GFP_KERNEL_ACCOUNT);
-	if (!new_ptr)
-		return -ENOMEM;
-
-	memcpy(&new_ptr[old_len], data, len);
-	blob->data = new_ptr;
-	blob->len = new_len;
-	return 0;
-}
-
-/**
- * vpd_blob_generate() - Construct a new &struct vpd_blob.
- * @generator: Function that supplies the blob data.
- * @arg:       Context pointer supplied by caller, passed to @generator.
- *
- * The @generator callback is invoked until it returns NULL. @arg is
- * passed to @generator in its first argument on each call. When
- * @generator returns data, it should store the data length in its
- * second argument.
- *
- * Context: May sleep.
- * Return: A completely populated &struct vpd_blob, or NULL on error.
- */
-static const struct vpd_blob *
-vpd_blob_generate(const char * (*generator)(void *, size_t *), void *arg)
-{
-	struct vpd_blob *blob;
-	const char *buf;
-	size_t len;
-	int err = 0;
-
-	blob  = kzalloc(sizeof(*blob), GFP_KERNEL_ACCOUNT);
-	if (!blob)
-		return NULL;
-
-	while (err == 0 && (buf = generator(arg, &len)))
-		err = vpd_blob_extend(blob, buf, len);
-
-	if (err != 0 || !vpd_blob_has_data(blob))
-		goto free_blob;
-
-	return blob;
-free_blob:
-	vpd_blob_free(blob);
-	return NULL;
-}
-
 /*
  * Internal VPD sequence APIs. A VPD sequence is a series of calls to
  * ibm,get-vpd for a given location code. The sequence ends when an
@@ -210,21 +126,10 @@ vpd_blob_generate(const char * (*generator)(void *, size_t *), void *arg)
  * returned.
  */
 
-/**
- * struct vpd_sequence - State for managing a VPD sequence.
- * @error:  Shall be zero as long as the sequence has not encountered an error,
- *          -ve errno otherwise. Use vpd_sequence_set_err() to update this.
- * @params: Parameter block to pass to rtas_ibm_get_vpd().
- */
-struct vpd_sequence {
-	int error;
-	struct rtas_ibm_get_vpd_params params;
-};
-
 /**
  * vpd_sequence_begin() - Begin a VPD retrieval sequence.
- * @seq:      Uninitialized sequence state.
- * @loc_code: Location code that defines the scope of the VPD to return.
+ * @seq:   Uninitialized sequence state.
+ * @param: Location code that defines the scope of the VPD to return.
  *
  * Initializes @seq with the resources necessary to carry out a VPD
  * sequence. Callers must pass @seq to vpd_sequence_end() regardless
@@ -232,9 +137,9 @@ struct vpd_sequence {
  *
  * Context: May sleep.
  */
-static void vpd_sequence_begin(struct vpd_sequence *seq,
-			       const struct papr_location_code *loc_code)
+static void vpd_sequence_begin(struct papr_rtas_sequence *seq, void *param)
 {
+	struct rtas_ibm_get_vpd_params *vpd_params;
 	/*
 	 * Use a static data structure for the location code passed to
 	 * RTAS to ensure it's in the RMA and avoid a separate work
@@ -249,14 +154,12 @@ static void vpd_sequence_begin(struct vpd_sequence *seq,
 	 * allocate the work area under the lock.
 	 */
 	mutex_lock(&rtas_ibm_get_vpd_lock);
-	static_loc_code = *loc_code;
-	*seq = (struct vpd_sequence) {
-		.params = {
-			.work_area = rtas_work_area_alloc(SZ_4K),
-			.loc_code = &static_loc_code,
-			.sequence = 1,
-		},
-	};
+	static_loc_code = *(struct papr_location_code *)param;
+	vpd_params =  (struct rtas_ibm_get_vpd_params *)seq->params;
+	vpd_params->work_area = rtas_work_area_alloc(SZ_4K);
+	vpd_params->loc_code = &static_loc_code;
+	vpd_params->sequence = 1;
+	vpd_params->status = 0;
 }
 
 /**
@@ -265,9 +168,12 @@ static void vpd_sequence_begin(struct vpd_sequence *seq,
  *
  * Releases resources obtained by vpd_sequence_begin().
  */
-static void vpd_sequence_end(struct vpd_sequence *seq)
+static void vpd_sequence_end(struct papr_rtas_sequence *seq)
 {
-	rtas_work_area_free(seq->params.work_area);
+	struct rtas_ibm_get_vpd_params *vpd_params;
+
+	vpd_params =  (struct rtas_ibm_get_vpd_params *)seq->params;
+	rtas_work_area_free(vpd_params->work_area);
 	mutex_unlock(&rtas_ibm_get_vpd_lock);
 }
 
@@ -283,16 +189,19 @@ static void vpd_sequence_end(struct vpd_sequence *seq)
  * Return: True if the sequence has encountered an error or if all VPD for
  *         this sequence has been retrieved. False otherwise.
  */
-static bool vpd_sequence_should_stop(const struct vpd_sequence *seq)
+static bool vpd_sequence_should_stop(const struct papr_rtas_sequence *seq)
 {
+	struct rtas_ibm_get_vpd_params *vpd_params;
 	bool done;
 
 	if (seq->error)
 		return true;
 
-	switch (seq->params.status) {
+	vpd_params =  (struct rtas_ibm_get_vpd_params *)seq->params;
+
+	switch (vpd_params->status) {
 	case 0:
-		if (seq->params.written == 0)
+		if (vpd_params->written == 0)
 			done = false; /* Initial state. */
 		else
 			done = true; /* All data consumed. */
@@ -308,26 +217,19 @@ static bool vpd_sequence_should_stop(const struct vpd_sequence *seq)
 	return done;
 }
 
-static int vpd_sequence_set_err(struct vpd_sequence *seq, int err)
-{
-	/* Preserve the first error recorded. */
-	if (seq->error == 0)
-		seq->error = err;
-
-	return seq->error;
-}
-
 /*
- * Generator function to be passed to vpd_blob_generate().
+ * Generator function to be passed to papr_rtas_blob_generate().
  */
-static const char *vpd_sequence_fill_work_area(void *arg, size_t *len)
+static const char *vpd_sequence_fill_work_area(struct papr_rtas_sequence *seq,
+						size_t *len)
 {
-	struct vpd_sequence *seq = arg;
-	struct rtas_ibm_get_vpd_params *p = &seq->params;
+	struct rtas_ibm_get_vpd_params *p;
+
+	p = (struct rtas_ibm_get_vpd_params *)seq->params;
 
 	if (vpd_sequence_should_stop(seq))
 		return NULL;
-	if (vpd_sequence_set_err(seq, rtas_ibm_get_vpd(p)))
+	if (papr_rtas_sequence_set_err(seq, rtas_ibm_get_vpd(p)))
 		return NULL;
 	*len = p->written;
 	return rtas_work_area_raw_buf(p->work_area);
@@ -335,82 +237,16 @@ static const char *vpd_sequence_fill_work_area(void *arg, size_t *len)
 
 /*
  * Higher-level VPD retrieval code below. These functions use the
- * vpd_blob_* and vpd_sequence_* APIs defined above to create fd-based
+ * papr_rtas_blob_* and vpd_sequence_* APIs defined above to create fd-based
  * VPD handles for consumption by user space.
  */
 
-/**
- * papr_vpd_run_sequence() - Run a single VPD retrieval sequence.
- * @loc_code: Location code that defines the scope of VPD to return.
- *
- * Context: May sleep. Holds a mutex and an RTAS work area for its
- *          duration. Typically performs multiple sleepable slab
- *          allocations.
- *
- * Return: A populated &struct vpd_blob on success. Encoded error
- * pointer otherwise.
- */
-static const struct vpd_blob *papr_vpd_run_sequence(const struct papr_location_code *loc_code)
-{
-	const struct vpd_blob *blob;
-	struct vpd_sequence seq;
-
-	vpd_sequence_begin(&seq, loc_code);
-	blob = vpd_blob_generate(vpd_sequence_fill_work_area, &seq);
-	if (!blob)
-		vpd_sequence_set_err(&seq, -ENOMEM);
-	vpd_sequence_end(&seq);
-
-	if (seq.error) {
-		vpd_blob_free(blob);
-		return ERR_PTR(seq.error);
-	}
-
-	return blob;
-}
-
-/**
- * papr_vpd_retrieve() - Return the VPD for a location code.
- * @loc_code: Location code that defines the scope of VPD to return.
- *
- * Run VPD sequences against @loc_code until a blob is successfully
- * instantiated, or a hard error is encountered, or a fatal signal is
- * pending.
- *
- * Context: May sleep.
- * Return: A fully populated VPD blob when successful. Encoded error
- * pointer otherwise.
- */
-static const struct vpd_blob *papr_vpd_retrieve(const struct papr_location_code *loc_code)
-{
-	const struct vpd_blob *blob;
-
-	/*
-	 * EAGAIN means the sequence errored with a -4 (VPD changed)
-	 * status from ibm,get-vpd, and we should attempt a new
-	 * sequence. PAPR+ v2.13 R1–7.3.20–5 indicates that this
-	 * should be a transient condition, not something that happens
-	 * continuously. But we'll stop trying on a fatal signal.
-	 */
-	do {
-		blob = papr_vpd_run_sequence(loc_code);
-		if (!IS_ERR(blob)) /* Success. */
-			break;
-		if (PTR_ERR(blob) != -EAGAIN) /* Hard error. */
-			break;
-		pr_info_ratelimited("VPD changed during retrieval, retrying\n");
-		cond_resched();
-	} while (!fatal_signal_pending(current));
-
-	return blob;
-}
-
 static ssize_t papr_vpd_handle_read(struct file *file, char __user *buf, size_t size, loff_t *off)
 {
-	const struct vpd_blob *blob = file->private_data;
+	const struct papr_rtas_blob *blob = file->private_data;
 
 	/* bug: we should not instantiate a handle without any data attached. */
-	if (!vpd_blob_has_data(blob)) {
+	if (!papr_rtas_blob_has_data(blob)) {
 		pr_err_once("handle without data\n");
 		return -EIO;
 	}
@@ -420,16 +256,16 @@ static ssize_t papr_vpd_handle_read(struct file *file, char __user *buf, size_t
 
 static int papr_vpd_handle_release(struct inode *inode, struct file *file)
 {
-	const struct vpd_blob *blob = file->private_data;
+	const struct papr_rtas_blob *blob = file->private_data;
 
-	vpd_blob_free(blob);
+	papr_rtas_blob_free(blob);
 
 	return 0;
 }
 
 static loff_t papr_vpd_handle_seek(struct file *file, loff_t off, int whence)
 {
-	const struct vpd_blob *blob = file->private_data;
+	const struct papr_rtas_blob *blob = file->private_data;
 
 	return fixed_size_llseek(file, off, whence, blob->len);
 }
@@ -460,10 +296,9 @@ static const struct file_operations papr_vpd_handle_ops = {
  */
 static long papr_vpd_create_handle(struct papr_location_code __user *ulc)
 {
+	struct rtas_ibm_get_vpd_params vpd_params = {};
+	struct papr_rtas_sequence seq = {};
 	struct papr_location_code klc;
-	const struct vpd_blob *blob;
-	struct file *file;
-	long err;
 	int fd;
 
 	if (copy_from_user(&klc, ulc, sizeof(klc)))
@@ -472,31 +307,18 @@ static long papr_vpd_create_handle(struct papr_location_code __user *ulc)
 	if (!string_is_terminated(klc.str, ARRAY_SIZE(klc.str)))
 		return -EINVAL;
 
-	blob = papr_vpd_retrieve(&klc);
-	if (IS_ERR(blob))
-		return PTR_ERR(blob);
+	seq = (struct papr_rtas_sequence) {
+		.begin = vpd_sequence_begin,
+		.end = vpd_sequence_end,
+		.work = vpd_sequence_fill_work_area,
+	};
 
-	fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC);
-	if (fd < 0) {
-		err = fd;
-		goto free_blob;
-	}
+	seq.params = (void *)&vpd_params;
 
-	file = anon_inode_getfile("[papr-vpd]", &papr_vpd_handle_ops,
-				  (void *)blob, O_RDONLY);
-	if (IS_ERR(file)) {
-		err = PTR_ERR(file);
-		goto put_fd;
-	}
+	fd = papr_rtas_setup_file_interface(&seq, &klc,
+			&papr_vpd_handle_ops, "[papr-vpd]");
 
-	file->f_mode |= FMODE_LSEEK | FMODE_PREAD;
-	fd_install(fd, file);
 	return fd;
-put_fd:
-	put_unused_fd(fd);
-free_blob:
-	vpd_blob_free(blob);
-	return err;
 }
 
 /*
-- 
2.43.5

[PATCH v2 2/6] powerpc/pseries: Define papr_indices_io_block for papr-indices ioctls

[Haren Myneni]

To issue ibm,get-indices, ibm,set-dynamic-indicator and
ibm,get-dynamic-sensor-state in the user space, the RMO buffer is
allocated for the work area which is restricted under system
lockdown. So instead of user space execution, the kernel will
provide /dev/papr-indices interface to execute these RTAS calls.

The user space assigns data in papr_indices_io_block struct
depends on the specific HCALL and passes to the following ioctls:

PAPR_INDICES_IOC_GET:		Use for ibm,get-indices. Returns a
				get-indices handle fd to read data.
PAPR_DYNAMIC_SENSOR_IOC_GET:	Use for  ibm,get-dynamic-sensor-state.
				Updates the sensor state in
				papr_indices_io_block.dynamic_param.state

PAPR_DYNAMIC_INDICATOR_IOC_SET:	Use for ibm,set-dynamic-indicator.
				Sets the new state for the input
				indicator.

Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/include/uapi/asm/papr-indices.h | 41 ++++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 arch/powerpc/include/uapi/asm/papr-indices.h

diff --git a/arch/powerpc/include/uapi/asm/papr-indices.h b/arch/powerpc/include/uapi/asm/papr-indices.h
new file mode 100644
index 000000000000..c580025fe201
--- /dev/null
+++ b/arch/powerpc/include/uapi/asm/papr-indices.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_PAPR_INDICES_H_
+#define _UAPI_PAPR_INDICES_H_
+
+#include <linux/types.h>
+#include <asm/ioctl.h>
+#include <asm/papr-miscdev.h>
+
+#define LOC_CODE_SIZE			80
+#define RTAS_GET_INDICES_BUF_SIZE	SZ_4K
+
+struct papr_indices_io_block {
+	union {
+		struct {
+			__u8 is_sensor; /* 0 for indicator and 1 for sensor */
+			__u32 indice_type;
+		} indices;
+		struct {
+			__u32 token; /* Sensor or indicator token */
+			__u32 state; /* get / set state */
+			/*
+			 * PAPR+ 12.3.2.4 Converged Location Code Rules - Length
+			 * Restrictions. 79 characters plus null.
+			 */
+			char location_code_str[LOC_CODE_SIZE]; /* location code */
+		} dynamic_param;
+	};
+};
+
+/*
+ * ioctls for /dev/papr-indices.
+ * PAPR_INDICES_IOC_GET:	Returns a get-indices handle fd to read data
+ * PAPR_DYNAMIC_SENSOR_IOC_GET:	Gets the state of the input sensor
+ * PAPR_DYNAMIC_INDICATOR_IOC_SET: Sets the new state for the input indicator
+ */
+#define PAPR_INDICES_IOC_GET		_IOW(PAPR_MISCDEV_IOC_ID, 3, struct papr_indices_io_block)
+#define PAPR_DYNAMIC_SENSOR_IOC_GET	_IOWR(PAPR_MISCDEV_IOC_ID, 4, struct papr_indices_io_block)
+#define PAPR_DYNAMIC_INDICATOR_IOC_SET	_IOW(PAPR_MISCDEV_IOC_ID, 5, struct papr_indices_io_block)
+
+
+#endif /* _UAPI_PAPR_INDICES_H_ */
-- 
2.43.5

[PATCH v2 3/6] powerpc/pseries: Add papr-indices char driver for ibm,get-indices

[Haren Myneni]

The RTAS call ibm,get-indices is used to obtain indices and
location codes for a specified indicator or sensor token. The
current implementation uses rtas_get_indices() API provided by
librtas library which allocates RMO buffer and issue this RTAS
call in the user space. But writable mapping /dev/mem access by
the user space is prohibited under system lockdown.

To overcome the restricted access in the user space, the kernel
provide interfaces to collect indices data from the hypervisor.
This patch adds papr-indices character driver and expose standard
interfaces such as open / ioctl/ read to user space in ways that
are compatible with lockdown.

PAPR (2.13 7.3.17 ibm,get-indices RTAS Call) describes the
following steps to retrieve all indices data:
- User input parameters to the RTAS call: sensor or indicator,
  and indice type
- ibm,get-indices is sequence RTAS call which means has to issue
  multiple times to get the entire list of indicators or sensors
  of a particular type. The hypervisor expects the first RTAS call
  with the sequence 1 and the subsequent calls with the sequence
  number returned from the previous calls.
- The OS may not interleave calls to ibm,get-indices for different
  indicator or sensor types. Means other RTAS calls with different
  type should not be issued while the previous type sequence is in
  progress. So collect the entire list of indices and copied to
  buffer BLOB during ioctl() and expose this buffer to the user
  space with the file descriptor.
- The hypervisor fills the work area with a specific format but
  does not return the number of bytes written to the buffer.
  Instead of parsing the data for each call to determine the data
  length, copy the work area size (RTAS_GET_INDICES_BUF_SIZE) to
  the buffer. Return work-area size of data to the user space for
  each read() call.

Expose these interfaces to user space with a /dev/papr-indices
character device using the following programming model:

 int devfd = open("/dev/papr-indices", O_RDONLY);
 int fd = ioctl(devfd, PAPR_INDICES_IOC_GET,
		struct papr_indices_io_block)
  - Collect all indices data for the specified token to the buffer
 char *buf = malloc(RTAS_GET_INDICES_BUF_SIZE);
 length = read(fd, buf,  RTAS_GET_INDICES_BUF_SIZE)
  - RTAS_GET_INDICES_BUF_SIZE of data is returned to the user
    space.
  - The user space retrieves the indices and their location codes
    from the buffer
  - Should issue multiple read() calls until reaches the end of
    BLOB buffer.

The read() should use the file descriptor obtained from ioctl to
get the data that is exposed to file descriptor. Implemented
support in librtas (rtas_get_indices()) for this new ABI for
system lockdown.

Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/include/asm/rtas.h               |   1 +
 arch/powerpc/kernel/rtas.c                    |   2 +-
 arch/powerpc/platforms/pseries/Makefile       |   2 +-
 arch/powerpc/platforms/pseries/papr-indices.c | 369 ++++++++++++++++++
 4 files changed, 372 insertions(+), 2 deletions(-)
 create mode 100644 arch/powerpc/platforms/pseries/papr-indices.c

diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index 04406162fc5a..7dc527a5aaac 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -515,6 +515,7 @@ extern char rtas_data_buf[RTAS_DATA_BUF_SIZE];
 extern unsigned long rtas_rmo_buf;
 
 extern struct mutex rtas_ibm_get_vpd_lock;
+extern struct mutex rtas_ibm_get_indices_lock;
 
 #define GLOBAL_INTERRUPT_QUEUE 9005
 
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index d31c9799cab2..76c634b92cb2 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -93,11 +93,11 @@ struct rtas_function {
  */
 static DEFINE_MUTEX(rtas_ibm_activate_firmware_lock);
 static DEFINE_MUTEX(rtas_ibm_get_dynamic_sensor_state_lock);
-static DEFINE_MUTEX(rtas_ibm_get_indices_lock);
 static DEFINE_MUTEX(rtas_ibm_lpar_perftools_lock);
 static DEFINE_MUTEX(rtas_ibm_physical_attestation_lock);
 static DEFINE_MUTEX(rtas_ibm_set_dynamic_indicator_lock);
 DEFINE_MUTEX(rtas_ibm_get_vpd_lock);
+DEFINE_MUTEX(rtas_ibm_get_indices_lock);
 
 static struct rtas_function rtas_function_table[] __ro_after_init = {
 	[RTAS_FNIDX__CHECK_EXCEPTION] = {
diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
index 697c216b70dc..e1db61877bb9 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -3,7 +3,7 @@ ccflags-$(CONFIG_PPC_PSERIES_DEBUG)	+= -DDEBUG
 
 obj-y			:= lpar.o hvCall.o nvram.o reconfig.o \
 			   of_helpers.o rtas-work-area.o papr-sysparm.o \
-			   papr-rtas-common.o papr-vpd.o \
+			   papr-rtas-common.o papr-vpd.o papr-indices.o \
 			   setup.o iommu.o event_sources.o ras.o \
 			   firmware.o power.o dlpar.o mobility.o rng.o \
 			   pci.o pci_dlpar.o eeh_pseries.o msi.o \
diff --git a/arch/powerpc/platforms/pseries/papr-indices.c b/arch/powerpc/platforms/pseries/papr-indices.c
new file mode 100644
index 000000000000..c5cd63dae609
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/papr-indices.c
@@ -0,0 +1,369 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#define pr_fmt(fmt) "papr-indices: " fmt
+
+#include <linux/build_bug.h>
+#include <linux/file.h>
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/lockdep.h>
+#include <linux/kernel.h>
+#include <linux/miscdevice.h>
+#include <linux/signal.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/string_helpers.h>
+#include <linux/uaccess.h>
+#include <asm/machdep.h>
+#include <asm/rtas-work-area.h>
+#include <asm/rtas.h>
+#include <uapi/asm/papr-indices.h>
+#include "papr-rtas-common.h"
+/*
+ * Function-specific return values for ibm,get-indices, derived from PAPR+
+ * v2.13 7.3.20 "ibm,get-indices RTAS Call".
+ */
+#define RTAS_IBM_GET_INDICES_COMPLETE    0 /* indices data has been retrieved. */
+#define RTAS_IBM_GET_INDICES_MORE_DATA   1 /* More data is available. */
+#define RTAS_IBM_GET_INDICES_START_OVER -4 /* Indices list changed, restart call sequence. */
+
+/**
+ * struct rtas_get_indices_params - Parameters (in and out) for
+ *                                      ibm,get-indices.
+ * @is_sensor:	In: Caller-provided whether sensor or indicator.
+ * @indice_type:In: Caller-provided indice (sensor or indicator) token
+ * @work_area:	In: Caller-provided work area buffer for results.
+ * @next:	In: Sequence number. Out: Next sequence number.
+ * @status:	Out: RTAS call status.
+ */
+struct rtas_get_indices_params {
+	u8 is_sensor;
+	u32 indice_type;
+	struct rtas_work_area *work_area;
+	u32 next;
+	s32 status;
+};
+
+/*
+ * rtas_ibm_get_indices() - Call ibm,get-indices to fill a work area buffer.
+ * @params: See &struct rtas_ibm_get_indices_params.
+ *
+ * Calls ibm,get-indices until it errors or successfully deposits data
+ * into the supplied work area. Handles RTAS retry statuses. Maps RTAS
+ * error statuses to reasonable errno values.
+ *
+ * The caller is expected to invoke rtas_ibm_get_indices() multiple times
+ * to retrieve all indices data for the provided indice type. Only one
+ * sequence should be in progress at any time; starting a new sequence
+ * will disrupt any sequence already in progress. Serialization of
+ * indices retrieval sequences is the responsibility of the caller.
+ *
+ * The caller should inspect @params.status to determine whether more
+ * calls are needed to complete the sequence.
+ *
+ * Context: May sleep.
+ * Return: -ve on error, 0 otherwise.
+ */
+static int rtas_ibm_get_indices(struct rtas_get_indices_params *params)
+{
+	struct rtas_work_area *work_area = params->work_area;
+	const s32 token = rtas_function_token(RTAS_FN_IBM_GET_INDICES);
+	u32 rets;
+	s32 fwrc;
+	int ret;
+
+	if (token == RTAS_UNKNOWN_SERVICE)
+		return -ENOENT;
+
+	lockdep_assert_held(&rtas_ibm_get_indices_lock);
+
+	do {
+		fwrc = rtas_call(token, 5, 2, &rets, params->is_sensor,
+				params->indice_type,
+				rtas_work_area_phys(work_area),
+				rtas_work_area_size(work_area),
+				params->next);
+	} while (rtas_busy_delay(fwrc));
+
+	switch (fwrc) {
+	case RTAS_HARDWARE_ERROR:
+		ret = -EIO;
+		break;
+	case RTAS_INVALID_PARAMETER: /* Indicator type is not supported */
+		ret = -EINVAL;
+		break;
+	case RTAS_IBM_GET_INDICES_START_OVER:
+		ret = -EAGAIN;
+		pr_info_ratelimited("Indices changed during retrieval, retrying\n");
+		params->next = 1;
+		break;
+	case RTAS_IBM_GET_INDICES_MORE_DATA:
+		params->next = rets;
+		fallthrough;
+	case RTAS_IBM_GET_INDICES_COMPLETE:
+		params->next = 0;
+		ret = 0;
+		break;
+	default:
+		ret = -EIO;
+		pr_err_ratelimited("unexpected ibm,get-indices status %d\n", fwrc);
+		break;
+	}
+
+	params->status = fwrc;
+	return ret;
+}
+
+/*
+ * Internal indices sequence APIs. A sequence is a series of calls to
+ * ibm,get-indices for a given location code. The sequence ends when
+ * an error is encountered or all indices for the input has been
+ * returned.
+ */
+
+/**
+ * indices_sequence_begin() - Begin a indices retrieval sequence.
+ * @seq:      Uninitialized sequence state.
+ * @params: Indices call parameters and initialization
+ *
+ * Initializes @seq with the resources necessary to carry out indices
+ * call sequence. Callers must pass @seq to indices_sequence_end()
+ * regardless of whether the sequence succeeds.
+ *
+ * Context: May sleep.
+ */
+static void indices_sequence_begin(struct papr_rtas_sequence *seq,
+				void *params)
+{
+	struct rtas_get_indices_params  *param;
+
+	param = (struct rtas_get_indices_params *)params;
+	/*
+	 * We could allocate the work area before acquiring the
+	 * function lock, but that would allow concurrent requests to
+	 * exhaust the limited work area pool for no benefit. So
+	 * allocate the work area under the lock.
+	 */
+	mutex_lock(&rtas_ibm_get_indices_lock);
+	param->work_area = rtas_work_area_alloc(RTAS_GET_INDICES_BUF_SIZE);
+	param->next = 1;
+	param->status = 0;
+	seq->params = param;
+}
+
+/**
+ * indices_sequence_end() - Finalize a indices retrieval sequence.
+ * @seq: indices call parameters from sequence struct
+ *
+ * Releases resources obtained by indices_sequence_begin().
+ */
+static void indices_sequence_end(struct papr_rtas_sequence *seq)
+{
+	struct rtas_get_indices_params *param;
+
+	param =  (struct rtas_get_indices_params *)seq->params;
+	rtas_work_area_free(param->work_area);
+	mutex_unlock(&rtas_ibm_get_indices_lock);
+}
+
+/**
+ * indices_sequence_should_stop() - Determine whether a indices
+ *                                  retrieval sequence should continue.
+ * @seq: indices sequence state.
+ *
+ * Examines the sequence error state and outputs of the last call to
+ * ibm,get-indices to determine whether the sequence in progress should
+ * continue or stop.
+ *
+ * Return: True if the sequence has encountered an error or if all
+ *         indices for this sequence has been retrieved. False otherwise.
+ */
+static bool indices_sequence_should_stop(const struct papr_rtas_sequence *seq)
+{
+	struct rtas_get_indices_params *param;
+	bool done;
+
+	if (seq->error)
+		return true;
+
+	param =  (struct rtas_get_indices_params *)seq->params;
+
+	switch (param->status) {
+	case RTAS_IBM_GET_INDICES_COMPLETE:
+		if (param->next == 1)
+			done = false; /* Initial state. */
+		else
+			done = true; /* All data consumed. */
+		break;
+	case RTAS_IBM_GET_INDICES_MORE_DATA:
+		done = false; /* More data available. */
+		break;
+	default:
+		done = true; /* Error encountered. */
+		break;
+	}
+
+	return done;
+}
+
+/*
+ * Work function to be passed to papr_rtas_blob_generate().
+ *
+ * ibm,get-indices RTAS call fills the work area with the certain
+ * format but does not return the bytes written in the buffer. So
+ * instead of kernel parsing this work area to determine the buffer
+ * length, copy the complete work area (RTAS_GET_INDICES_BUF_SIZE)
+ * to the blob and let the user space to obtain the data.
+ * Means RTAS_GET_INDICES_BUF_SIZE data will be returned for each
+ * read().
+ */
+
+static const char *indices_sequence_fill_work_area(struct papr_rtas_sequence *seq,
+						size_t *len)
+{
+	struct rtas_get_indices_params *p;
+
+	p = (struct rtas_get_indices_params *)seq->params;
+
+	if (indices_sequence_should_stop(seq))
+		return NULL;
+	if (papr_rtas_sequence_set_err(seq, rtas_ibm_get_indices(p)))
+		return NULL;
+	*len = RTAS_GET_INDICES_BUF_SIZE;
+	return rtas_work_area_raw_buf(p->work_area);
+}
+
+/*
+ * papr_indices_handle_read - returns indices blob data to the user space
+ *
+ * ibm,get-indices RTAS call fills the work area with the certian
+ * format but does not return the bytes written in the buffer and
+ * copied RTAS_GET_INDICES_BUF_SIZE data to the blob for each RTAS
+ * call. So send RTAS_GET_INDICES_BUF_SIZE buffer to the user space
+ * for each read().
+ */
+static ssize_t papr_indices_handle_read(struct file *file,
+		char __user *buf, size_t size, loff_t *off)
+{
+	const struct papr_rtas_blob *blob = file->private_data;
+
+	/* we should not instantiate a handle without any data attached. */
+	if (!papr_rtas_blob_has_data(blob)) {
+		pr_err_once("handle without data\n");
+		return -EIO;
+	}
+
+	if (size < RTAS_GET_INDICES_BUF_SIZE) {
+		pr_err_once("Invalid buffer length %ld, expect %d\n",
+				size, RTAS_GET_INDICES_BUF_SIZE);
+		return -EINVAL;
+	} else if (size > RTAS_GET_INDICES_BUF_SIZE)
+		size = RTAS_GET_INDICES_BUF_SIZE;
+
+	return simple_read_from_buffer(buf, size, off, blob->data, blob->len);
+}
+
+static int papr_indices_handle_release(struct inode *inode, struct file *file)
+{
+	const struct papr_rtas_blob *blob = file->private_data;
+
+	papr_rtas_blob_free(blob);
+
+	return 0;
+}
+
+static loff_t papr_indices_handle_seek(struct file *file, loff_t off,
+					int whence)
+{
+	const struct papr_rtas_blob *blob = file->private_data;
+
+	return fixed_size_llseek(file, off, whence, blob->len);
+}
+
+static const struct file_operations papr_indices_handle_ops = {
+	.read = papr_indices_handle_read,
+	.llseek = papr_indices_handle_seek,
+	.release = papr_indices_handle_release,
+};
+
+/*
+ * papr_indices_create_handle() - Create a fd-based handle for reading
+ *                                indices data
+ * @ubuf: Input parameters to RTAS call such as whether sensor or indicator
+ *        and indice type in user memory
+ *
+ * Handler for PAPR_INDICES_IOC_GET ioctl command. Validates @ubuf
+ * and instantiates an immutable indices "blob" for it. The blob is
+ * attached to a file descriptor for reading by user space. The memory
+ * backing the blob is freed when the file is released.
+ *
+ * The entire requested indices is retrieved by this call and all
+ * necessary RTAS interactions are performed before returning the fd
+ * to user space. This keeps the read handler simple and ensures that
+ * the kernel can prevent interleaving of ibm,get-indices call sequences.
+ *
+ * Return: The installed fd number if successful, -ve errno otherwise.
+ */
+static long papr_indices_create_handle(struct papr_indices_io_block __user *ubuf)
+{
+	struct papr_rtas_sequence seq = {};
+	struct rtas_get_indices_params params = {};
+	int fd;
+
+	if (get_user(params.is_sensor, &ubuf->indices.is_sensor))
+		return -EFAULT;
+
+	if (get_user(params.indice_type, &ubuf->indices.indice_type))
+		return -EFAULT;
+
+	seq = (struct papr_rtas_sequence) {
+		.begin = indices_sequence_begin,
+		.end = indices_sequence_end,
+		.work = indices_sequence_fill_work_area,
+	};
+
+	fd = papr_rtas_setup_file_interface(&seq, &params,
+			&papr_indices_handle_ops, "[papr-indices]");
+
+	return fd;
+}
+
+/*
+ * Top-level ioctl handler for /dev/papr-indices.
+ */
+static long papr_indices_dev_ioctl(struct file *filp, unsigned int ioctl,
+				unsigned long arg)
+{
+	void __user *argp = (__force void __user *)arg;
+	long ret;
+
+	switch (ioctl) {
+	case PAPR_INDICES_IOC_GET:
+		ret = papr_indices_create_handle(argp);
+		break;
+	default:
+		ret = -ENOIOCTLCMD;
+		break;
+	}
+
+	return ret;
+}
+
+static const struct file_operations papr_indices_ops = {
+	.unlocked_ioctl = papr_indices_dev_ioctl,
+};
+
+static struct miscdevice papr_indices_dev = {
+	.minor = MISC_DYNAMIC_MINOR,
+	.name = "papr-indices",
+	.fops = &papr_indices_ops,
+};
+
+static __init int papr_indices_init(void)
+{
+	if (!rtas_function_implemented(RTAS_FN_IBM_GET_INDICES))
+		return -ENODEV;
+
+	return misc_register(&papr_indices_dev);
+}
+machine_device_initcall(pseries, papr_indices_init);
-- 
2.43.5

[PATCH v2 4/6] powerpc/pseries: Add ibm,set-dynamic-indicator RTAS call support

[Haren Myneni]

The RTAS call ibm,set-dynamic-indicator is used to set the new
indicator state identified by a location code. The current
implementation uses rtas_set_dynamic_indicator() API provided by
librtas library which allocates RMO buffer and issue this RTAS
call in the user space. But /dev/mem access by the user space
is prohibited under system lockdown.

This patch provides an interface with new ioctl
PAPR_DYNAMIC_INDICATOR_IOC_SET to the papr-indices character
driver and expose this interface to the user space that is
compatible with lockdown.

Refer PAPR 7.3.18 ibm,set-dynamic-indicator for more
information on this RTAS call.
-  User input parameters to the RTAS call: location code
   string, indicator token and new state

Expose these interfaces to user space with a /dev/papr-indices
character device using the following programming model:
 int fd = open("/dev/papr-indices", O_RDWR);
 int ret = ioctl(fd, PAPR_DYNAMIC_INDICATOR_IOC_SET,
		struct papr_indices_io_block)
  - The user space passes input parameters in papr_indices_io_block
    struct

Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/include/asm/rtas.h               |   1 +
 arch/powerpc/kernel/rtas.c                    |   2 +-
 arch/powerpc/platforms/pseries/papr-indices.c | 122 ++++++++++++++++++
 3 files changed, 124 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index 7dc527a5aaac..2da52f59e4c6 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -516,6 +516,7 @@ extern unsigned long rtas_rmo_buf;
 
 extern struct mutex rtas_ibm_get_vpd_lock;
 extern struct mutex rtas_ibm_get_indices_lock;
+extern struct mutex rtas_ibm_set_dynamic_indicator_lock;
 
 #define GLOBAL_INTERRUPT_QUEUE 9005
 
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 76c634b92cb2..88fa416730af 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -95,9 +95,9 @@ static DEFINE_MUTEX(rtas_ibm_activate_firmware_lock);
 static DEFINE_MUTEX(rtas_ibm_get_dynamic_sensor_state_lock);
 static DEFINE_MUTEX(rtas_ibm_lpar_perftools_lock);
 static DEFINE_MUTEX(rtas_ibm_physical_attestation_lock);
-static DEFINE_MUTEX(rtas_ibm_set_dynamic_indicator_lock);
 DEFINE_MUTEX(rtas_ibm_get_vpd_lock);
 DEFINE_MUTEX(rtas_ibm_get_indices_lock);
+DEFINE_MUTEX(rtas_ibm_set_dynamic_indicator_lock);
 
 static struct rtas_function rtas_function_table[] __ro_after_init = {
 	[RTAS_FNIDX__CHECK_EXCEPTION] = {
diff --git a/arch/powerpc/platforms/pseries/papr-indices.c b/arch/powerpc/platforms/pseries/papr-indices.c
index c5cd63dae609..e4b4eb8ece84 100644
--- a/arch/powerpc/platforms/pseries/papr-indices.c
+++ b/arch/powerpc/platforms/pseries/papr-indices.c
@@ -27,6 +27,15 @@
 #define RTAS_IBM_GET_INDICES_MORE_DATA   1 /* More data is available. */
 #define RTAS_IBM_GET_INDICES_START_OVER -4 /* Indices list changed, restart call sequence. */
 
+/*
+ * Function-specific return values for ibm,set-dynamic-indicator and
+ * ibm,get-dynamic-sensor-state RTAS calls, drived from PAPR+
+ * v2.13 7.3.18 and 7.3.19.
+ */
+#define RTAS_IBM_DYNAMIC_INDICE_SUCCESS		0
+#define RTAS_IBM_DYNAMIC_INDICE_HW_ERROR	-1
+#define RTAS_IBM_DYNAMIC_INDICE_NO_INDICATOR	-3
+
 /**
  * struct rtas_get_indices_params - Parameters (in and out) for
  *                                      ibm,get-indices.
@@ -328,6 +337,110 @@ static long papr_indices_create_handle(struct papr_indices_io_block __user *ubuf
 	return fd;
 }
 
+/*
+ * Create work area with the input parameters. This function is used
+ * for both ibm,set-dynamic-indicator and ibm,get-dynamic-sensor-state
+ * RTAS Calls.
+ */
+static struct rtas_work_area *
+papr_dynamic_indice_buf_from_user(struct papr_indices_io_block __user *ubuf,
+				struct papr_indices_io_block *kbuf)
+{
+	struct rtas_work_area *work_area;
+	u32 length;
+	__be32 len_be;
+
+	if (copy_from_user(kbuf, ubuf, sizeof(*kbuf)))
+		return ERR_PTR(-EFAULT);
+
+
+	if (!string_is_terminated(kbuf->dynamic_param.location_code_str,
+			ARRAY_SIZE(kbuf->dynamic_param.location_code_str)))
+		return ERR_PTR(-EINVAL);
+
+	/*
+	 * The input data in the work area should be as follows:
+	 * - 32-bit integer length of the location code string,
+	 *   including NULL.
+	 * - Location code string, NULL terminated, identifying the
+	 *   token (sensor or indicator).
+	 * PAPR 2.13 - R1–7.3.18–5 ibm,set-dynamic-indicator
+	 *           - R1–7.3.19–5 ibm,get-dynamic-sensor-state
+	 */
+	/*
+	 * Length that user space passed should also include NULL
+	 * terminator.
+	 */
+	length = strlen(kbuf->dynamic_param.location_code_str) + 1;
+	if (length > LOC_CODE_SIZE)
+		return ERR_PTR(-EINVAL);
+
+	len_be = cpu_to_be32(length);
+
+	work_area = rtas_work_area_alloc(LOC_CODE_SIZE + sizeof(u32));
+	memcpy(rtas_work_area_raw_buf(work_area), &len_be, sizeof(u32));
+	memcpy((rtas_work_area_raw_buf(work_area) + sizeof(u32)),
+			&kbuf->dynamic_param.location_code_str, length);
+
+	return work_area;
+}
+
+/**
+ * papr_dynamic_indicator_ioc_set - ibm,set-dynamic-indicator RTAS Call
+ * PAPR 2.13 7.3.18
+ *
+ * @ubuf: Input parameters to RTAS call such as indicator token and
+ *        new state.
+ *
+ * Returns success or -errno.
+ */
+static long papr_dynamic_indicator_ioc_set(struct papr_indices_io_block __user *ubuf)
+{
+	struct papr_indices_io_block kbuf;
+	struct rtas_work_area *work_area;
+	s32 fwrc, token, ret;
+
+	token = rtas_function_token(RTAS_FN_IBM_SET_DYNAMIC_INDICATOR);
+	if (token == RTAS_UNKNOWN_SERVICE)
+		return -ENOENT;
+
+	mutex_lock(&rtas_ibm_set_dynamic_indicator_lock);
+	work_area = papr_dynamic_indice_buf_from_user(ubuf, &kbuf);
+	if (IS_ERR(work_area)) {
+		ret = PTR_ERR(work_area);
+		goto out;
+	}
+
+	do {
+		fwrc = rtas_call(token, 3, 1, NULL,
+				kbuf.dynamic_param.token,
+				kbuf.dynamic_param.state,
+				rtas_work_area_phys(work_area));
+	} while (rtas_busy_delay(fwrc));
+
+	rtas_work_area_free(work_area);
+
+	switch (fwrc) {
+	case RTAS_IBM_DYNAMIC_INDICE_SUCCESS:
+		ret = 0;
+		break;
+	case RTAS_IBM_DYNAMIC_INDICE_NO_INDICATOR:	/* No such indicator */
+		ret = -EOPNOTSUPP;
+		break;
+	default:
+		pr_err("unexpected ibm,set-dynamic-indicator result %d\n",
+			fwrc);
+		fallthrough;
+	case RTAS_IBM_DYNAMIC_INDICE_HW_ERROR:	/* Hardware/platform error */
+		ret = -EIO;
+		break;
+	}
+
+out:
+	mutex_unlock(&rtas_ibm_set_dynamic_indicator_lock);
+	return ret;
+}
+
 /*
  * Top-level ioctl handler for /dev/papr-indices.
  */
@@ -341,6 +454,12 @@ static long papr_indices_dev_ioctl(struct file *filp, unsigned int ioctl,
 	case PAPR_INDICES_IOC_GET:
 		ret = papr_indices_create_handle(argp);
 		break;
+	case PAPR_DYNAMIC_INDICATOR_IOC_SET:
+		if (filp->f_mode & FMODE_WRITE)
+			ret = papr_dynamic_indicator_ioc_set(argp);
+		else
+			ret = -EBADF;
+		break;
 	default:
 		ret = -ENOIOCTLCMD;
 		break;
@@ -364,6 +483,9 @@ static __init int papr_indices_init(void)
 	if (!rtas_function_implemented(RTAS_FN_IBM_GET_INDICES))
 		return -ENODEV;
 
+	if (!rtas_function_implemented(RTAS_FN_IBM_SET_DYNAMIC_INDICATOR))
+		return -ENODEV;
+
 	return misc_register(&papr_indices_dev);
 }
 machine_device_initcall(pseries, papr_indices_init);
-- 
2.43.5

[PATCH v2 5/6] powerpc/pseries: Add ibm,get-dynamic-sensor-state RTAS call support

[Haren Myneni]

The RTAS call ibm,get-dynamic-sensor-state is used to get the
sensor state identified by the location code and the sensor
token. The librtas library provides an API
rtas_get_dynamic_sensor() which uses /dev/mem access for work
area allocation but is restricted under system lockdown.

This patch provides an interface with new ioctl
 PAPR_DYNAMIC_SENSOR_IOC_GET to the papr-indices character
driver which executes this HCALL and copies the sensor state
in the user specified ioctl buffer.

Refer PAPR 7.3.19 ibm,get-dynamic-sensor-state for more
information on this RTAS call.
- User input parameters to the RTAS call: location code string
  and the sensor token

Expose these interfaces to user space with a /dev/papr-indices
character device using the following programming model:
 int fd = open("/dev/papr-indices", O_RDWR);
 int ret = ioctl(fd, PAPR_DYNAMIC_SENSOR_IOC_GET,
		struct papr_indices_io_block)
  - The user space specifies input parameters in
    papr_indices_io_block struct
  - Returned state for the specified sensor is copied to
    papr_indices_io_block.dynamic_param.state

Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/include/asm/rtas.h               |  1 +
 arch/powerpc/kernel/rtas.c                    |  2 +-
 arch/powerpc/platforms/pseries/papr-indices.c | 67 +++++++++++++++++++
 3 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index 2da52f59e4c6..fcd822f0e1d7 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -517,6 +517,7 @@ extern unsigned long rtas_rmo_buf;
 extern struct mutex rtas_ibm_get_vpd_lock;
 extern struct mutex rtas_ibm_get_indices_lock;
 extern struct mutex rtas_ibm_set_dynamic_indicator_lock;
+extern struct mutex rtas_ibm_get_dynamic_sensor_state_lock;
 
 #define GLOBAL_INTERRUPT_QUEUE 9005
 
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 88fa416730af..a4848e7f248e 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -92,12 +92,12 @@ struct rtas_function {
  * Per-function locks for sequence-based RTAS functions.
  */
 static DEFINE_MUTEX(rtas_ibm_activate_firmware_lock);
-static DEFINE_MUTEX(rtas_ibm_get_dynamic_sensor_state_lock);
 static DEFINE_MUTEX(rtas_ibm_lpar_perftools_lock);
 static DEFINE_MUTEX(rtas_ibm_physical_attestation_lock);
 DEFINE_MUTEX(rtas_ibm_get_vpd_lock);
 DEFINE_MUTEX(rtas_ibm_get_indices_lock);
 DEFINE_MUTEX(rtas_ibm_set_dynamic_indicator_lock);
+DEFINE_MUTEX(rtas_ibm_get_dynamic_sensor_state_lock);
 
 static struct rtas_function rtas_function_table[] __ro_after_init = {
 	[RTAS_FNIDX__CHECK_EXCEPTION] = {
diff --git a/arch/powerpc/platforms/pseries/papr-indices.c b/arch/powerpc/platforms/pseries/papr-indices.c
index e4b4eb8ece84..ca6ff2529017 100644
--- a/arch/powerpc/platforms/pseries/papr-indices.c
+++ b/arch/powerpc/platforms/pseries/papr-indices.c
@@ -441,6 +441,67 @@ static long papr_dynamic_indicator_ioc_set(struct papr_indices_io_block __user *
 	return ret;
 }
 
+/**
+ * papr_dynamic_sensor_ioc_get - ibm,get-dynamic-sensor-state RTAS Call
+ * PAPR 2.13 7.3.19
+ *
+ * @ubuf: Input parameters to RTAS call such as sensor token
+ *        Copies the state in user space buffer.
+ *
+ *
+ * Returns success or -errno.
+ */
+
+static long papr_dynamic_sensor_ioc_get(struct papr_indices_io_block __user *ubuf)
+{
+	struct papr_indices_io_block kbuf;
+	struct rtas_work_area *work_area;
+	s32 fwrc, token, ret;
+	u32 rets;
+
+	token = rtas_function_token(RTAS_FN_IBM_GET_DYNAMIC_SENSOR_STATE);
+	if (token == RTAS_UNKNOWN_SERVICE)
+		return -ENOENT;
+
+	mutex_lock(&rtas_ibm_get_dynamic_sensor_state_lock);
+	work_area = papr_dynamic_indice_buf_from_user(ubuf, &kbuf);
+	if (IS_ERR(work_area)) {
+		ret = PTR_ERR(work_area);
+		goto out;
+	}
+
+	do {
+		fwrc = rtas_call(token, 2, 2, &rets,
+				kbuf.dynamic_param.token,
+				rtas_work_area_phys(work_area));
+	} while (rtas_busy_delay(fwrc));
+
+	rtas_work_area_free(work_area);
+
+	switch (fwrc) {
+	case RTAS_IBM_DYNAMIC_INDICE_SUCCESS:
+		if (put_user(rets, &ubuf->dynamic_param.state))
+			ret = -EFAULT;
+		else
+			ret = 0;
+		break;
+	case RTAS_IBM_DYNAMIC_INDICE_NO_INDICATOR:	/* No such indicator */
+		ret = -EOPNOTSUPP;
+		break;
+	default:
+		pr_err("unexpected ibm,get-dynamic-sensor result %d\n",
+				fwrc);
+		fallthrough;
+	case RTAS_IBM_DYNAMIC_INDICE_HW_ERROR:	/* Hardware/platform error */
+		ret = -EIO;
+		break;
+	}
+
+out:
+	mutex_unlock(&rtas_ibm_get_dynamic_sensor_state_lock);
+	return ret;
+}
+
 /*
  * Top-level ioctl handler for /dev/papr-indices.
  */
@@ -454,6 +515,9 @@ static long papr_indices_dev_ioctl(struct file *filp, unsigned int ioctl,
 	case PAPR_INDICES_IOC_GET:
 		ret = papr_indices_create_handle(argp);
 		break;
+	case PAPR_DYNAMIC_SENSOR_IOC_GET:
+		ret = papr_dynamic_sensor_ioc_get(argp);
+		break;
 	case PAPR_DYNAMIC_INDICATOR_IOC_SET:
 		if (filp->f_mode & FMODE_WRITE)
 			ret = papr_dynamic_indicator_ioc_set(argp);
@@ -486,6 +550,9 @@ static __init int papr_indices_init(void)
 	if (!rtas_function_implemented(RTAS_FN_IBM_SET_DYNAMIC_INDICATOR))
 		return -ENODEV;
 
+	if (!rtas_function_implemented(RTAS_FN_IBM_GET_DYNAMIC_SENSOR_STATE))
+		return -ENODEV;
+
 	return misc_register(&papr_indices_dev);
 }
 machine_device_initcall(pseries, papr_indices_init);
-- 
2.43.5

[PATCH v2 6/6] powerpc/pseries: Add papr-platform-dump character driver for dump retrieval

[Haren Myneni]

ibm,platform-dump RTAS call in combination with writable mapping
/dev/mem is issued to collect platform dump from the hypervisor
and may need multiple calls to get the complete dump. The current
implementation uses rtas_platform_dump() API provided by librtas
library to issue these RTAS calls. But /dev/mem access by the
user space is prohibited under system lockdown.

The solution should be to restrict access to RTAS function in user
space and provide kernel interfaces to collect dump. This patch
adds papr-platform-dump character driver and expose standard
interfaces such as open / ioctl/ read to user space in ways that
are compatible with lockdown.

PAPR (7.3.3.4.1 ibm,platform-dump) provides a method to obtain
the complete dump:
- Each dump will be identified by ID called dump tag.
- A sequence of RTAS calls have to be issued until retrieve the
  complete dump. The hypervisor expects the first RTAS call with
  the sequence 0 and the subsequent calls with the sequence
  number returned from the previous calls.
- The hypervisor returns "dump complete" status once the complete
  dump is retrieved. But expects one more RTAS call from the
  partition with the NULL buffer to invalidate dump which means
  the dump will be removed in the hypervisor.
- Sequence of calls are allowed with different dump IDs at the
  same time but not with the same dump ID.

Expose these interfaces to user space with a /dev/papr-platform-dump
character device using the following programming model:

   int devfd = open("/dev/papr-platform-dump", O_RDONLY);
   int fd = ioctl(devfd,PAPR_PLATFORM_DUMP_IOC_CREATE_HANDLE, &dump_id)
	- Restrict user space to access with the same dump ID.
          Typically we do not expect user space requests the dump
          again for the same dump ID.
   char *buf = malloc(size);
   length = read(fd, buf, size);
        - size should be minimum 1K based on PAPR and  <= 4K based
          on RTAS work area size. It will be restrict to RTAS work
          area size. Using 4K work area based on the current
          implementation in librtas library
        - Each read call issue RTAS call to get the data based on
          the size requirement and returns bytes returned from the
          hypervisor
        - If the previous call returns dump complete status, the
          next read returns 0 like EOF.
   ret = ioctl(PAPR_PLATFORM_DUMP_IOC_INVALIDATE, &dump_id)
	- RTAS call with NULL buffer to invalidates the dump.

The read API should use the file descriptor obtained from ioctl
based on dump ID so that gets dump contents for the corresponding
dump ID. Implemented support in librtas (rtas_platform_dump()) for
this new ABI to support system lockdown.

Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 .../include/uapi/asm/papr-platform-dump.h     |  15 +
 arch/powerpc/platforms/pseries/Makefile       |   1 +
 .../platforms/pseries/papr-platform-dump.c    | 408 ++++++++++++++++++
 3 files changed, 424 insertions(+)
 create mode 100644 arch/powerpc/include/uapi/asm/papr-platform-dump.h
 create mode 100644 arch/powerpc/platforms/pseries/papr-platform-dump.c

diff --git a/arch/powerpc/include/uapi/asm/papr-platform-dump.h b/arch/powerpc/include/uapi/asm/papr-platform-dump.h
new file mode 100644
index 000000000000..3a0f152e3ce8
--- /dev/null
+++ b/arch/powerpc/include/uapi/asm/papr-platform-dump.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_PAPR_PLATFORM_DUMP_H_
+#define _UAPI_PAPR_PLATFORM_DUMP_H_
+
+#include <asm/ioctl.h>
+#include <asm/papr-miscdev.h>
+
+/*
+ * ioctl for /dev/papr-platform-dump. Returns a VPD handle fd corresponding to
+ * the location code.
+ */
+#define PAPR_PLATFORM_DUMP_IOC_CREATE_HANDLE _IOW(PAPR_MISCDEV_IOC_ID, 6, __u64)
+#define PAPR_PLATFORM_DUMP_IOC_INVALIDATE    _IOW(PAPR_MISCDEV_IOC_ID, 7, __u64)
+
+#endif /* _UAPI_PAPR_PLATFORM_DUMP_H_ */
diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
index e1db61877bb9..c82c94e0a73c 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -4,6 +4,7 @@ ccflags-$(CONFIG_PPC_PSERIES_DEBUG)	+= -DDEBUG
 obj-y			:= lpar.o hvCall.o nvram.o reconfig.o \
 			   of_helpers.o rtas-work-area.o papr-sysparm.o \
 			   papr-rtas-common.o papr-vpd.o papr-indices.o \
+			   papr-platform-dump.o \
 			   setup.o iommu.o event_sources.o ras.o \
 			   firmware.o power.o dlpar.o mobility.o rng.o \
 			   pci.o pci_dlpar.o eeh_pseries.o msi.o \
diff --git a/arch/powerpc/platforms/pseries/papr-platform-dump.c b/arch/powerpc/platforms/pseries/papr-platform-dump.c
new file mode 100644
index 000000000000..13a418d7c37e
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/papr-platform-dump.c
@@ -0,0 +1,408 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#define pr_fmt(fmt) "papr-platform-dump: " fmt
+
+#include <linux/anon_inodes.h>
+#include <linux/file.h>
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/miscdevice.h>
+#include <asm/machdep.h>
+#include <asm/rtas-work-area.h>
+#include <asm/rtas.h>
+#include <uapi/asm/papr-platform-dump.h>
+
+/*
+ * Function-specific return values for ibm,platform-dump, derived from
+ * PAPR+ v2.13 7.3.3.4.1 "ibm,platform-dump RTAS Call".
+ */
+#define	RTAS_IBM_PLATFORM_DUMP_COMPLETE	0	/* Complete dump retrieved. */
+#define	RTAS_IBM_PLATFORM_DUMP_CONTINUE	1	/* Continue dump */
+#define	RTAS_NOT_AUTHORIZED		-9002	/* Not Authorized */
+
+#define	RTAS_IBM_PLATFORM_DUMP_START	2 /* Linux status to start dump */
+
+/**
+ * struct ibm_platform_dump_params - Parameters (in and out) for
+ *                                              ibm,platform-dump
+ * @work_area:		In: work area buffer for results.
+ * @buf_length:		In: work area buffer length in bytes
+ * @dump_tag_hi:	In: Most-significant 32 bits of a Dump_Tag representing
+ *                      an id of the dump being processed.
+ * @dump_tag_lo:	In: Least-significant 32 bits of a Dump_Tag representing
+ *                      an id of the dump being processed.
+ * @sequence_hi:	In: Sequence number in most-significant 32 bits.
+ *                      Out: Next sequence number in most-significant 32 bits.
+ * @sequence_lo:	In: Sequence number in Least-significant 32 bits
+ *                      Out: Next sequence number in Least-significant 32 bits.
+ * @bytes_ret_hi:	Out: Bytes written in most-significant 32 bits.
+ * @bytes_ret_lo:	Out: Bytes written in Least-significant 32 bits.
+ * @status:		Out: RTAS call status.
+ * @list:		Maintain the list of dumps are in progress. Can
+ *                      retrieve multiple dumps with different dump IDs at
+ *                      the same time but not with the same dump ID. This list
+ *                      is used to determine whether the dump for the same ID
+ *                      is in progress.
+ */
+struct ibm_platform_dump_params {
+	struct rtas_work_area	*work_area;
+	u32			buf_length;
+	u32			dump_tag_hi;
+	u32			dump_tag_lo;
+	u32			sequence_hi;
+	u32			sequence_lo;
+	u32			bytes_ret_hi;
+	u32			bytes_ret_lo;
+	s32			status;
+	struct list_head	list;
+};
+
+/*
+ * Multiple dumps with different dump IDs can be retrieved at the same
+ * time, but not with dame dump ID. platform_dump_list_mutex and
+ * platform_dump_list are used to prevent this behavior.
+ */
+static DEFINE_MUTEX(platform_dump_list_mutex);
+static LIST_HEAD(platform_dump_list);
+
+/**
+ * rtas_ibm_platform_dump() - Call ibm,platform-dump to fill a work area
+ * buffer.
+ * @params: See &struct ibm_platform_dump_params.
+ * @buf_addr: Address of dump buffer (work_area)
+ * @buf_length: Length of the buffer in bytes (min. 1024)
+ *
+ * Calls ibm,platform-dump until it errors or successfully deposits data
+ * into the supplied work area. Handles RTAS retry statuses. Maps RTAS
+ * error statuses to reasonable errno values.
+ *
+ * Can request multiple dumps with different dump IDs at the same time,
+ * but not with the same dump ID which is prevented with the check in
+ * the ioctl code (papr_platform_dump_create_handle()).
+ *
+ * The caller should inspect @params.status to determine whether more
+ * calls are needed to complete the sequence.
+ *
+ * Context: May sleep.
+ * Return: -ve on error, 0 for dump complete and 1 for continue dump
+ */
+static int rtas_ibm_platform_dump(struct ibm_platform_dump_params *params,
+				phys_addr_t buf_addr, u32 buf_length)
+{
+	u32 rets[4];
+	s32 fwrc;
+	int ret = 0;
+
+	do {
+		fwrc = rtas_call(rtas_function_token(RTAS_FN_IBM_PLATFORM_DUMP),
+				6, 5,
+				rets,
+				params->dump_tag_hi,
+				params->dump_tag_lo,
+				params->sequence_hi,
+				params->sequence_lo,
+				buf_addr,
+				buf_length);
+	} while (rtas_busy_delay(fwrc));
+
+	switch (fwrc) {
+	case RTAS_HARDWARE_ERROR:
+		ret = -EIO;
+		break;
+	case RTAS_NOT_AUTHORIZED:
+		ret = -EPERM;
+		break;
+	case RTAS_IBM_PLATFORM_DUMP_CONTINUE:
+	case RTAS_IBM_PLATFORM_DUMP_COMPLETE:
+		params->sequence_hi = rets[0];
+		params->sequence_lo = rets[1];
+		params->bytes_ret_hi = rets[2];
+		params->bytes_ret_lo = rets[3];
+		break;
+	default:
+		ret = -EIO;
+		pr_err_ratelimited("unexpected ibm,platform-dump status %d\n",
+				fwrc);
+		break;
+	}
+
+	params->status = fwrc;
+	return ret;
+}
+
+/*
+ * Platform dump is used with multiple RTAS calls to retrieve the
+ * complete dump for the provided dump ID. Once the complete dump is
+ * retrieved, the hypervisor returns dump complete status (0) for the
+ * last RTAS call and expects the caller issues one more call with
+ * NULL buffer to invalidate the dump so that the hypervisor can remove
+ * the dump.
+ *
+ * After the specific dump is invalidated in the hypervisor, expect the
+ * dump complete status for the new sequence - the user space initiates
+ * new request for the same dump ID.
+ */
+static ssize_t papr_platform_dump_handle_read(struct file *file,
+		char __user *buf, size_t size, loff_t *off)
+{
+	struct ibm_platform_dump_params *params = file->private_data;
+	u64 total_bytes;
+	s32 fwrc;
+
+	/*
+	 * Dump already completed with the previous read calls.
+	 * In case if the user space issues further reads, returns
+	 * -EINVAL.
+	 */
+	if (!params->buf_length) {
+		pr_warn_once("Platform dump completed for dump ID %llu\n",
+			(u64) (((u64)params->dump_tag_hi << 32) |
+				params->dump_tag_lo));
+		return -EINVAL;
+	}
+
+	/*
+	 * The hypervisor returns status 0 if no more data available to
+	 * download. The dump will be invalidated with ioctl (see below).
+	 */
+	if (params->status == RTAS_IBM_PLATFORM_DUMP_COMPLETE) {
+		params->buf_length = 0;
+		/*
+		 * Returns 0 to the user space so that user
+		 * space read stops.
+		 */
+		return 0;
+	}
+
+	if (size < SZ_1K) {
+		pr_err_once("Buffer length should be minimum 1024 bytes\n");
+		return -EINVAL;
+	} else if (size > params->buf_length) {
+		/*
+		 * Allocate 4K work area. So if the user requests > 4K,
+		 * resize the buffer length.
+		 */
+		size = params->buf_length;
+	}
+
+	fwrc = rtas_ibm_platform_dump(params,
+			rtas_work_area_phys(params->work_area),
+			size);
+	if (fwrc < 0)
+		return fwrc;
+
+	total_bytes = (u64) (((u64)params->bytes_ret_hi << 32) |
+			params->bytes_ret_lo);
+
+	/*
+	 * Kernel or firmware bug, do not continue.
+	 */
+	if (WARN(total_bytes > size, "possible write beyond end of work area"))
+		return -EFAULT;
+
+	if (copy_to_user(buf, rtas_work_area_raw_buf(params->work_area),
+			total_bytes))
+		return -EFAULT;
+
+	return total_bytes;
+}
+
+static int papr_platform_dump_handle_release(struct inode *inode,
+					struct file *file)
+{
+	struct ibm_platform_dump_params *params = file->private_data;
+
+	if (params->work_area)
+		rtas_work_area_free(params->work_area);
+
+	mutex_lock(&platform_dump_list_mutex);
+	list_del(&params->list);
+	mutex_unlock(&platform_dump_list_mutex);
+
+	kfree(params);
+	file->private_data = NULL;
+	return 0;
+}
+
+/*
+ * This ioctl is used to invalidate the dump assuming the user space
+ * issue this ioctl after obtain the complete dump.
+ * Issue the last RTAS call with NULL buffer to invalidate the dump
+ * which means dump will be freed in the hypervisor.
+ */
+static long papr_platform_dump_invalidate_ioctl(struct file *file,
+				unsigned int ioctl, unsigned long arg)
+{
+	struct ibm_platform_dump_params *params;
+	u64 __user *argp = (void __user *)arg;
+	u64 param_dump_tag, dump_tag;
+
+	if (ioctl != PAPR_PLATFORM_DUMP_IOC_INVALIDATE)
+		return -ENOIOCTLCMD;
+
+	if (get_user(dump_tag, argp))
+		return -EFAULT;
+
+	/*
+	 * private_data is freeded during release(), so should not
+	 * happen.
+	 */
+	if (!file->private_data) {
+		pr_err("No valid FD to invalidate dump for the ID(%llu)\n",
+				dump_tag);
+		return -EINVAL;
+	}
+
+	params = file->private_data;
+	param_dump_tag = (u64) (((u64)params->dump_tag_hi << 32) |
+				params->dump_tag_lo);
+	if (dump_tag != param_dump_tag) {
+		pr_err("Invalid dump ID(%llu) to invalidate dump\n",
+				dump_tag);
+		return -EINVAL;
+	}
+
+	if (params->status != RTAS_IBM_PLATFORM_DUMP_COMPLETE)
+		pr_warn("Platform dump is not complete, but requested "
+			"to invalidate dump for ID(%llu)\n",
+			dump_tag);
+
+	return rtas_ibm_platform_dump(params, 0, 0);
+}
+
+static const struct file_operations papr_platform_dump_handle_ops = {
+	.read = papr_platform_dump_handle_read,
+	.release = papr_platform_dump_handle_release,
+	.unlocked_ioctl	= papr_platform_dump_invalidate_ioctl,
+};
+
+/**
+ * papr_platform_dump_create_handle() - Create a fd-based handle for
+ * reading platform dump
+ *
+ * Handler for PAPR_PLATFORM_DUMP_IOC_CREATE_HANDLE ioctl command
+ * Allocates RTAS parameter struct and work area and attached to the
+ * file descriptor for reading by user space with the multiple RTAS
+ * calls until the dump is completed. This memory allocation is freed
+ * when the file is released.
+ *
+ * Multiple dump requests with different IDs are allowed at the same
+ * time, but not with the same dump ID. So if the user space is
+ * already opened file descriptor for the specific dump ID, return
+ * -EALREADY for the next request.
+ *
+ * @dump_tag: Dump ID for the dump requested to retrieve from the
+ *		hypervisor
+ *
+ * Return: The installed fd number if successful, -ve errno otherwise.
+ */
+static long papr_platform_dump_create_handle(u64 dump_tag)
+{
+	struct ibm_platform_dump_params *params;
+	u64 param_dump_tag;
+	struct file *file;
+	long err;
+	int fd;
+
+	/*
+	 * Return failure if the user space is already opened FD for
+	 * the specific dump ID. This check will prevent multiple dump
+	 * requests for the same dump ID at the same time. Generally
+	 * should not expect this, but in case.
+	 */
+	list_for_each_entry(params, &platform_dump_list, list) {
+		param_dump_tag = (u64) (((u64)params->dump_tag_hi << 32) |
+					params->dump_tag_lo);
+		if (dump_tag == param_dump_tag) {
+			pr_err("Platform dump for ID(%llu) is already in progress\n",
+					dump_tag);
+			return -EALREADY;
+		}
+	}
+
+	params =  kzalloc(sizeof(struct ibm_platform_dump_params),
+			GFP_KERNEL_ACCOUNT);
+	if (!params)
+		return -ENOMEM;
+
+	params->work_area = rtas_work_area_alloc(SZ_4K);
+	params->buf_length = SZ_4K;
+	params->dump_tag_hi = (u32)(dump_tag >> 32);
+	params->dump_tag_lo = (u32)(dump_tag & 0x00000000ffffffffULL);
+	params->status = RTAS_IBM_PLATFORM_DUMP_START;
+
+	fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC);
+	if (fd < 0) {
+		err = fd;
+		goto put_fd;
+	}
+
+	file = anon_inode_getfile("[papr-platform-dump]",
+				&papr_platform_dump_handle_ops,
+				(void *)params, O_RDONLY);
+	if (IS_ERR(file)) {
+		err = PTR_ERR(file);
+		goto put_fd;
+	}
+
+	file->f_mode |= FMODE_LSEEK | FMODE_PREAD;
+	fd_install(fd, file);
+
+	list_add(&params->list, &platform_dump_list);
+
+	pr_info("%s (%d) initiated platform dump for dump tag %llu\n",
+		current->comm, current->pid, dump_tag);
+	return fd;
+put_fd:
+	rtas_work_area_free(params->work_area);
+	kfree(params);
+	put_unused_fd(fd);
+	return err;
+}
+
+/*
+ * Top-level ioctl handler for /dev/papr-platform-dump.
+ */
+static long papr_platform_dump_dev_ioctl(struct file *filp,
+					unsigned int ioctl,
+					unsigned long arg)
+{
+	u64 __user *argp = (void __user *)arg;
+	u64 dump_tag;
+	long ret;
+
+	if (get_user(dump_tag, argp))
+		return -EFAULT;
+
+	switch (ioctl) {
+	case PAPR_PLATFORM_DUMP_IOC_CREATE_HANDLE:
+		mutex_lock(&platform_dump_list_mutex);
+		ret = papr_platform_dump_create_handle(dump_tag);
+		mutex_unlock(&platform_dump_list_mutex);
+		break;
+	default:
+		ret = -ENOIOCTLCMD;
+		break;
+	}
+	return ret;
+}
+
+static const struct file_operations papr_platform_dump_ops = {
+	.unlocked_ioctl = papr_platform_dump_dev_ioctl,
+};
+
+static struct miscdevice papr_platform_dump_dev = {
+	.minor = MISC_DYNAMIC_MINOR,
+	.name = "papr-platform-dump",
+	.fops = &papr_platform_dump_ops,
+};
+
+static __init int papr_platform_dump_init(void)
+{
+	if (!rtas_function_implemented(RTAS_FN_IBM_PLATFORM_DUMP))
+		return -ENODEV;
+
+	return misc_register(&papr_platform_dump_dev);
+}
+machine_device_initcall(pseries, papr_platform_dump_init);
-- 
2.43.5

Re: [PATCH v2 6/6] powerpc/pseries: Add papr-platform-dump character driver for dump retrieval

[Michal Suchánek]

Hello,

thanks for working on this!

I see in thes version you ended up reusing the existing RTAS call code
which looks better.

From the past discussion it sounds like the get-indices call can list
the available dumps, and I do not see this connection documented.

Also the part about it not being used in practice by the service that
retrieves the dumps because it gets a message from the hypervisor with
the dump id when a dump is available.


On Fri, Jan 10, 2025 at 04:30:08PM -0800, Haren Myneni wrote:
> ibm,platform-dump RTAS call in combination with writable mapping
> /dev/mem is issued to collect platform dump from the hypervisor
> and may need multiple calls to get the complete dump. The current
> implementation uses rtas_platform_dump() API provided by librtas
> library to issue these RTAS calls. But /dev/mem access by the
> user space is prohibited under system lockdown.
> 
> The solution should be to restrict access to RTAS function in user
> space and provide kernel interfaces to collect dump. This patch
> adds papr-platform-dump character driver and expose standard
> interfaces such as open / ioctl/ read to user space in ways that
> are compatible with lockdown.
> 
> PAPR (7.3.3.4.1 ibm,platform-dump) provides a method to obtain
> the complete dump:
> - Each dump will be identified by ID called dump tag.
> - A sequence of RTAS calls have to be issued until retrieve the
>   complete dump. The hypervisor expects the first RTAS call with
>   the sequence 0 and the subsequent calls with the sequence
>   number returned from the previous calls.
> - The hypervisor returns "dump complete" status once the complete
>   dump is retrieved. But expects one more RTAS call from the
>   partition with the NULL buffer to invalidate dump which means
>   the dump will be removed in the hypervisor.
> - Sequence of calls are allowed with different dump IDs at the
>   same time but not with the same dump ID.
> 
> Expose these interfaces to user space with a /dev/papr-platform-dump
> character device using the following programming model:
> 
>    int devfd = open("/dev/papr-platform-dump", O_RDONLY);
>    int fd = ioctl(devfd,PAPR_PLATFORM_DUMP_IOC_CREATE_HANDLE, &dump_id)
> 	- Restrict user space to access with the same dump ID.
>           Typically we do not expect user space requests the dump
>           again for the same dump ID.
>    char *buf = malloc(size);
>    length = read(fd, buf, size);
>         - size should be minimum 1K based on PAPR and  <= 4K based
>           on RTAS work area size. It will be restrict to RTAS work
>           area size. Using 4K work area based on the current
>           implementation in librtas library
>         - Each read call issue RTAS call to get the data based on
>           the size requirement and returns bytes returned from the
>           hypervisor
>         - If the previous call returns dump complete status, the
>           next read returns 0 like EOF.
>    ret = ioctl(PAPR_PLATFORM_DUMP_IOC_INVALIDATE, &dump_id)
> 	- RTAS call with NULL buffer to invalidates the dump.
> 
> The read API should use the file descriptor obtained from ioctl
> based on dump ID so that gets dump contents for the corresponding
> dump ID. Implemented support in librtas (rtas_platform_dump()) for
> this new ABI to support system lockdown.
> 
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  .../include/uapi/asm/papr-platform-dump.h     |  15 +
>  arch/powerpc/platforms/pseries/Makefile       |   1 +
>  .../platforms/pseries/papr-platform-dump.c    | 408 ++++++++++++++++++
>  3 files changed, 424 insertions(+)
>  create mode 100644 arch/powerpc/include/uapi/asm/papr-platform-dump.h
>  create mode 100644 arch/powerpc/platforms/pseries/papr-platform-dump.c
> 
> diff --git a/arch/powerpc/include/uapi/asm/papr-platform-dump.h b/arch/powerpc/include/uapi/asm/papr-platform-dump.h
> new file mode 100644
> index 000000000000..3a0f152e3ce8
> --- /dev/null
> +++ b/arch/powerpc/include/uapi/asm/papr-platform-dump.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +#ifndef _UAPI_PAPR_PLATFORM_DUMP_H_
> +#define _UAPI_PAPR_PLATFORM_DUMP_H_
> +
> +#include <asm/ioctl.h>
> +#include <asm/papr-miscdev.h>
> +
> +/*
> + * ioctl for /dev/papr-platform-dump. Returns a VPD handle fd corresponding to
> + * the location code.
> + */
> +#define PAPR_PLATFORM_DUMP_IOC_CREATE_HANDLE _IOW(PAPR_MISCDEV_IOC_ID, 6, __u64)
> +#define PAPR_PLATFORM_DUMP_IOC_INVALIDATE    _IOW(PAPR_MISCDEV_IOC_ID, 7, __u64)
> +
> +#endif /* _UAPI_PAPR_PLATFORM_DUMP_H_ */
> diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
> index e1db61877bb9..c82c94e0a73c 100644
> --- a/arch/powerpc/platforms/pseries/Makefile
> +++ b/arch/powerpc/platforms/pseries/Makefile
> @@ -4,6 +4,7 @@ ccflags-$(CONFIG_PPC_PSERIES_DEBUG)	+= -DDEBUG
>  obj-y			:= lpar.o hvCall.o nvram.o reconfig.o \
>  			   of_helpers.o rtas-work-area.o papr-sysparm.o \
>  			   papr-rtas-common.o papr-vpd.o papr-indices.o \
> +			   papr-platform-dump.o \
>  			   setup.o iommu.o event_sources.o ras.o \
>  			   firmware.o power.o dlpar.o mobility.o rng.o \
>  			   pci.o pci_dlpar.o eeh_pseries.o msi.o \
> diff --git a/arch/powerpc/platforms/pseries/papr-platform-dump.c b/arch/powerpc/platforms/pseries/papr-platform-dump.c
> new file mode 100644
> index 000000000000..13a418d7c37e
> --- /dev/null
> +++ b/arch/powerpc/platforms/pseries/papr-platform-dump.c
> @@ -0,0 +1,408 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#define pr_fmt(fmt) "papr-platform-dump: " fmt
> +
> +#include <linux/anon_inodes.h>
> +#include <linux/file.h>
> +#include <linux/fs.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/miscdevice.h>
> +#include <asm/machdep.h>
> +#include <asm/rtas-work-area.h>
> +#include <asm/rtas.h>
> +#include <uapi/asm/papr-platform-dump.h>
> +
> +/*
> + * Function-specific return values for ibm,platform-dump, derived from
> + * PAPR+ v2.13 7.3.3.4.1 "ibm,platform-dump RTAS Call".
> + */
> +#define	RTAS_IBM_PLATFORM_DUMP_COMPLETE	0	/* Complete dump retrieved. */
> +#define	RTAS_IBM_PLATFORM_DUMP_CONTINUE	1	/* Continue dump */
> +#define	RTAS_NOT_AUTHORIZED		-9002	/* Not Authorized */
> +
> +#define	RTAS_IBM_PLATFORM_DUMP_START	2 /* Linux status to start dump */
> +
> +/**
> + * struct ibm_platform_dump_params - Parameters (in and out) for
> + *                                              ibm,platform-dump
> + * @work_area:		In: work area buffer for results.
> + * @buf_length:		In: work area buffer length in bytes
> + * @dump_tag_hi:	In: Most-significant 32 bits of a Dump_Tag representing
> + *                      an id of the dump being processed.
> + * @dump_tag_lo:	In: Least-significant 32 bits of a Dump_Tag representing
> + *                      an id of the dump being processed.
> + * @sequence_hi:	In: Sequence number in most-significant 32 bits.
> + *                      Out: Next sequence number in most-significant 32 bits.
> + * @sequence_lo:	In: Sequence number in Least-significant 32 bits
> + *                      Out: Next sequence number in Least-significant 32 bits.
> + * @bytes_ret_hi:	Out: Bytes written in most-significant 32 bits.
> + * @bytes_ret_lo:	Out: Bytes written in Least-significant 32 bits.
> + * @status:		Out: RTAS call status.
> + * @list:		Maintain the list of dumps are in progress. Can
> + *                      retrieve multiple dumps with different dump IDs at
> + *                      the same time but not with the same dump ID. This list
> + *                      is used to determine whether the dump for the same ID
> + *                      is in progress.
> + */
> +struct ibm_platform_dump_params {
> +	struct rtas_work_area	*work_area;
> +	u32			buf_length;
> +	u32			dump_tag_hi;
> +	u32			dump_tag_lo;
> +	u32			sequence_hi;
> +	u32			sequence_lo;
> +	u32			bytes_ret_hi;
> +	u32			bytes_ret_lo;
> +	s32			status;
> +	struct list_head	list;
> +};
> +
> +/*
> + * Multiple dumps with different dump IDs can be retrieved at the same
> + * time, but not with dame dump ID. platform_dump_list_mutex and
> + * platform_dump_list are used to prevent this behavior.
> + */
> +static DEFINE_MUTEX(platform_dump_list_mutex);
> +static LIST_HEAD(platform_dump_list);
> +
> +/**
> + * rtas_ibm_platform_dump() - Call ibm,platform-dump to fill a work area
> + * buffer.
> + * @params: See &struct ibm_platform_dump_params.
> + * @buf_addr: Address of dump buffer (work_area)
> + * @buf_length: Length of the buffer in bytes (min. 1024)
> + *
> + * Calls ibm,platform-dump until it errors or successfully deposits data
> + * into the supplied work area. Handles RTAS retry statuses. Maps RTAS
> + * error statuses to reasonable errno values.
> + *
> + * Can request multiple dumps with different dump IDs at the same time,
> + * but not with the same dump ID which is prevented with the check in
> + * the ioctl code (papr_platform_dump_create_handle()).
> + *
> + * The caller should inspect @params.status to determine whether more
> + * calls are needed to complete the sequence.
> + *
> + * Context: May sleep.
> + * Return: -ve on error, 0 for dump complete and 1 for continue dump
> + */
> +static int rtas_ibm_platform_dump(struct ibm_platform_dump_params *params,
> +				phys_addr_t buf_addr, u32 buf_length)
> +{
> +	u32 rets[4];
> +	s32 fwrc;
> +	int ret = 0;
> +
> +	do {
> +		fwrc = rtas_call(rtas_function_token(RTAS_FN_IBM_PLATFORM_DUMP),
> +				6, 5,
> +				rets,
> +				params->dump_tag_hi,
> +				params->dump_tag_lo,
> +				params->sequence_hi,
> +				params->sequence_lo,
> +				buf_addr,
> +				buf_length);
> +	} while (rtas_busy_delay(fwrc));
> +
> +	switch (fwrc) {
> +	case RTAS_HARDWARE_ERROR:
> +		ret = -EIO;
> +		break;
> +	case RTAS_NOT_AUTHORIZED:
> +		ret = -EPERM;
> +		break;
> +	case RTAS_IBM_PLATFORM_DUMP_CONTINUE:
> +	case RTAS_IBM_PLATFORM_DUMP_COMPLETE:
> +		params->sequence_hi = rets[0];
> +		params->sequence_lo = rets[1];
> +		params->bytes_ret_hi = rets[2];
> +		params->bytes_ret_lo = rets[3];
> +		break;
> +	default:
> +		ret = -EIO;
> +		pr_err_ratelimited("unexpected ibm,platform-dump status %d\n",
> +				fwrc);
> +		break;
> +	}
> +
> +	params->status = fwrc;
> +	return ret;
> +}
> +
> +/*
> + * Platform dump is used with multiple RTAS calls to retrieve the
> + * complete dump for the provided dump ID. Once the complete dump is
> + * retrieved, the hypervisor returns dump complete status (0) for the
> + * last RTAS call and expects the caller issues one more call with
> + * NULL buffer to invalidate the dump so that the hypervisor can remove
> + * the dump.
> + *
> + * After the specific dump is invalidated in the hypervisor, expect the
> + * dump complete status for the new sequence - the user space initiates
> + * new request for the same dump ID.
> + */
> +static ssize_t papr_platform_dump_handle_read(struct file *file,
> +		char __user *buf, size_t size, loff_t *off)
> +{
> +	struct ibm_platform_dump_params *params = file->private_data;
> +	u64 total_bytes;
> +	s32 fwrc;
> +
> +	/*
> +	 * Dump already completed with the previous read calls.
> +	 * In case if the user space issues further reads, returns
> +	 * -EINVAL.
> +	 */
> +	if (!params->buf_length) {
> +		pr_warn_once("Platform dump completed for dump ID %llu\n",
> +			(u64) (((u64)params->dump_tag_hi << 32) |
> +				params->dump_tag_lo));
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * The hypervisor returns status 0 if no more data available to
> +	 * download. The dump will be invalidated with ioctl (see below).
> +	 */
> +	if (params->status == RTAS_IBM_PLATFORM_DUMP_COMPLETE) {
> +		params->buf_length = 0;
> +		/*
> +		 * Returns 0 to the user space so that user
> +		 * space read stops.
> +		 */
> +		return 0;
> +	}
> +
> +	if (size < SZ_1K) {
> +		pr_err_once("Buffer length should be minimum 1024 bytes\n");
> +		return -EINVAL;
> +	} else if (size > params->buf_length) {
> +		/*
> +		 * Allocate 4K work area. So if the user requests > 4K,
> +		 * resize the buffer length.
> +		 */
> +		size = params->buf_length;
> +	}
> +
> +	fwrc = rtas_ibm_platform_dump(params,
> +			rtas_work_area_phys(params->work_area),
> +			size);
> +	if (fwrc < 0)
> +		return fwrc;
> +
> +	total_bytes = (u64) (((u64)params->bytes_ret_hi << 32) |
> +			params->bytes_ret_lo);
> +
> +	/*
> +	 * Kernel or firmware bug, do not continue.
> +	 */
> +	if (WARN(total_bytes > size, "possible write beyond end of work area"))
> +		return -EFAULT;
> +
> +	if (copy_to_user(buf, rtas_work_area_raw_buf(params->work_area),
> +			total_bytes))
> +		return -EFAULT;
> +
> +	return total_bytes;
> +}
> +
> +static int papr_platform_dump_handle_release(struct inode *inode,
> +					struct file *file)
> +{
> +	struct ibm_platform_dump_params *params = file->private_data;
> +
> +	if (params->work_area)
> +		rtas_work_area_free(params->work_area);
> +
> +	mutex_lock(&platform_dump_list_mutex);
> +	list_del(&params->list);
> +	mutex_unlock(&platform_dump_list_mutex);
> +
> +	kfree(params);
> +	file->private_data = NULL;
> +	return 0;
> +}
> +
> +/*
> + * This ioctl is used to invalidate the dump assuming the user space
> + * issue this ioctl after obtain the complete dump.
> + * Issue the last RTAS call with NULL buffer to invalidate the dump
> + * which means dump will be freed in the hypervisor.
> + */
> +static long papr_platform_dump_invalidate_ioctl(struct file *file,
> +				unsigned int ioctl, unsigned long arg)
> +{
> +	struct ibm_platform_dump_params *params;
> +	u64 __user *argp = (void __user *)arg;
> +	u64 param_dump_tag, dump_tag;
> +
> +	if (ioctl != PAPR_PLATFORM_DUMP_IOC_INVALIDATE)
> +		return -ENOIOCTLCMD;
> +
> +	if (get_user(dump_tag, argp))
> +		return -EFAULT;
> +
> +	/*
> +	 * private_data is freeded during release(), so should not
			   freed?
> +	 * happen.
> +	 */
> +	if (!file->private_data) {
> +		pr_err("No valid FD to invalidate dump for the ID(%llu)\n",
> +				dump_tag);
> +		return -EINVAL;
> +	}
> +
> +	params = file->private_data;
> +	param_dump_tag = (u64) (((u64)params->dump_tag_hi << 32) |
> +				params->dump_tag_lo);
> +	if (dump_tag != param_dump_tag) {
> +		pr_err("Invalid dump ID(%llu) to invalidate dump\n",
> +				dump_tag);
> +		return -EINVAL;
> +	}
> +
> +	if (params->status != RTAS_IBM_PLATFORM_DUMP_COMPLETE)
> +		pr_warn("Platform dump is not complete, but requested "
> +			"to invalidate dump for ID(%llu)\n",
> +			dump_tag);

Not sure if something should be done here or if relying on translation
of the error from the RTAS call is advisable.

Thanks

Michal

Re: [PATCH v2 6/6] powerpc/pseries: Add papr-platform-dump character driver for dump retrieval

[Haren Myneni]

On Wed, 2025-02-05 at 15:28 +0100, Michal Suchánek wrote:
> Hello,
> 
> thanks for working on this!
> 
> I see in thes version you ended up reusing the existing RTAS call
> code
> which looks better.
> 
> From the past discussion it sounds like the get-indices call can list
> the available dumps, and I do not see this connection documented.
> 
> Also the part about it not being used in practice by the service that
> retrieves the dumps because it gets a message from the hypervisor
> with
> the dump id when a dump is available.

ibm,get-indices is used to obtain data based on indicator and sensor,
not related to platform dump. ibm,platform-dump is used only on non-HMC 
based systems and BMC interface initiates manually to save the dump on
tne partition. Sorry in case caused confusion in the previous
discussion.

> 
> 
> On Fri, Jan 10, 2025 at 04:30:08PM -0800, Haren Myneni wrote:
> > ibm,platform-dump RTAS call in combination with writable mapping
> > /dev/mem is issued to collect platform dump from the hypervisor
> > and may need multiple calls to get the complete dump. The current
> > implementation uses rtas_platform_dump() API provided by librtas
> > library to issue these RTAS calls. But /dev/mem access by the
> > user space is prohibited under system lockdown.
> > 
> > The solution should be to restrict access to RTAS function in user
> > space and provide kernel interfaces to collect dump. This patch
> > adds papr-platform-dump character driver and expose standard
> > interfaces such as open / ioctl/ read to user space in ways that
> > are compatible with lockdown.
> > 
> > PAPR (7.3.3.4.1 ibm,platform-dump) provides a method to obtain
> > the complete dump:
> > - Each dump will be identified by ID called dump tag.
> > - A sequence of RTAS calls have to be issued until retrieve the
> >   complete dump. The hypervisor expects the first RTAS call with
> >   the sequence 0 and the subsequent calls with the sequence
> >   number returned from the previous calls.
> > - The hypervisor returns "dump complete" status once the complete
> >   dump is retrieved. But expects one more RTAS call from the
> >   partition with the NULL buffer to invalidate dump which means
> >   the dump will be removed in the hypervisor.
> > - Sequence of calls are allowed with different dump IDs at the
> >   same time but not with the same dump ID.
> > 
> > Expose these interfaces to user space with a /dev/papr-platform-
> > dump
> > character device using the following programming model:
> > 
> >    int devfd = open("/dev/papr-platform-dump", O_RDONLY);
> >    int fd = ioctl(devfd,PAPR_PLATFORM_DUMP_IOC_CREATE_HANDLE,
> > &dump_id)
> > 	- Restrict user space to access with the same dump ID.
> >           Typically we do not expect user space requests the dump
> >           again for the same dump ID.
> >    char *buf = malloc(size);
> >    length = read(fd, buf, size);
> >         - size should be minimum 1K based on PAPR and  <= 4K based
> >           on RTAS work area size. It will be restrict to RTAS work
> >           area size. Using 4K work area based on the current
> >           implementation in librtas library
> >         - Each read call issue RTAS call to get the data based on
> >           the size requirement and returns bytes returned from the
> >           hypervisor
> >         - If the previous call returns dump complete status, the
> >           next read returns 0 like EOF.
> >    ret = ioctl(PAPR_PLATFORM_DUMP_IOC_INVALIDATE, &dump_id)
> > 	- RTAS call with NULL buffer to invalidates the dump.
> > 
> > The read API should use the file descriptor obtained from ioctl
> > based on dump ID so that gets dump contents for the corresponding
> > dump ID. Implemented support in librtas (rtas_platform_dump()) for
> > this new ABI to support system lockdown.
> > 
> > Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> > ---
> >  .../include/uapi/asm/papr-platform-dump.h     |  15 +
> >  arch/powerpc/platforms/pseries/Makefile       |   1 +
> >  .../platforms/pseries/papr-platform-dump.c    | 408
> > ++++++++++++++++++
> >  3 files changed, 424 insertions(+)
> >  create mode 100644 arch/powerpc/include/uapi/asm/papr-platform-
> > dump.h
> >  create mode 100644 arch/powerpc/platforms/pseries/papr-platform-
> > dump.c
> > 
> > diff --git a/arch/powerpc/include/uapi/asm/papr-platform-dump.h
> > b/arch/powerpc/include/uapi/asm/papr-platform-dump.h
> > new file mode 100644
> > index 000000000000..3a0f152e3ce8
> > --- /dev/null
> > +++ b/arch/powerpc/include/uapi/asm/papr-platform-dump.h
> > @@ -0,0 +1,15 @@
> > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > +#ifndef _UAPI_PAPR_PLATFORM_DUMP_H_
> > +#define _UAPI_PAPR_PLATFORM_DUMP_H_
> > +
> > +#include <asm/ioctl.h>
> > +#include <asm/papr-miscdev.h>
> > +
> > +/*
> > + * ioctl for /dev/papr-platform-dump. Returns a VPD handle fd
> > corresponding to
> > + * the location code.
> > + */
> > +#define PAPR_PLATFORM_DUMP_IOC_CREATE_HANDLE
> > _IOW(PAPR_MISCDEV_IOC_ID, 6, __u64)
> > +#define
> > PAPR_PLATFORM_DUMP_IOC_INVALIDATE    _IOW(PAPR_MISCDEV_IOC_ID, 7,
> > __u64)
> > +
> > +#endif /* _UAPI_PAPR_PLATFORM_DUMP_H_ */
> > diff --git a/arch/powerpc/platforms/pseries/Makefile
> > b/arch/powerpc/platforms/pseries/Makefile
> > index e1db61877bb9..c82c94e0a73c 100644
> > --- a/arch/powerpc/platforms/pseries/Makefile
> > +++ b/arch/powerpc/platforms/pseries/Makefile
> > @@ -4,6 +4,7 @@ ccflags-$(CONFIG_PPC_PSERIES_DEBUG)	+= -DDEBUG
> >  obj-y			:= lpar.o hvCall.o nvram.o reconfig.o \
> >  			   of_helpers.o rtas-work-area.o papr-sysparm.o 
> > \
> >  			   papr-rtas-common.o papr-vpd.o papr-indices.o 
> > \
> > +			   papr-platform-dump.o \
> >  			   setup.o iommu.o event_sources.o ras.o \
> >  			   firmware.o power.o dlpar.o mobility.o rng.o
> > \
> >  			   pci.o pci_dlpar.o eeh_pseries.o msi.o \
> > diff --git a/arch/powerpc/platforms/pseries/papr-platform-dump.c
> > b/arch/powerpc/platforms/pseries/papr-platform-dump.c
> > new file mode 100644
> > index 000000000000..13a418d7c37e
> > --- /dev/null
> > +++ b/arch/powerpc/platforms/pseries/papr-platform-dump.c
> > @@ -0,0 +1,408 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +
> > +#define pr_fmt(fmt) "papr-platform-dump: " fmt
> > +
> > +#include <linux/anon_inodes.h>
> > +#include <linux/file.h>
> > +#include <linux/fs.h>
> > +#include <linux/init.h>
> > +#include <linux/kernel.h>
> > +#include <linux/miscdevice.h>
> > +#include <asm/machdep.h>
> > +#include <asm/rtas-work-area.h>
> > +#include <asm/rtas.h>
> > +#include <uapi/asm/papr-platform-dump.h>
> > +
> > +/*
> > + * Function-specific return values for ibm,platform-dump, derived
> > from
> > + * PAPR+ v2.13 7.3.3.4.1 "ibm,platform-dump RTAS Call".
> > + */
> > +#define	RTAS_IBM_PLATFORM_DUMP_COMPLETE	0	/* Complete
> > dump retrieved. */
> > +#define	RTAS_IBM_PLATFORM_DUMP_CONTINUE	1	/* Continue
> > dump */
> > +#define	RTAS_NOT_AUTHORIZED		-9002	/* Not Authorized
> > */
> > +
> > +#define	RTAS_IBM_PLATFORM_DUMP_START	2 /* Linux status
> > to start dump */
> > +
> > +/**
> > + * struct ibm_platform_dump_params - Parameters (in and out) for
> > + *                                              ibm,platform-dump
> > + * @work_area:		In: work area buffer for results.
> > + * @buf_length:		In: work area buffer length in bytes
> > + * @dump_tag_hi:	In: Most-significant 32 bits of a Dump_Tag
> > representing
> > + *                      an id of the dump being processed.
> > + * @dump_tag_lo:	In: Least-significant 32 bits of a Dump_Tag
> > representing
> > + *                      an id of the dump being processed.
> > + * @sequence_hi:	In: Sequence number in most-significant 32
> > bits.
> > + *                      Out: Next sequence number in most-
> > significant 32 bits.
> > + * @sequence_lo:	In: Sequence number in Least-significant 32
> > bits
> > + *                      Out: Next sequence number in Least-
> > significant 32 bits.
> > + * @bytes_ret_hi:	Out: Bytes written in most-significant 32 bits.
> > + * @bytes_ret_lo:	Out: Bytes written in Least-significant 32
> > bits.
> > + * @status:		Out: RTAS call status.
> > + * @list:		Maintain the list of dumps are in progress. Can
> > + *                      retrieve multiple dumps with different
> > dump IDs at
> > + *                      the same time but not with the same dump
> > ID. This list
> > + *                      is used to determine whether the dump for
> > the same ID
> > + *                      is in progress.
> > + */
> > +struct ibm_platform_dump_params {
> > +	struct rtas_work_area	*work_area;
> > +	u32			buf_length;
> > +	u32			dump_tag_hi;
> > +	u32			dump_tag_lo;
> > +	u32			sequence_hi;
> > +	u32			sequence_lo;
> > +	u32			bytes_ret_hi;
> > +	u32			bytes_ret_lo;
> > +	s32			status;
> > +	struct list_head	list;
> > +};
> > +
> > +/*
> > + * Multiple dumps with different dump IDs can be retrieved at the
> > same
> > + * time, but not with dame dump ID. platform_dump_list_mutex and
> > + * platform_dump_list are used to prevent this behavior.
> > + */
> > +static DEFINE_MUTEX(platform_dump_list_mutex);
> > +static LIST_HEAD(platform_dump_list);
> > +
> > +/**
> > + * rtas_ibm_platform_dump() - Call ibm,platform-dump to fill a
> > work area
> > + * buffer.
> > + * @params: See &struct ibm_platform_dump_params.
> > + * @buf_addr: Address of dump buffer (work_area)
> > + * @buf_length: Length of the buffer in bytes (min. 1024)
> > + *
> > + * Calls ibm,platform-dump until it errors or successfully
> > deposits data
> > + * into the supplied work area. Handles RTAS retry statuses. Maps
> > RTAS
> > + * error statuses to reasonable errno values.
> > + *
> > + * Can request multiple dumps with different dump IDs at the same
> > time,
> > + * but not with the same dump ID which is prevented with the check
> > in
> > + * the ioctl code (papr_platform_dump_create_handle()).
> > + *
> > + * The caller should inspect @params.status to determine whether
> > more
> > + * calls are needed to complete the sequence.
> > + *
> > + * Context: May sleep.
> > + * Return: -ve on error, 0 for dump complete and 1 for continue
> > dump
> > + */
> > +static int rtas_ibm_platform_dump(struct ibm_platform_dump_params
> > *params,
> > +				phys_addr_t buf_addr, u32 buf_length)
> > +{
> > +	u32 rets[4];
> > +	s32 fwrc;
> > +	int ret = 0;
> > +
> > +	do {
> > +		fwrc =
> > rtas_call(rtas_function_token(RTAS_FN_IBM_PLATFORM_DUMP),
> > +				6, 5,
> > +				rets,
> > +				params->dump_tag_hi,
> > +				params->dump_tag_lo,
> > +				params->sequence_hi,
> > +				params->sequence_lo,
> > +				buf_addr,
> > +				buf_length);
> > +	} while (rtas_busy_delay(fwrc));
> > +
> > +	switch (fwrc) {
> > +	case RTAS_HARDWARE_ERROR:
> > +		ret = -EIO;
> > +		break;
> > +	case RTAS_NOT_AUTHORIZED:
> > +		ret = -EPERM;
> > +		break;
> > +	case RTAS_IBM_PLATFORM_DUMP_CONTINUE:
> > +	case RTAS_IBM_PLATFORM_DUMP_COMPLETE:
> > +		params->sequence_hi = rets[0];
> > +		params->sequence_lo = rets[1];
> > +		params->bytes_ret_hi = rets[2];
> > +		params->bytes_ret_lo = rets[3];
> > +		break;
> > +	default:
> > +		ret = -EIO;
> > +		pr_err_ratelimited("unexpected ibm,platform-dump status
> > %d\n",
> > +				fwrc);
> > +		break;
> > +	}
> > +
> > +	params->status = fwrc;
> > +	return ret;
> > +}
> > +
> > +/*
> > + * Platform dump is used with multiple RTAS calls to retrieve the
> > + * complete dump for the provided dump ID. Once the complete dump
> > is
> > + * retrieved, the hypervisor returns dump complete status (0) for
> > the
> > + * last RTAS call and expects the caller issues one more call with
> > + * NULL buffer to invalidate the dump so that the hypervisor can
> > remove
> > + * the dump.
> > + *
> > + * After the specific dump is invalidated in the hypervisor,
> > expect the
> > + * dump complete status for the new sequence - the user space
> > initiates
> > + * new request for the same dump ID.
> > + */
> > +static ssize_t papr_platform_dump_handle_read(struct file *file,
> > +		char __user *buf, size_t size, loff_t *off)
> > +{
> > +	struct ibm_platform_dump_params *params = file->private_data;
> > +	u64 total_bytes;
> > +	s32 fwrc;
> > +
> > +	/*
> > +	 * Dump already completed with the previous read calls.
> > +	 * In case if the user space issues further reads, returns
> > +	 * -EINVAL.
> > +	 */
> > +	if (!params->buf_length) {
> > +		pr_warn_once("Platform dump completed for dump ID
> > %llu\n",
> > +			(u64) (((u64)params->dump_tag_hi << 32) |
> > +				params->dump_tag_lo));
> > +		return -EINVAL;
> > +	}
> > +
> > +	/*
> > +	 * The hypervisor returns status 0 if no more data available to
> > +	 * download. The dump will be invalidated with ioctl (see
> > below).
> > +	 */
> > +	if (params->status == RTAS_IBM_PLATFORM_DUMP_COMPLETE) {
> > +		params->buf_length = 0;
> > +		/*
> > +		 * Returns 0 to the user space so that user
> > +		 * space read stops.
> > +		 */
> > +		return 0;
> > +	}
> > +
> > +	if (size < SZ_1K) {
> > +		pr_err_once("Buffer length should be minimum 1024
> > bytes\n");
> > +		return -EINVAL;
> > +	} else if (size > params->buf_length) {
> > +		/*
> > +		 * Allocate 4K work area. So if the user requests > 4K,
> > +		 * resize the buffer length.
> > +		 */
> > +		size = params->buf_length;
> > +	}
> > +
> > +	fwrc = rtas_ibm_platform_dump(params,
> > +			rtas_work_area_phys(params->work_area),
> > +			size);
> > +	if (fwrc < 0)
> > +		return fwrc;
> > +
> > +	total_bytes = (u64) (((u64)params->bytes_ret_hi << 32) |
> > +			params->bytes_ret_lo);
> > +
> > +	/*
> > +	 * Kernel or firmware bug, do not continue.
> > +	 */
> > +	if (WARN(total_bytes > size, "possible write beyond end of work
> > area"))
> > +		return -EFAULT;
> > +
> > +	if (copy_to_user(buf, rtas_work_area_raw_buf(params-
> > >work_area),
> > +			total_bytes))
> > +		return -EFAULT;
> > +
> > +	return total_bytes;
> > +}
> > +
> > +static int papr_platform_dump_handle_release(struct inode *inode,
> > +					struct file *file)
> > +{
> > +	struct ibm_platform_dump_params *params = file->private_data;
> > +
> > +	if (params->work_area)
> > +		rtas_work_area_free(params->work_area);
> > +
> > +	mutex_lock(&platform_dump_list_mutex);
> > +	list_del(&params->list);
> > +	mutex_unlock(&platform_dump_list_mutex);
> > +
> > +	kfree(params);
> > +	file->private_data = NULL;
> > +	return 0;
> > +}
> > +
> > +/*
> > + * This ioctl is used to invalidate the dump assuming the user
> > space
> > + * issue this ioctl after obtain the complete dump.
> > + * Issue the last RTAS call with NULL buffer to invalidate the
> > dump
> > + * which means dump will be freed in the hypervisor.
> > + */
> > +static long papr_platform_dump_invalidate_ioctl(struct file *file,
> > +				unsigned int ioctl, unsigned long arg)
> > +{
> > +	struct ibm_platform_dump_params *params;
> > +	u64 __user *argp = (void __user *)arg;
> > +	u64 param_dump_tag, dump_tag;
> > +
> > +	if (ioctl != PAPR_PLATFORM_DUMP_IOC_INVALIDATE)
> > +		return -ENOIOCTLCMD;
> > +
> > +	if (get_user(dump_tag, argp))
> > +		return -EFAULT;
> > +
> > +	/*
> > +	 * private_data is freeded during release(), so should not
> 			   freed?
> > +	 * happen.
> > +	 */
> > +	if (!file->private_data) {
> > +		pr_err("No valid FD to invalidate dump for the
> > ID(%llu)\n",
> > +				dump_tag);
> > +		return -EINVAL;
> > +	}
> > +
> > +	params = file->private_data;
> > +	param_dump_tag = (u64) (((u64)params->dump_tag_hi << 32) |
> > +				params->dump_tag_lo);
> > +	if (dump_tag != param_dump_tag) {
> > +		pr_err("Invalid dump ID(%llu) to invalidate dump\n",
> > +				dump_tag);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (params->status != RTAS_IBM_PLATFORM_DUMP_COMPLETE)
> > +		pr_warn("Platform dump is not complete, but requested "
> > +			"to invalidate dump for ID(%llu)\n",
> > +			dump_tag);
> 
> Not sure if something should be done here or if relying on
> translation
> of the error from the RTAS call is advisable.

This check just diplays message in case if the user initiated to
invalidate the dump without saving it completely. Then invalidates the
dump with RTAS call and retuns the RTAS return value. 

As mentioned above, platform-dump is available only on non-HMC based
systems. So invoke the collection of dump by BMC based interface, not
widely used. I can remove this check if preferred.

Thanks
Haren

> 
> Thanks
> 
> Michal

Re: [PATCH v2 6/6] powerpc/pseries: Add papr-platform-dump character driver for dump retrieval

[Michal Suchánek]

On Wed, Feb 05, 2025 at 11:51:19PM -0800, Haren Myneni wrote:
> On Wed, 2025-02-05 at 15:28 +0100, Michal Suchánek wrote:
> > Hello,
> > 
> > thanks for working on this!
> > 
> > I see in thes version you ended up reusing the existing RTAS call
> > code
> > which looks better.
> > 
> > From the past discussion it sounds like the get-indices call can list
> > the available dumps, and I do not see this connection documented.
> > 
> > Also the part about it not being used in practice by the service that
> > retrieves the dumps because it gets a message from the hypervisor
> > with
> > the dump id when a dump is available.
> 
> ibm,get-indices is used to obtain data based on indicator and sensor,
> not related to platform dump. ibm,platform-dump is used only on non-HMC 
> based systems and BMC interface initiates manually to save the dump on
> tne partition. Sorry in case caused confusion in the previous
> discussion.
> 
> > 
> > 
> > On Fri, Jan 10, 2025 at 04:30:08PM -0800, Haren Myneni wrote:
> > > ibm,platform-dump RTAS call in combination with writable mapping
> > > /dev/mem is issued to collect platform dump from the hypervisor
> > > and may need multiple calls to get the complete dump. The current
> > > implementation uses rtas_platform_dump() API provided by librtas
> > > library to issue these RTAS calls. But /dev/mem access by the
> > > user space is prohibited under system lockdown.
> > > 
> > > The solution should be to restrict access to RTAS function in user
> > > space and provide kernel interfaces to collect dump. This patch
> > > adds papr-platform-dump character driver and expose standard
> > > interfaces such as open / ioctl/ read to user space in ways that
> > > are compatible with lockdown.
> > > 
> > > PAPR (7.3.3.4.1 ibm,platform-dump) provides a method to obtain
> > > the complete dump:
> > > - Each dump will be identified by ID called dump tag.
> > > - A sequence of RTAS calls have to be issued until retrieve the
> > >   complete dump. The hypervisor expects the first RTAS call with
> > >   the sequence 0 and the subsequent calls with the sequence
> > >   number returned from the previous calls.
> > > - The hypervisor returns "dump complete" status once the complete
> > >   dump is retrieved. But expects one more RTAS call from the
> > >   partition with the NULL buffer to invalidate dump which means
> > >   the dump will be removed in the hypervisor.
> > > - Sequence of calls are allowed with different dump IDs at the
> > >   same time but not with the same dump ID.
> > > 
> > > Expose these interfaces to user space with a /dev/papr-platform-
> > > dump
> > > character device using the following programming model:
> > > 
> > >    int devfd = open("/dev/papr-platform-dump", O_RDONLY);
> > >    int fd = ioctl(devfd,PAPR_PLATFORM_DUMP_IOC_CREATE_HANDLE,
> > > &dump_id)
> > > 	- Restrict user space to access with the same dump ID.
> > >           Typically we do not expect user space requests the dump
> > >           again for the same dump ID.
> > >    char *buf = malloc(size);
> > >    length = read(fd, buf, size);
> > >         - size should be minimum 1K based on PAPR and  <= 4K based
> > >           on RTAS work area size. It will be restrict to RTAS work
> > >           area size. Using 4K work area based on the current
> > >           implementation in librtas library
> > >         - Each read call issue RTAS call to get the data based on
> > >           the size requirement and returns bytes returned from the
> > >           hypervisor
> > >         - If the previous call returns dump complete status, the
> > >           next read returns 0 like EOF.
> > >    ret = ioctl(PAPR_PLATFORM_DUMP_IOC_INVALIDATE, &dump_id)
> > > 	- RTAS call with NULL buffer to invalidates the dump.
> > > 
> > > The read API should use the file descriptor obtained from ioctl
> > > based on dump ID so that gets dump contents for the corresponding
> > > dump ID. Implemented support in librtas (rtas_platform_dump()) for
> > > this new ABI to support system lockdown.
> > > 
> > > Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> > > ---
> > >  .../include/uapi/asm/papr-platform-dump.h     |  15 +
> > >  arch/powerpc/platforms/pseries/Makefile       |   1 +
> > >  .../platforms/pseries/papr-platform-dump.c    | 408
> > > ++++++++++++++++++
> > >  3 files changed, 424 insertions(+)
> > >  create mode 100644 arch/powerpc/include/uapi/asm/papr-platform-
> > > dump.h
> > >  create mode 100644 arch/powerpc/platforms/pseries/papr-platform-
> > > dump.c
> > > 
> > > diff --git a/arch/powerpc/include/uapi/asm/papr-platform-dump.h
> > > b/arch/powerpc/include/uapi/asm/papr-platform-dump.h
> > > new file mode 100644
> > > index 000000000000..3a0f152e3ce8
> > > --- /dev/null
> > > +++ b/arch/powerpc/include/uapi/asm/papr-platform-dump.h
> > > @@ -0,0 +1,15 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > +#ifndef _UAPI_PAPR_PLATFORM_DUMP_H_
> > > +#define _UAPI_PAPR_PLATFORM_DUMP_H_
> > > +
> > > +#include <asm/ioctl.h>
> > > +#include <asm/papr-miscdev.h>
> > > +
> > > +/*
> > > + * ioctl for /dev/papr-platform-dump. Returns a VPD handle fd
> > > corresponding to
> > > + * the location code.
> > > + */
> > > +#define PAPR_PLATFORM_DUMP_IOC_CREATE_HANDLE
> > > _IOW(PAPR_MISCDEV_IOC_ID, 6, __u64)
> > > +#define
> > > PAPR_PLATFORM_DUMP_IOC_INVALIDATE    _IOW(PAPR_MISCDEV_IOC_ID, 7,
> > > __u64)
> > > +
> > > +#endif /* _UAPI_PAPR_PLATFORM_DUMP_H_ */
> > > diff --git a/arch/powerpc/platforms/pseries/Makefile
> > > b/arch/powerpc/platforms/pseries/Makefile
> > > index e1db61877bb9..c82c94e0a73c 100644
> > > --- a/arch/powerpc/platforms/pseries/Makefile
> > > +++ b/arch/powerpc/platforms/pseries/Makefile
> > > @@ -4,6 +4,7 @@ ccflags-$(CONFIG_PPC_PSERIES_DEBUG)	+= -DDEBUG
> > >  obj-y			:= lpar.o hvCall.o nvram.o reconfig.o \
> > >  			   of_helpers.o rtas-work-area.o papr-sysparm.o 
> > > \
> > >  			   papr-rtas-common.o papr-vpd.o papr-indices.o 
> > > \
> > > +			   papr-platform-dump.o \
> > >  			   setup.o iommu.o event_sources.o ras.o \
> > >  			   firmware.o power.o dlpar.o mobility.o rng.o
> > > \
> > >  			   pci.o pci_dlpar.o eeh_pseries.o msi.o \
> > > diff --git a/arch/powerpc/platforms/pseries/papr-platform-dump.c
> > > b/arch/powerpc/platforms/pseries/papr-platform-dump.c
> > > new file mode 100644
> > > index 000000000000..13a418d7c37e
> > > --- /dev/null
> > > +++ b/arch/powerpc/platforms/pseries/papr-platform-dump.c
> > > @@ -0,0 +1,408 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +
> > > +#define pr_fmt(fmt) "papr-platform-dump: " fmt
> > > +
> > > +#include <linux/anon_inodes.h>
> > > +#include <linux/file.h>
> > > +#include <linux/fs.h>
> > > +#include <linux/init.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/miscdevice.h>
> > > +#include <asm/machdep.h>
> > > +#include <asm/rtas-work-area.h>
> > > +#include <asm/rtas.h>
> > > +#include <uapi/asm/papr-platform-dump.h>
> > > +
> > > +/*
> > > + * Function-specific return values for ibm,platform-dump, derived
> > > from
> > > + * PAPR+ v2.13 7.3.3.4.1 "ibm,platform-dump RTAS Call".
> > > + */
> > > +#define	RTAS_IBM_PLATFORM_DUMP_COMPLETE	0	/* Complete
> > > dump retrieved. */
> > > +#define	RTAS_IBM_PLATFORM_DUMP_CONTINUE	1	/* Continue
> > > dump */
> > > +#define	RTAS_NOT_AUTHORIZED		-9002	/* Not Authorized
> > > */
> > > +
> > > +#define	RTAS_IBM_PLATFORM_DUMP_START	2 /* Linux status
> > > to start dump */
> > > +
> > > +/**
> > > + * struct ibm_platform_dump_params - Parameters (in and out) for
> > > + *                                              ibm,platform-dump
> > > + * @work_area:		In: work area buffer for results.
> > > + * @buf_length:		In: work area buffer length in bytes
> > > + * @dump_tag_hi:	In: Most-significant 32 bits of a Dump_Tag
> > > representing
> > > + *                      an id of the dump being processed.
> > > + * @dump_tag_lo:	In: Least-significant 32 bits of a Dump_Tag
> > > representing
> > > + *                      an id of the dump being processed.
> > > + * @sequence_hi:	In: Sequence number in most-significant 32
> > > bits.
> > > + *                      Out: Next sequence number in most-
> > > significant 32 bits.
> > > + * @sequence_lo:	In: Sequence number in Least-significant 32
> > > bits
> > > + *                      Out: Next sequence number in Least-
> > > significant 32 bits.
> > > + * @bytes_ret_hi:	Out: Bytes written in most-significant 32 bits.
> > > + * @bytes_ret_lo:	Out: Bytes written in Least-significant 32
> > > bits.
> > > + * @status:		Out: RTAS call status.
> > > + * @list:		Maintain the list of dumps are in progress. Can
> > > + *                      retrieve multiple dumps with different
> > > dump IDs at
> > > + *                      the same time but not with the same dump
> > > ID. This list
> > > + *                      is used to determine whether the dump for
> > > the same ID
> > > + *                      is in progress.
> > > + */
> > > +struct ibm_platform_dump_params {
> > > +	struct rtas_work_area	*work_area;
> > > +	u32			buf_length;
> > > +	u32			dump_tag_hi;
> > > +	u32			dump_tag_lo;
> > > +	u32			sequence_hi;
> > > +	u32			sequence_lo;
> > > +	u32			bytes_ret_hi;
> > > +	u32			bytes_ret_lo;
> > > +	s32			status;
> > > +	struct list_head	list;
> > > +};
> > > +
> > > +/*
> > > + * Multiple dumps with different dump IDs can be retrieved at the
> > > same
> > > + * time, but not with dame dump ID. platform_dump_list_mutex and
> > > + * platform_dump_list are used to prevent this behavior.
> > > + */
> > > +static DEFINE_MUTEX(platform_dump_list_mutex);
> > > +static LIST_HEAD(platform_dump_list);
> > > +
> > > +/**
> > > + * rtas_ibm_platform_dump() - Call ibm,platform-dump to fill a
> > > work area
> > > + * buffer.
> > > + * @params: See &struct ibm_platform_dump_params.
> > > + * @buf_addr: Address of dump buffer (work_area)
> > > + * @buf_length: Length of the buffer in bytes (min. 1024)
> > > + *
> > > + * Calls ibm,platform-dump until it errors or successfully
> > > deposits data
> > > + * into the supplied work area. Handles RTAS retry statuses. Maps
> > > RTAS
> > > + * error statuses to reasonable errno values.
> > > + *
> > > + * Can request multiple dumps with different dump IDs at the same
> > > time,
> > > + * but not with the same dump ID which is prevented with the check
> > > in
> > > + * the ioctl code (papr_platform_dump_create_handle()).
> > > + *
> > > + * The caller should inspect @params.status to determine whether
> > > more
> > > + * calls are needed to complete the sequence.
> > > + *
> > > + * Context: May sleep.
> > > + * Return: -ve on error, 0 for dump complete and 1 for continue
> > > dump
> > > + */
> > > +static int rtas_ibm_platform_dump(struct ibm_platform_dump_params
> > > *params,
> > > +				phys_addr_t buf_addr, u32 buf_length)
> > > +{
> > > +	u32 rets[4];
> > > +	s32 fwrc;
> > > +	int ret = 0;
> > > +
> > > +	do {
> > > +		fwrc =
> > > rtas_call(rtas_function_token(RTAS_FN_IBM_PLATFORM_DUMP),
> > > +				6, 5,
> > > +				rets,
> > > +				params->dump_tag_hi,
> > > +				params->dump_tag_lo,
> > > +				params->sequence_hi,
> > > +				params->sequence_lo,
> > > +				buf_addr,
> > > +				buf_length);
> > > +	} while (rtas_busy_delay(fwrc));
> > > +
> > > +	switch (fwrc) {
> > > +	case RTAS_HARDWARE_ERROR:
> > > +		ret = -EIO;
> > > +		break;
> > > +	case RTAS_NOT_AUTHORIZED:
> > > +		ret = -EPERM;
> > > +		break;
> > > +	case RTAS_IBM_PLATFORM_DUMP_CONTINUE:
> > > +	case RTAS_IBM_PLATFORM_DUMP_COMPLETE:
> > > +		params->sequence_hi = rets[0];
> > > +		params->sequence_lo = rets[1];
> > > +		params->bytes_ret_hi = rets[2];
> > > +		params->bytes_ret_lo = rets[3];
> > > +		break;
> > > +	default:
> > > +		ret = -EIO;
> > > +		pr_err_ratelimited("unexpected ibm,platform-dump status
> > > %d\n",
> > > +				fwrc);
> > > +		break;
> > > +	}
> > > +
> > > +	params->status = fwrc;
> > > +	return ret;
> > > +}
> > > +
> > > +/*
> > > + * Platform dump is used with multiple RTAS calls to retrieve the
> > > + * complete dump for the provided dump ID. Once the complete dump
> > > is
> > > + * retrieved, the hypervisor returns dump complete status (0) for
> > > the
> > > + * last RTAS call and expects the caller issues one more call with
> > > + * NULL buffer to invalidate the dump so that the hypervisor can
> > > remove
> > > + * the dump.
> > > + *
> > > + * After the specific dump is invalidated in the hypervisor,
> > > expect the
> > > + * dump complete status for the new sequence - the user space
> > > initiates
> > > + * new request for the same dump ID.
> > > + */
> > > +static ssize_t papr_platform_dump_handle_read(struct file *file,
> > > +		char __user *buf, size_t size, loff_t *off)
> > > +{
> > > +	struct ibm_platform_dump_params *params = file->private_data;
> > > +	u64 total_bytes;
> > > +	s32 fwrc;
> > > +
> > > +	/*
> > > +	 * Dump already completed with the previous read calls.
> > > +	 * In case if the user space issues further reads, returns
> > > +	 * -EINVAL.
> > > +	 */
> > > +	if (!params->buf_length) {
> > > +		pr_warn_once("Platform dump completed for dump ID
> > > %llu\n",
> > > +			(u64) (((u64)params->dump_tag_hi << 32) |
> > > +				params->dump_tag_lo));
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	/*
> > > +	 * The hypervisor returns status 0 if no more data available to
> > > +	 * download. The dump will be invalidated with ioctl (see
> > > below).
> > > +	 */
> > > +	if (params->status == RTAS_IBM_PLATFORM_DUMP_COMPLETE) {
> > > +		params->buf_length = 0;
> > > +		/*
> > > +		 * Returns 0 to the user space so that user
> > > +		 * space read stops.
> > > +		 */
> > > +		return 0;
> > > +	}
> > > +
> > > +	if (size < SZ_1K) {
> > > +		pr_err_once("Buffer length should be minimum 1024
> > > bytes\n");
> > > +		return -EINVAL;
> > > +	} else if (size > params->buf_length) {
> > > +		/*
> > > +		 * Allocate 4K work area. So if the user requests > 4K,
> > > +		 * resize the buffer length.
> > > +		 */
> > > +		size = params->buf_length;
> > > +	}
> > > +
> > > +	fwrc = rtas_ibm_platform_dump(params,
> > > +			rtas_work_area_phys(params->work_area),
> > > +			size);
> > > +	if (fwrc < 0)
> > > +		return fwrc;
> > > +
> > > +	total_bytes = (u64) (((u64)params->bytes_ret_hi << 32) |
> > > +			params->bytes_ret_lo);
> > > +
> > > +	/*
> > > +	 * Kernel or firmware bug, do not continue.
> > > +	 */
> > > +	if (WARN(total_bytes > size, "possible write beyond end of work
> > > area"))
> > > +		return -EFAULT;
> > > +
> > > +	if (copy_to_user(buf, rtas_work_area_raw_buf(params-
> > > >work_area),
> > > +			total_bytes))
> > > +		return -EFAULT;
> > > +
> > > +	return total_bytes;
> > > +}
> > > +
> > > +static int papr_platform_dump_handle_release(struct inode *inode,
> > > +					struct file *file)
> > > +{
> > > +	struct ibm_platform_dump_params *params = file->private_data;
> > > +
> > > +	if (params->work_area)
> > > +		rtas_work_area_free(params->work_area);
> > > +
> > > +	mutex_lock(&platform_dump_list_mutex);
> > > +	list_del(&params->list);
> > > +	mutex_unlock(&platform_dump_list_mutex);
> > > +
> > > +	kfree(params);
> > > +	file->private_data = NULL;
> > > +	return 0;
> > > +}
> > > +
> > > +/*
> > > + * This ioctl is used to invalidate the dump assuming the user
> > > space
> > > + * issue this ioctl after obtain the complete dump.
> > > + * Issue the last RTAS call with NULL buffer to invalidate the
> > > dump
> > > + * which means dump will be freed in the hypervisor.
> > > + */
> > > +static long papr_platform_dump_invalidate_ioctl(struct file *file,
> > > +				unsigned int ioctl, unsigned long arg)
> > > +{
> > > +	struct ibm_platform_dump_params *params;
> > > +	u64 __user *argp = (void __user *)arg;
> > > +	u64 param_dump_tag, dump_tag;
> > > +
> > > +	if (ioctl != PAPR_PLATFORM_DUMP_IOC_INVALIDATE)
> > > +		return -ENOIOCTLCMD;
> > > +
> > > +	if (get_user(dump_tag, argp))
> > > +		return -EFAULT;
> > > +
> > > +	/*
> > > +	 * private_data is freeded during release(), so should not
> > 			   freed?
> > > +	 * happen.
> > > +	 */
> > > +	if (!file->private_data) {
> > > +		pr_err("No valid FD to invalidate dump for the
> > > ID(%llu)\n",
> > > +				dump_tag);
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	params = file->private_data;
> > > +	param_dump_tag = (u64) (((u64)params->dump_tag_hi << 32) |
> > > +				params->dump_tag_lo);
> > > +	if (dump_tag != param_dump_tag) {
> > > +		pr_err("Invalid dump ID(%llu) to invalidate dump\n",
> > > +				dump_tag);
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	if (params->status != RTAS_IBM_PLATFORM_DUMP_COMPLETE)
> > > +		pr_warn("Platform dump is not complete, but requested "
> > > +			"to invalidate dump for ID(%llu)\n",
> > > +			dump_tag);
> > 
> > Not sure if something should be done here or if relying on
> > translation
> > of the error from the RTAS call is advisable.
> 
> This check just diplays message in case if the user initiated to
> invalidate the dump without saving it completely. Then invalidates the
> dump with RTAS call and retuns the RTAS return value. 
> 
> As mentioned above, platform-dump is available only on non-HMC based
> systems. So invoke the collection of dump by BMC based interface, not
> widely used. I can remove this check if preferred.

From the previous discussion it sounds like trying to invalidate the
dump without first reading it in full is an error.

The state to detect this error is tracked which makes it possible to
produce this warning.

Then it's also possible to handle the error without roundtrip to the
hypervisor.

Thanks

Michal

> 
> Thanks
> Haren
> 
> > 
> > Thanks
> > 
> > Michal
> 

Re: [PATCH v2 6/6] powerpc/pseries: Add papr-platform-dump character driver for dump retrieval

[Haren Myneni]

On Thu, 2025-02-06 at 10:18 +0100, Michal Suchánek wrote:
> On Wed, Feb 05, 2025 at 11:51:19PM -0800, Haren Myneni wrote:
> > On Wed, 2025-02-05 at 15:28 +0100, Michal Suchánek wrote:
> > > Hello,
> > > 
> > > thanks for working on this!
> > > 
> > > I see in thes version you ended up reusing the existing RTAS call
> > > code
> > > which looks better.
> > > 
> > > From the past discussion it sounds like the get-indices call can
> > > list
> > > the available dumps, and I do not see this connection documented.
> > > 
> > > Also the part about it not being used in practice by the service
> > > that
> > > retrieves the dumps because it gets a message from the hypervisor
> > > with
> > > the dump id when a dump is available.
> > 
> > ibm,get-indices is used to obtain data based on indicator and
> > sensor,
> > not related to platform dump. ibm,platform-dump is used only on
> > non-HMC 
> > based systems and BMC interface initiates manually to save the dump
> > on
> > tne partition. Sorry in case caused confusion in the previous
> > discussion.
> > 
> > > 
> > > On Fri, Jan 10, 2025 at 04:30:08PM -0800, Haren Myneni wrote:
> > > > ibm,platform-dump RTAS call in combination with writable
> > > > mapping
> > > > /dev/mem is issued to collect platform dump from the hypervisor
> > > > and may need multiple calls to get the complete dump. The
> > > > current
> > > > implementation uses rtas_platform_dump() API provided by
> > > > librtas
> > > > library to issue these RTAS calls. But /dev/mem access by the
> > > > user space is prohibited under system lockdown.
> > > > 
> > > > The solution should be to restrict access to RTAS function in
> > > > user
> > > > space and provide kernel interfaces to collect dump. This patch
> > > > adds papr-platform-dump character driver and expose standard
> > > > interfaces such as open / ioctl/ read to user space in ways
> > > > that
> > > > are compatible with lockdown.
> > > > 
> > > > PAPR (7.3.3.4.1 ibm,platform-dump) provides a method to obtain
> > > > the complete dump:
> > > > - Each dump will be identified by ID called dump tag.
> > > > - A sequence of RTAS calls have to be issued until retrieve the
> > > >   complete dump. The hypervisor expects the first RTAS call
> > > > with
> > > >   the sequence 0 and the subsequent calls with the sequence
> > > >   number returned from the previous calls.
> > > > - The hypervisor returns "dump complete" status once the
> > > > complete
> > > >   dump is retrieved. But expects one more RTAS call from the
> > > >   partition with the NULL buffer to invalidate dump which means
> > > >   the dump will be removed in the hypervisor.
> > > > - Sequence of calls are allowed with different dump IDs at the
> > > >   same time but not with the same dump ID.
> > > > 
> > > > Expose these interfaces to user space with a /dev/papr-
> > > > platform-
> > > > dump
> > > > character device using the following programming model:
> > > > 
> > > >    int devfd = open("/dev/papr-platform-dump", O_RDONLY);
> > > >    int fd = ioctl(devfd,PAPR_PLATFORM_DUMP_IOC_CREATE_HANDLE,
> > > > &dump_id)
> > > > 	- Restrict user space to access with the same dump ID.
> > > >           Typically we do not expect user space requests the
> > > > dump
> > > >           again for the same dump ID.
> > > >    char *buf = malloc(size);
> > > >    length = read(fd, buf, size);
> > > >         - size should be minimum 1K based on PAPR and  <= 4K
> > > > based
> > > >           on RTAS work area size. It will be restrict to RTAS
> > > > work
> > > >           area size. Using 4K work area based on the current
> > > >           implementation in librtas library
> > > >         - Each read call issue RTAS call to get the data based
> > > > on
> > > >           the size requirement and returns bytes returned from
> > > > the
> > > >           hypervisor
> > > >         - If the previous call returns dump complete status,
> > > > the
> > > >           next read returns 0 like EOF.
> > > >    ret = ioctl(PAPR_PLATFORM_DUMP_IOC_INVALIDATE, &dump_id)
> > > > 	- RTAS call with NULL buffer to invalidates the dump.
> > > > 
> > > > The read API should use the file descriptor obtained from ioctl
> > > > based on dump ID so that gets dump contents for the
> > > > corresponding
> > > > dump ID. Implemented support in librtas (rtas_platform_dump())
> > > > for
> > > > this new ABI to support system lockdown.
> > > > 
> > > > Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> > > > ---
> > > >  .../include/uapi/asm/papr-platform-dump.h     |  15 +
> > > >  arch/powerpc/platforms/pseries/Makefile       |   1 +
> > > >  .../platforms/pseries/papr-platform-dump.c    | 408
> > > > ++++++++++++++++++
> > > >  3 files changed, 424 insertions(+)
> > > >  create mode 100644 arch/powerpc/include/uapi/asm/papr-
> > > > platform-
> > > > dump.h
> > > >  create mode 100644 arch/powerpc/platforms/pseries/papr-
> > > > platform-
> > > > dump.c
> > > > 
> > > > diff --git a/arch/powerpc/include/uapi/asm/papr-platform-dump.h
> > > > b/arch/powerpc/include/uapi/asm/papr-platform-dump.h
> > > > new file mode 100644
> > > > index 000000000000..3a0f152e3ce8
> > > > --- /dev/null
> > > > +++ b/arch/powerpc/include/uapi/asm/papr-platform-dump.h
> > > > @@ -0,0 +1,15 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > > +#ifndef _UAPI_PAPR_PLATFORM_DUMP_H_
> > > > +#define _UAPI_PAPR_PLATFORM_DUMP_H_
> > > > +
> > > > +#include <asm/ioctl.h>
> > > > +#include <asm/papr-miscdev.h>
> > > > +
> > > > +/*
> > > > + * ioctl for /dev/papr-platform-dump. Returns a VPD handle fd
> > > > corresponding to
> > > > + * the location code.
> > > > + */
> > > > +#define PAPR_PLATFORM_DUMP_IOC_CREATE_HANDLE
> > > > _IOW(PAPR_MISCDEV_IOC_ID, 6, __u64)
> > > > +#define
> > > > PAPR_PLATFORM_DUMP_IOC_INVALIDATE    _IOW(PAPR_MISCDEV_IOC_ID,
> > > > 7,
> > > > __u64)
> > > > +
> > > > +#endif /* _UAPI_PAPR_PLATFORM_DUMP_H_ */
> > > > diff --git a/arch/powerpc/platforms/pseries/Makefile
> > > > b/arch/powerpc/platforms/pseries/Makefile
> > > > index e1db61877bb9..c82c94e0a73c 100644
> > > > --- a/arch/powerpc/platforms/pseries/Makefile
> > > > +++ b/arch/powerpc/platforms/pseries/Makefile
> > > > @@ -4,6 +4,7 @@ ccflags-$(CONFIG_PPC_PSERIES_DEBUG)	+=
> > > > -DDEBUG
> > > >  obj-y			:= lpar.o hvCall.o nvram.o reconfig.o \
> > > >  			   of_helpers.o rtas-work-area.o papr-
> > > > sysparm.o 
> > > > \
> > > >  			   papr-rtas-common.o papr-vpd.o papr-
> > > > indices.o 
> > > > \
> > > > +			   papr-platform-dump.o \
> > > >  			   setup.o iommu.o event_sources.o
> > > > ras.o \
> > > >  			   firmware.o power.o dlpar.o
> > > > mobility.o rng.o
> > > > \
> > > >  			   pci.o pci_dlpar.o eeh_pseries.o
> > > > msi.o \
> > > > diff --git a/arch/powerpc/platforms/pseries/papr-platform-
> > > > dump.c
> > > > b/arch/powerpc/platforms/pseries/papr-platform-dump.c
> > > > new file mode 100644
> > > > index 000000000000..13a418d7c37e
> > > > --- /dev/null
> > > > +++ b/arch/powerpc/platforms/pseries/papr-platform-dump.c
> > > > @@ -0,0 +1,408 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > +
> > > > +#define pr_fmt(fmt) "papr-platform-dump: " fmt
> > > > +
> > > > +#include <linux/anon_inodes.h>
> > > > +#include <linux/file.h>
> > > > +#include <linux/fs.h>
> > > > +#include <linux/init.h>
> > > > +#include <linux/kernel.h>
> > > > +#include <linux/miscdevice.h>
> > > > +#include <asm/machdep.h>
> > > > +#include <asm/rtas-work-area.h>
> > > > +#include <asm/rtas.h>
> > > > +#include <uapi/asm/papr-platform-dump.h>
> > > > +
> > > > +/*
> > > > + * Function-specific return values for ibm,platform-dump,
> > > > derived
> > > > from
> > > > + * PAPR+ v2.13 7.3.3.4.1 "ibm,platform-dump RTAS Call".
> > > > + */
> > > > +#define	RTAS_IBM_PLATFORM_DUMP_COMPLETE	0	/*
> > > > Complete
> > > > dump retrieved. */
> > > > +#define	RTAS_IBM_PLATFORM_DUMP_CONTINUE	1	/*
> > > > Continue
> > > > dump */
> > > > +#define	RTAS_NOT_AUTHORIZED		-9002	/* Not
> > > > Authorized
> > > > */
> > > > +
> > > > +#define	RTAS_IBM_PLATFORM_DUMP_START	2 /* Linux
> > > > status
> > > > to start dump */
> > > > +
> > > > +/**
> > > > + * struct ibm_platform_dump_params - Parameters (in and out)
> > > > for
> > > > + *                                              ibm,platform-
> > > > dump
> > > > + * @work_area:		In: work area buffer for results.
> > > > + * @buf_length:		In: work area buffer length in
> > > > bytes
> > > > + * @dump_tag_hi:	In: Most-significant 32 bits of a
> > > > Dump_Tag
> > > > representing
> > > > + *                      an id of the dump being processed.
> > > > + * @dump_tag_lo:	In: Least-significant 32 bits of a
> > > > Dump_Tag
> > > > representing
> > > > + *                      an id of the dump being processed.
> > > > + * @sequence_hi:	In: Sequence number in most-significant 
> > > > 32
> > > > bits.
> > > > + *                      Out: Next sequence number in most-
> > > > significant 32 bits.
> > > > + * @sequence_lo:	In: Sequence number in Least-
> > > > significant 32
> > > > bits
> > > > + *                      Out: Next sequence number in Least-
> > > > significant 32 bits.
> > > > + * @bytes_ret_hi:	Out: Bytes written in most-significant
> > > > 32 bits.
> > > > + * @bytes_ret_lo:	Out: Bytes written in Least-significant 
> > > > 32
> > > > bits.
> > > > + * @status:		Out: RTAS call status.
> > > > + * @list:		Maintain the list of dumps are in
> > > > progress. Can
> > > > + *                      retrieve multiple dumps with different
> > > > dump IDs at
> > > > + *                      the same time but not with the same
> > > > dump
> > > > ID. This list
> > > > + *                      is used to determine whether the dump
> > > > for
> > > > the same ID
> > > > + *                      is in progress.
> > > > + */
> > > > +struct ibm_platform_dump_params {
> > > > +	struct rtas_work_area	*work_area;
> > > > +	u32			buf_length;
> > > > +	u32			dump_tag_hi;
> > > > +	u32			dump_tag_lo;
> > > > +	u32			sequence_hi;
> > > > +	u32			sequence_lo;
> > > > +	u32			bytes_ret_hi;
> > > > +	u32			bytes_ret_lo;
> > > > +	s32			status;
> > > > +	struct list_head	list;
> > > > +};
> > > > +
> > > > +/*
> > > > + * Multiple dumps with different dump IDs can be retrieved at
> > > > the
> > > > same
> > > > + * time, but not with dame dump ID. platform_dump_list_mutex
> > > > and
> > > > + * platform_dump_list are used to prevent this behavior.
> > > > + */
> > > > +static DEFINE_MUTEX(platform_dump_list_mutex);
> > > > +static LIST_HEAD(platform_dump_list);
> > > > +
> > > > +/**
> > > > + * rtas_ibm_platform_dump() - Call ibm,platform-dump to fill a
> > > > work area
> > > > + * buffer.
> > > > + * @params: See &struct ibm_platform_dump_params.
> > > > + * @buf_addr: Address of dump buffer (work_area)
> > > > + * @buf_length: Length of the buffer in bytes (min. 1024)
> > > > + *
> > > > + * Calls ibm,platform-dump until it errors or successfully
> > > > deposits data
> > > > + * into the supplied work area. Handles RTAS retry statuses.
> > > > Maps
> > > > RTAS
> > > > + * error statuses to reasonable errno values.
> > > > + *
> > > > + * Can request multiple dumps with different dump IDs at the
> > > > same
> > > > time,
> > > > + * but not with the same dump ID which is prevented with the
> > > > check
> > > > in
> > > > + * the ioctl code (papr_platform_dump_create_handle()).
> > > > + *
> > > > + * The caller should inspect @params.status to determine
> > > > whether
> > > > more
> > > > + * calls are needed to complete the sequence.
> > > > + *
> > > > + * Context: May sleep.
> > > > + * Return: -ve on error, 0 for dump complete and 1 for
> > > > continue
> > > > dump
> > > > + */
> > > > +static int rtas_ibm_platform_dump(struct
> > > > ibm_platform_dump_params
> > > > *params,
> > > > +				phys_addr_t buf_addr, u32
> > > > buf_length)
> > > > +{
> > > > +	u32 rets[4];
> > > > +	s32 fwrc;
> > > > +	int ret = 0;
> > > > +
> > > > +	do {
> > > > +		fwrc =
> > > > rtas_call(rtas_function_token(RTAS_FN_IBM_PLATFORM_DUMP),
> > > > +				6, 5,
> > > > +				rets,
> > > > +				params->dump_tag_hi,
> > > > +				params->dump_tag_lo,
> > > > +				params->sequence_hi,
> > > > +				params->sequence_lo,
> > > > +				buf_addr,
> > > > +				buf_length);
> > > > +	} while (rtas_busy_delay(fwrc));
> > > > +
> > > > +	switch (fwrc) {
> > > > +	case RTAS_HARDWARE_ERROR:
> > > > +		ret = -EIO;
> > > > +		break;
> > > > +	case RTAS_NOT_AUTHORIZED:
> > > > +		ret = -EPERM;
> > > > +		break;
> > > > +	case RTAS_IBM_PLATFORM_DUMP_CONTINUE:
> > > > +	case RTAS_IBM_PLATFORM_DUMP_COMPLETE:
> > > > +		params->sequence_hi = rets[0];
> > > > +		params->sequence_lo = rets[1];
> > > > +		params->bytes_ret_hi = rets[2];
> > > > +		params->bytes_ret_lo = rets[3];
> > > > +		break;
> > > > +	default:
> > > > +		ret = -EIO;
> > > > +		pr_err_ratelimited("unexpected ibm,platform-
> > > > dump status
> > > > %d\n",
> > > > +				fwrc);
> > > > +		break;
> > > > +	}
> > > > +
> > > > +	params->status = fwrc;
> > > > +	return ret;
> > > > +}
> > > > +
> > > > +/*
> > > > + * Platform dump is used with multiple RTAS calls to retrieve
> > > > the
> > > > + * complete dump for the provided dump ID. Once the complete
> > > > dump
> > > > is
> > > > + * retrieved, the hypervisor returns dump complete status (0)
> > > > for
> > > > the
> > > > + * last RTAS call and expects the caller issues one more call
> > > > with
> > > > + * NULL buffer to invalidate the dump so that the hypervisor
> > > > can
> > > > remove
> > > > + * the dump.
> > > > + *
> > > > + * After the specific dump is invalidated in the hypervisor,
> > > > expect the
> > > > + * dump complete status for the new sequence - the user space
> > > > initiates
> > > > + * new request for the same dump ID.
> > > > + */
> > > > +static ssize_t papr_platform_dump_handle_read(struct file
> > > > *file,
> > > > +		char __user *buf, size_t size, loff_t *off)
> > > > +{
> > > > +	struct ibm_platform_dump_params *params = file-
> > > > >private_data;
> > > > +	u64 total_bytes;
> > > > +	s32 fwrc;
> > > > +
> > > > +	/*
> > > > +	 * Dump already completed with the previous read calls.
> > > > +	 * In case if the user space issues further reads,
> > > > returns
> > > > +	 * -EINVAL.
> > > > +	 */
> > > > +	if (!params->buf_length) {
> > > > +		pr_warn_once("Platform dump completed for dump
> > > > ID
> > > > %llu\n",
> > > > +			(u64) (((u64)params->dump_tag_hi << 32)
> > > > |
> > > > +				params->dump_tag_lo));
> > > > +		return -EINVAL;
> > > > +	}
> > > > +
> > > > +	/*
> > > > +	 * The hypervisor returns status 0 if no more data
> > > > available to
> > > > +	 * download. The dump will be invalidated with ioctl
> > > > (see
> > > > below).
> > > > +	 */
> > > > +	if (params->status == RTAS_IBM_PLATFORM_DUMP_COMPLETE)
> > > > {
> > > > +		params->buf_length = 0;
> > > > +		/*
> > > > +		 * Returns 0 to the user space so that user
> > > > +		 * space read stops.
> > > > +		 */
> > > > +		return 0;
> > > > +	}
> > > > +
> > > > +	if (size < SZ_1K) {
> > > > +		pr_err_once("Buffer length should be minimum
> > > > 1024
> > > > bytes\n");
> > > > +		return -EINVAL;
> > > > +	} else if (size > params->buf_length) {
> > > > +		/*
> > > > +		 * Allocate 4K work area. So if the user
> > > > requests > 4K,
> > > > +		 * resize the buffer length.
> > > > +		 */
> > > > +		size = params->buf_length;
> > > > +	}
> > > > +
> > > > +	fwrc = rtas_ibm_platform_dump(params,
> > > > +			rtas_work_area_phys(params->work_area),
> > > > +			size);
> > > > +	if (fwrc < 0)
> > > > +		return fwrc;
> > > > +
> > > > +	total_bytes = (u64) (((u64)params->bytes_ret_hi << 32)
> > > > |
> > > > +			params->bytes_ret_lo);
> > > > +
> > > > +	/*
> > > > +	 * Kernel or firmware bug, do not continue.
> > > > +	 */
> > > > +	if (WARN(total_bytes > size, "possible write beyond end
> > > > of work
> > > > area"))
> > > > +		return -EFAULT;
> > > > +
> > > > +	if (copy_to_user(buf, rtas_work_area_raw_buf(params-
> > > > > work_area),
> > > > +			total_bytes))
> > > > +		return -EFAULT;
> > > > +
> > > > +	return total_bytes;
> > > > +}
> > > > +
> > > > +static int papr_platform_dump_handle_release(struct inode
> > > > *inode,
> > > > +					struct file *file)
> > > > +{
> > > > +	struct ibm_platform_dump_params *params = file-
> > > > >private_data;
> > > > +
> > > > +	if (params->work_area)
> > > > +		rtas_work_area_free(params->work_area);
> > > > +
> > > > +	mutex_lock(&platform_dump_list_mutex);
> > > > +	list_del(&params->list);
> > > > +	mutex_unlock(&platform_dump_list_mutex);
> > > > +
> > > > +	kfree(params);
> > > > +	file->private_data = NULL;
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +/*
> > > > + * This ioctl is used to invalidate the dump assuming the user
> > > > space
> > > > + * issue this ioctl after obtain the complete dump.
> > > > + * Issue the last RTAS call with NULL buffer to invalidate the
> > > > dump
> > > > + * which means dump will be freed in the hypervisor.
> > > > + */
> > > > +static long papr_platform_dump_invalidate_ioctl(struct file
> > > > *file,
> > > > +				unsigned int ioctl, unsigned
> > > > long arg)
> > > > +{
> > > > +	struct ibm_platform_dump_params *params;
> > > > +	u64 __user *argp = (void __user *)arg;
> > > > +	u64 param_dump_tag, dump_tag;
> > > > +
> > > > +	if (ioctl != PAPR_PLATFORM_DUMP_IOC_INVALIDATE)
> > > > +		return -ENOIOCTLCMD;
> > > > +
> > > > +	if (get_user(dump_tag, argp))
> > > > +		return -EFAULT;
> > > > +
> > > > +	/*
> > > > +	 * private_data is freeded during release(), so should
> > > > not
> > > 			   freed?
> > > > +	 * happen.
> > > > +	 */
> > > > +	if (!file->private_data) {
> > > > +		pr_err("No valid FD to invalidate dump for the
> > > > ID(%llu)\n",
> > > > +				dump_tag);
> > > > +		return -EINVAL;
> > > > +	}
> > > > +
> > > > +	params = file->private_data;
> > > > +	param_dump_tag = (u64) (((u64)params->dump_tag_hi <<
> > > > 32) |
> > > > +				params->dump_tag_lo);
> > > > +	if (dump_tag != param_dump_tag) {
> > > > +		pr_err("Invalid dump ID(%llu) to invalidate
> > > > dump\n",
> > > > +				dump_tag);
> > > > +		return -EINVAL;
> > > > +	}
> > > > +
> > > > +	if (params->status != RTAS_IBM_PLATFORM_DUMP_COMPLETE)
> > > > +		pr_warn("Platform dump is not complete, but
> > > > requested "
> > > > +			"to invalidate dump for ID(%llu)\n",
> > > > +			dump_tag);
> > > 
> > > Not sure if something should be done here or if relying on
> > > translation
> > > of the error from the RTAS call is advisable.
> > 
> > This check just diplays message in case if the user initiated to
> > invalidate the dump without saving it completely. Then invalidates
> > the
> > dump with RTAS call and retuns the RTAS return value. 
> > 
> > As mentioned above, platform-dump is available only on non-HMC
> > based
> > systems. So invoke the collection of dump by BMC based interface,
> > not
> > widely used. I can remove this check if preferred.
> 
> From the previous discussion it sounds like trying to invalidate the
> dump without first reading it in full is an error.

Thanks for your suggestions.

Yes, it was doing as part of read() calls. But explicit ioctl to
invalidate here. I was thinking like user space removing FD without
reading or writing operation.

> 
> The state to detect this error is tracked which makes it possible to
> produce this warning.
> 
> Then it's also possible to handle the error without roundtrip to the
> hypervisor.

Do you prefer return en error without invalidating if the dump is not
read completely? Sure we can.

if (params->status != RTAS_IBM_PLATFORM_DUMP_COMPLETE) {
	pr_err("Platform dump is not complete, but requested "
		"to invalidate dump for ID(%llu)\n",
		dump_tag);
	return -EPERM;
}

Thanks
Haren

> 
> Thanks
> 
> Michal
> 
> > Thanks
> > Haren
> > 
> > > Thanks
> > > 
> > > Michal

Re: [PATCH v2 6/6] powerpc/pseries: Add papr-platform-dump character driver for dump retrieval

[Michal Suchánek]

On Thu, Feb 06, 2025 at 07:28:14AM -0800, Haren Myneni wrote:
> On Thu, 2025-02-06 at 10:18 +0100, Michal Suchánek wrote:
> > On Wed, Feb 05, 2025 at 11:51:19PM -0800, Haren Myneni wrote:
> > > On Wed, 2025-02-05 at 15:28 +0100, Michal Suchánek wrote:
> > > > Hello,
> > > > 
> > > > thanks for working on this!
> > > > 
> > > > I see in thes version you ended up reusing the existing RTAS call
> > > > code
> > > > which looks better.
> > > > 
> > > > From the past discussion it sounds like the get-indices call can
> > > > list
> > > > the available dumps, and I do not see this connection documented.
> > > > 
> > > > Also the part about it not being used in practice by the service
> > > > that
> > > > retrieves the dumps because it gets a message from the hypervisor
> > > > with
> > > > the dump id when a dump is available.
> > > 
> > > ibm,get-indices is used to obtain data based on indicator and
> > > sensor,
> > > not related to platform dump. ibm,platform-dump is used only on
> > > non-HMC 
> > > based systems and BMC interface initiates manually to save the dump
> > > on
> > > tne partition. Sorry in case caused confusion in the previous
> > > discussion.
> > > 
> > > > 
> > > > On Fri, Jan 10, 2025 at 04:30:08PM -0800, Haren Myneni wrote:
> > > > > ibm,platform-dump RTAS call in combination with writable
> > > > > mapping
> > > > > /dev/mem is issued to collect platform dump from the hypervisor
> > > > > and may need multiple calls to get the complete dump. The
> > > > > current
> > > > > implementation uses rtas_platform_dump() API provided by
> > > > > librtas
> > > > > library to issue these RTAS calls. But /dev/mem access by the
> > > > > user space is prohibited under system lockdown.
> > > > > 
> > > > > The solution should be to restrict access to RTAS function in
> > > > > user
> > > > > space and provide kernel interfaces to collect dump. This patch
> > > > > adds papr-platform-dump character driver and expose standard
> > > > > interfaces such as open / ioctl/ read to user space in ways
> > > > > that
> > > > > are compatible with lockdown.
> > > > > 
> > > > > PAPR (7.3.3.4.1 ibm,platform-dump) provides a method to obtain
> > > > > the complete dump:
> > > > > - Each dump will be identified by ID called dump tag.
> > > > > - A sequence of RTAS calls have to be issued until retrieve the
> > > > >   complete dump. The hypervisor expects the first RTAS call
> > > > > with
> > > > >   the sequence 0 and the subsequent calls with the sequence
> > > > >   number returned from the previous calls.
> > > > > - The hypervisor returns "dump complete" status once the
> > > > > complete
> > > > >   dump is retrieved. But expects one more RTAS call from the
> > > > >   partition with the NULL buffer to invalidate dump which means
> > > > >   the dump will be removed in the hypervisor.
> > > > > - Sequence of calls are allowed with different dump IDs at the
> > > > >   same time but not with the same dump ID.
> > > > > 
> > > > > Expose these interfaces to user space with a /dev/papr-
> > > > > platform-
> > > > > dump
> > > > > character device using the following programming model:
> > > > > 
> > > > >    int devfd = open("/dev/papr-platform-dump", O_RDONLY);
> > > > >    int fd = ioctl(devfd,PAPR_PLATFORM_DUMP_IOC_CREATE_HANDLE,
> > > > > &dump_id)
> > > > > 	- Restrict user space to access with the same dump ID.
> > > > >           Typically we do not expect user space requests the
> > > > > dump
> > > > >           again for the same dump ID.
> > > > >    char *buf = malloc(size);
> > > > >    length = read(fd, buf, size);
> > > > >         - size should be minimum 1K based on PAPR and  <= 4K
> > > > > based
> > > > >           on RTAS work area size. It will be restrict to RTAS
> > > > > work
> > > > >           area size. Using 4K work area based on the current
> > > > >           implementation in librtas library
> > > > >         - Each read call issue RTAS call to get the data based
> > > > > on
> > > > >           the size requirement and returns bytes returned from
> > > > > the
> > > > >           hypervisor
> > > > >         - If the previous call returns dump complete status,
> > > > > the
> > > > >           next read returns 0 like EOF.
> > > > >    ret = ioctl(PAPR_PLATFORM_DUMP_IOC_INVALIDATE, &dump_id)
> > > > > 	- RTAS call with NULL buffer to invalidates the dump.
> > > > > 
> > > > > The read API should use the file descriptor obtained from ioctl
> > > > > based on dump ID so that gets dump contents for the
> > > > > corresponding
> > > > > dump ID. Implemented support in librtas (rtas_platform_dump())
> > > > > for
> > > > > this new ABI to support system lockdown.
> > > > > 
> > > > > Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> > > > > ---
> > > > >  .../include/uapi/asm/papr-platform-dump.h     |  15 +
> > > > >  arch/powerpc/platforms/pseries/Makefile       |   1 +
> > > > >  .../platforms/pseries/papr-platform-dump.c    | 408
> > > > > ++++++++++++++++++
> > > > >  3 files changed, 424 insertions(+)
> > > > >  create mode 100644 arch/powerpc/include/uapi/asm/papr-
> > > > > platform-
> > > > > dump.h
> > > > >  create mode 100644 arch/powerpc/platforms/pseries/papr-
> > > > > platform-
> > > > > dump.c
> > > > > 
> > > > > diff --git a/arch/powerpc/include/uapi/asm/papr-platform-dump.h
> > > > > b/arch/powerpc/include/uapi/asm/papr-platform-dump.h
> > > > > new file mode 100644
> > > > > index 000000000000..3a0f152e3ce8
> > > > > --- /dev/null
> > > > > +++ b/arch/powerpc/include/uapi/asm/papr-platform-dump.h
> > > > > @@ -0,0 +1,15 @@
> > > > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > > > +#ifndef _UAPI_PAPR_PLATFORM_DUMP_H_
> > > > > +#define _UAPI_PAPR_PLATFORM_DUMP_H_
> > > > > +
> > > > > +#include <asm/ioctl.h>
> > > > > +#include <asm/papr-miscdev.h>
> > > > > +
> > > > > +/*
> > > > > + * ioctl for /dev/papr-platform-dump. Returns a VPD handle fd
> > > > > corresponding to
> > > > > + * the location code.
> > > > > + */
> > > > > +#define PAPR_PLATFORM_DUMP_IOC_CREATE_HANDLE
> > > > > _IOW(PAPR_MISCDEV_IOC_ID, 6, __u64)
> > > > > +#define
> > > > > PAPR_PLATFORM_DUMP_IOC_INVALIDATE    _IOW(PAPR_MISCDEV_IOC_ID,
> > > > > 7,
> > > > > __u64)
> > > > > +
> > > > > +#endif /* _UAPI_PAPR_PLATFORM_DUMP_H_ */
> > > > > diff --git a/arch/powerpc/platforms/pseries/Makefile
> > > > > b/arch/powerpc/platforms/pseries/Makefile
> > > > > index e1db61877bb9..c82c94e0a73c 100644
> > > > > --- a/arch/powerpc/platforms/pseries/Makefile
> > > > > +++ b/arch/powerpc/platforms/pseries/Makefile
> > > > > @@ -4,6 +4,7 @@ ccflags-$(CONFIG_PPC_PSERIES_DEBUG)	+=
> > > > > -DDEBUG
> > > > >  obj-y			:= lpar.o hvCall.o nvram.o reconfig.o \
> > > > >  			   of_helpers.o rtas-work-area.o papr-
> > > > > sysparm.o 
> > > > > \
> > > > >  			   papr-rtas-common.o papr-vpd.o papr-
> > > > > indices.o 
> > > > > \
> > > > > +			   papr-platform-dump.o \
> > > > >  			   setup.o iommu.o event_sources.o
> > > > > ras.o \
> > > > >  			   firmware.o power.o dlpar.o
> > > > > mobility.o rng.o
> > > > > \
> > > > >  			   pci.o pci_dlpar.o eeh_pseries.o
> > > > > msi.o \
> > > > > diff --git a/arch/powerpc/platforms/pseries/papr-platform-
> > > > > dump.c
> > > > > b/arch/powerpc/platforms/pseries/papr-platform-dump.c
> > > > > new file mode 100644
> > > > > index 000000000000..13a418d7c37e
> > > > > --- /dev/null
> > > > > +++ b/arch/powerpc/platforms/pseries/papr-platform-dump.c
> > > > > @@ -0,0 +1,408 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > > +
> > > > > +#define pr_fmt(fmt) "papr-platform-dump: " fmt
> > > > > +
> > > > > +#include <linux/anon_inodes.h>
> > > > > +#include <linux/file.h>
> > > > > +#include <linux/fs.h>
> > > > > +#include <linux/init.h>
> > > > > +#include <linux/kernel.h>
> > > > > +#include <linux/miscdevice.h>
> > > > > +#include <asm/machdep.h>
> > > > > +#include <asm/rtas-work-area.h>
> > > > > +#include <asm/rtas.h>
> > > > > +#include <uapi/asm/papr-platform-dump.h>
> > > > > +
> > > > > +/*
> > > > > + * Function-specific return values for ibm,platform-dump,
> > > > > derived
> > > > > from
> > > > > + * PAPR+ v2.13 7.3.3.4.1 "ibm,platform-dump RTAS Call".
> > > > > + */
> > > > > +#define	RTAS_IBM_PLATFORM_DUMP_COMPLETE	0	/*
> > > > > Complete
> > > > > dump retrieved. */
> > > > > +#define	RTAS_IBM_PLATFORM_DUMP_CONTINUE	1	/*
> > > > > Continue
> > > > > dump */
> > > > > +#define	RTAS_NOT_AUTHORIZED		-9002	/* Not
> > > > > Authorized
> > > > > */
> > > > > +
> > > > > +#define	RTAS_IBM_PLATFORM_DUMP_START	2 /* Linux
> > > > > status
> > > > > to start dump */
> > > > > +
> > > > > +/**
> > > > > + * struct ibm_platform_dump_params - Parameters (in and out)
> > > > > for
> > > > > + *                                              ibm,platform-
> > > > > dump
> > > > > + * @work_area:		In: work area buffer for results.
> > > > > + * @buf_length:		In: work area buffer length in
> > > > > bytes
> > > > > + * @dump_tag_hi:	In: Most-significant 32 bits of a
> > > > > Dump_Tag
> > > > > representing
> > > > > + *                      an id of the dump being processed.
> > > > > + * @dump_tag_lo:	In: Least-significant 32 bits of a
> > > > > Dump_Tag
> > > > > representing
> > > > > + *                      an id of the dump being processed.
> > > > > + * @sequence_hi:	In: Sequence number in most-significant 
> > > > > 32
> > > > > bits.
> > > > > + *                      Out: Next sequence number in most-
> > > > > significant 32 bits.
> > > > > + * @sequence_lo:	In: Sequence number in Least-
> > > > > significant 32
> > > > > bits
> > > > > + *                      Out: Next sequence number in Least-
> > > > > significant 32 bits.
> > > > > + * @bytes_ret_hi:	Out: Bytes written in most-significant
> > > > > 32 bits.
> > > > > + * @bytes_ret_lo:	Out: Bytes written in Least-significant 
> > > > > 32
> > > > > bits.
> > > > > + * @status:		Out: RTAS call status.
> > > > > + * @list:		Maintain the list of dumps are in
> > > > > progress. Can
> > > > > + *                      retrieve multiple dumps with different
> > > > > dump IDs at
> > > > > + *                      the same time but not with the same
> > > > > dump
> > > > > ID. This list
> > > > > + *                      is used to determine whether the dump
> > > > > for
> > > > > the same ID
> > > > > + *                      is in progress.
> > > > > + */
> > > > > +struct ibm_platform_dump_params {
> > > > > +	struct rtas_work_area	*work_area;
> > > > > +	u32			buf_length;
> > > > > +	u32			dump_tag_hi;
> > > > > +	u32			dump_tag_lo;
> > > > > +	u32			sequence_hi;
> > > > > +	u32			sequence_lo;
> > > > > +	u32			bytes_ret_hi;
> > > > > +	u32			bytes_ret_lo;
> > > > > +	s32			status;
> > > > > +	struct list_head	list;
> > > > > +};
> > > > > +
> > > > > +/*
> > > > > + * Multiple dumps with different dump IDs can be retrieved at
> > > > > the
> > > > > same
> > > > > + * time, but not with dame dump ID. platform_dump_list_mutex
> > > > > and
> > > > > + * platform_dump_list are used to prevent this behavior.
> > > > > + */
> > > > > +static DEFINE_MUTEX(platform_dump_list_mutex);
> > > > > +static LIST_HEAD(platform_dump_list);
> > > > > +
> > > > > +/**
> > > > > + * rtas_ibm_platform_dump() - Call ibm,platform-dump to fill a
> > > > > work area
> > > > > + * buffer.
> > > > > + * @params: See &struct ibm_platform_dump_params.
> > > > > + * @buf_addr: Address of dump buffer (work_area)
> > > > > + * @buf_length: Length of the buffer in bytes (min. 1024)
> > > > > + *
> > > > > + * Calls ibm,platform-dump until it errors or successfully
> > > > > deposits data
> > > > > + * into the supplied work area. Handles RTAS retry statuses.
> > > > > Maps
> > > > > RTAS
> > > > > + * error statuses to reasonable errno values.
> > > > > + *
> > > > > + * Can request multiple dumps with different dump IDs at the
> > > > > same
> > > > > time,
> > > > > + * but not with the same dump ID which is prevented with the
> > > > > check
> > > > > in
> > > > > + * the ioctl code (papr_platform_dump_create_handle()).
> > > > > + *
> > > > > + * The caller should inspect @params.status to determine
> > > > > whether
> > > > > more
> > > > > + * calls are needed to complete the sequence.
> > > > > + *
> > > > > + * Context: May sleep.
> > > > > + * Return: -ve on error, 0 for dump complete and 1 for
> > > > > continue
> > > > > dump
> > > > > + */
> > > > > +static int rtas_ibm_platform_dump(struct
> > > > > ibm_platform_dump_params
> > > > > *params,
> > > > > +				phys_addr_t buf_addr, u32
> > > > > buf_length)
> > > > > +{
> > > > > +	u32 rets[4];
> > > > > +	s32 fwrc;
> > > > > +	int ret = 0;
> > > > > +
> > > > > +	do {
> > > > > +		fwrc =
> > > > > rtas_call(rtas_function_token(RTAS_FN_IBM_PLATFORM_DUMP),
> > > > > +				6, 5,
> > > > > +				rets,
> > > > > +				params->dump_tag_hi,
> > > > > +				params->dump_tag_lo,
> > > > > +				params->sequence_hi,
> > > > > +				params->sequence_lo,
> > > > > +				buf_addr,
> > > > > +				buf_length);
> > > > > +	} while (rtas_busy_delay(fwrc));
> > > > > +
> > > > > +	switch (fwrc) {
> > > > > +	case RTAS_HARDWARE_ERROR:
> > > > > +		ret = -EIO;
> > > > > +		break;
> > > > > +	case RTAS_NOT_AUTHORIZED:
> > > > > +		ret = -EPERM;
> > > > > +		break;
> > > > > +	case RTAS_IBM_PLATFORM_DUMP_CONTINUE:
> > > > > +	case RTAS_IBM_PLATFORM_DUMP_COMPLETE:
> > > > > +		params->sequence_hi = rets[0];
> > > > > +		params->sequence_lo = rets[1];
> > > > > +		params->bytes_ret_hi = rets[2];
> > > > > +		params->bytes_ret_lo = rets[3];
> > > > > +		break;
> > > > > +	default:
> > > > > +		ret = -EIO;
> > > > > +		pr_err_ratelimited("unexpected ibm,platform-
> > > > > dump status
> > > > > %d\n",
> > > > > +				fwrc);
> > > > > +		break;
> > > > > +	}
> > > > > +
> > > > > +	params->status = fwrc;
> > > > > +	return ret;
> > > > > +}
> > > > > +
> > > > > +/*
> > > > > + * Platform dump is used with multiple RTAS calls to retrieve
> > > > > the
> > > > > + * complete dump for the provided dump ID. Once the complete
> > > > > dump
> > > > > is
> > > > > + * retrieved, the hypervisor returns dump complete status (0)
> > > > > for
> > > > > the
> > > > > + * last RTAS call and expects the caller issues one more call
> > > > > with
> > > > > + * NULL buffer to invalidate the dump so that the hypervisor
> > > > > can
> > > > > remove
> > > > > + * the dump.
> > > > > + *
> > > > > + * After the specific dump is invalidated in the hypervisor,
> > > > > expect the
> > > > > + * dump complete status for the new sequence - the user space
> > > > > initiates
> > > > > + * new request for the same dump ID.
> > > > > + */
> > > > > +static ssize_t papr_platform_dump_handle_read(struct file
> > > > > *file,
> > > > > +		char __user *buf, size_t size, loff_t *off)
> > > > > +{
> > > > > +	struct ibm_platform_dump_params *params = file-
> > > > > >private_data;
> > > > > +	u64 total_bytes;
> > > > > +	s32 fwrc;
> > > > > +
> > > > > +	/*
> > > > > +	 * Dump already completed with the previous read calls.
> > > > > +	 * In case if the user space issues further reads,
> > > > > returns
> > > > > +	 * -EINVAL.
> > > > > +	 */
> > > > > +	if (!params->buf_length) {
> > > > > +		pr_warn_once("Platform dump completed for dump
> > > > > ID
> > > > > %llu\n",
> > > > > +			(u64) (((u64)params->dump_tag_hi << 32)
> > > > > |
> > > > > +				params->dump_tag_lo));
> > > > > +		return -EINVAL;
> > > > > +	}
> > > > > +
> > > > > +	/*
> > > > > +	 * The hypervisor returns status 0 if no more data
> > > > > available to
> > > > > +	 * download. The dump will be invalidated with ioctl
> > > > > (see
> > > > > below).
> > > > > +	 */
> > > > > +	if (params->status == RTAS_IBM_PLATFORM_DUMP_COMPLETE)
> > > > > {
> > > > > +		params->buf_length = 0;
> > > > > +		/*
> > > > > +		 * Returns 0 to the user space so that user
> > > > > +		 * space read stops.
> > > > > +		 */
> > > > > +		return 0;
> > > > > +	}
> > > > > +
> > > > > +	if (size < SZ_1K) {
> > > > > +		pr_err_once("Buffer length should be minimum
> > > > > 1024
> > > > > bytes\n");
> > > > > +		return -EINVAL;
> > > > > +	} else if (size > params->buf_length) {
> > > > > +		/*
> > > > > +		 * Allocate 4K work area. So if the user
> > > > > requests > 4K,
> > > > > +		 * resize the buffer length.
> > > > > +		 */
> > > > > +		size = params->buf_length;
> > > > > +	}
> > > > > +
> > > > > +	fwrc = rtas_ibm_platform_dump(params,
> > > > > +			rtas_work_area_phys(params->work_area),
> > > > > +			size);
> > > > > +	if (fwrc < 0)
> > > > > +		return fwrc;
> > > > > +
> > > > > +	total_bytes = (u64) (((u64)params->bytes_ret_hi << 32)
> > > > > |
> > > > > +			params->bytes_ret_lo);
> > > > > +
> > > > > +	/*
> > > > > +	 * Kernel or firmware bug, do not continue.
> > > > > +	 */
> > > > > +	if (WARN(total_bytes > size, "possible write beyond end
> > > > > of work
> > > > > area"))
> > > > > +		return -EFAULT;
> > > > > +
> > > > > +	if (copy_to_user(buf, rtas_work_area_raw_buf(params-
> > > > > > work_area),
> > > > > +			total_bytes))
> > > > > +		return -EFAULT;
> > > > > +
> > > > > +	return total_bytes;
> > > > > +}
> > > > > +
> > > > > +static int papr_platform_dump_handle_release(struct inode
> > > > > *inode,
> > > > > +					struct file *file)
> > > > > +{
> > > > > +	struct ibm_platform_dump_params *params = file-
> > > > > >private_data;
> > > > > +
> > > > > +	if (params->work_area)
> > > > > +		rtas_work_area_free(params->work_area);
> > > > > +
> > > > > +	mutex_lock(&platform_dump_list_mutex);
> > > > > +	list_del(&params->list);
> > > > > +	mutex_unlock(&platform_dump_list_mutex);
> > > > > +
> > > > > +	kfree(params);
> > > > > +	file->private_data = NULL;
> > > > > +	return 0;
> > > > > +}
> > > > > +
> > > > > +/*
> > > > > + * This ioctl is used to invalidate the dump assuming the user
> > > > > space
> > > > > + * issue this ioctl after obtain the complete dump.
> > > > > + * Issue the last RTAS call with NULL buffer to invalidate the
> > > > > dump
> > > > > + * which means dump will be freed in the hypervisor.
> > > > > + */
> > > > > +static long papr_platform_dump_invalidate_ioctl(struct file
> > > > > *file,
> > > > > +				unsigned int ioctl, unsigned
> > > > > long arg)
> > > > > +{
> > > > > +	struct ibm_platform_dump_params *params;
> > > > > +	u64 __user *argp = (void __user *)arg;
> > > > > +	u64 param_dump_tag, dump_tag;
> > > > > +
> > > > > +	if (ioctl != PAPR_PLATFORM_DUMP_IOC_INVALIDATE)
> > > > > +		return -ENOIOCTLCMD;
> > > > > +
> > > > > +	if (get_user(dump_tag, argp))
> > > > > +		return -EFAULT;
> > > > > +
> > > > > +	/*
> > > > > +	 * private_data is freeded during release(), so should
> > > > > not
> > > > 			   freed?
> > > > > +	 * happen.
> > > > > +	 */
> > > > > +	if (!file->private_data) {
> > > > > +		pr_err("No valid FD to invalidate dump for the
> > > > > ID(%llu)\n",
> > > > > +				dump_tag);
> > > > > +		return -EINVAL;
> > > > > +	}
> > > > > +
> > > > > +	params = file->private_data;
> > > > > +	param_dump_tag = (u64) (((u64)params->dump_tag_hi <<
> > > > > 32) |
> > > > > +				params->dump_tag_lo);
> > > > > +	if (dump_tag != param_dump_tag) {
> > > > > +		pr_err("Invalid dump ID(%llu) to invalidate
> > > > > dump\n",
> > > > > +				dump_tag);
> > > > > +		return -EINVAL;
> > > > > +	}
> > > > > +
> > > > > +	if (params->status != RTAS_IBM_PLATFORM_DUMP_COMPLETE)
> > > > > +		pr_warn("Platform dump is not complete, but
> > > > > requested "
> > > > > +			"to invalidate dump for ID(%llu)\n",
> > > > > +			dump_tag);
> > > > 
> > > > Not sure if something should be done here or if relying on
> > > > translation
> > > > of the error from the RTAS call is advisable.
> > > 
> > > This check just diplays message in case if the user initiated to
> > > invalidate the dump without saving it completely. Then invalidates
> > > the
> > > dump with RTAS call and retuns the RTAS return value. 
> > > 
> > > As mentioned above, platform-dump is available only on non-HMC
> > > based
> > > systems. So invoke the collection of dump by BMC based interface,
> > > not
> > > widely used. I can remove this check if preferred.
> > 
> > From the previous discussion it sounds like trying to invalidate the
> > dump without first reading it in full is an error.
> 
> Thanks for your suggestions.
> 
> Yes, it was doing as part of read() calls. But explicit ioctl to
> invalidate here. I was thinking like user space removing FD without
> reading or writing operation.

And is it possible to invalidate the dump without reading it fully
first?

If not then there is no point trying to do the call that is known to
fail anyway.

Thanks

Michal

> 
> > 
> > The state to detect this error is tracked which makes it possible to
> > produce this warning.
> > 
> > Then it's also possible to handle the error without roundtrip to the
> > hypervisor.
> 
> Do you prefer return en error without invalidating if the dump is not
> read completely? Sure we can.
> 
> if (params->status != RTAS_IBM_PLATFORM_DUMP_COMPLETE) {
> 	pr_err("Platform dump is not complete, but requested "
> 		"to invalidate dump for ID(%llu)\n",
> 		dump_tag);
> 	return -EPERM;
> }
> 
> Thanks
> Haren
> 
> > 
> > Thanks
> > 
> > Michal
> > 
> > > Thanks
> > > Haren
> > > 
> > > > Thanks
> > > > 
> > > > Michal
> 

Re: [PATCH v2 6/6] powerpc/pseries: Add papr-platform-dump character driver for dump retrieval

[Haren Myneni]

On Thu, 2025-02-06 at 16:32 +0100, Michal Suchánek wrote:
> On Thu, Feb 06, 2025 at 07:28:14AM -0800, Haren Myneni wrote:
> > On Thu, 2025-02-06 at 10:18 +0100, Michal Suchánek wrote:
> > > On Wed, Feb 05, 2025 at 11:51:19PM -0800, Haren Myneni wrote:
> > > > On Wed, 2025-02-05 at 15:28 +0100, Michal Suchánek wrote:
> > > > > Hello,
> > > > > 
> > > > > thanks for working on this!
> > > > > 
> > > > > I see in thes version you ended up reusing the existing RTAS
> > > > > call
> > > > > code
> > > > > which looks better.
> > > > > 
> > > > > From the past discussion it sounds like the get-indices call
> > > > > can
> > > > > list
> > > > > the available dumps, and I do not see this connection
> > > > > documented.
> > > > > 
> > > > > Also the part about it not being used in practice by the
> > > > > service
> > > > > that
> > > > > retrieves the dumps because it gets a message from the
> > > > > hypervisor
> > > > > with
> > > > > the dump id when a dump is available.
> > > > 
> > > > ibm,get-indices is used to obtain data based on indicator and
> > > > sensor,
> > > > not related to platform dump. ibm,platform-dump is used only on
> > > > non-HMC 
> > > > based systems and BMC interface initiates manually to save the
> > > > dump
> > > > on
> > > > tne partition. Sorry in case caused confusion in the previous
> > > > discussion.
> > > > 
> > > > > On Fri, Jan 10, 2025 at 04:30:08PM -0800, Haren Myneni wrote:
> > > > > > ibm,platform-dump RTAS call in combination with writable
> > > > > > mapping
> > > > > > /dev/mem is issued to collect platform dump from the
> > > > > > hypervisor
> > > > > > and may need multiple calls to get the complete dump. The
> > > > > > current
> > > > > > implementation uses rtas_platform_dump() API provided by
> > > > > > librtas
> > > > > > library to issue these RTAS calls. But /dev/mem access by
> > > > > > the
> > > > > > user space is prohibited under system lockdown.
> > > > > > 
> > > > > > The solution should be to restrict access to RTAS function
> > > > > > in
> > > > > > user
> > > > > > space and provide kernel interfaces to collect dump. This
> > > > > > patch
> > > > > > adds papr-platform-dump character driver and expose
> > > > > > standard
> > > > > > interfaces such as open / ioctl/ read to user space in ways
> > > > > > that
> > > > > > are compatible with lockdown.
> > > > > > 
> > > > > > PAPR (7.3.3.4.1 ibm,platform-dump) provides a method to
> > > > > > obtain
> > > > > > the complete dump:
> > > > > > - Each dump will be identified by ID called dump tag.
> > > > > > - A sequence of RTAS calls have to be issued until retrieve
> > > > > > the
> > > > > >   complete dump. The hypervisor expects the first RTAS call
> > > > > > with
> > > > > >   the sequence 0 and the subsequent calls with the sequence
> > > > > >   number returned from the previous calls.
> > > > > > - The hypervisor returns "dump complete" status once the
> > > > > > complete
> > > > > >   dump is retrieved. But expects one more RTAS call from
> > > > > > the
> > > > > >   partition with the NULL buffer to invalidate dump which
> > > > > > means
> > > > > >   the dump will be removed in the hypervisor.
> > > > > > - Sequence of calls are allowed with different dump IDs at
> > > > > > the
> > > > > >   same time but not with the same dump ID.
> > > > > > 
> > > > > > Expose these interfaces to user space with a /dev/papr-
> > > > > > platform-
> > > > > > dump
> > > > > > character device using the following programming model:
> > > > > > 
> > > > > >    int devfd = open("/dev/papr-platform-dump", O_RDONLY);
> > > > > >    int fd =
> > > > > > ioctl(devfd,PAPR_PLATFORM_DUMP_IOC_CREATE_HANDLE,
> > > > > > &dump_id)
> > > > > > 	- Restrict user space to access with the same dump ID.
> > > > > >           Typically we do not expect user space requests
> > > > > > the
> > > > > > dump
> > > > > >           again for the same dump ID.
> > > > > >    char *buf = malloc(size);
> > > > > >    length = read(fd, buf, size);
> > > > > >         - size should be minimum 1K based on PAPR and  <=
> > > > > > 4K
> > > > > > based
> > > > > >           on RTAS work area size. It will be restrict to
> > > > > > RTAS
> > > > > > work
> > > > > >           area size. Using 4K work area based on the
> > > > > > current
> > > > > >           implementation in librtas library
> > > > > >         - Each read call issue RTAS call to get the data
> > > > > > based
> > > > > > on
> > > > > >           the size requirement and returns bytes returned
> > > > > > from
> > > > > > the
> > > > > >           hypervisor
> > > > > >         - If the previous call returns dump complete
> > > > > > status,
> > > > > > the
> > > > > >           next read returns 0 like EOF.
> > > > > >    ret = ioctl(PAPR_PLATFORM_DUMP_IOC_INVALIDATE, &dump_id)
> > > > > > 	- RTAS call with NULL buffer to invalidates the dump.
> > > > > > 
> > > > > > The read API should use the file descriptor obtained from
> > > > > > ioctl
> > > > > > based on dump ID so that gets dump contents for the
> > > > > > corresponding
> > > > > > dump ID. Implemented support in librtas
> > > > > > (rtas_platform_dump())
> > > > > > for
> > > > > > this new ABI to support system lockdown.
> > > > > > 
> > > > > > Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> > > > > > ---
> > > > > >  .../include/uapi/asm/papr-platform-dump.h     |  15 +
> > > > > >  arch/powerpc/platforms/pseries/Makefile       |   1 +
> > > > > >  .../platforms/pseries/papr-platform-dump.c    | 408
> > > > > > ++++++++++++++++++
> > > > > >  3 files changed, 424 insertions(+)
> > > > > >  create mode 100644 arch/powerpc/include/uapi/asm/papr-
> > > > > > platform-
> > > > > > dump.h
> > > > > >  create mode 100644 arch/powerpc/platforms/pseries/papr-
> > > > > > platform-
> > > > > > dump.c
> > > > > > 
> > > > > > diff --git a/arch/powerpc/include/uapi/asm/papr-platform-
> > > > > > dump.h
> > > > > > b/arch/powerpc/include/uapi/asm/papr-platform-dump.h
> > > > > > new file mode 100644
> > > > > > index 000000000000..3a0f152e3ce8
> > > > > > --- /dev/null
> > > > > > +++ b/arch/powerpc/include/uapi/asm/papr-platform-dump.h
> > > > > > @@ -0,0 +1,15 @@
> > > > > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-
> > > > > > note */
> > > > > > +#ifndef _UAPI_PAPR_PLATFORM_DUMP_H_
> > > > > > +#define _UAPI_PAPR_PLATFORM_DUMP_H_
> > > > > > +
> > > > > > +#include <asm/ioctl.h>
> > > > > > +#include <asm/papr-miscdev.h>
> > > > > > +
> > > > > > +/*
> > > > > > + * ioctl for /dev/papr-platform-dump. Returns a VPD handle
> > > > > > fd
> > > > > > corresponding to
> > > > > > + * the location code.
> > > > > > + */
> > > > > > +#define PAPR_PLATFORM_DUMP_IOC_CREATE_HANDLE
> > > > > > _IOW(PAPR_MISCDEV_IOC_ID, 6, __u64)
> > > > > > +#define
> > > > > > PAPR_PLATFORM_DUMP_IOC_INVALIDATE    _IOW(PAPR_MISCDEV_IOC_
> > > > > > ID,
> > > > > > 7,
> > > > > > __u64)
> > > > > > +
> > > > > > +#endif /* _UAPI_PAPR_PLATFORM_DUMP_H_ */
> > > > > > diff --git a/arch/powerpc/platforms/pseries/Makefile
> > > > > > b/arch/powerpc/platforms/pseries/Makefile
> > > > > > index e1db61877bb9..c82c94e0a73c 100644
> > > > > > --- a/arch/powerpc/platforms/pseries/Makefile
> > > > > > +++ b/arch/powerpc/platforms/pseries/Makefile
> > > > > > @@ -4,6 +4,7 @@ ccflags-$(CONFIG_PPC_PSERIES_DEBUG)	+=
> > > > > > -DDEBUG
> > > > > >  obj-y			:= lpar.o hvCall.o nvram.o
> > > > > > reconfig.o \
> > > > > >  			   of_helpers.o rtas-work-area.o papr-
> > > > > > sysparm.o 
> > > > > > \
> > > > > >  			   papr-rtas-common.o papr-vpd.o papr-
> > > > > > indices.o 
> > > > > > \
> > > > > > +			   papr-platform-dump.o \
> > > > > >  			   setup.o iommu.o event_sources.o
> > > > > > ras.o \
> > > > > >  			   firmware.o power.o dlpar.o
> > > > > > mobility.o rng.o
> > > > > > \
> > > > > >  			   pci.o pci_dlpar.o eeh_pseries.o
> > > > > > msi.o \
> > > > > > diff --git a/arch/powerpc/platforms/pseries/papr-platform-
> > > > > > dump.c
> > > > > > b/arch/powerpc/platforms/pseries/papr-platform-dump.c
> > > > > > new file mode 100644
> > > > > > index 000000000000..13a418d7c37e
> > > > > > --- /dev/null
> > > > > > +++ b/arch/powerpc/platforms/pseries/papr-platform-dump.c
> > > > > > @@ -0,0 +1,408 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > > > +
> > > > > > +#define pr_fmt(fmt) "papr-platform-dump: " fmt
> > > > > > +
> > > > > > +#include <linux/anon_inodes.h>
> > > > > > +#include <linux/file.h>
> > > > > > +#include <linux/fs.h>
> > > > > > +#include <linux/init.h>
> > > > > > +#include <linux/kernel.h>
> > > > > > +#include <linux/miscdevice.h>
> > > > > > +#include <asm/machdep.h>
> > > > > > +#include <asm/rtas-work-area.h>
> > > > > > +#include <asm/rtas.h>
> > > > > > +#include <uapi/asm/papr-platform-dump.h>
> > > > > > +
> > > > > > +/*
> > > > > > + * Function-specific return values for ibm,platform-dump,
> > > > > > derived
> > > > > > from
> > > > > > + * PAPR+ v2.13 7.3.3.4.1 "ibm,platform-dump RTAS Call".
> > > > > > + */
> > > > > > +#define	RTAS_IBM_PLATFORM_DUMP_COMPLETE	0	/*
> > > > > > Complete
> > > > > > dump retrieved. */
> > > > > > +#define	RTAS_IBM_PLATFORM_DUMP_CONTINUE	1	/*
> > > > > > Continue
> > > > > > dump */
> > > > > > +#define	RTAS_NOT_AUTHORIZED		-9002	/* Not
> > > > > > Authorized
> > > > > > */
> > > > > > +
> > > > > > +#define	RTAS_IBM_PLATFORM_DUMP_START	2 /* Linux
> > > > > > status
> > > > > > to start dump */
> > > > > > +
> > > > > > +/**
> > > > > > + * struct ibm_platform_dump_params - Parameters (in and
> > > > > > out)
> > > > > > for
> > > > > > +
> > > > > > *                                              ibm,platform
> > > > > > -
> > > > > > dump
> > > > > > + * @work_area:		In: work area buffer for
> > > > > > results.
> > > > > > + * @buf_length:		In: work area buffer length in
> > > > > > bytes
> > > > > > + * @dump_tag_hi:	In: Most-significant 32 bits of a
> > > > > > Dump_Tag
> > > > > > representing
> > > > > > + *                      an id of the dump being processed.
> > > > > > + * @dump_tag_lo:	In: Least-significant 32 bits of a
> > > > > > Dump_Tag
> > > > > > representing
> > > > > > + *                      an id of the dump being processed.
> > > > > > + * @sequence_hi:	In: Sequence number in most-
> > > > > > significant 
> > > > > > 32
> > > > > > bits.
> > > > > > + *                      Out: Next sequence number in most-
> > > > > > significant 32 bits.
> > > > > > + * @sequence_lo:	In: Sequence number in Least-
> > > > > > significant 32
> > > > > > bits
> > > > > > + *                      Out: Next sequence number in
> > > > > > Least-
> > > > > > significant 32 bits.
> > > > > > + * @bytes_ret_hi:	Out: Bytes written in most-significant
> > > > > > 32 bits.
> > > > > > + * @bytes_ret_lo:	Out: Bytes written in Least-
> > > > > > significant 
> > > > > > 32
> > > > > > bits.
> > > > > > + * @status:		Out: RTAS call status.
> > > > > > + * @list:		Maintain the list of dumps are in
> > > > > > progress. Can
> > > > > > + *                      retrieve multiple dumps with
> > > > > > different
> > > > > > dump IDs at
> > > > > > + *                      the same time but not with the
> > > > > > same
> > > > > > dump
> > > > > > ID. This list
> > > > > > + *                      is used to determine whether the
> > > > > > dump
> > > > > > for
> > > > > > the same ID
> > > > > > + *                      is in progress.
> > > > > > + */
> > > > > > +struct ibm_platform_dump_params {
> > > > > > +	struct rtas_work_area	*work_area;
> > > > > > +	u32			buf_length;
> > > > > > +	u32			dump_tag_hi;
> > > > > > +	u32			dump_tag_lo;
> > > > > > +	u32			sequence_hi;
> > > > > > +	u32			sequence_lo;
> > > > > > +	u32			bytes_ret_hi;
> > > > > > +	u32			bytes_ret_lo;
> > > > > > +	s32			status;
> > > > > > +	struct list_head	list;
> > > > > > +};
> > > > > > +
> > > > > > +/*
> > > > > > + * Multiple dumps with different dump IDs can be retrieved
> > > > > > at
> > > > > > the
> > > > > > same
> > > > > > + * time, but not with dame dump ID.
> > > > > > platform_dump_list_mutex
> > > > > > and
> > > > > > + * platform_dump_list are used to prevent this behavior.
> > > > > > + */
> > > > > > +static DEFINE_MUTEX(platform_dump_list_mutex);
> > > > > > +static LIST_HEAD(platform_dump_list);
> > > > > > +
> > > > > > +/**
> > > > > > + * rtas_ibm_platform_dump() - Call ibm,platform-dump to
> > > > > > fill a
> > > > > > work area
> > > > > > + * buffer.
> > > > > > + * @params: See &struct ibm_platform_dump_params.
> > > > > > + * @buf_addr: Address of dump buffer (work_area)
> > > > > > + * @buf_length: Length of the buffer in bytes (min. 1024)
> > > > > > + *
> > > > > > + * Calls ibm,platform-dump until it errors or successfully
> > > > > > deposits data
> > > > > > + * into the supplied work area. Handles RTAS retry
> > > > > > statuses.
> > > > > > Maps
> > > > > > RTAS
> > > > > > + * error statuses to reasonable errno values.
> > > > > > + *
> > > > > > + * Can request multiple dumps with different dump IDs at
> > > > > > the
> > > > > > same
> > > > > > time,
> > > > > > + * but not with the same dump ID which is prevented with
> > > > > > the
> > > > > > check
> > > > > > in
> > > > > > + * the ioctl code (papr_platform_dump_create_handle()).
> > > > > > + *
> > > > > > + * The caller should inspect @params.status to determine
> > > > > > whether
> > > > > > more
> > > > > > + * calls are needed to complete the sequence.
> > > > > > + *
> > > > > > + * Context: May sleep.
> > > > > > + * Return: -ve on error, 0 for dump complete and 1 for
> > > > > > continue
> > > > > > dump
> > > > > > + */
> > > > > > +static int rtas_ibm_platform_dump(struct
> > > > > > ibm_platform_dump_params
> > > > > > *params,
> > > > > > +				phys_addr_t buf_addr, u32
> > > > > > buf_length)
> > > > > > +{
> > > > > > +	u32 rets[4];
> > > > > > +	s32 fwrc;
> > > > > > +	int ret = 0;
> > > > > > +
> > > > > > +	do {
> > > > > > +		fwrc =
> > > > > > rtas_call(rtas_function_token(RTAS_FN_IBM_PLATFORM_DUMP),
> > > > > > +				6, 5,
> > > > > > +				rets,
> > > > > > +				params->dump_tag_hi,
> > > > > > +				params->dump_tag_lo,
> > > > > > +				params->sequence_hi,
> > > > > > +				params->sequence_lo,
> > > > > > +				buf_addr,
> > > > > > +				buf_length);
> > > > > > +	} while (rtas_busy_delay(fwrc));
> > > > > > +
> > > > > > +	switch (fwrc) {
> > > > > > +	case RTAS_HARDWARE_ERROR:
> > > > > > +		ret = -EIO;
> > > > > > +		break;
> > > > > > +	case RTAS_NOT_AUTHORIZED:
> > > > > > +		ret = -EPERM;
> > > > > > +		break;
> > > > > > +	case RTAS_IBM_PLATFORM_DUMP_CONTINUE:
> > > > > > +	case RTAS_IBM_PLATFORM_DUMP_COMPLETE:
> > > > > > +		params->sequence_hi = rets[0];
> > > > > > +		params->sequence_lo = rets[1];
> > > > > > +		params->bytes_ret_hi = rets[2];
> > > > > > +		params->bytes_ret_lo = rets[3];
> > > > > > +		break;
> > > > > > +	default:
> > > > > > +		ret = -EIO;
> > > > > > +		pr_err_ratelimited("unexpected ibm,platform-
> > > > > > dump status
> > > > > > %d\n",
> > > > > > +				fwrc);
> > > > > > +		break;
> > > > > > +	}
> > > > > > +
> > > > > > +	params->status = fwrc;
> > > > > > +	return ret;
> > > > > > +}
> > > > > > +
> > > > > > +/*
> > > > > > + * Platform dump is used with multiple RTAS calls to
> > > > > > retrieve
> > > > > > the
> > > > > > + * complete dump for the provided dump ID. Once the
> > > > > > complete
> > > > > > dump
> > > > > > is
> > > > > > + * retrieved, the hypervisor returns dump complete status
> > > > > > (0)
> > > > > > for
> > > > > > the
> > > > > > + * last RTAS call and expects the caller issues one more
> > > > > > call
> > > > > > with
> > > > > > + * NULL buffer to invalidate the dump so that the
> > > > > > hypervisor
> > > > > > can
> > > > > > remove
> > > > > > + * the dump.
> > > > > > + *
> > > > > > + * After the specific dump is invalidated in the
> > > > > > hypervisor,
> > > > > > expect the
> > > > > > + * dump complete status for the new sequence - the user
> > > > > > space
> > > > > > initiates
> > > > > > + * new request for the same dump ID.
> > > > > > + */
> > > > > > +static ssize_t papr_platform_dump_handle_read(struct file
> > > > > > *file,
> > > > > > +		char __user *buf, size_t size, loff_t *off)
> > > > > > +{
> > > > > > +	struct ibm_platform_dump_params *params = file-
> > > > > > > private_data;
> > > > > > +	u64 total_bytes;
> > > > > > +	s32 fwrc;
> > > > > > +
> > > > > > +	/*
> > > > > > +	 * Dump already completed with the previous read calls.
> > > > > > +	 * In case if the user space issues further reads,
> > > > > > returns
> > > > > > +	 * -EINVAL.
> > > > > > +	 */
> > > > > > +	if (!params->buf_length) {
> > > > > > +		pr_warn_once("Platform dump completed for dump
> > > > > > ID
> > > > > > %llu\n",
> > > > > > +			(u64) (((u64)params->dump_tag_hi << 32)
> > > > > > +				params->dump_tag_lo));
> > > > > > +		return -EINVAL;
> > > > > > +	}
> > > > > > +
> > > > > > +	/*
> > > > > > +	 * The hypervisor returns status 0 if no more data
> > > > > > available to
> > > > > > +	 * download. The dump will be invalidated with ioctl
> > > > > > (see
> > > > > > below).
> > > > > > +	 */
> > > > > > +	if (params->status == RTAS_IBM_PLATFORM_DUMP_COMPLETE)
> > > > > > {
> > > > > > +		params->buf_length = 0;
> > > > > > +		/*
> > > > > > +		 * Returns 0 to the user space so that user
> > > > > > +		 * space read stops.
> > > > > > +		 */
> > > > > > +		return 0;
> > > > > > +	}
> > > > > > +
> > > > > > +	if (size < SZ_1K) {
> > > > > > +		pr_err_once("Buffer length should be minimum
> > > > > > 1024
> > > > > > bytes\n");
> > > > > > +		return -EINVAL;
> > > > > > +	} else if (size > params->buf_length) {
> > > > > > +		/*
> > > > > > +		 * Allocate 4K work area. So if the user
> > > > > > requests > 4K,
> > > > > > +		 * resize the buffer length.
> > > > > > +		 */
> > > > > > +		size = params->buf_length;
> > > > > > +	}
> > > > > > +
> > > > > > +	fwrc = rtas_ibm_platform_dump(params,
> > > > > > +			rtas_work_area_phys(params->work_area),
> > > > > > +			size);
> > > > > > +	if (fwrc < 0)
> > > > > > +		return fwrc;
> > > > > > +
> > > > > > +	total_bytes = (u64) (((u64)params->bytes_ret_hi << 32)
> > > > > > +			params->bytes_ret_lo);
> > > > > > +
> > > > > > +	/*
> > > > > > +	 * Kernel or firmware bug, do not continue.
> > > > > > +	 */
> > > > > > +	if (WARN(total_bytes > size, "possible write beyond end
> > > > > > of work
> > > > > > area"))
> > > > > > +		return -EFAULT;
> > > > > > +
> > > > > > +	if (copy_to_user(buf, rtas_work_area_raw_buf(params-
> > > > > > > work_area),
> > > > > > +			total_bytes))
> > > > > > +		return -EFAULT;
> > > > > > +
> > > > > > +	return total_bytes;
> > > > > > +}
> > > > > > +
> > > > > > +static int papr_platform_dump_handle_release(struct inode
> > > > > > *inode,
> > > > > > +					struct file *file)
> > > > > > +{
> > > > > > +	struct ibm_platform_dump_params *params = file-
> > > > > > > private_data;
> > > > > > +
> > > > > > +	if (params->work_area)
> > > > > > +		rtas_work_area_free(params->work_area);
> > > > > > +
> > > > > > +	mutex_lock(&platform_dump_list_mutex);
> > > > > > +	list_del(&params->list);
> > > > > > +	mutex_unlock(&platform_dump_list_mutex);
> > > > > > +
> > > > > > +	kfree(params);
> > > > > > +	file->private_data = NULL;
> > > > > > +	return 0;
> > > > > > +}
> > > > > > +
> > > > > > +/*
> > > > > > + * This ioctl is used to invalidate the dump assuming the
> > > > > > user
> > > > > > space
> > > > > > + * issue this ioctl after obtain the complete dump.
> > > > > > + * Issue the last RTAS call with NULL buffer to invalidate
> > > > > > the
> > > > > > dump
> > > > > > + * which means dump will be freed in the hypervisor.
> > > > > > + */
> > > > > > +static long papr_platform_dump_invalidate_ioctl(struct
> > > > > > file
> > > > > > *file,
> > > > > > +				unsigned int ioctl, unsigned
> > > > > > long arg)
> > > > > > +{
> > > > > > +	struct ibm_platform_dump_params *params;
> > > > > > +	u64 __user *argp = (void __user *)arg;
> > > > > > +	u64 param_dump_tag, dump_tag;
> > > > > > +
> > > > > > +	if (ioctl != PAPR_PLATFORM_DUMP_IOC_INVALIDATE)
> > > > > > +		return -ENOIOCTLCMD;
> > > > > > +
> > > > > > +	if (get_user(dump_tag, argp))
> > > > > > +		return -EFAULT;
> > > > > > +
> > > > > > +	/*
> > > > > > +	 * private_data is freeded during release(), so should
> > > > > > not
> > > > > 			   freed?
> > > > > > +	 * happen.
> > > > > > +	 */
> > > > > > +	if (!file->private_data) {
> > > > > > +		pr_err("No valid FD to invalidate dump for the
> > > > > > ID(%llu)\n",
> > > > > > +				dump_tag);
> > > > > > +		return -EINVAL;
> > > > > > +	}
> > > > > > +
> > > > > > +	params = file->private_data;
> > > > > > +	param_dump_tag = (u64) (((u64)params->dump_tag_hi <<
> > > > > > 32) |
> > > > > > +				params->dump_tag_lo);
> > > > > > +	if (dump_tag != param_dump_tag) {
> > > > > > +		pr_err("Invalid dump ID(%llu) to invalidate
> > > > > > dump\n",
> > > > > > +				dump_tag);
> > > > > > +		return -EINVAL;
> > > > > > +	}
> > > > > > +
> > > > > > +	if (params->status != RTAS_IBM_PLATFORM_DUMP_COMPLETE)
> > > > > > +		pr_warn("Platform dump is not complete, but
> > > > > > requested "
> > > > > > +			"to invalidate dump for ID(%llu)\n",
> > > > > > +			dump_tag);
> > > > > 
> > > > > Not sure if something should be done here or if relying on
> > > > > translation
> > > > > of the error from the RTAS call is advisable.
> > > > 
> > > > This check just diplays message in case if the user initiated
> > > > to
> > > > invalidate the dump without saving it completely. Then
> > > > invalidates
> > > > the
> > > > dump with RTAS call and retuns the RTAS return value. 
> > > > 
> > > > As mentioned above, platform-dump is available only on non-HMC
> > > > based
> > > > systems. So invoke the collection of dump by BMC based
> > > > interface,
> > > > not
> > > > widely used. I can remove this check if preferred.
> > > 
> > > From the previous discussion it sounds like trying to invalidate
> > > the
> > > dump without first reading it in full is an error.
> > 
> > Thanks for your suggestions.
> > 
> > Yes, it was doing as part of read() calls. But explicit ioctl to
> > invalidate here. I was thinking like user space removing FD without
> > reading or writing operation.
> 
> And is it possible to invalidate the dump without reading it fully
> first?
> 
> If not then there is no point trying to do the call that is known to
> fail anyway.

Generally not possible if uses librtas API rtas_platform_dump() which
reads the dump completely and then the application calls this API
explicitly to invalidate the dump (with buffer NULL - as doing in the
current implementation). The current use case is extract_platdump
command (ppc64-diag package)

extract_platdump() {   /* we are not chamging this implementation */
 
status = rtas_platform_dump() - initial call
while !dump_complete {
	status = rtas_platform_dump()
	if (status < 0) failure
	if (status == 0) dump_complete = 1
}

status = rtas_platform_dump() -  to invalidate dump by passing buffer =
NULL

We should not expect using any command other than extract_platdump
since the use case of collecting platform dump is narrow and is only on
non-hmc based systems.

Hence added warning message if the dump is not completely read and
invalidate the dump like removing file by mistake.

But I like your suggestion of returning an error (-EPERM) if not saved
the dump completely.

Thanks
Haren

> 
> Thanks
> 
> Michal
> 
> > > The state to detect this error is tracked which makes it possible
> > > to
> > > produce this warning.
> > > 
> > > Then it's also possible to handle the error without roundtrip to
> > > the
> > > hypervisor.
> > 
> > Do you prefer return en error without invalidating if the dump is
> > not
> > read completely? Sure we can.
> > 
> > if (params->status != RTAS_IBM_PLATFORM_DUMP_COMPLETE) {
> > 	pr_err("Platform dump is not complete, but requested "
> > 		"to invalidate dump for ID(%llu)\n",
> > 		dump_tag);
> > 	return -EPERM;
> > }
> > 
> > Thanks
> > Haren
> > 
> > > Thanks
> > > 
> > > Michal
> > > 
> > > > Thanks
> > > > Haren
> > > > 
> > > > > Thanks
> > > > > 
> > > > > Michal

Re: [PATCH v2 6/6] powerpc/pseries: Add papr-platform-dump character driver for dump retrieval

[Michal Suchánek]

On Thu, Feb 06, 2025 at 10:34:42AM -0800, Haren Myneni wrote:
> On Thu, 2025-02-06 at 16:32 +0100, Michal Suchánek wrote:
> > On Thu, Feb 06, 2025 at 07:28:14AM -0800, Haren Myneni wrote:
> > > On Thu, 2025-02-06 at 10:18 +0100, Michal Suchánek wrote:
> > > > On Wed, Feb 05, 2025 at 11:51:19PM -0800, Haren Myneni wrote:
> > > > > On Wed, 2025-02-05 at 15:28 +0100, Michal Suchánek wrote:

> > > > > > > +
> > > > > > > +	if (params->status != RTAS_IBM_PLATFORM_DUMP_COMPLETE)
> > > > > > > +		pr_warn("Platform dump is not complete, but
> > > > > > > requested "
> > > > > > > +			"to invalidate dump for ID(%llu)\n",
> > > > > > > +			dump_tag);
> > > > > > 
> > > > > > Not sure if something should be done here or if relying on
> > > > > > translation
> > > > > > of the error from the RTAS call is advisable.
> > > > > 
> > > > > This check just diplays message in case if the user initiated
> > > > > to
> > > > > invalidate the dump without saving it completely. Then
> > > > > invalidates
> > > > > the
> > > > > dump with RTAS call and retuns the RTAS return value. 
> > > > > 
> > > > > As mentioned above, platform-dump is available only on non-HMC
> > > > > based
> > > > > systems. So invoke the collection of dump by BMC based
> > > > > interface,
> > > > > not
> > > > > widely used. I can remove this check if preferred.
> > > > 
> > > > From the previous discussion it sounds like trying to invalidate
> > > > the
> > > > dump without first reading it in full is an error.
> > > 
> > > Thanks for your suggestions.
> > > 
> > > Yes, it was doing as part of read() calls. But explicit ioctl to
> > > invalidate here. I was thinking like user space removing FD without
> > > reading or writing operation.
> > 
> > And is it possible to invalidate the dump without reading it fully
> > first?
> > 
> > If not then there is no point trying to do the call that is known to
> > fail anyway.
> 
> Generally not possible if uses librtas API rtas_platform_dump() which
> reads the dump completely and then the application calls this API
> explicitly to invalidate the dump (with buffer NULL - as doing in the
> current implementation). The current use case is extract_platdump
> command (ppc64-diag package)
> 
> extract_platdump() {   /* we are not chamging this implementation */
>  
> status = rtas_platform_dump() - initial call
> while !dump_complete {
> 	status = rtas_platform_dump()
> 	if (status < 0) failure
> 	if (status == 0) dump_complete = 1
> }
> 
> status = rtas_platform_dump() -  to invalidate dump by passing buffer =
> NULL
> 
> We should not expect using any command other than extract_platdump
> since the use case of collecting platform dump is narrow and is only on
> non-hmc based systems.
> 
> Hence added warning message if the dump is not completely read and
> invalidate the dump like removing file by mistake.
> 
> But I like your suggestion of returning an error (-EPERM) if not saved
> the dump completely.

I think EPERM is not correct in this case. It's not a problem of
permission but of incorrect state.

There are some errors around that like EBUSY or EINPROGRESS.

Thanks

Micahl

Re: [PATCH v2 6/6] powerpc/pseries: Add papr-platform-dump character driver for dump retrieval

[Haren Myneni]

On Thu, 2025-02-06 at 20:53 +0100, Michal Suchánek wrote:
> On Thu, Feb 06, 2025 at 10:34:42AM -0800, Haren Myneni wrote:
> > On Thu, 2025-02-06 at 16:32 +0100, Michal Suchánek wrote:
> > > On Thu, Feb 06, 2025 at 07:28:14AM -0800, Haren Myneni wrote:
> > > > On Thu, 2025-02-06 at 10:18 +0100, Michal Suchánek wrote:
> > > > > On Wed, Feb 05, 2025 at 11:51:19PM -0800, Haren Myneni wrote:
> > > > > > On Wed, 2025-02-05 at 15:28 +0100, Michal Suchánek wrote:
> > > > > > > > +
> > > > > > > > +	if (params->status !=
> > > > > > > > RTAS_IBM_PLATFORM_DUMP_COMPLETE)
> > > > > > > > +		pr_warn("Platform dump is not complete,
> > > > > > > > but
> > > > > > > > requested "
> > > > > > > > +			"to invalidate dump for
> > > > > > > > ID(%llu)\n",
> > > > > > > > +			dump_tag);
> > > > > > > 
> > > > > > > Not sure if something should be done here or if relying
> > > > > > > on
> > > > > > > translation
> > > > > > > of the error from the RTAS call is advisable.
> > > > > > 
> > > > > > This check just diplays message in case if the user
> > > > > > initiated
> > > > > > to
> > > > > > invalidate the dump without saving it completely. Then
> > > > > > invalidates
> > > > > > the
> > > > > > dump with RTAS call and retuns the RTAS return value. 
> > > > > > 
> > > > > > As mentioned above, platform-dump is available only on non-
> > > > > > HMC
> > > > > > based
> > > > > > systems. So invoke the collection of dump by BMC based
> > > > > > interface,
> > > > > > not
> > > > > > widely used. I can remove this check if preferred.
> > > > > 
> > > > > From the previous discussion it sounds like trying to
> > > > > invalidate
> > > > > the
> > > > > dump without first reading it in full is an error.
> > > > 
> > > > Thanks for your suggestions.
> > > > 
> > > > Yes, it was doing as part of read() calls. But explicit ioctl
> > > > to
> > > > invalidate here. I was thinking like user space removing FD
> > > > without
> > > > reading or writing operation.
> > > 
> > > And is it possible to invalidate the dump without reading it
> > > fully
> > > first?
> > > 
> > > If not then there is no point trying to do the call that is known
> > > to
> > > fail anyway.
> > 
> > Generally not possible if uses librtas API rtas_platform_dump()
> > which
> > reads the dump completely and then the application calls this API
> > explicitly to invalidate the dump (with buffer NULL - as doing in
> > the
> > current implementation). The current use case is extract_platdump
> > command (ppc64-diag package)
> > 
> > extract_platdump() {   /* we are not chamging this implementation
> > */
> >  
> > status = rtas_platform_dump() - initial call
> > while !dump_complete {
> > 	status = rtas_platform_dump()
> > 	if (status < 0) failure
> > 	if (status == 0) dump_complete = 1
> > }
> > 
> > status = rtas_platform_dump() -  to invalidate dump by passing
> > buffer =
> > NULL
> > 
> > We should not expect using any command other than extract_platdump
> > since the use case of collecting platform dump is narrow and is
> > only on
> > non-hmc based systems.
> > 
> > Hence added warning message if the dump is not completely read and
> > invalidate the dump like removing file by mistake.
> > 
> > But I like your suggestion of returning an error (-EPERM) if not
> > saved
> > the dump completely.
> 
> I think EPERM is not correct in this case. It's not a problem of
> permission but of incorrect state.
> 
> There are some errors around that like EBUSY or EINPROGRESS.

Thanks for your suggestions - will use EINPROGRESS. 

> 
> Thanks
> 
> Micahl