Hi Kevin, On Fri, May 19, 2023 at 08:42:07AM +0000, Tian, Kevin wrote: > > +}; > > +#define IOMMU_DEVICE_GET_HW_INFO _IO(IOMMUFD_TYPE, > > IOMMUFD_CMD_DEVICE_GET_HW_INFO) > > #endif > > Here we have a naming confusion. > > 'IOMMU' is the prefix of iommufd ioctls. > > 'DEVICE' is the subjective. > > Then "GET_HW_INFO" implies getting hardware info related to > this device. then it should not be restricted to the iommu info. > > with that it's clearer to call it IOMMU_DEVICE_GET_IOMMU_INFO. Though the entire ioctl is tied to the input "dev_id", I think it isn't really about the device corresponding to the dev_id, similar to the IOMMU_HWPT_ALLOC having a dev_id input too. So, I think the "IOMMU_DEVICE" here should be interpreted simply as "an iommu device". We could also highlight this somewhere in the header. With that being said, IOMMU_DEVICE_SET/UNSET_DATA should be renamed to IOMMU_DEVICE_SET/UNSET_DEV_DATA -- "DEVICE" is the iommu device while the "DEV_DATA" is a given device that's behind the iommu. > similarly for struct iommu_hw_info. > > 'iommu' is the prefix for all iommufd ioctl structures. > > then 'hw_info' is too broard. > > iommu_device_iommu_info reads better? though having two > iommu's in the name is a little bit annoying... How about: IOMMU_DEVICE_GET_FEATURES struct iommu_device_features ? Thanks Nic