RE: [PATCH v6 05/10] iommufd: Add fault and response message definitions

[Tian, Kevin]

> From: Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx>
> Sent: Thursday, June 6, 2024 2:28 PM
> 
> On 6/5/24 4:28 PM, Tian, Kevin wrote:
> >> From: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> >> Sent: Monday, May 27, 2024 12:05 PM
> >>
> >> +
> >> +/**
> >> + * struct iommu_hwpt_page_response - IOMMU page fault response
> >> + * @size: sizeof(struct iommu_hwpt_page_response)
> >> + * @flags: Must be set to 0
> >> + * @dev_id: device ID of target device for the response
> >> + * @pasid: Process Address Space ID
> >> + * @grpid: Page Request Group Index
> >> + * @code: One of response code in enum
> iommufd_page_response_code.
> >> + * @cookie: The kernel-managed cookie reported in the fault message.
> >> + */
> >> +struct iommu_hwpt_page_response {
> >> +	__u32 size;
> >> +	__u32 flags;
> >> +	__u32 dev_id;
> >> +	__u32 pasid;
> >> +	__u32 grpid;
> >> +	__u32 code;
> >> +	__u32 cookie;
> >> +	__u32 reserved;
> >> +};
> >
> > with the response queue per fault object we don't need all fields here,
> > e.g. dev_id, pasid, etc. Cookie is sufficient.
> 
> I prefer not to mess the definition of user API data and the kernel
> driver implementation. The kernel driver may change in the future, but
> the user API will remain stable for a long time.

sure it remains stable for reasonable reason. Here we defined some
fields but they are even not used and checked in the kernel. IMHO it
suggests redundant definition. If there is value to keep them, do we
need to at least verify them same as the completion record?

> 
> I am neutral about whether we could remove above fields, but I need all
> maintainers to agree on this, given that this has undergone five rounds
> of review. :-)
> 

sure let's hear their opinions.