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