Re: [PATCH v5 08/15] cxl: Add support for FWCTL get driver information callback

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Feb 13, 2025 at 10:33:54AM -0800, Dan Williams wrote:
> Jason Gunthorpe wrote:
> > On Wed, Feb 12, 2025 at 01:18:01PM -0800, Saeed Mahameed wrote:
> > > > +/**
> > > > + * struct fwctl_info_cxl - ioctl(FWCTL_INFO) out_device_data
> > > > + * @reserved: zero
> > > > + */
> > > > +struct fwctl_info_cxl {
> > > > +	__u32 reserved;
> > > > +};
> > > 
> > > I would add some useful info such as: num_user_features, mbox->feature_cap,
> > > etc ...
> > > 
> > > Maybe if the callback is not implemented by the fwctl driver, then fwctl
> > > subsystem should make sure to return a default info struct or some zero
> > > response buffer.
> > 
> > Yes, lets not implement an empty struct. The core should permit a
> > driver to nop this
> > 
> > Should I add this?
> > 
> > @@ -62,7 +62,11 @@ static int fwctl_cmd_info(struct fwctl_ucmd *ucmd)
> >         if (cmd->flags)
> >                 return -EOPNOTSUPP;
> >  
> > -       if (cmd->device_data_len) {
> > +       if (!fwctl->ops->info && cmd->device_data_len) {
> > +               if (clear_user(u64_to_user_ptr(cmd->out_device_data),
> > +                              cmd->device_data_len))
> > +                       return -EFAULT;
> > +       } else if (cmd->device_data_len) {
> >                 void *driver_info __free(kfree) =
> >                         fwctl->ops->info(ucmd->uctx, &driver_info_len);
> >                 if (IS_ERR(driver_info))
> 
> Yes, please. Let's not force the ABI to be chatty if documentation can
> alleviate the need for an 'info' command.

Done

Jason