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

[Dan Williams <dan.j.williams@intel.com>]

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.