On Tue, Aug 22, 2023 at 04:33:39PM -0500, Nathan Lynch via B4 Relay wrote: > From: Nathan Lynch <nath...@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_WRONLY); > 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 VPD, clients 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; > concurrent 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 start over. (The driver does not yet > handle this, but it should be easy to add.) > > 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. > > Likely remaining work: > > * Handle RTAS call status -4 (VPD changed) during ibm,get-vpd call > sequence. > * Prevent ibm,get-vpd calls via rtas(2) from disrupting ibm,get-vpd > call sequences in this driver. > * (Maybe) implement a poll method for delivering notifications of > potential changes to VPD, e.g. after a partition migration. > > Questions, points for discussion: > > * Am I allocating the ioctl numbers correctly? > * The only way to discover the size of a VPD buffer is > lseek(SEEK_END). fstat() doesn't work for anonymous fds like > this. Is this OK, or should the buffer length be discoverable some > other way? > > Signed-off-by: Nathan Lynch <nath...@linux.ibm.com> > --- > Documentation/userspace-api/ioctl/ioctl-number.rst | 2 + > arch/powerpc/include/uapi/asm/papr-vpd.h | 29 ++ > arch/powerpc/platforms/pseries/Makefile | 1 + > arch/powerpc/platforms/pseries/papr-vpd.c | 353 > +++++++++++++++++++++ > 4 files changed, 385 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:v...@ratio.de> > 0xB1 00-1F PPPoX > > <mailto:mostr...@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-g...@vger.kernel.org> > 0xB5 00-0F uapi/linux/rpmsg.h > <mailto:linux-remotep...@vger.kernel.org> > 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..aa33217ad5de > --- /dev/null > +++ b/arch/powerpc/include/uapi/asm/papr-vpd.h > @@ -0,0 +1,29 @@ > +// SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note > +#ifndef _UAPI_PAPR_VPD_H_ > +#define _UAPI_PAPR_VPD_H_ > + > +#include <linux/types.h> > +#include <asm/ioctl.h> > + > +struct papr_location_code { > + /* > + * PAPR+ 12.3.2.4 Converged Location Code Rules - Length > + * Restrictions. 79 characters plus nul. > + */ > + char str[80]; > +}; > + > +#define PAPR_VPD_IOCTL_BASE 0xb2 > + > +#define PAPR_VPD_IO(nr) _IO(PAPR_VPD_IOCTL_BASE, nr) > +#define PAPR_VPD_IOR(nr, type) _IOR(PAPR_VPD_IOCTL_BASE, nr, type) > +#define PAPR_VPD_IOW(nr, type) _IOW(PAPR_VPD_IOCTL_BASE, nr, type) > +#define PAPR_VPD_IOWR(nr, type) _IOWR(PAPR_VPD_IOCTL_BASE, nr, type) > + > +/* > + * ioctl for /dev/papr-vpd. Returns a VPD handle fd corresponding to > + * the location code. > + */ > +#define PAPR_VPD_CREATE_HANDLE PAPR_VPD_IOW(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..6fc9ee0c48ae > --- /dev/null > +++ b/arch/powerpc/platforms/pseries/papr-vpd.c > @@ -0,0 +1,353 @@ > +// 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/kernel.h> > +#include <linux/miscdevice.h> > +#include <linux/slab.h> > +#include <linux/string.h>
Should be linux/strin_helpers to get string_is_terminated explicitly and not randomly throuugh another header. Thanks Michal > +#include <asm/machdep.h> > +#include <asm/papr-vpd.h> > +#include <asm/rtas-work-area.h> > +#include <asm/rtas.h> > + > +/* > + * Internal VPD "blob" APIs: for accumulating successive ibm,get-vpd results > + * into a buffer to be attached to a file descriptor. > + */ > +struct vpd_blob { > + const char *data; > + size_t len; > +}; > + > +static struct vpd_blob *vpd_blob_new(void) > +{ > + return kzalloc(sizeof(struct vpd_blob), GFP_KERNEL_ACCOUNT); > +} > + > +static void vpd_blob_free(struct vpd_blob *blob) > +{ > + if (blob) { > + kvfree(blob->data); > + kfree(blob); > + } > +} > + > +static int vpd_blob_accumulate(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; > +} > + > +/** > + * struct rtas_ibm_get_vpd_params - Parameters (in and out) for ibm,get-vpd. > + * > + * @loc_code: In: Location code buffer. Must be RTAS-addressable. > + * @work_area: In: 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; > +}; > + > +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; > +} > + > +struct vpd_sequence_state { > + struct mutex *mutex; /* always &vpd_sequence_mutex */ > + struct pin_cookie cookie; > + int error; > + struct rtas_ibm_get_vpd_params params; > +}; > + > +static void vpd_sequence_begin(struct vpd_sequence_state *state, > + const struct papr_location_code *loc_code) > +{ > + static DEFINE_MUTEX(vpd_sequence_mutex); > + /* > + * 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. > + */ > + static struct papr_location_code static_loc_code; > + > + mutex_lock(&vpd_sequence_mutex); > + > + static_loc_code = *loc_code; > + *state = (struct vpd_sequence_state) { > + .mutex = &vpd_sequence_mutex, > + .cookie = lockdep_pin_lock(&vpd_sequence_mutex), > + .params = { > + .work_area = rtas_work_area_alloc(SZ_4K), > + .loc_code = &static_loc_code, > + .sequence = 1, > + }, > + }; > +} > + > +static bool vpd_sequence_done(const struct vpd_sequence_state *state) > +{ > + bool done; > + > + if (state->error) > + return true; > + > + switch (state->params.status) { > + case 0: > + if (state->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 bool vpd_sequence_advance(struct vpd_sequence_state *state) > +{ > + if (vpd_sequence_done(state)) > + return false; > + > + state->error = rtas_ibm_get_vpd(&state->params); > + > + return state->error == 0; > +} > + > +static size_t vpd_sequence_get_buffer(const struct vpd_sequence_state > *state, const char **buf) > +{ > + *buf = rtas_work_area_raw_buf(state->params.work_area); > + return state->params.written; > +} > + > +static void vpd_sequence_set_err(struct vpd_sequence_state *state, int err) > +{ > + state->error = err; > +} > + > +static void vpd_sequence_end(struct vpd_sequence_state *state) > +{ > + rtas_work_area_free(state->params.work_area); > + lockdep_unpin_lock(state->mutex, state->cookie); > + mutex_unlock(state->mutex); > +} > + > +static struct vpd_blob *papr_vpd_retrieve(const struct papr_location_code > *loc_code) > +{ > + struct vpd_sequence_state state; > + struct vpd_blob *blob; > + > + blob = vpd_blob_new(); > + if (!blob) > + return ERR_PTR(-ENOMEM); > + > + vpd_sequence_begin(&state, loc_code); > + > + while (vpd_sequence_advance(&state)) { > + const char *buf; > + const size_t len = vpd_sequence_get_buffer(&state, &buf); > + > + vpd_sequence_set_err(&state, vpd_blob_accumulate(blob, buf, > len)); > + } > + > + vpd_sequence_end(&state); > + > + if (!state.error) > + return blob; > + > + vpd_blob_free(blob); > + > + return ERR_PTR(state.error); > +} > + > +static ssize_t papr_vpd_handle_read(struct file *file, char __user *buf, > size_t size, loff_t *off) > +{ > + struct vpd_blob *blob = file->private_data; > + > + /* Blobs should always have a valid data pointer and nonzero size. */ > + if (WARN_ON_ONCE(!blob->data)) > + return -EIO; > + if (WARN_ON_ONCE(blob->len == 0)) > + 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) > +{ > + 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) > +{ > + 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, > +}; > + > +static long papr_vpd_ioctl_create_handle(struct papr_location_code __user > *ulc) > +{ > + struct papr_location_code klc; > + 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, > + 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; > +} > + > +/* 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_CREATE_HANDLE: > + ret = papr_vpd_ioctl_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 >