On Wed, 2025-08-27 at 23:45 +0530, Mahesh J Salgaonkar wrote:
> On 2025-08-12 15:57:07 Tue, Haren Myneni wrote:
> > The hypervisor provides ibm,send-hvpipe-msg and
> > ibm,receive-hvpipe-msg RTAS calls which can be used by the
> > partition to communicate through an inband hypervisor channel with
> > different external sources such as Hardware Management Console
> > (HMC). The information exchanged, whether it be messages, raw or
> > formatted data, etc., is only known to between applications in the
> > OS and the source (HMC). This patch adds papr-hvpipe character
> > driver and provides the standard interfaces such as open / ioctl/
> > read / write to user space for exchanging information with HMC
> > using send/recevive HVPIPE RTAS functions.
> > 
> > PAPR (7.3.32 Hypervisor Pipe Information Exchange) defines the
> > HVPIPE usage:
> > - The hypervisor has one HVPIPE per partition for all sources.
> > - OS can determine this feature’s availability by detecting the
> >   “ibm,hypervisor-pipe-capable” property in the /rtas node of the
> >   device tree.
> > - Each source is represented by the source ID which is used in
> >   send / recv HVPIPE RTAS. (Ex: source ID is the target for the
> >   payload in send RTAS).
> > - Return status of ibm,send-hvpipe-msg can be considered as
> >   delivered the payload.
> > - Return status of ibm,receive-hvpipe-msg can be considered as
> >   ACK to source.
> > - The hypervisor generates hvpipe message event interrupt when
> >   the partition has the payload to receive.
> > 
> > Provide the interfaces to the user space with /dev/papr-hvpipe
> > character device using the following programming model:
> > 
> > int devfd = open("/dev/papr-hvpipe")
> > int fd = ioctl(devfd, PAPR_HVPIPE_IOC_CREATE_HANDLE, &srcID);
> > - Restrict the user space to use the same source ID and do not
> >   expect more than one process access with the same source.
> > char *buf = malloc(size);
> > - SIZE should be 4K and the buffer contains header and the
> >   payload.
> > length = write(fd, buf, size);
> > - OS issues ibm,send-hvpipe-msg RTAS and returns the RTAS status
> >   to the user space.
> > ret = poll(fd,...)
> > - The HVPIPE event message IRQ wakes up for any waiting FDs.
> > length = read(fd, buf, size);
> > - OS issues ibm,receive-hvpipe-msg to receive payload from the
> >   hypervisor.
> > release(fd);
> > - OS issues ibm,receive-hvpipe-msg if any payload is pending so
> >   that pipe is not blocked.
> > 
> > The actual implementation of these calls are added in the
> > next patches.
> > 
> > Signed-off-by: Haren Myneni <ha...@linux.ibm.com>
> > ---
> >  arch/powerpc/platforms/pseries/Makefile      |   1 +
> >  arch/powerpc/platforms/pseries/papr-hvpipe.c | 274
> > +++++++++++++++++++
> >  arch/powerpc/platforms/pseries/papr-hvpipe.h |  14 +
> >  3 files changed, 289 insertions(+)
> >  create mode 100644 arch/powerpc/platforms/pseries/papr-hvpipe.c
> >  create mode 100644 arch/powerpc/platforms/pseries/papr-hvpipe.h
> > 
> [...]
> > +static int papr_hvpipe_dev_create_handle(u32 srcID)
> > +{
> > +   struct hvpipe_source_info *src_info;
> > +   struct file *file;
> > +   long err;
> > +   int fd;
> > +
> > +   spin_lock(&hvpipe_src_list_lock);
> > +   /*
> > +    * Do not allow more than one process communicates with
> > +    * each source.
> > +    */
> > +   src_info = hvpipe_find_source(srcID);
> > +   if (src_info) {
> > +           spin_unlock(&hvpipe_src_list_lock);
> > +           pr_err("pid(%d) is already using the
> > source(%d)\n",
> > +                           src_info->tsk->pid, srcID);
> > +           return -EALREADY;
> > +   }
> > +   spin_unlock(&hvpipe_src_list_lock);
> 
> What if two process simulteneously try to get handle ? The new
> src_info
> node being allocated below isn't yet on the list. Which means as soon
> as
> we unlock, there is a chance another process can still come here
> requesting for same srcID and eventually we will see duplicate or
> more
> entries for same srcID added to the list from different processes.

Thanks for finding this issue.

yes, generally the user space should not open with the same source
again if follows HVPIPE semantics, but it is possible. In this case,
end up notifying the first FD in the list for each receive HVPIPE data.

As you suggested, will check again before adding to the list.

Thanks
Haren

> 
> > +
> > +   src_info = kzalloc(sizeof(*src_info), GFP_KERNEL_ACCOUNT);
> > +   if (!src_info)
> > +           return -ENOMEM;
> > +
> > +   src_info->srcID = srcID;
> > +   src_info->tsk = current;
> > +   init_waitqueue_head(&src_info->recv_wqh);
> > +
> > +   fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC);
> > +   if (fd < 0) {
> > +           err = fd;
> > +           goto free_buf;
> > +   }
> > +
> > +   file = anon_inode_getfile("[papr-hvpipe]",
> > +                   &papr_hvpipe_handle_ops, (void *)src_info,
> > +                   O_RDWR);
> > +   if (IS_ERR(file)) {
> > +           err = PTR_ERR(file);
> > +           goto put_fd;
> > +   }
> > +
> > +   fd_install(fd, file);
> > +
> > +   spin_lock(&hvpipe_src_list_lock);
> 
> Should we check again here to make sure same srcID hasn't been
> already
> added to the list while we were unlocked and return -EALREADY ?
> 
> > +   list_add(&src_info->list, &hvpipe_src_list);
> > +   spin_unlock(&hvpipe_src_list_lock);
> > +
> > +   return fd;
> > +
> > +put_fd:
> > +   put_unused_fd(fd);
> > +free_buf:
> > +   kfree(src_info);
> > +   return err;
> > +}
> > +

Reply via email to