Re: [PATCH 3/6] iommufd: Add IOMMU_DEVICE_GET_INFO

From: Jason Gunthorpe
Date: Fri Feb 10 2023 - 16:00:01 EST


On Fri, Feb 10, 2023 at 07:55:42AM +0000, Tian, Kevin wrote:
> > From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> > Sent: Thursday, February 9, 2023 12:17 PM
> >
> > +int iommufd_device_get_info(struct iommufd_ucmd *ucmd)
> > +{
> > + struct iommu_device_info *cmd = ucmd->cmd;
> > + struct iommufd_object *dev_obj;
> > + struct device *dev;
> > + const struct iommu_ops *ops;
> > + void *data;
> > + unsigned int length, data_len;
> > + int rc;
> > +
> > + if (cmd->flags || cmd->__reserved || !cmd->data_len)
> > + return -EOPNOTSUPP;
>
> do we want !cmd->data_len being a way to check how many bytes are
> required in a following call to get the vendor info?

There is no reason, if the userspace doesn't have a struct large
enough then it also doesn't know what the extra bytes should even be
used for. No reason to read the.

> > +struct iommu_device_info {
> > + __u32 size;
> > + __u32 flags;
> > + __u32 dev_id;
> > + __u32 data_len;
> > + __aligned_u64 data_ptr;
>
> moving forward iommu hw cap is not the only information reported
> via this interface, e.g. it can be also used to report pasid mode.
>
> from this angle it's better to rename above two fields to be iommu
> specific, e.g.:
>
> __u32 iommu_data_len;
> __aligned_u64 iommu_data_ptr;

maybe call it hw info

Jason