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

[Dave Jiang <dave.jiang@intel.com>]

On 2/13/25 8:32 AM, 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?

I'm planning to add some information like Saeed suggested. But this may not be a bad idea to allow the driver to nop the info callback.

> 
> @@ -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))
> 
> Jason