* [PATCH 0/7] powerpc/pseries: new character devices for system parameters and VPD
@ 2023-10-06 21:01 ` Nathan Lynch via B4 Relay
0 siblings, 0 replies; 20+ messages in thread
From: Nathan Lynch @ 2023-10-06 21:01 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Michal Suchánek
Cc: linuxppc-dev, tyreld, gcwilson, Nathan Lynch
Add character devices that expose PAPR-specific system parameters and
VPD to user space.
The problem: important platform features are enabled on Linux VMs
through the powerpc-specific rtas() syscall in combination with
writeable mappings of /dev/mem. In typical usage, this is encapsulated
behind APIs provided by the librtas library. This paradigm is
incompatible with lockdown, which prohibits /dev/mem access. It also
is too low-level in many cases: a single logical operation may require
multiple sys_rtas() calls in succession to complete. This carries the
risk that a process may exit while leaving an operation unfinished. It
also means that callers must coordinate their use of the syscall for
functions that cannot tolerate multiple concurrent clients, such as
ibm,get-vpd.
The solution presented here is to add a pair of small pseries-specific
"drivers," one for VPD and one for system parameters. The new drivers
expose these facilities to user space in ways that are compatible with
lockdown and require no coordination between their clients.
Both drivers could potentially support poll() methods to notify
clients of changes to parameters or VPD that happen due to partition
migration and other events. But that should be safe to leave for
later, assuming there's any interest.
I have made changes to librtas to prefer the new interfaces and
verified that existing clients work correctly with the new code. A
draft PR for that work is here:
https://github.com/ibm-power-utilities/librtas/pull/36
I expect to propose at least one more small driver in this style for
platform dump retrieval in a separate submission in the future. I
consider the work in this series mature enough now to request review
for inclusion as-is.
---
Changes in v1 vs initial RFC:
- Add papr-sysparm driver and tests.
- Add a papr-miscdev.h uapi header.
- Prevent sys_rtas() from interfering with papr-vpd call sequences.
- Handle -4 ("VPD changed") status in papr-vpd.
- Include string_helpers.h in papr-vpd.c, per Michal Suchánek
- Link to RFC: https://lore.kernel.org/r/20230822-papr-sys_rtas-vs-lockdown-v1-0-932623cf3c7b@linux.ibm.com
---
Nathan Lynch (7):
powerpc/uapi: export papr-miscdev.h header
powerpc/pseries: papr-vpd char driver for VPD retrieval
powerpc/rtas: serialize ibm,get-vpd service with papr-vpd sequences
powerpc/pseries/papr-sysparm: validate buffer object lengths
powerpc/pseries/papr-sysparm: expose chardev API to user space
powerpc/selftests: add test for papr-vpd
powerpc/selftests: add test for papr-sysparm
Documentation/userspace-api/ioctl/ioctl-number.rst | 4 +
arch/powerpc/include/asm/papr-sysparm.h | 17 +-
arch/powerpc/include/asm/papr-vpd.h | 18 +
arch/powerpc/include/uapi/asm/papr-miscdev.h | 9 +
arch/powerpc/include/uapi/asm/papr-sysparm.h | 58 +++
arch/powerpc/include/uapi/asm/papr-vpd.h | 22 +
arch/powerpc/kernel/rtas.c | 26 +
arch/powerpc/platforms/pseries/Makefile | 1 +
arch/powerpc/platforms/pseries/papr-sysparm.c | 207 +++++++-
arch/powerpc/platforms/pseries/papr-vpd.c | 542 +++++++++++++++++++++
tools/testing/selftests/powerpc/Makefile | 2 +
.../selftests/powerpc/papr_sysparm/.gitignore | 1 +
.../selftests/powerpc/papr_sysparm/Makefile | 12 +
.../selftests/powerpc/papr_sysparm/papr_sysparm.c | 164 +++++++
.../testing/selftests/powerpc/papr_vpd/.gitignore | 1 +
tools/testing/selftests/powerpc/papr_vpd/Makefile | 12 +
.../testing/selftests/powerpc/papr_vpd/papr_vpd.c | 352 +++++++++++++
17 files changed, 1440 insertions(+), 8 deletions(-)
---
base-commit: eddc90ea2af5933249ea1a78119f2c8ef8d07156
change-id: 20230817-papr-sys_rtas-vs-lockdown-5c54505db792
Best regards,
--
Nathan Lynch <nathanl@linux.ibm.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/7] powerpc/pseries: new character devices for system parameters and VPD
@ 2023-10-06 21:01 ` Nathan Lynch via B4 Relay
0 siblings, 0 replies; 20+ messages in thread
From: Nathan Lynch via B4 Relay @ 2023-10-06 21:01 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Michal Suchánek
Cc: Nathan Lynch, tyreld, gcwilson, linuxppc-dev
Add character devices that expose PAPR-specific system parameters and
VPD to user space.
The problem: important platform features are enabled on Linux VMs
through the powerpc-specific rtas() syscall in combination with
writeable mappings of /dev/mem. In typical usage, this is encapsulated
behind APIs provided by the librtas library. This paradigm is
incompatible with lockdown, which prohibits /dev/mem access. It also
is too low-level in many cases: a single logical operation may require
multiple sys_rtas() calls in succession to complete. This carries the
risk that a process may exit while leaving an operation unfinished. It
also means that callers must coordinate their use of the syscall for
functions that cannot tolerate multiple concurrent clients, such as
ibm,get-vpd.
The solution presented here is to add a pair of small pseries-specific
"drivers," one for VPD and one for system parameters. The new drivers
expose these facilities to user space in ways that are compatible with
lockdown and require no coordination between their clients.
Both drivers could potentially support poll() methods to notify
clients of changes to parameters or VPD that happen due to partition
migration and other events. But that should be safe to leave for
later, assuming there's any interest.
I have made changes to librtas to prefer the new interfaces and
verified that existing clients work correctly with the new code. A
draft PR for that work is here:
https://github.com/ibm-power-utilities/librtas/pull/36
I expect to propose at least one more small driver in this style for
platform dump retrieval in a separate submission in the future. I
consider the work in this series mature enough now to request review
for inclusion as-is.
---
Changes in v1 vs initial RFC:
- Add papr-sysparm driver and tests.
- Add a papr-miscdev.h uapi header.
- Prevent sys_rtas() from interfering with papr-vpd call sequences.
- Handle -4 ("VPD changed") status in papr-vpd.
- Include string_helpers.h in papr-vpd.c, per Michal Suchánek
- Link to RFC: https://lore.kernel.org/r/20230822-papr-sys_rtas-vs-lockdown-v1-0-932623cf3c7b@linux.ibm.com
---
Nathan Lynch (7):
powerpc/uapi: export papr-miscdev.h header
powerpc/pseries: papr-vpd char driver for VPD retrieval
powerpc/rtas: serialize ibm,get-vpd service with papr-vpd sequences
powerpc/pseries/papr-sysparm: validate buffer object lengths
powerpc/pseries/papr-sysparm: expose chardev API to user space
powerpc/selftests: add test for papr-vpd
powerpc/selftests: add test for papr-sysparm
Documentation/userspace-api/ioctl/ioctl-number.rst | 4 +
arch/powerpc/include/asm/papr-sysparm.h | 17 +-
arch/powerpc/include/asm/papr-vpd.h | 18 +
arch/powerpc/include/uapi/asm/papr-miscdev.h | 9 +
arch/powerpc/include/uapi/asm/papr-sysparm.h | 58 +++
arch/powerpc/include/uapi/asm/papr-vpd.h | 22 +
arch/powerpc/kernel/rtas.c | 26 +
arch/powerpc/platforms/pseries/Makefile | 1 +
arch/powerpc/platforms/pseries/papr-sysparm.c | 207 +++++++-
arch/powerpc/platforms/pseries/papr-vpd.c | 542 +++++++++++++++++++++
tools/testing/selftests/powerpc/Makefile | 2 +
.../selftests/powerpc/papr_sysparm/.gitignore | 1 +
.../selftests/powerpc/papr_sysparm/Makefile | 12 +
.../selftests/powerpc/papr_sysparm/papr_sysparm.c | 164 +++++++
.../testing/selftests/powerpc/papr_vpd/.gitignore | 1 +
tools/testing/selftests/powerpc/papr_vpd/Makefile | 12 +
.../testing/selftests/powerpc/papr_vpd/papr_vpd.c | 352 +++++++++++++
17 files changed, 1440 insertions(+), 8 deletions(-)
---
base-commit: eddc90ea2af5933249ea1a78119f2c8ef8d07156
change-id: 20230817-papr-sys_rtas-vs-lockdown-5c54505db792
Best regards,
--
Nathan Lynch <nathanl@linux.ibm.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/7] powerpc/uapi: export papr-miscdev.h header
2023-10-06 21:01 ` Nathan Lynch via B4 Relay
@ 2023-10-06 21:01 ` Nathan Lynch
-1 siblings, 0 replies; 20+ messages in thread
From: Nathan Lynch via B4 Relay @ 2023-10-06 21:01 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Michal Suchánek
Cc: Nathan Lynch, tyreld, gcwilson, linuxppc-dev
From: Nathan Lynch <nathanl@linux.ibm.com>
Allocate one identifying code (the first column of the ioctl-number
table) for the collection of PAPR miscdev drivers to share.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
arch/powerpc/include/uapi/asm/papr-miscdev.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/powerpc/include/uapi/asm/papr-miscdev.h b/arch/powerpc/include/uapi/asm/papr-miscdev.h
new file mode 100644
index 000000000000..49a2a270b7f3
--- /dev/null
+++ b/arch/powerpc/include/uapi/asm/papr-miscdev.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_PAPR_MISCDEV_H_
+#define _UAPI_PAPR_MISCDEV_H_
+
+enum {
+ PAPR_MISCDEV_IOC_ID = 0xb2,
+};
+
+#endif /* _UAPI_PAPR_MISCDEV_H_ */
--
2.41.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 1/7] powerpc/uapi: export papr-miscdev.h header
@ 2023-10-06 21:01 ` Nathan Lynch
0 siblings, 0 replies; 20+ messages in thread
From: Nathan Lynch @ 2023-10-06 21:01 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Michal Suchánek
Cc: linuxppc-dev, tyreld, gcwilson, Nathan Lynch
Allocate one identifying code (the first column of the ioctl-number
table) for the collection of PAPR miscdev drivers to share.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
arch/powerpc/include/uapi/asm/papr-miscdev.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/powerpc/include/uapi/asm/papr-miscdev.h b/arch/powerpc/include/uapi/asm/papr-miscdev.h
new file mode 100644
index 000000000000..49a2a270b7f3
--- /dev/null
+++ b/arch/powerpc/include/uapi/asm/papr-miscdev.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_PAPR_MISCDEV_H_
+#define _UAPI_PAPR_MISCDEV_H_
+
+enum {
+ PAPR_MISCDEV_IOC_ID = 0xb2,
+};
+
+#endif /* _UAPI_PAPR_MISCDEV_H_ */
--
2.41.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/7] powerpc/pseries: papr-vpd char driver for VPD retrieval
2023-10-06 21:01 ` Nathan Lynch via B4 Relay
@ 2023-10-06 21:01 ` Nathan Lynch
-1 siblings, 0 replies; 20+ messages in thread
From: Nathan Lynch via B4 Relay @ 2023-10-06 21:01 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Michal Suchánek
Cc: Nathan Lynch, tyreld, gcwilson, linuxppc-dev
From: Nathan Lynch <nathanl@linux.ibm.com>
PowerVM LPARs may retrieve Vital Product Data (VPD) for system
components using the ibm,get-vpd RTAS function.
We can expose this to user space with a /dev/papr-vpd character
device, where the programming model is:
struct papr_location_code plc = { .str = "", }; /* obtain all VPD */
int devfd = open("/dev/papr-vpd", O_RDONLY);
int vpdfd = ioctl(devfd, PAPR_VPD_CREATE_HANDLE, &plc);
size_t size = lseek(vpdfd, 0, SEEK_END);
char *buf = malloc(size);
pread(devfd, buf, size, 0);
When a file descriptor is obtained from ioctl(PAPR_VPD_CREATE_HANDLE),
the file contains the result of a complete ibm,get-vpd sequence. The
file contents are immutable from the POV of user space. To get a new
view of the VPD, the client must create a new handle.
This design choice insulates user space from most of the complexities
that ibm,get-vpd brings:
* ibm,get-vpd must be called more than once to obtain complete
results.
* Only one ibm,get-vpd call sequence should be in progress at a time;
interleaved sequences will disrupt each other. Callers must have a
protocol for serializing their use of the function.
* A call sequence in progress may receive a "VPD changed, try again"
status, requiring the client to abandon the sequence and start
over.
The memory required for the VPD buffers seems acceptable, around 20KB
for all VPD on one of my systems. And the value of the
/rtas/ibm,vpd-size DT property (the estimated maximum size of VPD) is
consistently 300KB across various systems I've checked.
I've implemented support for this new ABI in the rtas_get_vpd()
function in librtas, which the vpdupdate command currently uses to
populate its VPD database. I've verified that an unmodified vpdupdate
binary generates an identical database when using a librtas.so that
prefers the new ABI.
Note that the driver needs to serialize its call sequences with legacy
sys_rtas(ibm,get-vpd) callers, so it exposes its internal lock for
sys_rtas.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
Documentation/userspace-api/ioctl/ioctl-number.rst | 2 +
arch/powerpc/include/asm/papr-vpd.h | 18 +
arch/powerpc/include/uapi/asm/papr-vpd.h | 22 +
arch/powerpc/platforms/pseries/Makefile | 1 +
arch/powerpc/platforms/pseries/papr-vpd.c | 542 +++++++++++++++++++++
5 files changed, 585 insertions(+)
diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
index 4ea5b837399a..a950545bf7cd 100644
--- a/Documentation/userspace-api/ioctl/ioctl-number.rst
+++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
@@ -349,6 +349,8 @@ Code Seq# Include File Comments
<mailto:vgo@ratio.de>
0xB1 00-1F PPPoX
<mailto:mostrows@styx.uwaterloo.ca>
+0xB2 00 arch/powerpc/include/uapi/asm/papr-vpd.h powerpc/pseries VPD API
+ <mailto:linuxppc-dev>
0xB3 00 linux/mmc/ioctl.h
0xB4 00-0F linux/gpio.h <mailto:linux-gpio@vger.kernel.org>
0xB5 00-0F uapi/linux/rpmsg.h <mailto:linux-remoteproc@vger.kernel.org>
diff --git a/arch/powerpc/include/asm/papr-vpd.h b/arch/powerpc/include/asm/papr-vpd.h
new file mode 100644
index 000000000000..bce13e63ae6b
--- /dev/null
+++ b/arch/powerpc/include/asm/papr-vpd.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _ASM_POWERPC_PAPR_VPD_H
+#define _ASM_POWERPC_PAPR_VPD_H
+
+/*
+ * This is here only so sys_rtas() can avoid disrupting VPD sequences
+ * in progress in the papr-vpd driver. There's no other reason that
+ * the rest of the kernel should call into papr-vpd.
+ */
+#ifdef CONFIG_PPC_PSERIES
+void papr_vpd_mutex_lock(void);
+void papr_vpd_mutex_unlock(void);
+#else /* CONFIG_PPC_PSERIES */
+static inline void papr_vpd_mutex_lock(void) {}
+static inline void papr_vpd_mutex_unlock(void) {}
+#endif /* CONFIG_PPC_PSERIES */
+
+#endif /* _ASM_POWERPC_PAPR_VPD_H */
diff --git a/arch/powerpc/include/uapi/asm/papr-vpd.h b/arch/powerpc/include/uapi/asm/papr-vpd.h
new file mode 100644
index 000000000000..b62e4f897a70
--- /dev/null
+++ b/arch/powerpc/include/uapi/asm/papr-vpd.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_PAPR_VPD_H_
+#define _UAPI_PAPR_VPD_H_
+
+#include <asm/ioctl.h>
+#include <asm/papr-miscdev.h>
+
+struct papr_location_code {
+ /*
+ * PAPR+ 12.3.2.4 Converged Location Code Rules - Length
+ * Restrictions. 79 characters plus nul.
+ */
+ char str[80];
+};
+
+/*
+ * ioctl for /dev/papr-vpd. Returns a VPD handle fd corresponding to
+ * the location code.
+ */
+#define PAPR_VPD_IOC_CREATE_HANDLE _IOW(PAPR_MISCDEV_IOC_ID, 0, struct papr_location_code)
+
+#endif /* _UAPI_PAPR_VPD_H_ */
diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
index 53c3b91af2f7..cbcaa102e2b4 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-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-vpd.c b/arch/powerpc/platforms/pseries/papr-vpd.c
new file mode 100644
index 000000000000..06a60e1d1290
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/papr-vpd.c
@@ -0,0 +1,542 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#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>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/miscdevice.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/string_helpers.h>
+#include <asm/machdep.h>
+#include <asm/papr-vpd.h>
+#include <asm/rtas-work-area.h>
+#include <asm/rtas.h>
+#include <uapi/asm/papr-vpd.h>
+
+/**
+ * struct rtas_ibm_get_vpd_params - Parameters (in and out) for ibm,get-vpd.
+ * @loc_code: In: Caller-provided location code buffer. Must be RTAS-addressable.
+ * @work_area: In: Caller-provided work area buffer for results.
+ * @sequence: In: Sequence number. Out: Next sequence number.
+ * @written: Out: Bytes written by ibm,get-vpd to @work_area.
+ * @status: Out: RTAS call status.
+ */
+struct rtas_ibm_get_vpd_params {
+ const struct papr_location_code *loc_code;
+ struct rtas_work_area *work_area;
+ u32 sequence;
+ u32 written;
+ s32 status;
+};
+
+/**
+ * rtas_ibm_get_vpd() - Call ibm,get-vpd to fill a work area buffer.
+ * @params: See &struct rtas_ibm_get_vpd_params.
+ *
+ * Calls ibm,get-vpd 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_vpd() multiple times
+ * to retrieve all the VPD for the provided location code. Only one
+ * sequence should be in progress at any time; starting a new sequence
+ * will disrupt any sequence already in progress. Serialization of VPD
+ * 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_vpd(struct rtas_ibm_get_vpd_params *params)
+{
+ const struct papr_location_code *loc_code = params->loc_code;
+ struct rtas_work_area *work_area = params->work_area;
+ u32 rets[2];
+ s32 fwrc;
+ int ret;
+
+ do {
+ fwrc = rtas_call(rtas_function_token(RTAS_FN_IBM_GET_VPD), 4, 3,
+ rets,
+ __pa(loc_code),
+ rtas_work_area_phys(work_area),
+ rtas_work_area_size(work_area),
+ params->sequence);
+ } while (rtas_busy_delay(fwrc));
+
+ switch (fwrc) {
+ case -1:
+ ret = -EIO;
+ break;
+ case -3:
+ ret = -EINVAL;
+ break;
+ case -4:
+ ret = -EAGAIN;
+ break;
+ case 1:
+ params->sequence = rets[0];
+ fallthrough;
+ case 0:
+ params->written = rets[1];
+ /*
+ * Kernel or firmware bug, do not continue.
+ */
+ if (WARN(params->written > rtas_work_area_size(work_area),
+ "possible write beyond end of work area"))
+ ret = -EFAULT;
+ else
+ ret = 0;
+ break;
+ default:
+ ret = -EIO;
+ pr_err_ratelimited("unexpected ibm,get-vpd status %d\n", fwrc);
+ break;
+ }
+
+ params->status = fwrc;
+ 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 = old_ptr ?
+ kvrealloc(old_ptr, old_len, new_len, GFP_KERNEL_ACCOUNT) :
+ kvmalloc(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;
+}
+
+/*
+ * vpd_sequence_mutex must be held while a sequence is in progress.
+ */
+static DEFINE_MUTEX(vpd_sequence_mutex);
+
+/*
+ * papr_vpd_mutex_lock/unlock() are the only public APIs, and they're
+ * exposed for the sole purpose of preventing sys_rtas() from messing
+ * up in-progress VPD sequences in this code.
+ */
+void papr_vpd_mutex_lock(void)
+{
+ mutex_lock(&vpd_sequence_mutex);
+}
+
+void papr_vpd_mutex_unlock(void)
+{
+ mutex_unlock(&vpd_sequence_mutex);
+}
+
+/*
+ * 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
+ * error is encountered or all VPD for the location code has been
+ * 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.
+ *
+ * Initializes @seq with the resources necessary to carry out a VPD
+ * sequence. Callers must pass @seq to vpd_sequence_end() regardless
+ * of whether the sequence succeeds.
+ *
+ * Context: May sleep.
+ */
+static void vpd_sequence_begin(struct vpd_sequence *seq,
+ const struct papr_location_code *loc_code)
+{
+ /*
+ * Use a static data structure for the location code passed to
+ * RTAS to ensure it's in the RMA and avoid a separate work
+ * area allocation. Guarded by vpd_sequence_mutex.
+ */
+ static struct papr_location_code static_loc_code;
+
+ /*
+ * We could allocate the work area before acquiring the mutex,
+ * but that would allow concurrent requests to exhaust the
+ * limited work area pool for no benefit. So allocate the work
+ * area under the vpd sequence lock.
+ */
+ papr_vpd_mutex_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,
+ },
+ };
+}
+
+/**
+ * vpd_sequence_end() - Finalize a VPD retrieval sequence.
+ * @seq: Sequence state.
+ *
+ * Releases resources obtained by vpd_sequence_begin().
+ */
+static void vpd_sequence_end(struct vpd_sequence *seq)
+{
+ rtas_work_area_free(seq->params.work_area);
+ papr_vpd_mutex_unlock();
+}
+
+/**
+ * vpd_sequence_should_stop() - Determine whether a VPD retrieval sequence
+ * should continue.
+ * @seq: VPD sequence state.
+ *
+ * Examines the sequence error state and outputs of the last call to
+ * ibm,get-vpd to determine whether the sequence in progress should
+ * continue or stop.
+ *
+ * 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)
+{
+ bool done;
+
+ if (seq->error)
+ return true;
+
+ switch (seq->params.status) {
+ case 0:
+ if (seq->params.written == 0)
+ done = false; /* Initial state. */
+ else
+ done = true; /* All data consumed. */
+ break;
+ case 1:
+ done = false; /* More data available. */
+ break;
+ default:
+ done = true; /* Error encountered. */
+ break;
+ }
+
+ 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().
+ */
+static const char *vpd_sequence_fill_work_area(void *arg, size_t *len)
+{
+ struct vpd_sequence *seq = arg;
+ struct rtas_ibm_get_vpd_params *p = &seq->params;
+
+ if (vpd_sequence_should_stop(seq))
+ return NULL;
+ if (vpd_sequence_set_err(seq, rtas_ibm_get_vpd(p)))
+ return NULL;
+ *len = p->written;
+ return rtas_work_area_raw_buf(p->work_area);
+}
+
+/*
+ * Higher-level VPD retrieval code below. These functions use the
+ * vpd_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 driver-private 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;
+
+ /* bug: we should not instantiate a handle without any data attached. */
+ if (!vpd_blob_has_data(blob)) {
+ pr_err_once("handle without data\n");
+ return -EIO;
+ }
+
+ return simple_read_from_buffer(buf, size, off, blob->data, blob->len);
+}
+
+static int papr_vpd_handle_release(struct inode *inode, struct file *file)
+{
+ const struct vpd_blob *blob = file->private_data;
+
+ vpd_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;
+
+ return fixed_size_llseek(file, off, whence, blob->len);
+}
+
+
+static const struct file_operations papr_vpd_handle_ops = {
+ .read = papr_vpd_handle_read,
+ .llseek = papr_vpd_handle_seek,
+ .release = papr_vpd_handle_release,
+};
+
+/**
+ * papr_vpd_create_handle() - Create a fd-based handle for reading VPD.
+ * @ulc: Location code in user memory; defines the scope of the VPD to retrieve.
+ *
+ * Handler for PAPR_VPD_IOC_CREATE_HANDLE ioctl command. Validates
+ * @ulc and instantiates a VPD "blob" for it. The blob is attached to
+ * a file descriptor for reading by user space. The memory associated
+ * with the blob is freed when the file is released.
+ *
+ * Return: The installed fd number if successful, -ve errno otherwise.
+ */
+static long papr_vpd_create_handle(struct papr_location_code __user *ulc)
+{
+ 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)))
+ return -EFAULT;
+
+ 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);
+
+ fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC);
+ if (fd < 0) {
+ err = fd;
+ goto free_blob;
+ }
+
+ 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;
+ }
+
+ 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;
+}
+
+/*
+ * Top-level ioctl handler for /dev/papr-vpd.
+ */
+static long papr_vpd_dev_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
+{
+ void __user *argp = (__force void __user *)arg;
+ long ret;
+
+ switch (ioctl) {
+ case PAPR_VPD_IOC_CREATE_HANDLE:
+ ret = papr_vpd_create_handle(argp);
+ break;
+ default:
+ ret = -ENOIOCTLCMD;
+ break;
+ }
+ return ret;
+}
+
+static const struct file_operations papr_vpd_ops = {
+ .unlocked_ioctl = papr_vpd_dev_ioctl,
+};
+
+static struct miscdevice papr_vpd_dev = {
+ .minor = MISC_DYNAMIC_MINOR,
+ .name = "papr-vpd",
+ .fops = &papr_vpd_ops,
+};
+
+static __init int papr_vpd_init(void)
+{
+ if (!rtas_function_implemented(RTAS_FN_IBM_GET_VPD))
+ return -ENODEV;
+
+ return misc_register(&papr_vpd_dev);
+}
+machine_device_initcall(pseries, papr_vpd_init);
--
2.41.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/7] powerpc/pseries: papr-vpd char driver for VPD retrieval
@ 2023-10-06 21:01 ` Nathan Lynch
0 siblings, 0 replies; 20+ messages in thread
From: Nathan Lynch @ 2023-10-06 21:01 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Michal Suchánek
Cc: linuxppc-dev, tyreld, gcwilson, Nathan Lynch
PowerVM LPARs may retrieve Vital Product Data (VPD) for system
components using the ibm,get-vpd RTAS function.
We can expose this to user space with a /dev/papr-vpd character
device, where the programming model is:
struct papr_location_code plc = { .str = "", }; /* obtain all VPD */
int devfd = open("/dev/papr-vpd", O_RDONLY);
int vpdfd = ioctl(devfd, PAPR_VPD_CREATE_HANDLE, &plc);
size_t size = lseek(vpdfd, 0, SEEK_END);
char *buf = malloc(size);
pread(devfd, buf, size, 0);
When a file descriptor is obtained from ioctl(PAPR_VPD_CREATE_HANDLE),
the file contains the result of a complete ibm,get-vpd sequence. The
file contents are immutable from the POV of user space. To get a new
view of the VPD, the client must create a new handle.
This design choice insulates user space from most of the complexities
that ibm,get-vpd brings:
* ibm,get-vpd must be called more than once to obtain complete
results.
* Only one ibm,get-vpd call sequence should be in progress at a time;
interleaved sequences will disrupt each other. Callers must have a
protocol for serializing their use of the function.
* A call sequence in progress may receive a "VPD changed, try again"
status, requiring the client to abandon the sequence and start
over.
The memory required for the VPD buffers seems acceptable, around 20KB
for all VPD on one of my systems. And the value of the
/rtas/ibm,vpd-size DT property (the estimated maximum size of VPD) is
consistently 300KB across various systems I've checked.
I've implemented support for this new ABI in the rtas_get_vpd()
function in librtas, which the vpdupdate command currently uses to
populate its VPD database. I've verified that an unmodified vpdupdate
binary generates an identical database when using a librtas.so that
prefers the new ABI.
Note that the driver needs to serialize its call sequences with legacy
sys_rtas(ibm,get-vpd) callers, so it exposes its internal lock for
sys_rtas.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
Documentation/userspace-api/ioctl/ioctl-number.rst | 2 +
arch/powerpc/include/asm/papr-vpd.h | 18 +
arch/powerpc/include/uapi/asm/papr-vpd.h | 22 +
arch/powerpc/platforms/pseries/Makefile | 1 +
arch/powerpc/platforms/pseries/papr-vpd.c | 542 +++++++++++++++++++++
5 files changed, 585 insertions(+)
diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
index 4ea5b837399a..a950545bf7cd 100644
--- a/Documentation/userspace-api/ioctl/ioctl-number.rst
+++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
@@ -349,6 +349,8 @@ Code Seq# Include File Comments
<mailto:vgo@ratio.de>
0xB1 00-1F PPPoX
<mailto:mostrows@styx.uwaterloo.ca>
+0xB2 00 arch/powerpc/include/uapi/asm/papr-vpd.h powerpc/pseries VPD API
+ <mailto:linuxppc-dev>
0xB3 00 linux/mmc/ioctl.h
0xB4 00-0F linux/gpio.h <mailto:linux-gpio@vger.kernel.org>
0xB5 00-0F uapi/linux/rpmsg.h <mailto:linux-remoteproc@vger.kernel.org>
diff --git a/arch/powerpc/include/asm/papr-vpd.h b/arch/powerpc/include/asm/papr-vpd.h
new file mode 100644
index 000000000000..bce13e63ae6b
--- /dev/null
+++ b/arch/powerpc/include/asm/papr-vpd.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _ASM_POWERPC_PAPR_VPD_H
+#define _ASM_POWERPC_PAPR_VPD_H
+
+/*
+ * This is here only so sys_rtas() can avoid disrupting VPD sequences
+ * in progress in the papr-vpd driver. There's no other reason that
+ * the rest of the kernel should call into papr-vpd.
+ */
+#ifdef CONFIG_PPC_PSERIES
+void papr_vpd_mutex_lock(void);
+void papr_vpd_mutex_unlock(void);
+#else /* CONFIG_PPC_PSERIES */
+static inline void papr_vpd_mutex_lock(void) {}
+static inline void papr_vpd_mutex_unlock(void) {}
+#endif /* CONFIG_PPC_PSERIES */
+
+#endif /* _ASM_POWERPC_PAPR_VPD_H */
diff --git a/arch/powerpc/include/uapi/asm/papr-vpd.h b/arch/powerpc/include/uapi/asm/papr-vpd.h
new file mode 100644
index 000000000000..b62e4f897a70
--- /dev/null
+++ b/arch/powerpc/include/uapi/asm/papr-vpd.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_PAPR_VPD_H_
+#define _UAPI_PAPR_VPD_H_
+
+#include <asm/ioctl.h>
+#include <asm/papr-miscdev.h>
+
+struct papr_location_code {
+ /*
+ * PAPR+ 12.3.2.4 Converged Location Code Rules - Length
+ * Restrictions. 79 characters plus nul.
+ */
+ char str[80];
+};
+
+/*
+ * ioctl for /dev/papr-vpd. Returns a VPD handle fd corresponding to
+ * the location code.
+ */
+#define PAPR_VPD_IOC_CREATE_HANDLE _IOW(PAPR_MISCDEV_IOC_ID, 0, struct papr_location_code)
+
+#endif /* _UAPI_PAPR_VPD_H_ */
diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
index 53c3b91af2f7..cbcaa102e2b4 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-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-vpd.c b/arch/powerpc/platforms/pseries/papr-vpd.c
new file mode 100644
index 000000000000..06a60e1d1290
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/papr-vpd.c
@@ -0,0 +1,542 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#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>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/miscdevice.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/string_helpers.h>
+#include <asm/machdep.h>
+#include <asm/papr-vpd.h>
+#include <asm/rtas-work-area.h>
+#include <asm/rtas.h>
+#include <uapi/asm/papr-vpd.h>
+
+/**
+ * struct rtas_ibm_get_vpd_params - Parameters (in and out) for ibm,get-vpd.
+ * @loc_code: In: Caller-provided location code buffer. Must be RTAS-addressable.
+ * @work_area: In: Caller-provided work area buffer for results.
+ * @sequence: In: Sequence number. Out: Next sequence number.
+ * @written: Out: Bytes written by ibm,get-vpd to @work_area.
+ * @status: Out: RTAS call status.
+ */
+struct rtas_ibm_get_vpd_params {
+ const struct papr_location_code *loc_code;
+ struct rtas_work_area *work_area;
+ u32 sequence;
+ u32 written;
+ s32 status;
+};
+
+/**
+ * rtas_ibm_get_vpd() - Call ibm,get-vpd to fill a work area buffer.
+ * @params: See &struct rtas_ibm_get_vpd_params.
+ *
+ * Calls ibm,get-vpd 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_vpd() multiple times
+ * to retrieve all the VPD for the provided location code. Only one
+ * sequence should be in progress at any time; starting a new sequence
+ * will disrupt any sequence already in progress. Serialization of VPD
+ * 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_vpd(struct rtas_ibm_get_vpd_params *params)
+{
+ const struct papr_location_code *loc_code = params->loc_code;
+ struct rtas_work_area *work_area = params->work_area;
+ u32 rets[2];
+ s32 fwrc;
+ int ret;
+
+ do {
+ fwrc = rtas_call(rtas_function_token(RTAS_FN_IBM_GET_VPD), 4, 3,
+ rets,
+ __pa(loc_code),
+ rtas_work_area_phys(work_area),
+ rtas_work_area_size(work_area),
+ params->sequence);
+ } while (rtas_busy_delay(fwrc));
+
+ switch (fwrc) {
+ case -1:
+ ret = -EIO;
+ break;
+ case -3:
+ ret = -EINVAL;
+ break;
+ case -4:
+ ret = -EAGAIN;
+ break;
+ case 1:
+ params->sequence = rets[0];
+ fallthrough;
+ case 0:
+ params->written = rets[1];
+ /*
+ * Kernel or firmware bug, do not continue.
+ */
+ if (WARN(params->written > rtas_work_area_size(work_area),
+ "possible write beyond end of work area"))
+ ret = -EFAULT;
+ else
+ ret = 0;
+ break;
+ default:
+ ret = -EIO;
+ pr_err_ratelimited("unexpected ibm,get-vpd status %d\n", fwrc);
+ break;
+ }
+
+ params->status = fwrc;
+ 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 = old_ptr ?
+ kvrealloc(old_ptr, old_len, new_len, GFP_KERNEL_ACCOUNT) :
+ kvmalloc(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;
+}
+
+/*
+ * vpd_sequence_mutex must be held while a sequence is in progress.
+ */
+static DEFINE_MUTEX(vpd_sequence_mutex);
+
+/*
+ * papr_vpd_mutex_lock/unlock() are the only public APIs, and they're
+ * exposed for the sole purpose of preventing sys_rtas() from messing
+ * up in-progress VPD sequences in this code.
+ */
+void papr_vpd_mutex_lock(void)
+{
+ mutex_lock(&vpd_sequence_mutex);
+}
+
+void papr_vpd_mutex_unlock(void)
+{
+ mutex_unlock(&vpd_sequence_mutex);
+}
+
+/*
+ * 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
+ * error is encountered or all VPD for the location code has been
+ * 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.
+ *
+ * Initializes @seq with the resources necessary to carry out a VPD
+ * sequence. Callers must pass @seq to vpd_sequence_end() regardless
+ * of whether the sequence succeeds.
+ *
+ * Context: May sleep.
+ */
+static void vpd_sequence_begin(struct vpd_sequence *seq,
+ const struct papr_location_code *loc_code)
+{
+ /*
+ * Use a static data structure for the location code passed to
+ * RTAS to ensure it's in the RMA and avoid a separate work
+ * area allocation. Guarded by vpd_sequence_mutex.
+ */
+ static struct papr_location_code static_loc_code;
+
+ /*
+ * We could allocate the work area before acquiring the mutex,
+ * but that would allow concurrent requests to exhaust the
+ * limited work area pool for no benefit. So allocate the work
+ * area under the vpd sequence lock.
+ */
+ papr_vpd_mutex_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,
+ },
+ };
+}
+
+/**
+ * vpd_sequence_end() - Finalize a VPD retrieval sequence.
+ * @seq: Sequence state.
+ *
+ * Releases resources obtained by vpd_sequence_begin().
+ */
+static void vpd_sequence_end(struct vpd_sequence *seq)
+{
+ rtas_work_area_free(seq->params.work_area);
+ papr_vpd_mutex_unlock();
+}
+
+/**
+ * vpd_sequence_should_stop() - Determine whether a VPD retrieval sequence
+ * should continue.
+ * @seq: VPD sequence state.
+ *
+ * Examines the sequence error state and outputs of the last call to
+ * ibm,get-vpd to determine whether the sequence in progress should
+ * continue or stop.
+ *
+ * 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)
+{
+ bool done;
+
+ if (seq->error)
+ return true;
+
+ switch (seq->params.status) {
+ case 0:
+ if (seq->params.written == 0)
+ done = false; /* Initial state. */
+ else
+ done = true; /* All data consumed. */
+ break;
+ case 1:
+ done = false; /* More data available. */
+ break;
+ default:
+ done = true; /* Error encountered. */
+ break;
+ }
+
+ 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().
+ */
+static const char *vpd_sequence_fill_work_area(void *arg, size_t *len)
+{
+ struct vpd_sequence *seq = arg;
+ struct rtas_ibm_get_vpd_params *p = &seq->params;
+
+ if (vpd_sequence_should_stop(seq))
+ return NULL;
+ if (vpd_sequence_set_err(seq, rtas_ibm_get_vpd(p)))
+ return NULL;
+ *len = p->written;
+ return rtas_work_area_raw_buf(p->work_area);
+}
+
+/*
+ * Higher-level VPD retrieval code below. These functions use the
+ * vpd_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 driver-private 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;
+
+ /* bug: we should not instantiate a handle without any data attached. */
+ if (!vpd_blob_has_data(blob)) {
+ pr_err_once("handle without data\n");
+ return -EIO;
+ }
+
+ return simple_read_from_buffer(buf, size, off, blob->data, blob->len);
+}
+
+static int papr_vpd_handle_release(struct inode *inode, struct file *file)
+{
+ const struct vpd_blob *blob = file->private_data;
+
+ vpd_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;
+
+ return fixed_size_llseek(file, off, whence, blob->len);
+}
+
+
+static const struct file_operations papr_vpd_handle_ops = {
+ .read = papr_vpd_handle_read,
+ .llseek = papr_vpd_handle_seek,
+ .release = papr_vpd_handle_release,
+};
+
+/**
+ * papr_vpd_create_handle() - Create a fd-based handle for reading VPD.
+ * @ulc: Location code in user memory; defines the scope of the VPD to retrieve.
+ *
+ * Handler for PAPR_VPD_IOC_CREATE_HANDLE ioctl command. Validates
+ * @ulc and instantiates a VPD "blob" for it. The blob is attached to
+ * a file descriptor for reading by user space. The memory associated
+ * with the blob is freed when the file is released.
+ *
+ * Return: The installed fd number if successful, -ve errno otherwise.
+ */
+static long papr_vpd_create_handle(struct papr_location_code __user *ulc)
+{
+ 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)))
+ return -EFAULT;
+
+ 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);
+
+ fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC);
+ if (fd < 0) {
+ err = fd;
+ goto free_blob;
+ }
+
+ 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;
+ }
+
+ 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;
+}
+
+/*
+ * Top-level ioctl handler for /dev/papr-vpd.
+ */
+static long papr_vpd_dev_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
+{
+ void __user *argp = (__force void __user *)arg;
+ long ret;
+
+ switch (ioctl) {
+ case PAPR_VPD_IOC_CREATE_HANDLE:
+ ret = papr_vpd_create_handle(argp);
+ break;
+ default:
+ ret = -ENOIOCTLCMD;
+ break;
+ }
+ return ret;
+}
+
+static const struct file_operations papr_vpd_ops = {
+ .unlocked_ioctl = papr_vpd_dev_ioctl,
+};
+
+static struct miscdevice papr_vpd_dev = {
+ .minor = MISC_DYNAMIC_MINOR,
+ .name = "papr-vpd",
+ .fops = &papr_vpd_ops,
+};
+
+static __init int papr_vpd_init(void)
+{
+ if (!rtas_function_implemented(RTAS_FN_IBM_GET_VPD))
+ return -ENODEV;
+
+ return misc_register(&papr_vpd_dev);
+}
+machine_device_initcall(pseries, papr_vpd_init);
--
2.41.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/7] powerpc/rtas: serialize ibm,get-vpd service with papr-vpd sequences
2023-10-06 21:01 ` Nathan Lynch via B4 Relay
@ 2023-10-06 21:01 ` Nathan Lynch via B4 Relay
-1 siblings, 0 replies; 20+ messages in thread
From: Nathan Lynch @ 2023-10-06 21:01 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Michal Suchánek
Cc: linuxppc-dev, tyreld, gcwilson, Nathan Lynch
Take the papr-vpd driver's internal mutex when sys_rtas performs
ibm,get-vpd calls. This prevents sys_rtas(ibm,get-vpd) calls from
interleaving with sequences performed by the driver, ensuring that
such sequences are not disrupted.
However, it cannot prevent the driver from interleaving with sequences
that are initiated via sys_rtas, since control returns to user space
with each sys_rtas(ibm,get-vpd) call.
Emit a notice on first use of sys_rtas(ibm,get-vpd) encouraging users
to migrate to /dev/papr-vpd.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
arch/powerpc/kernel/rtas.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index eddc031c4b95..70ae118d2a13 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -37,6 +37,7 @@
#include <asm/machdep.h>
#include <asm/mmu.h>
#include <asm/page.h>
+#include <asm/papr-vpd.h>
#include <asm/rtas-work-area.h>
#include <asm/rtas.h>
#include <asm/time.h>
@@ -1861,6 +1862,28 @@ SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
goto copy_return;
}
+ if (token == rtas_function_token(RTAS_FN_IBM_GET_VPD)) {
+ /*
+ * ibm,get-vpd potentially needs to be invoked
+ * multiple times to obtain complete results.
+ * Interleaved ibm,get-vpd sequences disrupt each
+ * other.
+ *
+ * /dev/papr-vpd doesn't have this problem and users
+ * do not need to be aware of each other to use it
+ * safely.
+ *
+ * We can prevent this call from disrupting a
+ * /dev/papr-vpd-initiated sequence in progress by
+ * reaching into the driver to take its internal
+ * lock. Unfortunately there is no way to prevent
+ * interference in the other direction without
+ * resorting to even worse hacks.
+ */
+ pr_notice_once("Calling ibm,get-vpd via sys_rtas is allowed but deprecated. Use /dev/papr-vpd instead.\n");
+ papr_vpd_mutex_lock();
+ }
+
buff_copy = get_errorlog_buffer();
raw_spin_lock_irqsave(&rtas_lock, flags);
@@ -1870,6 +1893,9 @@ SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
do_enter_rtas(&rtas_args);
args = rtas_args;
+ if (token == rtas_function_token(RTAS_FN_IBM_GET_VPD))
+ papr_vpd_mutex_unlock();
+
/* A -1 return code indicates that the last command couldn't
be completed due to a hardware error. */
if (be32_to_cpu(args.rets[0]) == -1)
--
2.41.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/7] powerpc/rtas: serialize ibm,get-vpd service with papr-vpd sequences
@ 2023-10-06 21:01 ` Nathan Lynch via B4 Relay
0 siblings, 0 replies; 20+ messages in thread
From: Nathan Lynch via B4 Relay @ 2023-10-06 21:01 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Michal Suchánek
Cc: Nathan Lynch, tyreld, gcwilson, linuxppc-dev
From: Nathan Lynch <nathanl@linux.ibm.com>
Take the papr-vpd driver's internal mutex when sys_rtas performs
ibm,get-vpd calls. This prevents sys_rtas(ibm,get-vpd) calls from
interleaving with sequences performed by the driver, ensuring that
such sequences are not disrupted.
However, it cannot prevent the driver from interleaving with sequences
that are initiated via sys_rtas, since control returns to user space
with each sys_rtas(ibm,get-vpd) call.
Emit a notice on first use of sys_rtas(ibm,get-vpd) encouraging users
to migrate to /dev/papr-vpd.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
arch/powerpc/kernel/rtas.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index eddc031c4b95..70ae118d2a13 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -37,6 +37,7 @@
#include <asm/machdep.h>
#include <asm/mmu.h>
#include <asm/page.h>
+#include <asm/papr-vpd.h>
#include <asm/rtas-work-area.h>
#include <asm/rtas.h>
#include <asm/time.h>
@@ -1861,6 +1862,28 @@ SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
goto copy_return;
}
+ if (token == rtas_function_token(RTAS_FN_IBM_GET_VPD)) {
+ /*
+ * ibm,get-vpd potentially needs to be invoked
+ * multiple times to obtain complete results.
+ * Interleaved ibm,get-vpd sequences disrupt each
+ * other.
+ *
+ * /dev/papr-vpd doesn't have this problem and users
+ * do not need to be aware of each other to use it
+ * safely.
+ *
+ * We can prevent this call from disrupting a
+ * /dev/papr-vpd-initiated sequence in progress by
+ * reaching into the driver to take its internal
+ * lock. Unfortunately there is no way to prevent
+ * interference in the other direction without
+ * resorting to even worse hacks.
+ */
+ pr_notice_once("Calling ibm,get-vpd via sys_rtas is allowed but deprecated. Use /dev/papr-vpd instead.\n");
+ papr_vpd_mutex_lock();
+ }
+
buff_copy = get_errorlog_buffer();
raw_spin_lock_irqsave(&rtas_lock, flags);
@@ -1870,6 +1893,9 @@ SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
do_enter_rtas(&rtas_args);
args = rtas_args;
+ if (token == rtas_function_token(RTAS_FN_IBM_GET_VPD))
+ papr_vpd_mutex_unlock();
+
/* A -1 return code indicates that the last command couldn't
be completed due to a hardware error. */
if (be32_to_cpu(args.rets[0]) == -1)
--
2.41.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 4/7] powerpc/pseries/papr-sysparm: validate buffer object lengths
2023-10-06 21:01 ` Nathan Lynch via B4 Relay
@ 2023-10-06 21:01 ` Nathan Lynch via B4 Relay
-1 siblings, 0 replies; 20+ messages in thread
From: Nathan Lynch @ 2023-10-06 21:01 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Michal Suchánek
Cc: linuxppc-dev, tyreld, gcwilson, Nathan Lynch
The ability to get and set system parameters will be exposed to user
space, so let's get a little more strict about malformed
papr_sysparm_buf objects.
* Create accessors for the length field of struct papr_sysparm_buf.
The length is always stored in MSB order and this is better than
spreading the necessary conversions all over.
* Reject attempts to submit invalid buffers to RTAS.
* Warn if RTAS returns a buffer with an invalid length, clamping the
returned length to a safe value that won't overrun the buffer.
These are meant as precautionary measures to mitigate both firmware
and kernel bugs in this area, should they arise, but I am not aware of
any.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
arch/powerpc/platforms/pseries/papr-sysparm.c | 47 +++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/papr-sysparm.c b/arch/powerpc/platforms/pseries/papr-sysparm.c
index fedc61599e6c..a1e7aeac7416 100644
--- a/arch/powerpc/platforms/pseries/papr-sysparm.c
+++ b/arch/powerpc/platforms/pseries/papr-sysparm.c
@@ -23,6 +23,46 @@ void papr_sysparm_buf_free(struct papr_sysparm_buf *buf)
kfree(buf);
}
+static size_t papr_sysparm_buf_get_length(const struct papr_sysparm_buf *buf)
+{
+ return be16_to_cpu(buf->len);
+}
+
+static void papr_sysparm_buf_set_length(struct papr_sysparm_buf *buf, size_t length)
+{
+ WARN_ONCE(length > sizeof(buf->val),
+ "bogus length %zu, clamping to safe value", length);
+ length = min(sizeof(buf->val), length);
+ buf->len = cpu_to_be16(length);
+}
+
+/*
+ * For use on buffers returned from ibm,get-system-parameter before
+ * returning them to callers. Ensures the encoded length of valid data
+ * cannot overrun buf->val[].
+ */
+static void papr_sysparm_buf_clamp_length(struct papr_sysparm_buf *buf)
+{
+ papr_sysparm_buf_set_length(buf, papr_sysparm_buf_get_length(buf));
+}
+
+/*
+ * Perform some basic diligence on the system parameter buffer before
+ * submitting it to RTAS.
+ */
+static bool papr_sysparm_buf_can_submit(const struct papr_sysparm_buf *buf)
+{
+ /*
+ * Firmware ought to reject buffer lengths that exceed the
+ * maximum specified in PAPR, but there's no reason for the
+ * kernel to allow them either.
+ */
+ if (papr_sysparm_buf_get_length(buf) > sizeof(buf->val))
+ return false;
+
+ return true;
+}
+
/**
* papr_sysparm_get() - Retrieve the value of a PAPR system parameter.
* @param: PAPR system parameter token as described in
@@ -63,6 +103,9 @@ int papr_sysparm_get(papr_sysparm_t param, struct papr_sysparm_buf *buf)
if (token == RTAS_UNKNOWN_SERVICE)
return -ENOENT;
+ if (!papr_sysparm_buf_can_submit(buf))
+ return -EINVAL;
+
work_area = rtas_work_area_alloc(sizeof(*buf));
memcpy(rtas_work_area_raw_buf(work_area), buf, sizeof(*buf));
@@ -77,6 +120,7 @@ int papr_sysparm_get(papr_sysparm_t param, struct papr_sysparm_buf *buf)
case 0:
ret = 0;
memcpy(buf, rtas_work_area_raw_buf(work_area), sizeof(*buf));
+ papr_sysparm_buf_clamp_length(buf);
break;
case -3: /* parameter not implemented */
ret = -EOPNOTSUPP;
@@ -115,6 +159,9 @@ int papr_sysparm_set(papr_sysparm_t param, const struct papr_sysparm_buf *buf)
if (token == RTAS_UNKNOWN_SERVICE)
return -ENOENT;
+ if (!papr_sysparm_buf_can_submit(buf))
+ return -EINVAL;
+
work_area = rtas_work_area_alloc(sizeof(*buf));
memcpy(rtas_work_area_raw_buf(work_area), buf, sizeof(*buf));
--
2.41.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 4/7] powerpc/pseries/papr-sysparm: validate buffer object lengths
@ 2023-10-06 21:01 ` Nathan Lynch via B4 Relay
0 siblings, 0 replies; 20+ messages in thread
From: Nathan Lynch via B4 Relay @ 2023-10-06 21:01 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Michal Suchánek
Cc: Nathan Lynch, tyreld, gcwilson, linuxppc-dev
From: Nathan Lynch <nathanl@linux.ibm.com>
The ability to get and set system parameters will be exposed to user
space, so let's get a little more strict about malformed
papr_sysparm_buf objects.
* Create accessors for the length field of struct papr_sysparm_buf.
The length is always stored in MSB order and this is better than
spreading the necessary conversions all over.
* Reject attempts to submit invalid buffers to RTAS.
* Warn if RTAS returns a buffer with an invalid length, clamping the
returned length to a safe value that won't overrun the buffer.
These are meant as precautionary measures to mitigate both firmware
and kernel bugs in this area, should they arise, but I am not aware of
any.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
arch/powerpc/platforms/pseries/papr-sysparm.c | 47 +++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/papr-sysparm.c b/arch/powerpc/platforms/pseries/papr-sysparm.c
index fedc61599e6c..a1e7aeac7416 100644
--- a/arch/powerpc/platforms/pseries/papr-sysparm.c
+++ b/arch/powerpc/platforms/pseries/papr-sysparm.c
@@ -23,6 +23,46 @@ void papr_sysparm_buf_free(struct papr_sysparm_buf *buf)
kfree(buf);
}
+static size_t papr_sysparm_buf_get_length(const struct papr_sysparm_buf *buf)
+{
+ return be16_to_cpu(buf->len);
+}
+
+static void papr_sysparm_buf_set_length(struct papr_sysparm_buf *buf, size_t length)
+{
+ WARN_ONCE(length > sizeof(buf->val),
+ "bogus length %zu, clamping to safe value", length);
+ length = min(sizeof(buf->val), length);
+ buf->len = cpu_to_be16(length);
+}
+
+/*
+ * For use on buffers returned from ibm,get-system-parameter before
+ * returning them to callers. Ensures the encoded length of valid data
+ * cannot overrun buf->val[].
+ */
+static void papr_sysparm_buf_clamp_length(struct papr_sysparm_buf *buf)
+{
+ papr_sysparm_buf_set_length(buf, papr_sysparm_buf_get_length(buf));
+}
+
+/*
+ * Perform some basic diligence on the system parameter buffer before
+ * submitting it to RTAS.
+ */
+static bool papr_sysparm_buf_can_submit(const struct papr_sysparm_buf *buf)
+{
+ /*
+ * Firmware ought to reject buffer lengths that exceed the
+ * maximum specified in PAPR, but there's no reason for the
+ * kernel to allow them either.
+ */
+ if (papr_sysparm_buf_get_length(buf) > sizeof(buf->val))
+ return false;
+
+ return true;
+}
+
/**
* papr_sysparm_get() - Retrieve the value of a PAPR system parameter.
* @param: PAPR system parameter token as described in
@@ -63,6 +103,9 @@ int papr_sysparm_get(papr_sysparm_t param, struct papr_sysparm_buf *buf)
if (token == RTAS_UNKNOWN_SERVICE)
return -ENOENT;
+ if (!papr_sysparm_buf_can_submit(buf))
+ return -EINVAL;
+
work_area = rtas_work_area_alloc(sizeof(*buf));
memcpy(rtas_work_area_raw_buf(work_area), buf, sizeof(*buf));
@@ -77,6 +120,7 @@ int papr_sysparm_get(papr_sysparm_t param, struct papr_sysparm_buf *buf)
case 0:
ret = 0;
memcpy(buf, rtas_work_area_raw_buf(work_area), sizeof(*buf));
+ papr_sysparm_buf_clamp_length(buf);
break;
case -3: /* parameter not implemented */
ret = -EOPNOTSUPP;
@@ -115,6 +159,9 @@ int papr_sysparm_set(papr_sysparm_t param, const struct papr_sysparm_buf *buf)
if (token == RTAS_UNKNOWN_SERVICE)
return -ENOENT;
+ if (!papr_sysparm_buf_can_submit(buf))
+ return -EINVAL;
+
work_area = rtas_work_area_alloc(sizeof(*buf));
memcpy(rtas_work_area_raw_buf(work_area), buf, sizeof(*buf));
--
2.41.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 5/7] powerpc/pseries/papr-sysparm: expose chardev API to user space
2023-10-06 21:01 ` Nathan Lynch via B4 Relay
@ 2023-10-06 21:01 ` Nathan Lynch via B4 Relay
-1 siblings, 0 replies; 20+ messages in thread
From: Nathan Lynch @ 2023-10-06 21:01 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Michal Suchánek
Cc: linuxppc-dev, tyreld, gcwilson, Nathan Lynch
Until now the papr_sysparm APIs have been kernel-internal. But user
space needs access to PAPR system parameters too. The only method
available to user space today to get or set system parameters is using
sys_rtas() and /dev/mem to pass RTAS-addressable buffers between user
space and firmware. This is incompatible with lockdown and should be
deprecated.
So provide an alternative ABI to user space in the form of a
/dev/papr-sysparm character device with just two ioctl commands (get
and set). The data payloads involved are small enough to fit in the
ioctl argument buffer, making the code relatively simple.
Exposing the system parameters through sysfs has been considered but
it would be too awkward:
* The kernel currently does not have to contain an exhaustive list of
defined system parameters. This is a convenient property to maintain
because we don't have to update the kernel whenever a new parameter
is added to PAPR. Exporting a named attribute in sysfs for each
parameter would negate this.
* Some system parameters are text-based and some are not.
* Retrieval of at least one system parameter requires input data,
which a simple read-oriented interface can't support.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
Documentation/userspace-api/ioctl/ioctl-number.rst | 2 +
arch/powerpc/include/asm/papr-sysparm.h | 17 ++-
arch/powerpc/include/uapi/asm/papr-sysparm.h | 58 ++++++++
arch/powerpc/platforms/pseries/papr-sysparm.c | 160 ++++++++++++++++++++-
4 files changed, 229 insertions(+), 8 deletions(-)
diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
index a950545bf7cd..d8b6cb1a3636 100644
--- a/Documentation/userspace-api/ioctl/ioctl-number.rst
+++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
@@ -351,6 +351,8 @@ Code Seq# Include File Comments
<mailto:mostrows@styx.uwaterloo.ca>
0xB2 00 arch/powerpc/include/uapi/asm/papr-vpd.h powerpc/pseries VPD API
<mailto:linuxppc-dev>
+0xB2 01-02 arch/powerpc/include/uapi/asm/papr-sysparm.h powerpc/pseries system parameter API
+ <mailto:linuxppc-dev>
0xB3 00 linux/mmc/ioctl.h
0xB4 00-0F linux/gpio.h <mailto:linux-gpio@vger.kernel.org>
0xB5 00-0F uapi/linux/rpmsg.h <mailto:linux-remoteproc@vger.kernel.org>
diff --git a/arch/powerpc/include/asm/papr-sysparm.h b/arch/powerpc/include/asm/papr-sysparm.h
index f5fdbd8ae9db..0dbbff59101d 100644
--- a/arch/powerpc/include/asm/papr-sysparm.h
+++ b/arch/powerpc/include/asm/papr-sysparm.h
@@ -2,8 +2,10 @@
#ifndef _ASM_POWERPC_PAPR_SYSPARM_H
#define _ASM_POWERPC_PAPR_SYSPARM_H
+#include <uapi/asm/papr-sysparm.h>
+
typedef struct {
- const u32 token;
+ u32 token;
} papr_sysparm_t;
#define mk_papr_sysparm(x_) ((papr_sysparm_t){ .token = x_, })
@@ -20,11 +22,14 @@ typedef struct {
#define PAPR_SYSPARM_TLB_BLOCK_INVALIDATE_ATTRS mk_papr_sysparm(50)
#define PAPR_SYSPARM_LPAR_NAME mk_papr_sysparm(55)
-enum {
- PAPR_SYSPARM_MAX_INPUT = 1024,
- PAPR_SYSPARM_MAX_OUTPUT = 4000,
-};
-
+/**
+ * struct papr_sysparm_buf - RTAS work area layout for system parameter functions.
+ *
+ * This is the memory layout of the buffers passed to/from
+ * ibm,get-system-parameter and ibm,set-system-parameter. It is
+ * distinct from the papr_sysparm_io_block structure that is passed
+ * between user space and the kernel.
+ */
struct papr_sysparm_buf {
__be16 len;
char val[PAPR_SYSPARM_MAX_OUTPUT];
diff --git a/arch/powerpc/include/uapi/asm/papr-sysparm.h b/arch/powerpc/include/uapi/asm/papr-sysparm.h
new file mode 100644
index 000000000000..9f9a0f267ea5
--- /dev/null
+++ b/arch/powerpc/include/uapi/asm/papr-sysparm.h
@@ -0,0 +1,58 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_PAPR_SYSPARM_H_
+#define _UAPI_PAPR_SYSPARM_H_
+
+#include <linux/types.h>
+#include <asm/ioctl.h>
+#include <asm/papr-miscdev.h>
+
+enum {
+ PAPR_SYSPARM_MAX_INPUT = 1024,
+ PAPR_SYSPARM_MAX_OUTPUT = 4000,
+};
+
+struct papr_sysparm_io_block {
+ __u32 parameter;
+ __u16 length;
+ char data[PAPR_SYSPARM_MAX_OUTPUT];
+};
+
+/**
+ * PAPR_SYSPARM_IOC_GET - Retrieve the value of a PAPR system parameter.
+ *
+ * Uses _IOWR because of one corner case: Retrieving the value of the
+ * "OS Service Entitlement Status" parameter (60) requires the caller
+ * to supply input data (a date string) in the buffer passed to
+ * firmware. So the @length and @data of the incoming
+ * papr_sysparm_io_block are always used to initialize the work area
+ * supplied to ibm,get-system-parameter. No other parameters are known
+ * to parameterize the result this way, and callers are encouraged
+ * (but not required) to zero-initialize @length and @data in the
+ * common case.
+ *
+ * On error the contents of the ioblock are indeterminate.
+ *
+ * Return:
+ * 0: Success; @length is the length of valid data in @data, not to exceed @PAPR_SYSPARM_MAX_OUTPUT.
+ * -EIO: Platform error. (-1)
+ * -EINVAL: Incorrect data length or format. (-9999)
+ * -EPERM: The calling partition is not allowed to access this parameter. (-9002)
+ * -EOPNOTSUPP: Parameter not supported on this platform (-3)
+ */
+#define PAPR_SYSPARM_IOC_GET _IOWR(PAPR_MISCDEV_IOC_ID, 1, struct papr_sysparm_io_block)
+
+/**
+ * PAPR_SYSPARM_IOC_SET - Update the value of a PAPR system parameter.
+ *
+ * The contents of the ioblock are unchanged regardless of success.
+ *
+ * Return:
+ * 0: Success; the parameter has been updated.
+ * -EIO: Platform error. (-1)
+ * -EINVAL: Incorrect data length or format. (-9999)
+ * -EPERM: The calling partition is not allowed to access this parameter. (-9002)
+ * -EOPNOTSUPP: Parameter not supported on this platform (-3)
+ */
+#define PAPR_SYSPARM_IOC_SET _IOW(PAPR_MISCDEV_IOC_ID, 2, struct papr_sysparm_io_block)
+
+#endif /* _UAPI_PAPR_SYSPARM_H_ */
diff --git a/arch/powerpc/platforms/pseries/papr-sysparm.c b/arch/powerpc/platforms/pseries/papr-sysparm.c
index a1e7aeac7416..cc600f89246c 100644
--- a/arch/powerpc/platforms/pseries/papr-sysparm.c
+++ b/arch/powerpc/platforms/pseries/papr-sysparm.c
@@ -2,14 +2,19 @@
#define pr_fmt(fmt) "papr-sysparm: " fmt
+#include <linux/anon_inodes.h>
#include <linux/bug.h>
+#include <linux/file.h>
+#include <linux/fs.h>
#include <linux/init.h>
#include <linux/kernel.h>
+#include <linux/miscdevice.h>
#include <linux/printk.h>
#include <linux/slab.h>
-#include <asm/rtas.h>
+#include <asm/machdep.h>
#include <asm/papr-sysparm.h>
#include <asm/rtas-work-area.h>
+#include <asm/rtas.h>
struct papr_sysparm_buf *papr_sysparm_buf_alloc(void)
{
@@ -87,7 +92,6 @@ static bool papr_sysparm_buf_can_submit(const struct papr_sysparm_buf *buf)
*
* Return: 0 on success, -errno otherwise. @buf is unmodified on error.
*/
-
int papr_sysparm_get(papr_sysparm_t param, struct papr_sysparm_buf *buf)
{
const s32 token = rtas_function_token(RTAS_FN_IBM_GET_SYSTEM_PARAMETER);
@@ -196,3 +200,155 @@ int papr_sysparm_set(papr_sysparm_t param, const struct papr_sysparm_buf *buf)
return ret;
}
+
+static struct papr_sysparm_buf *
+papr_sysparm_buf_from_user(const struct papr_sysparm_io_block __user *user_iob)
+{
+ struct papr_sysparm_buf *kern_spbuf;
+ papr_sysparm_t param;
+ long err;
+ u16 len;
+
+ /*
+ * The parameter index that userspace wants to query or update.
+ */
+ if (get_user(param.token, &user_iob->parameter))
+ return ERR_PTR(-EFAULT);
+ /*
+ * The length of valid data that userspace claims to be in
+ * user_iob->data[].
+ */
+ if (get_user(len, &user_iob->length))
+ return ERR_PTR(-EFAULT);
+
+ static_assert(sizeof(user_iob->data) >= PAPR_SYSPARM_MAX_INPUT);
+ static_assert(sizeof(kern_spbuf->val) >= PAPR_SYSPARM_MAX_INPUT);
+
+ if (len > PAPR_SYSPARM_MAX_INPUT)
+ return ERR_PTR(-EINVAL);
+
+ kern_spbuf = papr_sysparm_buf_alloc();
+ if (!kern_spbuf)
+ return ERR_PTR(-ENOMEM);
+
+ papr_sysparm_buf_set_length(kern_spbuf, len);
+
+ if (len > 0 && copy_from_user(kern_spbuf->val, user_iob->data, len)) {
+ err = -EFAULT;
+ goto free_sysparm_buf;
+ }
+
+ return kern_spbuf;
+
+free_sysparm_buf:
+ papr_sysparm_buf_free(kern_spbuf);
+ return ERR_PTR(err);
+}
+
+static int papr_sysparm_buf_to_user(const struct papr_sysparm_buf *kern_spbuf,
+ struct papr_sysparm_io_block __user *user_iob)
+{
+ u16 len_out = papr_sysparm_buf_get_length(kern_spbuf);
+
+ if (put_user(len_out, &user_iob->length))
+ return -EFAULT;
+
+ static_assert(sizeof(user_iob->data) >= PAPR_SYSPARM_MAX_OUTPUT);
+ static_assert(sizeof(kern_spbuf->val) >= PAPR_SYSPARM_MAX_OUTPUT);
+
+ if (copy_to_user(user_iob->data, kern_spbuf->val, PAPR_SYSPARM_MAX_OUTPUT))
+ return -EFAULT;
+
+ return 0;
+}
+
+static long papr_sysparm_ioctl_get(struct papr_sysparm_io_block __user *user_iob)
+{
+ struct papr_sysparm_buf *kern_spbuf;
+ papr_sysparm_t param;
+ long ret;
+
+ if (get_user(param.token, &user_iob->parameter))
+ return -EFAULT;
+
+ kern_spbuf = papr_sysparm_buf_from_user(user_iob);
+ if (IS_ERR(kern_spbuf))
+ return PTR_ERR(kern_spbuf);
+
+ ret = papr_sysparm_get(param, kern_spbuf);
+ if (ret)
+ goto free_sysparm_buf;
+
+ ret = papr_sysparm_buf_to_user(kern_spbuf, user_iob);
+ if (ret)
+ goto free_sysparm_buf;
+
+ ret = 0;
+
+free_sysparm_buf:
+ papr_sysparm_buf_free(kern_spbuf);
+ return ret;
+}
+
+
+static long papr_sysparm_ioctl_set(struct papr_sysparm_io_block __user *user_iob)
+{
+ struct papr_sysparm_buf *kern_spbuf;
+ papr_sysparm_t param;
+ long ret;
+
+ if (get_user(param.token, &user_iob->parameter))
+ return -EFAULT;
+
+ kern_spbuf = papr_sysparm_buf_from_user(user_iob);
+ if (IS_ERR(kern_spbuf))
+ return PTR_ERR(kern_spbuf);
+
+ ret = papr_sysparm_set(param, kern_spbuf);
+ if (ret)
+ goto free_sysparm_buf;
+
+ ret = 0;
+
+free_sysparm_buf:
+ papr_sysparm_buf_free(kern_spbuf);
+ return ret;
+}
+
+static long papr_sysparm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
+{
+ void __user *argp = (__force void __user *)arg;
+ long ret;
+
+ switch (ioctl) {
+ case PAPR_SYSPARM_IOC_GET:
+ ret = papr_sysparm_ioctl_get(argp);
+ break;
+ case PAPR_SYSPARM_IOC_SET:
+ ret = papr_sysparm_ioctl_set(argp);
+ break;
+ default:
+ ret = -ENOIOCTLCMD;
+ break;
+ }
+ return ret;
+}
+
+static const struct file_operations papr_sysparm_ops = {
+ .unlocked_ioctl = papr_sysparm_ioctl,
+};
+
+static struct miscdevice papr_sysparm_dev = {
+ .minor = MISC_DYNAMIC_MINOR,
+ .name = "papr-sysparm",
+ .fops = &papr_sysparm_ops,
+};
+
+static __init int papr_sysparm_init(void)
+{
+ if (!rtas_function_implemented(RTAS_FN_IBM_GET_SYSTEM_PARAMETER))
+ return -ENODEV;
+
+ return misc_register(&papr_sysparm_dev);
+}
+machine_device_initcall(pseries, papr_sysparm_init);
--
2.41.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 5/7] powerpc/pseries/papr-sysparm: expose chardev API to user space
@ 2023-10-06 21:01 ` Nathan Lynch via B4 Relay
0 siblings, 0 replies; 20+ messages in thread
From: Nathan Lynch via B4 Relay @ 2023-10-06 21:01 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Michal Suchánek
Cc: Nathan Lynch, tyreld, gcwilson, linuxppc-dev
From: Nathan Lynch <nathanl@linux.ibm.com>
Until now the papr_sysparm APIs have been kernel-internal. But user
space needs access to PAPR system parameters too. The only method
available to user space today to get or set system parameters is using
sys_rtas() and /dev/mem to pass RTAS-addressable buffers between user
space and firmware. This is incompatible with lockdown and should be
deprecated.
So provide an alternative ABI to user space in the form of a
/dev/papr-sysparm character device with just two ioctl commands (get
and set). The data payloads involved are small enough to fit in the
ioctl argument buffer, making the code relatively simple.
Exposing the system parameters through sysfs has been considered but
it would be too awkward:
* The kernel currently does not have to contain an exhaustive list of
defined system parameters. This is a convenient property to maintain
because we don't have to update the kernel whenever a new parameter
is added to PAPR. Exporting a named attribute in sysfs for each
parameter would negate this.
* Some system parameters are text-based and some are not.
* Retrieval of at least one system parameter requires input data,
which a simple read-oriented interface can't support.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
Documentation/userspace-api/ioctl/ioctl-number.rst | 2 +
arch/powerpc/include/asm/papr-sysparm.h | 17 ++-
arch/powerpc/include/uapi/asm/papr-sysparm.h | 58 ++++++++
arch/powerpc/platforms/pseries/papr-sysparm.c | 160 ++++++++++++++++++++-
4 files changed, 229 insertions(+), 8 deletions(-)
diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
index a950545bf7cd..d8b6cb1a3636 100644
--- a/Documentation/userspace-api/ioctl/ioctl-number.rst
+++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
@@ -351,6 +351,8 @@ Code Seq# Include File Comments
<mailto:mostrows@styx.uwaterloo.ca>
0xB2 00 arch/powerpc/include/uapi/asm/papr-vpd.h powerpc/pseries VPD API
<mailto:linuxppc-dev>
+0xB2 01-02 arch/powerpc/include/uapi/asm/papr-sysparm.h powerpc/pseries system parameter API
+ <mailto:linuxppc-dev>
0xB3 00 linux/mmc/ioctl.h
0xB4 00-0F linux/gpio.h <mailto:linux-gpio@vger.kernel.org>
0xB5 00-0F uapi/linux/rpmsg.h <mailto:linux-remoteproc@vger.kernel.org>
diff --git a/arch/powerpc/include/asm/papr-sysparm.h b/arch/powerpc/include/asm/papr-sysparm.h
index f5fdbd8ae9db..0dbbff59101d 100644
--- a/arch/powerpc/include/asm/papr-sysparm.h
+++ b/arch/powerpc/include/asm/papr-sysparm.h
@@ -2,8 +2,10 @@
#ifndef _ASM_POWERPC_PAPR_SYSPARM_H
#define _ASM_POWERPC_PAPR_SYSPARM_H
+#include <uapi/asm/papr-sysparm.h>
+
typedef struct {
- const u32 token;
+ u32 token;
} papr_sysparm_t;
#define mk_papr_sysparm(x_) ((papr_sysparm_t){ .token = x_, })
@@ -20,11 +22,14 @@ typedef struct {
#define PAPR_SYSPARM_TLB_BLOCK_INVALIDATE_ATTRS mk_papr_sysparm(50)
#define PAPR_SYSPARM_LPAR_NAME mk_papr_sysparm(55)
-enum {
- PAPR_SYSPARM_MAX_INPUT = 1024,
- PAPR_SYSPARM_MAX_OUTPUT = 4000,
-};
-
+/**
+ * struct papr_sysparm_buf - RTAS work area layout for system parameter functions.
+ *
+ * This is the memory layout of the buffers passed to/from
+ * ibm,get-system-parameter and ibm,set-system-parameter. It is
+ * distinct from the papr_sysparm_io_block structure that is passed
+ * between user space and the kernel.
+ */
struct papr_sysparm_buf {
__be16 len;
char val[PAPR_SYSPARM_MAX_OUTPUT];
diff --git a/arch/powerpc/include/uapi/asm/papr-sysparm.h b/arch/powerpc/include/uapi/asm/papr-sysparm.h
new file mode 100644
index 000000000000..9f9a0f267ea5
--- /dev/null
+++ b/arch/powerpc/include/uapi/asm/papr-sysparm.h
@@ -0,0 +1,58 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_PAPR_SYSPARM_H_
+#define _UAPI_PAPR_SYSPARM_H_
+
+#include <linux/types.h>
+#include <asm/ioctl.h>
+#include <asm/papr-miscdev.h>
+
+enum {
+ PAPR_SYSPARM_MAX_INPUT = 1024,
+ PAPR_SYSPARM_MAX_OUTPUT = 4000,
+};
+
+struct papr_sysparm_io_block {
+ __u32 parameter;
+ __u16 length;
+ char data[PAPR_SYSPARM_MAX_OUTPUT];
+};
+
+/**
+ * PAPR_SYSPARM_IOC_GET - Retrieve the value of a PAPR system parameter.
+ *
+ * Uses _IOWR because of one corner case: Retrieving the value of the
+ * "OS Service Entitlement Status" parameter (60) requires the caller
+ * to supply input data (a date string) in the buffer passed to
+ * firmware. So the @length and @data of the incoming
+ * papr_sysparm_io_block are always used to initialize the work area
+ * supplied to ibm,get-system-parameter. No other parameters are known
+ * to parameterize the result this way, and callers are encouraged
+ * (but not required) to zero-initialize @length and @data in the
+ * common case.
+ *
+ * On error the contents of the ioblock are indeterminate.
+ *
+ * Return:
+ * 0: Success; @length is the length of valid data in @data, not to exceed @PAPR_SYSPARM_MAX_OUTPUT.
+ * -EIO: Platform error. (-1)
+ * -EINVAL: Incorrect data length or format. (-9999)
+ * -EPERM: The calling partition is not allowed to access this parameter. (-9002)
+ * -EOPNOTSUPP: Parameter not supported on this platform (-3)
+ */
+#define PAPR_SYSPARM_IOC_GET _IOWR(PAPR_MISCDEV_IOC_ID, 1, struct papr_sysparm_io_block)
+
+/**
+ * PAPR_SYSPARM_IOC_SET - Update the value of a PAPR system parameter.
+ *
+ * The contents of the ioblock are unchanged regardless of success.
+ *
+ * Return:
+ * 0: Success; the parameter has been updated.
+ * -EIO: Platform error. (-1)
+ * -EINVAL: Incorrect data length or format. (-9999)
+ * -EPERM: The calling partition is not allowed to access this parameter. (-9002)
+ * -EOPNOTSUPP: Parameter not supported on this platform (-3)
+ */
+#define PAPR_SYSPARM_IOC_SET _IOW(PAPR_MISCDEV_IOC_ID, 2, struct papr_sysparm_io_block)
+
+#endif /* _UAPI_PAPR_SYSPARM_H_ */
diff --git a/arch/powerpc/platforms/pseries/papr-sysparm.c b/arch/powerpc/platforms/pseries/papr-sysparm.c
index a1e7aeac7416..cc600f89246c 100644
--- a/arch/powerpc/platforms/pseries/papr-sysparm.c
+++ b/arch/powerpc/platforms/pseries/papr-sysparm.c
@@ -2,14 +2,19 @@
#define pr_fmt(fmt) "papr-sysparm: " fmt
+#include <linux/anon_inodes.h>
#include <linux/bug.h>
+#include <linux/file.h>
+#include <linux/fs.h>
#include <linux/init.h>
#include <linux/kernel.h>
+#include <linux/miscdevice.h>
#include <linux/printk.h>
#include <linux/slab.h>
-#include <asm/rtas.h>
+#include <asm/machdep.h>
#include <asm/papr-sysparm.h>
#include <asm/rtas-work-area.h>
+#include <asm/rtas.h>
struct papr_sysparm_buf *papr_sysparm_buf_alloc(void)
{
@@ -87,7 +92,6 @@ static bool papr_sysparm_buf_can_submit(const struct papr_sysparm_buf *buf)
*
* Return: 0 on success, -errno otherwise. @buf is unmodified on error.
*/
-
int papr_sysparm_get(papr_sysparm_t param, struct papr_sysparm_buf *buf)
{
const s32 token = rtas_function_token(RTAS_FN_IBM_GET_SYSTEM_PARAMETER);
@@ -196,3 +200,155 @@ int papr_sysparm_set(papr_sysparm_t param, const struct papr_sysparm_buf *buf)
return ret;
}
+
+static struct papr_sysparm_buf *
+papr_sysparm_buf_from_user(const struct papr_sysparm_io_block __user *user_iob)
+{
+ struct papr_sysparm_buf *kern_spbuf;
+ papr_sysparm_t param;
+ long err;
+ u16 len;
+
+ /*
+ * The parameter index that userspace wants to query or update.
+ */
+ if (get_user(param.token, &user_iob->parameter))
+ return ERR_PTR(-EFAULT);
+ /*
+ * The length of valid data that userspace claims to be in
+ * user_iob->data[].
+ */
+ if (get_user(len, &user_iob->length))
+ return ERR_PTR(-EFAULT);
+
+ static_assert(sizeof(user_iob->data) >= PAPR_SYSPARM_MAX_INPUT);
+ static_assert(sizeof(kern_spbuf->val) >= PAPR_SYSPARM_MAX_INPUT);
+
+ if (len > PAPR_SYSPARM_MAX_INPUT)
+ return ERR_PTR(-EINVAL);
+
+ kern_spbuf = papr_sysparm_buf_alloc();
+ if (!kern_spbuf)
+ return ERR_PTR(-ENOMEM);
+
+ papr_sysparm_buf_set_length(kern_spbuf, len);
+
+ if (len > 0 && copy_from_user(kern_spbuf->val, user_iob->data, len)) {
+ err = -EFAULT;
+ goto free_sysparm_buf;
+ }
+
+ return kern_spbuf;
+
+free_sysparm_buf:
+ papr_sysparm_buf_free(kern_spbuf);
+ return ERR_PTR(err);
+}
+
+static int papr_sysparm_buf_to_user(const struct papr_sysparm_buf *kern_spbuf,
+ struct papr_sysparm_io_block __user *user_iob)
+{
+ u16 len_out = papr_sysparm_buf_get_length(kern_spbuf);
+
+ if (put_user(len_out, &user_iob->length))
+ return -EFAULT;
+
+ static_assert(sizeof(user_iob->data) >= PAPR_SYSPARM_MAX_OUTPUT);
+ static_assert(sizeof(kern_spbuf->val) >= PAPR_SYSPARM_MAX_OUTPUT);
+
+ if (copy_to_user(user_iob->data, kern_spbuf->val, PAPR_SYSPARM_MAX_OUTPUT))
+ return -EFAULT;
+
+ return 0;
+}
+
+static long papr_sysparm_ioctl_get(struct papr_sysparm_io_block __user *user_iob)
+{
+ struct papr_sysparm_buf *kern_spbuf;
+ papr_sysparm_t param;
+ long ret;
+
+ if (get_user(param.token, &user_iob->parameter))
+ return -EFAULT;
+
+ kern_spbuf = papr_sysparm_buf_from_user(user_iob);
+ if (IS_ERR(kern_spbuf))
+ return PTR_ERR(kern_spbuf);
+
+ ret = papr_sysparm_get(param, kern_spbuf);
+ if (ret)
+ goto free_sysparm_buf;
+
+ ret = papr_sysparm_buf_to_user(kern_spbuf, user_iob);
+ if (ret)
+ goto free_sysparm_buf;
+
+ ret = 0;
+
+free_sysparm_buf:
+ papr_sysparm_buf_free(kern_spbuf);
+ return ret;
+}
+
+
+static long papr_sysparm_ioctl_set(struct papr_sysparm_io_block __user *user_iob)
+{
+ struct papr_sysparm_buf *kern_spbuf;
+ papr_sysparm_t param;
+ long ret;
+
+ if (get_user(param.token, &user_iob->parameter))
+ return -EFAULT;
+
+ kern_spbuf = papr_sysparm_buf_from_user(user_iob);
+ if (IS_ERR(kern_spbuf))
+ return PTR_ERR(kern_spbuf);
+
+ ret = papr_sysparm_set(param, kern_spbuf);
+ if (ret)
+ goto free_sysparm_buf;
+
+ ret = 0;
+
+free_sysparm_buf:
+ papr_sysparm_buf_free(kern_spbuf);
+ return ret;
+}
+
+static long papr_sysparm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
+{
+ void __user *argp = (__force void __user *)arg;
+ long ret;
+
+ switch (ioctl) {
+ case PAPR_SYSPARM_IOC_GET:
+ ret = papr_sysparm_ioctl_get(argp);
+ break;
+ case PAPR_SYSPARM_IOC_SET:
+ ret = papr_sysparm_ioctl_set(argp);
+ break;
+ default:
+ ret = -ENOIOCTLCMD;
+ break;
+ }
+ return ret;
+}
+
+static const struct file_operations papr_sysparm_ops = {
+ .unlocked_ioctl = papr_sysparm_ioctl,
+};
+
+static struct miscdevice papr_sysparm_dev = {
+ .minor = MISC_DYNAMIC_MINOR,
+ .name = "papr-sysparm",
+ .fops = &papr_sysparm_ops,
+};
+
+static __init int papr_sysparm_init(void)
+{
+ if (!rtas_function_implemented(RTAS_FN_IBM_GET_SYSTEM_PARAMETER))
+ return -ENODEV;
+
+ return misc_register(&papr_sysparm_dev);
+}
+machine_device_initcall(pseries, papr_sysparm_init);
--
2.41.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 6/7] powerpc/selftests: add test for papr-vpd
2023-10-06 21:01 ` Nathan Lynch via B4 Relay
@ 2023-10-06 21:01 ` Nathan Lynch via B4 Relay
-1 siblings, 0 replies; 20+ messages in thread
From: Nathan Lynch @ 2023-10-06 21:01 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Michal Suchánek
Cc: linuxppc-dev, tyreld, gcwilson, Nathan Lynch
Add selftests for /dev/papr-vpd, exercising the common expected use
cases:
* Retrieve all VPD by passing an empty location code.
* Retrieve the "system VPD" by passing a location code derived from DT
root node properties, as done by the vpdupdate command.
The tests also verify that certain intended properties of the driver
hold:
* Passing an unterminated location code to PAPR_VPD_CREATE_HANDLE gets
EINVAL.
* Passing a NULL location code pointer to PAPR_VPD_CREATE_HANDLE gets
EFAULT.
* Closing the device node without first issuing a
PAPR_VPD_CREATE_HANDLE command to it succeeds.
* Releasing a handle without first consuming any data from it
succeeds.
* Re-reading the contents of a handle returns the same data as the
first time.
Some minimal validation of the returned data is performed.
The tests are skipped on systems where the papr-vpd driver does not
initialize, making this useful only on PowerVM LPARs at this point.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
tools/testing/selftests/powerpc/Makefile | 1 +
.../testing/selftests/powerpc/papr_vpd/.gitignore | 1 +
tools/testing/selftests/powerpc/papr_vpd/Makefile | 12 +
.../testing/selftests/powerpc/papr_vpd/papr_vpd.c | 352 +++++++++++++++++++++
4 files changed, 366 insertions(+)
diff --git a/tools/testing/selftests/powerpc/Makefile b/tools/testing/selftests/powerpc/Makefile
index 7ea42fa02eab..05fc68d446c2 100644
--- a/tools/testing/selftests/powerpc/Makefile
+++ b/tools/testing/selftests/powerpc/Makefile
@@ -32,6 +32,7 @@ SUB_DIRS = alignment \
vphn \
math \
papr_attributes \
+ papr_vpd \
ptrace \
security \
mce
diff --git a/tools/testing/selftests/powerpc/papr_vpd/.gitignore b/tools/testing/selftests/powerpc/papr_vpd/.gitignore
new file mode 100644
index 000000000000..49285031a656
--- /dev/null
+++ b/tools/testing/selftests/powerpc/papr_vpd/.gitignore
@@ -0,0 +1 @@
+/papr_vpd
diff --git a/tools/testing/selftests/powerpc/papr_vpd/Makefile b/tools/testing/selftests/powerpc/papr_vpd/Makefile
new file mode 100644
index 000000000000..06b719703bfd
--- /dev/null
+++ b/tools/testing/selftests/powerpc/papr_vpd/Makefile
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0
+noarg:
+ $(MAKE) -C ../
+
+TEST_GEN_PROGS := papr_vpd
+
+top_srcdir = ../../../../..
+include ../../lib.mk
+
+$(TEST_GEN_PROGS): ../harness.c ../utils.c
+
+$(OUTPUT)/papr_vpd: CFLAGS += $(KHDR_INCLUDES)
diff --git a/tools/testing/selftests/powerpc/papr_vpd/papr_vpd.c b/tools/testing/selftests/powerpc/papr_vpd/papr_vpd.c
new file mode 100644
index 000000000000..98cbb9109ee6
--- /dev/null
+++ b/tools/testing/selftests/powerpc/papr_vpd/papr_vpd.c
@@ -0,0 +1,352 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#define _GNU_SOURCE
+#include <errno.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <unistd.h>
+
+#include <asm/papr-vpd.h>
+
+#include "utils.h"
+
+#define DEVPATH "/dev/papr-vpd"
+
+static int dev_papr_vpd_open_close(void)
+{
+ const int devfd = open(DEVPATH, O_RDONLY);
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+ FAIL_IF(close(devfd) != 0);
+
+ return 0;
+}
+
+static int dev_papr_vpd_get_handle_all(void)
+{
+ const int devfd = open(DEVPATH, O_RDONLY);
+ struct papr_location_code lc = { .str = "", };
+ off_t size;
+ int fd;
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+
+ errno = 0;
+ fd = ioctl(devfd, PAPR_VPD_IOC_CREATE_HANDLE, &lc);
+ FAIL_IF(errno != 0);
+ FAIL_IF(fd < 0);
+
+ FAIL_IF(close(devfd) != 0);
+
+ size = lseek(fd, 0, SEEK_END);
+ FAIL_IF(size <= 0);
+
+ void *buf = malloc((size_t)size);
+ FAIL_IF(!buf);
+
+ ssize_t consumed = pread(fd, buf, size, 0);
+ FAIL_IF(consumed != size);
+
+ /* Ensure EOF */
+ FAIL_IF(read(fd, buf, size) != 0);
+ FAIL_IF(close(fd));
+
+ /* Verify that the buffer looks like VPD */
+ static const char needle[] = "System VPD";
+ FAIL_IF(!memmem(buf, size, needle, strlen(needle)));
+
+ return 0;
+}
+
+static int dev_papr_vpd_get_handle_byte_at_a_time(void)
+{
+ const int devfd = open(DEVPATH, O_RDONLY);
+ struct papr_location_code lc = { .str = "", };
+ int fd;
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+
+ errno = 0;
+ fd = ioctl(devfd, PAPR_VPD_IOC_CREATE_HANDLE, &lc);
+ FAIL_IF(errno != 0);
+ FAIL_IF(fd < 0);
+
+ FAIL_IF(close(devfd) != 0);
+
+ size_t consumed = 0;
+ while (1) {
+ ssize_t res;
+ char c;
+
+ errno = 0;
+ res = read(fd, &c, sizeof(c));
+ FAIL_IF(res > sizeof(c));
+ FAIL_IF(res < 0);
+ FAIL_IF(errno != 0);
+ consumed += res;
+ if (res == 0)
+ break;
+ }
+
+ FAIL_IF(consumed != lseek(fd, 0, SEEK_END));
+
+ FAIL_IF(close(fd));
+
+ return 0;
+}
+
+
+static int dev_papr_vpd_unterm_loc_code(void)
+{
+ const int devfd = open(DEVPATH, O_RDONLY);
+ struct papr_location_code lc = {};
+ int fd;
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+
+ /*
+ * Place a non-null byte in every element of loc_code; the
+ * driver should reject this input.
+ */
+ memset(lc.str, 'x', ARRAY_SIZE(lc.str));
+
+ errno = 0;
+ fd = ioctl(devfd, PAPR_VPD_IOC_CREATE_HANDLE, &lc);
+ FAIL_IF(fd != -1);
+ FAIL_IF(errno != EINVAL);
+
+ FAIL_IF(close(devfd) != 0);
+ return 0;
+}
+
+static int dev_papr_vpd_null_handle(void)
+{
+ const int devfd = open(DEVPATH, O_RDONLY);
+ int rc;
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+
+ errno = 0;
+ rc = ioctl(devfd, PAPR_VPD_IOC_CREATE_HANDLE, NULL);
+ FAIL_IF(rc != -1);
+ FAIL_IF(errno != EFAULT);
+
+ FAIL_IF(close(devfd) != 0);
+ return 0;
+}
+
+static int papr_vpd_close_handle_without_reading(void)
+{
+ const int devfd = open(DEVPATH, O_RDONLY);
+ struct papr_location_code lc;
+ int fd;
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+
+ errno = 0;
+ fd = ioctl(devfd, PAPR_VPD_IOC_CREATE_HANDLE, &lc);
+ FAIL_IF(errno != 0);
+ FAIL_IF(fd < 0);
+
+ /* close the handle without reading it */
+ FAIL_IF(close(fd) != 0);
+
+ FAIL_IF(close(devfd) != 0);
+ return 0;
+}
+
+static int papr_vpd_reread(void)
+{
+ const int devfd = open(DEVPATH, O_RDONLY);
+ struct papr_location_code lc = { .str = "", };
+ int fd;
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+
+ errno = 0;
+ fd = ioctl(devfd, PAPR_VPD_IOC_CREATE_HANDLE, &lc);
+ FAIL_IF(errno != 0);
+ FAIL_IF(fd < 0);
+
+ FAIL_IF(close(devfd) != 0);
+
+ const off_t size = lseek(fd, 0, SEEK_END);
+ FAIL_IF(size <= 0);
+
+ char *bufs[2];
+
+ for (size_t i = 0; i < ARRAY_SIZE(bufs); ++i) {
+ bufs[i] = malloc(size);
+ FAIL_IF(!bufs[i]);
+ ssize_t consumed = pread(fd, bufs[i], size, 0);
+ FAIL_IF(consumed != size);
+ }
+
+ FAIL_IF(memcmp(bufs[0], bufs[1], size));
+
+ FAIL_IF(close(fd) != 0);
+
+ return 0;
+}
+
+static int get_system_loc_code(struct papr_location_code *lc)
+{
+ static const char system_id_path[] = "/sys/firmware/devicetree/base/system-id";
+ static const char model_path[] = "/sys/firmware/devicetree/base/model";
+ char *system_id;
+ char *model;
+ int err = -1;
+
+ if (read_file_alloc(model_path, &model, NULL))
+ return err;
+
+ if (read_file_alloc(system_id_path, &system_id, NULL))
+ goto free_model;
+
+ char *mtm;
+ int sscanf_ret = sscanf(model, "IBM,%ms", &mtm);
+ if (sscanf_ret != 1)
+ goto free_system_id;
+
+ char *plant_and_seq;
+ if (sscanf(system_id, "IBM,%*c%*c%ms", &plant_and_seq) != 1)
+ goto free_mtm;
+ /*
+ * Replace - with . to build location code.
+ */
+ char *sep = strchr(mtm, '-');
+ if (!sep)
+ goto free_mtm;
+ else
+ *sep = '.';
+
+ snprintf(lc->str, sizeof(lc->str),
+ "U%s.%s", mtm, plant_and_seq);
+ err = 0;
+
+ free(plant_and_seq);
+free_mtm:
+ free(mtm);
+free_system_id:
+ free(system_id);
+free_model:
+ free(model);
+ return err;
+}
+
+static int papr_vpd_system_loc_code(void)
+{
+ struct papr_location_code lc;
+ const int devfd = open(DEVPATH, O_RDONLY);
+ off_t size;
+ int fd;
+
+ SKIP_IF_MSG(get_system_loc_code(&lc),
+ "Cannot determine system location code");
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+
+ errno = 0;
+ fd = ioctl(devfd, PAPR_VPD_IOC_CREATE_HANDLE, &lc);
+ FAIL_IF(errno != 0);
+ FAIL_IF(fd < 0);
+
+ FAIL_IF(close(devfd) != 0);
+
+ size = lseek(fd, 0, SEEK_END);
+ FAIL_IF(size <= 0);
+
+ void *buf = malloc((size_t)size);
+ FAIL_IF(!buf);
+
+ ssize_t consumed = pread(fd, buf, size, 0);
+ FAIL_IF(consumed != size);
+
+ /* Ensure EOF */
+ FAIL_IF(read(fd, buf, size) != 0);
+ FAIL_IF(close(fd));
+
+ /* Verify that the buffer looks like VPD */
+ static const char needle[] = "System VPD";
+ FAIL_IF(!memmem(buf, size, needle, strlen(needle)));
+
+ return 0;
+}
+
+struct vpd_test {
+ int (*function)(void);
+ const char *description;
+};
+
+static const struct vpd_test vpd_tests[] = {
+ {
+ .function = dev_papr_vpd_open_close,
+ .description = "open/close " DEVPATH,
+ },
+ {
+ .function = dev_papr_vpd_unterm_loc_code,
+ .description = "ensure EINVAL on unterminated location code",
+ },
+ {
+ .function = dev_papr_vpd_null_handle,
+ .description = "ensure EFAULT on bad handle addr",
+ },
+ {
+ .function = dev_papr_vpd_get_handle_all,
+ .description = "get handle for all VPD"
+ },
+ {
+ .function = papr_vpd_close_handle_without_reading,
+ .description = "close handle without consuming VPD"
+ },
+ {
+ .function = dev_papr_vpd_get_handle_byte_at_a_time,
+ .description = "read all VPD one byte at a time"
+ },
+ {
+ .function = papr_vpd_reread,
+ .description = "ensure re-read yields same results"
+ },
+ {
+ .function = papr_vpd_system_loc_code,
+ .description = "get handle for system VPD"
+ },
+};
+
+int main(void)
+{
+ size_t fails = 0;
+
+ for (size_t i = 0; i < ARRAY_SIZE(vpd_tests); ++i) {
+ const struct vpd_test *t = &vpd_tests[i];
+
+ if (test_harness(t->function, t->description))
+ ++fails;
+ }
+
+ return fails == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
+}
--
2.41.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 6/7] powerpc/selftests: add test for papr-vpd
@ 2023-10-06 21:01 ` Nathan Lynch via B4 Relay
0 siblings, 0 replies; 20+ messages in thread
From: Nathan Lynch via B4 Relay @ 2023-10-06 21:01 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Michal Suchánek
Cc: Nathan Lynch, tyreld, gcwilson, linuxppc-dev
From: Nathan Lynch <nathanl@linux.ibm.com>
Add selftests for /dev/papr-vpd, exercising the common expected use
cases:
* Retrieve all VPD by passing an empty location code.
* Retrieve the "system VPD" by passing a location code derived from DT
root node properties, as done by the vpdupdate command.
The tests also verify that certain intended properties of the driver
hold:
* Passing an unterminated location code to PAPR_VPD_CREATE_HANDLE gets
EINVAL.
* Passing a NULL location code pointer to PAPR_VPD_CREATE_HANDLE gets
EFAULT.
* Closing the device node without first issuing a
PAPR_VPD_CREATE_HANDLE command to it succeeds.
* Releasing a handle without first consuming any data from it
succeeds.
* Re-reading the contents of a handle returns the same data as the
first time.
Some minimal validation of the returned data is performed.
The tests are skipped on systems where the papr-vpd driver does not
initialize, making this useful only on PowerVM LPARs at this point.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
tools/testing/selftests/powerpc/Makefile | 1 +
.../testing/selftests/powerpc/papr_vpd/.gitignore | 1 +
tools/testing/selftests/powerpc/papr_vpd/Makefile | 12 +
.../testing/selftests/powerpc/papr_vpd/papr_vpd.c | 352 +++++++++++++++++++++
4 files changed, 366 insertions(+)
diff --git a/tools/testing/selftests/powerpc/Makefile b/tools/testing/selftests/powerpc/Makefile
index 7ea42fa02eab..05fc68d446c2 100644
--- a/tools/testing/selftests/powerpc/Makefile
+++ b/tools/testing/selftests/powerpc/Makefile
@@ -32,6 +32,7 @@ SUB_DIRS = alignment \
vphn \
math \
papr_attributes \
+ papr_vpd \
ptrace \
security \
mce
diff --git a/tools/testing/selftests/powerpc/papr_vpd/.gitignore b/tools/testing/selftests/powerpc/papr_vpd/.gitignore
new file mode 100644
index 000000000000..49285031a656
--- /dev/null
+++ b/tools/testing/selftests/powerpc/papr_vpd/.gitignore
@@ -0,0 +1 @@
+/papr_vpd
diff --git a/tools/testing/selftests/powerpc/papr_vpd/Makefile b/tools/testing/selftests/powerpc/papr_vpd/Makefile
new file mode 100644
index 000000000000..06b719703bfd
--- /dev/null
+++ b/tools/testing/selftests/powerpc/papr_vpd/Makefile
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0
+noarg:
+ $(MAKE) -C ../
+
+TEST_GEN_PROGS := papr_vpd
+
+top_srcdir = ../../../../..
+include ../../lib.mk
+
+$(TEST_GEN_PROGS): ../harness.c ../utils.c
+
+$(OUTPUT)/papr_vpd: CFLAGS += $(KHDR_INCLUDES)
diff --git a/tools/testing/selftests/powerpc/papr_vpd/papr_vpd.c b/tools/testing/selftests/powerpc/papr_vpd/papr_vpd.c
new file mode 100644
index 000000000000..98cbb9109ee6
--- /dev/null
+++ b/tools/testing/selftests/powerpc/papr_vpd/papr_vpd.c
@@ -0,0 +1,352 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#define _GNU_SOURCE
+#include <errno.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <unistd.h>
+
+#include <asm/papr-vpd.h>
+
+#include "utils.h"
+
+#define DEVPATH "/dev/papr-vpd"
+
+static int dev_papr_vpd_open_close(void)
+{
+ const int devfd = open(DEVPATH, O_RDONLY);
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+ FAIL_IF(close(devfd) != 0);
+
+ return 0;
+}
+
+static int dev_papr_vpd_get_handle_all(void)
+{
+ const int devfd = open(DEVPATH, O_RDONLY);
+ struct papr_location_code lc = { .str = "", };
+ off_t size;
+ int fd;
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+
+ errno = 0;
+ fd = ioctl(devfd, PAPR_VPD_IOC_CREATE_HANDLE, &lc);
+ FAIL_IF(errno != 0);
+ FAIL_IF(fd < 0);
+
+ FAIL_IF(close(devfd) != 0);
+
+ size = lseek(fd, 0, SEEK_END);
+ FAIL_IF(size <= 0);
+
+ void *buf = malloc((size_t)size);
+ FAIL_IF(!buf);
+
+ ssize_t consumed = pread(fd, buf, size, 0);
+ FAIL_IF(consumed != size);
+
+ /* Ensure EOF */
+ FAIL_IF(read(fd, buf, size) != 0);
+ FAIL_IF(close(fd));
+
+ /* Verify that the buffer looks like VPD */
+ static const char needle[] = "System VPD";
+ FAIL_IF(!memmem(buf, size, needle, strlen(needle)));
+
+ return 0;
+}
+
+static int dev_papr_vpd_get_handle_byte_at_a_time(void)
+{
+ const int devfd = open(DEVPATH, O_RDONLY);
+ struct papr_location_code lc = { .str = "", };
+ int fd;
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+
+ errno = 0;
+ fd = ioctl(devfd, PAPR_VPD_IOC_CREATE_HANDLE, &lc);
+ FAIL_IF(errno != 0);
+ FAIL_IF(fd < 0);
+
+ FAIL_IF(close(devfd) != 0);
+
+ size_t consumed = 0;
+ while (1) {
+ ssize_t res;
+ char c;
+
+ errno = 0;
+ res = read(fd, &c, sizeof(c));
+ FAIL_IF(res > sizeof(c));
+ FAIL_IF(res < 0);
+ FAIL_IF(errno != 0);
+ consumed += res;
+ if (res == 0)
+ break;
+ }
+
+ FAIL_IF(consumed != lseek(fd, 0, SEEK_END));
+
+ FAIL_IF(close(fd));
+
+ return 0;
+}
+
+
+static int dev_papr_vpd_unterm_loc_code(void)
+{
+ const int devfd = open(DEVPATH, O_RDONLY);
+ struct papr_location_code lc = {};
+ int fd;
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+
+ /*
+ * Place a non-null byte in every element of loc_code; the
+ * driver should reject this input.
+ */
+ memset(lc.str, 'x', ARRAY_SIZE(lc.str));
+
+ errno = 0;
+ fd = ioctl(devfd, PAPR_VPD_IOC_CREATE_HANDLE, &lc);
+ FAIL_IF(fd != -1);
+ FAIL_IF(errno != EINVAL);
+
+ FAIL_IF(close(devfd) != 0);
+ return 0;
+}
+
+static int dev_papr_vpd_null_handle(void)
+{
+ const int devfd = open(DEVPATH, O_RDONLY);
+ int rc;
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+
+ errno = 0;
+ rc = ioctl(devfd, PAPR_VPD_IOC_CREATE_HANDLE, NULL);
+ FAIL_IF(rc != -1);
+ FAIL_IF(errno != EFAULT);
+
+ FAIL_IF(close(devfd) != 0);
+ return 0;
+}
+
+static int papr_vpd_close_handle_without_reading(void)
+{
+ const int devfd = open(DEVPATH, O_RDONLY);
+ struct papr_location_code lc;
+ int fd;
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+
+ errno = 0;
+ fd = ioctl(devfd, PAPR_VPD_IOC_CREATE_HANDLE, &lc);
+ FAIL_IF(errno != 0);
+ FAIL_IF(fd < 0);
+
+ /* close the handle without reading it */
+ FAIL_IF(close(fd) != 0);
+
+ FAIL_IF(close(devfd) != 0);
+ return 0;
+}
+
+static int papr_vpd_reread(void)
+{
+ const int devfd = open(DEVPATH, O_RDONLY);
+ struct papr_location_code lc = { .str = "", };
+ int fd;
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+
+ errno = 0;
+ fd = ioctl(devfd, PAPR_VPD_IOC_CREATE_HANDLE, &lc);
+ FAIL_IF(errno != 0);
+ FAIL_IF(fd < 0);
+
+ FAIL_IF(close(devfd) != 0);
+
+ const off_t size = lseek(fd, 0, SEEK_END);
+ FAIL_IF(size <= 0);
+
+ char *bufs[2];
+
+ for (size_t i = 0; i < ARRAY_SIZE(bufs); ++i) {
+ bufs[i] = malloc(size);
+ FAIL_IF(!bufs[i]);
+ ssize_t consumed = pread(fd, bufs[i], size, 0);
+ FAIL_IF(consumed != size);
+ }
+
+ FAIL_IF(memcmp(bufs[0], bufs[1], size));
+
+ FAIL_IF(close(fd) != 0);
+
+ return 0;
+}
+
+static int get_system_loc_code(struct papr_location_code *lc)
+{
+ static const char system_id_path[] = "/sys/firmware/devicetree/base/system-id";
+ static const char model_path[] = "/sys/firmware/devicetree/base/model";
+ char *system_id;
+ char *model;
+ int err = -1;
+
+ if (read_file_alloc(model_path, &model, NULL))
+ return err;
+
+ if (read_file_alloc(system_id_path, &system_id, NULL))
+ goto free_model;
+
+ char *mtm;
+ int sscanf_ret = sscanf(model, "IBM,%ms", &mtm);
+ if (sscanf_ret != 1)
+ goto free_system_id;
+
+ char *plant_and_seq;
+ if (sscanf(system_id, "IBM,%*c%*c%ms", &plant_and_seq) != 1)
+ goto free_mtm;
+ /*
+ * Replace - with . to build location code.
+ */
+ char *sep = strchr(mtm, '-');
+ if (!sep)
+ goto free_mtm;
+ else
+ *sep = '.';
+
+ snprintf(lc->str, sizeof(lc->str),
+ "U%s.%s", mtm, plant_and_seq);
+ err = 0;
+
+ free(plant_and_seq);
+free_mtm:
+ free(mtm);
+free_system_id:
+ free(system_id);
+free_model:
+ free(model);
+ return err;
+}
+
+static int papr_vpd_system_loc_code(void)
+{
+ struct papr_location_code lc;
+ const int devfd = open(DEVPATH, O_RDONLY);
+ off_t size;
+ int fd;
+
+ SKIP_IF_MSG(get_system_loc_code(&lc),
+ "Cannot determine system location code");
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+
+ errno = 0;
+ fd = ioctl(devfd, PAPR_VPD_IOC_CREATE_HANDLE, &lc);
+ FAIL_IF(errno != 0);
+ FAIL_IF(fd < 0);
+
+ FAIL_IF(close(devfd) != 0);
+
+ size = lseek(fd, 0, SEEK_END);
+ FAIL_IF(size <= 0);
+
+ void *buf = malloc((size_t)size);
+ FAIL_IF(!buf);
+
+ ssize_t consumed = pread(fd, buf, size, 0);
+ FAIL_IF(consumed != size);
+
+ /* Ensure EOF */
+ FAIL_IF(read(fd, buf, size) != 0);
+ FAIL_IF(close(fd));
+
+ /* Verify that the buffer looks like VPD */
+ static const char needle[] = "System VPD";
+ FAIL_IF(!memmem(buf, size, needle, strlen(needle)));
+
+ return 0;
+}
+
+struct vpd_test {
+ int (*function)(void);
+ const char *description;
+};
+
+static const struct vpd_test vpd_tests[] = {
+ {
+ .function = dev_papr_vpd_open_close,
+ .description = "open/close " DEVPATH,
+ },
+ {
+ .function = dev_papr_vpd_unterm_loc_code,
+ .description = "ensure EINVAL on unterminated location code",
+ },
+ {
+ .function = dev_papr_vpd_null_handle,
+ .description = "ensure EFAULT on bad handle addr",
+ },
+ {
+ .function = dev_papr_vpd_get_handle_all,
+ .description = "get handle for all VPD"
+ },
+ {
+ .function = papr_vpd_close_handle_without_reading,
+ .description = "close handle without consuming VPD"
+ },
+ {
+ .function = dev_papr_vpd_get_handle_byte_at_a_time,
+ .description = "read all VPD one byte at a time"
+ },
+ {
+ .function = papr_vpd_reread,
+ .description = "ensure re-read yields same results"
+ },
+ {
+ .function = papr_vpd_system_loc_code,
+ .description = "get handle for system VPD"
+ },
+};
+
+int main(void)
+{
+ size_t fails = 0;
+
+ for (size_t i = 0; i < ARRAY_SIZE(vpd_tests); ++i) {
+ const struct vpd_test *t = &vpd_tests[i];
+
+ if (test_harness(t->function, t->description))
+ ++fails;
+ }
+
+ return fails == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
+}
--
2.41.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 7/7] powerpc/selftests: add test for papr-sysparm
2023-10-06 21:01 ` Nathan Lynch via B4 Relay
@ 2023-10-06 21:01 ` Nathan Lynch via B4 Relay
-1 siblings, 0 replies; 20+ messages in thread
From: Nathan Lynch @ 2023-10-06 21:01 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Michal Suchánek
Cc: linuxppc-dev, tyreld, gcwilson, Nathan Lynch
Consistently testing system parameter access is a bit difficult by
nature -- the set of parameters available depends on the model and
system configuration, and updating a parameter should be considered a
destructive operation reserved for the admin.
So we validate some of the error paths and retrieve the SPLPAR
characteristics string, but not much else.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
tools/testing/selftests/powerpc/Makefile | 1 +
.../selftests/powerpc/papr_sysparm/.gitignore | 1 +
.../selftests/powerpc/papr_sysparm/Makefile | 12 ++
.../selftests/powerpc/papr_sysparm/papr_sysparm.c | 164 +++++++++++++++++++++
4 files changed, 178 insertions(+)
diff --git a/tools/testing/selftests/powerpc/Makefile b/tools/testing/selftests/powerpc/Makefile
index 05fc68d446c2..c376151982c4 100644
--- a/tools/testing/selftests/powerpc/Makefile
+++ b/tools/testing/selftests/powerpc/Makefile
@@ -33,6 +33,7 @@ SUB_DIRS = alignment \
math \
papr_attributes \
papr_vpd \
+ papr_sysparm \
ptrace \
security \
mce
diff --git a/tools/testing/selftests/powerpc/papr_sysparm/.gitignore b/tools/testing/selftests/powerpc/papr_sysparm/.gitignore
new file mode 100644
index 000000000000..f2a69bf59d40
--- /dev/null
+++ b/tools/testing/selftests/powerpc/papr_sysparm/.gitignore
@@ -0,0 +1 @@
+/papr_sysparm
diff --git a/tools/testing/selftests/powerpc/papr_sysparm/Makefile b/tools/testing/selftests/powerpc/papr_sysparm/Makefile
new file mode 100644
index 000000000000..7f79e437634a
--- /dev/null
+++ b/tools/testing/selftests/powerpc/papr_sysparm/Makefile
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0
+noarg:
+ $(MAKE) -C ../
+
+TEST_GEN_PROGS := papr_sysparm
+
+top_srcdir = ../../../../..
+include ../../lib.mk
+
+$(TEST_GEN_PROGS): ../harness.c ../utils.c
+
+$(OUTPUT)/papr_sysparm: CFLAGS += $(KHDR_INCLUDES)
diff --git a/tools/testing/selftests/powerpc/papr_sysparm/papr_sysparm.c b/tools/testing/selftests/powerpc/papr_sysparm/papr_sysparm.c
new file mode 100644
index 000000000000..fc25c03e8bc7
--- /dev/null
+++ b/tools/testing/selftests/powerpc/papr_sysparm/papr_sysparm.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <errno.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <sys/ioctl.h>
+#include <unistd.h>
+#include <asm/papr-sysparm.h>
+
+#include "utils.h"
+
+#define DEVPATH "/dev/papr-sysparm"
+
+static int open_close(void)
+{
+ const int devfd = open(DEVPATH, O_RDONLY);
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+ FAIL_IF(close(devfd) != 0);
+
+ return 0;
+}
+
+static int get_splpar(void)
+{
+ struct papr_sysparm_io_block sp = {
+ .parameter = 20, // SPLPAR characteristics
+ };
+ const int devfd = open(DEVPATH, O_RDONLY);
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+ FAIL_IF(ioctl(devfd, PAPR_SYSPARM_IOC_GET, &sp) != 0);
+ FAIL_IF(sp.length == 0);
+ FAIL_IF(sp.length > sizeof(sp.data));
+ FAIL_IF(close(devfd) != 0);
+
+ return 0;
+}
+
+static int get_bad_parameter(void)
+{
+ struct papr_sysparm_io_block sp = {
+ .parameter = UINT32_MAX, // there are only ~60 specified parameters
+ };
+ const int devfd = open(DEVPATH, O_RDONLY);
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+
+ // Ensure expected error
+ FAIL_IF(ioctl(devfd, PAPR_SYSPARM_IOC_GET, &sp) != -1);
+ FAIL_IF(errno != EOPNOTSUPP);
+
+ // Ensure the buffer is unchanged
+ FAIL_IF(sp.length != 0);
+ for (size_t i = 0; i < ARRAY_SIZE(sp.data); ++i)
+ FAIL_IF(sp.data[i] != 0);
+
+ FAIL_IF(close(devfd) != 0);
+
+ return 0;
+}
+
+static int check_efault_common(unsigned long cmd)
+{
+ const int devfd = open(DEVPATH, O_RDONLY);
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+
+ // Ensure expected error
+ FAIL_IF(ioctl(devfd, cmd, NULL) != -1);
+ FAIL_IF(errno != EFAULT);
+
+ FAIL_IF(close(devfd) != 0);
+
+ return 0;
+}
+
+static int check_efault_get(void)
+{
+ return check_efault_common(PAPR_SYSPARM_IOC_GET);
+}
+
+static int check_efault_set(void)
+{
+ return check_efault_common(PAPR_SYSPARM_IOC_SET);
+}
+
+static int set_hmc0(void)
+{
+ struct papr_sysparm_io_block sp = {
+ .parameter = 0, // HMC0, not a settable parameter
+ };
+ const int devfd = open(DEVPATH, O_RDONLY);
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+
+ // Ensure expected error
+ FAIL_IF(ioctl(devfd, PAPR_SYSPARM_IOC_SET, &sp) != -1);
+ FAIL_IF(errno != EPERM);
+
+ FAIL_IF(close(devfd) != 0);
+
+ return 0;
+}
+
+struct sysparm_test {
+ int (*function)(void);
+ const char *description;
+};
+
+static const struct sysparm_test sysparm_tests[] = {
+ {
+ .function = open_close,
+ .description = "open and close " DEVPATH " without issuing commands",
+ },
+ {
+ .function = get_splpar,
+ .description = "retrieve SPLPAR characteristics",
+ },
+ {
+ .function = get_bad_parameter,
+ .description = "verify EOPNOTSUPP for known-bad parameter",
+ },
+ {
+ .function = check_efault_get,
+ .description = "PAPR_SYSPARM_IOC_GET returns EFAULT on bad address",
+ },
+ {
+ .function = check_efault_set,
+ .description = "PAPR_SYSPARM_IOC_SET returns EFAULT on bad address",
+ },
+ {
+ .function = set_hmc0,
+ .description = "ensure EPERM on attempt to update HMC0",
+ },
+};
+
+int main(void)
+{
+ size_t fails = 0;
+
+ for (size_t i = 0; i < ARRAY_SIZE(sysparm_tests); ++i) {
+ const struct sysparm_test *t = &sysparm_tests[i];
+
+ if (test_harness(t->function, t->description))
+ ++fails;
+ }
+
+ return fails == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
+}
--
2.41.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 7/7] powerpc/selftests: add test for papr-sysparm
@ 2023-10-06 21:01 ` Nathan Lynch via B4 Relay
0 siblings, 0 replies; 20+ messages in thread
From: Nathan Lynch via B4 Relay @ 2023-10-06 21:01 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Michal Suchánek
Cc: Nathan Lynch, tyreld, gcwilson, linuxppc-dev
From: Nathan Lynch <nathanl@linux.ibm.com>
Consistently testing system parameter access is a bit difficult by
nature -- the set of parameters available depends on the model and
system configuration, and updating a parameter should be considered a
destructive operation reserved for the admin.
So we validate some of the error paths and retrieve the SPLPAR
characteristics string, but not much else.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
tools/testing/selftests/powerpc/Makefile | 1 +
.../selftests/powerpc/papr_sysparm/.gitignore | 1 +
.../selftests/powerpc/papr_sysparm/Makefile | 12 ++
.../selftests/powerpc/papr_sysparm/papr_sysparm.c | 164 +++++++++++++++++++++
4 files changed, 178 insertions(+)
diff --git a/tools/testing/selftests/powerpc/Makefile b/tools/testing/selftests/powerpc/Makefile
index 05fc68d446c2..c376151982c4 100644
--- a/tools/testing/selftests/powerpc/Makefile
+++ b/tools/testing/selftests/powerpc/Makefile
@@ -33,6 +33,7 @@ SUB_DIRS = alignment \
math \
papr_attributes \
papr_vpd \
+ papr_sysparm \
ptrace \
security \
mce
diff --git a/tools/testing/selftests/powerpc/papr_sysparm/.gitignore b/tools/testing/selftests/powerpc/papr_sysparm/.gitignore
new file mode 100644
index 000000000000..f2a69bf59d40
--- /dev/null
+++ b/tools/testing/selftests/powerpc/papr_sysparm/.gitignore
@@ -0,0 +1 @@
+/papr_sysparm
diff --git a/tools/testing/selftests/powerpc/papr_sysparm/Makefile b/tools/testing/selftests/powerpc/papr_sysparm/Makefile
new file mode 100644
index 000000000000..7f79e437634a
--- /dev/null
+++ b/tools/testing/selftests/powerpc/papr_sysparm/Makefile
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0
+noarg:
+ $(MAKE) -C ../
+
+TEST_GEN_PROGS := papr_sysparm
+
+top_srcdir = ../../../../..
+include ../../lib.mk
+
+$(TEST_GEN_PROGS): ../harness.c ../utils.c
+
+$(OUTPUT)/papr_sysparm: CFLAGS += $(KHDR_INCLUDES)
diff --git a/tools/testing/selftests/powerpc/papr_sysparm/papr_sysparm.c b/tools/testing/selftests/powerpc/papr_sysparm/papr_sysparm.c
new file mode 100644
index 000000000000..fc25c03e8bc7
--- /dev/null
+++ b/tools/testing/selftests/powerpc/papr_sysparm/papr_sysparm.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <errno.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <sys/ioctl.h>
+#include <unistd.h>
+#include <asm/papr-sysparm.h>
+
+#include "utils.h"
+
+#define DEVPATH "/dev/papr-sysparm"
+
+static int open_close(void)
+{
+ const int devfd = open(DEVPATH, O_RDONLY);
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+ FAIL_IF(close(devfd) != 0);
+
+ return 0;
+}
+
+static int get_splpar(void)
+{
+ struct papr_sysparm_io_block sp = {
+ .parameter = 20, // SPLPAR characteristics
+ };
+ const int devfd = open(DEVPATH, O_RDONLY);
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+ FAIL_IF(ioctl(devfd, PAPR_SYSPARM_IOC_GET, &sp) != 0);
+ FAIL_IF(sp.length == 0);
+ FAIL_IF(sp.length > sizeof(sp.data));
+ FAIL_IF(close(devfd) != 0);
+
+ return 0;
+}
+
+static int get_bad_parameter(void)
+{
+ struct papr_sysparm_io_block sp = {
+ .parameter = UINT32_MAX, // there are only ~60 specified parameters
+ };
+ const int devfd = open(DEVPATH, O_RDONLY);
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+
+ // Ensure expected error
+ FAIL_IF(ioctl(devfd, PAPR_SYSPARM_IOC_GET, &sp) != -1);
+ FAIL_IF(errno != EOPNOTSUPP);
+
+ // Ensure the buffer is unchanged
+ FAIL_IF(sp.length != 0);
+ for (size_t i = 0; i < ARRAY_SIZE(sp.data); ++i)
+ FAIL_IF(sp.data[i] != 0);
+
+ FAIL_IF(close(devfd) != 0);
+
+ return 0;
+}
+
+static int check_efault_common(unsigned long cmd)
+{
+ const int devfd = open(DEVPATH, O_RDONLY);
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+
+ // Ensure expected error
+ FAIL_IF(ioctl(devfd, cmd, NULL) != -1);
+ FAIL_IF(errno != EFAULT);
+
+ FAIL_IF(close(devfd) != 0);
+
+ return 0;
+}
+
+static int check_efault_get(void)
+{
+ return check_efault_common(PAPR_SYSPARM_IOC_GET);
+}
+
+static int check_efault_set(void)
+{
+ return check_efault_common(PAPR_SYSPARM_IOC_SET);
+}
+
+static int set_hmc0(void)
+{
+ struct papr_sysparm_io_block sp = {
+ .parameter = 0, // HMC0, not a settable parameter
+ };
+ const int devfd = open(DEVPATH, O_RDONLY);
+
+ SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
+ DEVPATH " not present");
+
+ FAIL_IF(devfd < 0);
+
+ // Ensure expected error
+ FAIL_IF(ioctl(devfd, PAPR_SYSPARM_IOC_SET, &sp) != -1);
+ FAIL_IF(errno != EPERM);
+
+ FAIL_IF(close(devfd) != 0);
+
+ return 0;
+}
+
+struct sysparm_test {
+ int (*function)(void);
+ const char *description;
+};
+
+static const struct sysparm_test sysparm_tests[] = {
+ {
+ .function = open_close,
+ .description = "open and close " DEVPATH " without issuing commands",
+ },
+ {
+ .function = get_splpar,
+ .description = "retrieve SPLPAR characteristics",
+ },
+ {
+ .function = get_bad_parameter,
+ .description = "verify EOPNOTSUPP for known-bad parameter",
+ },
+ {
+ .function = check_efault_get,
+ .description = "PAPR_SYSPARM_IOC_GET returns EFAULT on bad address",
+ },
+ {
+ .function = check_efault_set,
+ .description = "PAPR_SYSPARM_IOC_SET returns EFAULT on bad address",
+ },
+ {
+ .function = set_hmc0,
+ .description = "ensure EPERM on attempt to update HMC0",
+ },
+};
+
+int main(void)
+{
+ size_t fails = 0;
+
+ for (size_t i = 0; i < ARRAY_SIZE(sysparm_tests); ++i) {
+ const struct sysparm_test *t = &sysparm_tests[i];
+
+ if (test_harness(t->function, t->description))
+ ++fails;
+ }
+
+ return fails == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
+}
--
2.41.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 5/7] powerpc/pseries/papr-sysparm: expose chardev API to user space
2023-10-06 21:01 ` Nathan Lynch via B4 Relay
@ 2023-10-06 23:40 ` kernel test robot
-1 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2023-10-06 23:40 UTC (permalink / raw)
To: Nathan Lynch via B4 Relay, Michael Ellerman, Nicholas Piggin,
Michal Suchánek
Cc: oe-kbuild-all, Nathan Lynch, tyreld, gcwilson, linuxppc-dev
Hi Nathan,
kernel test robot noticed the following build warnings:
[auto build test WARNING on eddc90ea2af5933249ea1a78119f2c8ef8d07156]
url: https://github.com/intel-lab-lkp/linux/commits/Nathan-Lynch-via-B4-Relay/powerpc-uapi-export-papr-miscdev-h-header/20231007-050832
base: eddc90ea2af5933249ea1a78119f2c8ef8d07156
patch link: https://lore.kernel.org/r/20231006-papr-sys_rtas-vs-lockdown-v1-5-3a36bfb66e2e%40linux.ibm.com
patch subject: [PATCH 5/7] powerpc/pseries/papr-sysparm: expose chardev API to user space
config: powerpc-allyesconfig (https://download.01.org/0day-ci/archive/20231007/202310070732.jj0Y8Vkb-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231007/202310070732.jj0Y8Vkb-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310070732.jj0Y8Vkb-lkp@intel.com/
All warnings (new ones prefixed by >>):
arch/powerpc/platforms/pseries/papr-sysparm.c: In function 'papr_sysparm_buf_from_user':
>> arch/powerpc/platforms/pseries/papr-sysparm.c:208:24: warning: variable 'param' set but not used [-Wunused-but-set-variable]
208 | papr_sysparm_t param;
| ^~~~~
vim +/param +208 arch/powerpc/platforms/pseries/papr-sysparm.c
203
204 static struct papr_sysparm_buf *
205 papr_sysparm_buf_from_user(const struct papr_sysparm_io_block __user *user_iob)
206 {
207 struct papr_sysparm_buf *kern_spbuf;
> 208 papr_sysparm_t param;
209 long err;
210 u16 len;
211
212 /*
213 * The parameter index that userspace wants to query or update.
214 */
215 if (get_user(param.token, &user_iob->parameter))
216 return ERR_PTR(-EFAULT);
217 /*
218 * The length of valid data that userspace claims to be in
219 * user_iob->data[].
220 */
221 if (get_user(len, &user_iob->length))
222 return ERR_PTR(-EFAULT);
223
224 static_assert(sizeof(user_iob->data) >= PAPR_SYSPARM_MAX_INPUT);
225 static_assert(sizeof(kern_spbuf->val) >= PAPR_SYSPARM_MAX_INPUT);
226
227 if (len > PAPR_SYSPARM_MAX_INPUT)
228 return ERR_PTR(-EINVAL);
229
230 kern_spbuf = papr_sysparm_buf_alloc();
231 if (!kern_spbuf)
232 return ERR_PTR(-ENOMEM);
233
234 papr_sysparm_buf_set_length(kern_spbuf, len);
235
236 if (len > 0 && copy_from_user(kern_spbuf->val, user_iob->data, len)) {
237 err = -EFAULT;
238 goto free_sysparm_buf;
239 }
240
241 return kern_spbuf;
242
243 free_sysparm_buf:
244 papr_sysparm_buf_free(kern_spbuf);
245 return ERR_PTR(err);
246 }
247
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 5/7] powerpc/pseries/papr-sysparm: expose chardev API to user space
@ 2023-10-06 23:40 ` kernel test robot
0 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2023-10-06 23:40 UTC (permalink / raw)
To: Nathan Lynch via B4 Relay, Michael Ellerman, Nicholas Piggin,
Michal Suchánek
Cc: Nathan Lynch, tyreld, gcwilson, linuxppc-dev, oe-kbuild-all
Hi Nathan,
kernel test robot noticed the following build warnings:
[auto build test WARNING on eddc90ea2af5933249ea1a78119f2c8ef8d07156]
url: https://github.com/intel-lab-lkp/linux/commits/Nathan-Lynch-via-B4-Relay/powerpc-uapi-export-papr-miscdev-h-header/20231007-050832
base: eddc90ea2af5933249ea1a78119f2c8ef8d07156
patch link: https://lore.kernel.org/r/20231006-papr-sys_rtas-vs-lockdown-v1-5-3a36bfb66e2e%40linux.ibm.com
patch subject: [PATCH 5/7] powerpc/pseries/papr-sysparm: expose chardev API to user space
config: powerpc-allyesconfig (https://download.01.org/0day-ci/archive/20231007/202310070732.jj0Y8Vkb-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231007/202310070732.jj0Y8Vkb-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310070732.jj0Y8Vkb-lkp@intel.com/
All warnings (new ones prefixed by >>):
arch/powerpc/platforms/pseries/papr-sysparm.c: In function 'papr_sysparm_buf_from_user':
>> arch/powerpc/platforms/pseries/papr-sysparm.c:208:24: warning: variable 'param' set but not used [-Wunused-but-set-variable]
208 | papr_sysparm_t param;
| ^~~~~
vim +/param +208 arch/powerpc/platforms/pseries/papr-sysparm.c
203
204 static struct papr_sysparm_buf *
205 papr_sysparm_buf_from_user(const struct papr_sysparm_io_block __user *user_iob)
206 {
207 struct papr_sysparm_buf *kern_spbuf;
> 208 papr_sysparm_t param;
209 long err;
210 u16 len;
211
212 /*
213 * The parameter index that userspace wants to query or update.
214 */
215 if (get_user(param.token, &user_iob->parameter))
216 return ERR_PTR(-EFAULT);
217 /*
218 * The length of valid data that userspace claims to be in
219 * user_iob->data[].
220 */
221 if (get_user(len, &user_iob->length))
222 return ERR_PTR(-EFAULT);
223
224 static_assert(sizeof(user_iob->data) >= PAPR_SYSPARM_MAX_INPUT);
225 static_assert(sizeof(kern_spbuf->val) >= PAPR_SYSPARM_MAX_INPUT);
226
227 if (len > PAPR_SYSPARM_MAX_INPUT)
228 return ERR_PTR(-EINVAL);
229
230 kern_spbuf = papr_sysparm_buf_alloc();
231 if (!kern_spbuf)
232 return ERR_PTR(-ENOMEM);
233
234 papr_sysparm_buf_set_length(kern_spbuf, len);
235
236 if (len > 0 && copy_from_user(kern_spbuf->val, user_iob->data, len)) {
237 err = -EFAULT;
238 goto free_sysparm_buf;
239 }
240
241 return kern_spbuf;
242
243 free_sysparm_buf:
244 papr_sysparm_buf_free(kern_spbuf);
245 return ERR_PTR(err);
246 }
247
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 5/7] powerpc/pseries/papr-sysparm: expose chardev API to user space
2023-10-06 21:01 ` Nathan Lynch via B4 Relay
@ 2023-10-10 21:44 ` kernel test robot
-1 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2023-10-10 21:44 UTC (permalink / raw)
To: Nathan Lynch via B4 Relay, Michael Ellerman, Nicholas Piggin,
Michal Suchánek
Cc: oe-kbuild-all, Nathan Lynch, tyreld, gcwilson, linuxppc-dev
Hi Nathan,
kernel test robot noticed the following build errors:
[auto build test ERROR on eddc90ea2af5933249ea1a78119f2c8ef8d07156]
url: https://github.com/intel-lab-lkp/linux/commits/Nathan-Lynch-via-B4-Relay/powerpc-uapi-export-papr-miscdev-h-header/20231007-050832
base: eddc90ea2af5933249ea1a78119f2c8ef8d07156
patch link: https://lore.kernel.org/r/20231006-papr-sys_rtas-vs-lockdown-v1-5-3a36bfb66e2e%40linux.ibm.com
patch subject: [PATCH 5/7] powerpc/pseries/papr-sysparm: expose chardev API to user space
config: powerpc64-randconfig-002-20231010 (https://download.01.org/0day-ci/archive/20231011/202310110529.nINDz0Nx-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231011/202310110529.nINDz0Nx-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310110529.nINDz0Nx-lkp@intel.com/
All errors (new ones prefixed by >>):
arch/powerpc/platforms/pseries/papr-sysparm.c: In function 'papr_sysparm_buf_from_user':
>> arch/powerpc/platforms/pseries/papr-sysparm.c:208:24: error: variable 'param' set but not used [-Werror=unused-but-set-variable]
208 | papr_sysparm_t param;
| ^~~~~
cc1: all warnings being treated as errors
vim +/param +208 arch/powerpc/platforms/pseries/papr-sysparm.c
203
204 static struct papr_sysparm_buf *
205 papr_sysparm_buf_from_user(const struct papr_sysparm_io_block __user *user_iob)
206 {
207 struct papr_sysparm_buf *kern_spbuf;
> 208 papr_sysparm_t param;
209 long err;
210 u16 len;
211
212 /*
213 * The parameter index that userspace wants to query or update.
214 */
215 if (get_user(param.token, &user_iob->parameter))
216 return ERR_PTR(-EFAULT);
217 /*
218 * The length of valid data that userspace claims to be in
219 * user_iob->data[].
220 */
221 if (get_user(len, &user_iob->length))
222 return ERR_PTR(-EFAULT);
223
224 static_assert(sizeof(user_iob->data) >= PAPR_SYSPARM_MAX_INPUT);
225 static_assert(sizeof(kern_spbuf->val) >= PAPR_SYSPARM_MAX_INPUT);
226
227 if (len > PAPR_SYSPARM_MAX_INPUT)
228 return ERR_PTR(-EINVAL);
229
230 kern_spbuf = papr_sysparm_buf_alloc();
231 if (!kern_spbuf)
232 return ERR_PTR(-ENOMEM);
233
234 papr_sysparm_buf_set_length(kern_spbuf, len);
235
236 if (len > 0 && copy_from_user(kern_spbuf->val, user_iob->data, len)) {
237 err = -EFAULT;
238 goto free_sysparm_buf;
239 }
240
241 return kern_spbuf;
242
243 free_sysparm_buf:
244 papr_sysparm_buf_free(kern_spbuf);
245 return ERR_PTR(err);
246 }
247
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 5/7] powerpc/pseries/papr-sysparm: expose chardev API to user space
@ 2023-10-10 21:44 ` kernel test robot
0 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2023-10-10 21:44 UTC (permalink / raw)
To: Nathan Lynch via B4 Relay, Michael Ellerman, Nicholas Piggin,
Michal Suchánek
Cc: Nathan Lynch, tyreld, gcwilson, linuxppc-dev, oe-kbuild-all
Hi Nathan,
kernel test robot noticed the following build errors:
[auto build test ERROR on eddc90ea2af5933249ea1a78119f2c8ef8d07156]
url: https://github.com/intel-lab-lkp/linux/commits/Nathan-Lynch-via-B4-Relay/powerpc-uapi-export-papr-miscdev-h-header/20231007-050832
base: eddc90ea2af5933249ea1a78119f2c8ef8d07156
patch link: https://lore.kernel.org/r/20231006-papr-sys_rtas-vs-lockdown-v1-5-3a36bfb66e2e%40linux.ibm.com
patch subject: [PATCH 5/7] powerpc/pseries/papr-sysparm: expose chardev API to user space
config: powerpc64-randconfig-002-20231010 (https://download.01.org/0day-ci/archive/20231011/202310110529.nINDz0Nx-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231011/202310110529.nINDz0Nx-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310110529.nINDz0Nx-lkp@intel.com/
All errors (new ones prefixed by >>):
arch/powerpc/platforms/pseries/papr-sysparm.c: In function 'papr_sysparm_buf_from_user':
>> arch/powerpc/platforms/pseries/papr-sysparm.c:208:24: error: variable 'param' set but not used [-Werror=unused-but-set-variable]
208 | papr_sysparm_t param;
| ^~~~~
cc1: all warnings being treated as errors
vim +/param +208 arch/powerpc/platforms/pseries/papr-sysparm.c
203
204 static struct papr_sysparm_buf *
205 papr_sysparm_buf_from_user(const struct papr_sysparm_io_block __user *user_iob)
206 {
207 struct papr_sysparm_buf *kern_spbuf;
> 208 papr_sysparm_t param;
209 long err;
210 u16 len;
211
212 /*
213 * The parameter index that userspace wants to query or update.
214 */
215 if (get_user(param.token, &user_iob->parameter))
216 return ERR_PTR(-EFAULT);
217 /*
218 * The length of valid data that userspace claims to be in
219 * user_iob->data[].
220 */
221 if (get_user(len, &user_iob->length))
222 return ERR_PTR(-EFAULT);
223
224 static_assert(sizeof(user_iob->data) >= PAPR_SYSPARM_MAX_INPUT);
225 static_assert(sizeof(kern_spbuf->val) >= PAPR_SYSPARM_MAX_INPUT);
226
227 if (len > PAPR_SYSPARM_MAX_INPUT)
228 return ERR_PTR(-EINVAL);
229
230 kern_spbuf = papr_sysparm_buf_alloc();
231 if (!kern_spbuf)
232 return ERR_PTR(-ENOMEM);
233
234 papr_sysparm_buf_set_length(kern_spbuf, len);
235
236 if (len > 0 && copy_from_user(kern_spbuf->val, user_iob->data, len)) {
237 err = -EFAULT;
238 goto free_sysparm_buf;
239 }
240
241 return kern_spbuf;
242
243 free_sysparm_buf:
244 papr_sysparm_buf_free(kern_spbuf);
245 return ERR_PTR(err);
246 }
247
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2023-10-10 21:46 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-06 21:01 [PATCH 0/7] powerpc/pseries: new character devices for system parameters and VPD Nathan Lynch
2023-10-06 21:01 ` Nathan Lynch via B4 Relay
2023-10-06 21:01 ` [PATCH 1/7] powerpc/uapi: export papr-miscdev.h header Nathan Lynch via B4 Relay
2023-10-06 21:01 ` Nathan Lynch
2023-10-06 21:01 ` [PATCH 2/7] powerpc/pseries: papr-vpd char driver for VPD retrieval Nathan Lynch via B4 Relay
2023-10-06 21:01 ` Nathan Lynch
2023-10-06 21:01 ` [PATCH 3/7] powerpc/rtas: serialize ibm,get-vpd service with papr-vpd sequences Nathan Lynch
2023-10-06 21:01 ` Nathan Lynch via B4 Relay
2023-10-06 21:01 ` [PATCH 4/7] powerpc/pseries/papr-sysparm: validate buffer object lengths Nathan Lynch
2023-10-06 21:01 ` Nathan Lynch via B4 Relay
2023-10-06 21:01 ` [PATCH 5/7] powerpc/pseries/papr-sysparm: expose chardev API to user space Nathan Lynch
2023-10-06 21:01 ` Nathan Lynch via B4 Relay
2023-10-06 23:40 ` kernel test robot
2023-10-06 23:40 ` kernel test robot
2023-10-10 21:44 ` kernel test robot
2023-10-10 21:44 ` kernel test robot
2023-10-06 21:01 ` [PATCH 6/7] powerpc/selftests: add test for papr-vpd Nathan Lynch
2023-10-06 21:01 ` Nathan Lynch via B4 Relay
2023-10-06 21:01 ` [PATCH 7/7] powerpc/selftests: add test for papr-sysparm Nathan Lynch
2023-10-06 21:01 ` Nathan Lynch via B4 Relay
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.