On Thu, Aug 31, 2023 at 09:37:12PM +1000, Michael Ellerman wrote: > Michal Suchánek <msucha...@suse.de> writes: > > On Thu, Aug 31, 2023 at 03:34:37PM +1000, Michael Ellerman wrote: > >> Michal Suchánek <msucha...@suse.de> writes: > >> > 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 > >> > > >> > Could this be somewhat less obfuscated? > >> > > >> > What the caller wants is the result of "ibm,get-vpd", which is a > >> > well-known string identifier of the rtas call. > >> > >> Not really. What the caller wants is *the VPD*. Currently that's done > >> by calling the RTAS "ibm,get-vpd" function, but that could change in > >> future. There's RTAS calls that have been replaced with a "version 2" in > >> the past, that could happen here too. Or the RTAS call could be replaced > >> by a hypercall (though unlikely). > >> > >> But hopefully if the underlying mechanism changed the kernel would be > >> able to hide that detail behind this new API, and users would not need > >> to change at all. > >> > >> > Yet this identifier is never passed in. Instead we have this new > >> > PAPR_VPD_CREATE_HANDLE. This is a completely new identifier, specific to > >> > this call only as is the /dev/papr-vpd device name, another new > >> > identifier. > >> > > >> > Maybe the interface could provide a way to specify the service name? > >> > > >> >> file contents are immutable from the POV of user space. To get a new > >> >> view of VPD, clients must create a new handle. > >> > > >> > Which is basically the same as creating a file descriptor with open(). > >> > >> Sort of. But much cleaner becuase you don't need to create a file in the > >> filesystem and tell userspace how to find it. > > > > You very much do. There is the /dev/papr-vpd and PAPR_VPD_CREATE_HANDLE > > which userspace has to know about, the PAPR_VPD_CREATE_HANDLE is not > > even possible to find at all. > > Well yeah you need the device itself :)
And as named it's specific to this call, and new devices will be needed for any additional rtas called implemented. > > And yes the ioctl is defined in a header, not in the filesystem, but > that's entirely normal for an ioctl based API. Of course, because the ioctl API has no safe way of passing a string identifier for the function. Then it needs to create these obscure IDs. Other APIs that don't have this problem exist. Thanks Michal