Hello, On Fri, Nov 17, 2023 at 11:14:27PM -0600, 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_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 <nath...@linux.ibm.com> > --- > Documentation/userspace-api/ioctl/ioctl-number.rst | 2 + > arch/powerpc/include/uapi/asm/papr-vpd.h | 22 + > arch/powerpc/platforms/pseries/Makefile | 1 + > arch/powerpc/platforms/pseries/papr-vpd.c | 536 > +++++++++++++++++++++ > 4 files changed, 561 insertions(+)
> diff --git a/arch/powerpc/platforms/pseries/Makefile > b/arch/powerpc/platforms/pseries/Makefile > index 1476c5e4433c..f936962a2946 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..2bc52301a402 > --- /dev/null > +++ b/arch/powerpc/platforms/pseries/papr-vpd.c > @@ -0,0 +1,536 @@ > +// 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> ... > +/** > + * 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+ 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)); this is defined in linux/sched/signal.h which is not included. > + > + 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 an immutable VPD "blob" for it. The blob is > + * attached to a file descriptor for reading by user space. The memory > + * backing the blob is freed when the file is released. > + * > + * The entire requested VPD is retrieved by this call and all > + * necessary RTAS interactions are performed before returning the fd > + * to user space. This keeps the read handler simple and ensures that > + * the kernel can prevent interleaving of ibm,get-vpd call sequences. > + * > + * 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; This is defined in linux/uaccess.h which is not included. Same for the sysparm driver. Tested-by: Michal Suchánek <msucha...@suse.de> > + > + 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 >