On Mon, Oct 28, 2024 at 10:03:09AM -0300, Jason Gunthorpe wrote:
On Mon, Oct 28, 2024 at 11:24:10AM +0800, Zhangfei Gao wrote:
+/**
- iommufd_viommu_alloc_hwpt_nested() - Get a hwpt_nested for a vIOMMU
- @viommu: vIOMMU ojbect to associate the hwpt_nested/domain with
- @user_data: user_data pointer. Must be valid
- Allocate a new IOMMU_DOMAIN_NESTED for a vIOMMU and return it as a NESTED
- hw_pagetable.
- */
+static struct iommufd_hwpt_nested * +iommufd_viommu_alloc_hwpt_nested(struct iommufd_viommu *viommu, u32 flags,
const struct iommu_user_data *user_data)+{
struct iommufd_hwpt_nested *hwpt_nested;struct iommufd_hw_pagetable *hwpt;int rc;if (flags)return ERR_PTR(-EOPNOTSUPP);This check should be removed.
When a user page fault is required, IOMMU_HWPT_FAULT_ID_VALID is set. if (cmd->flags & IOMMU_HWPT_FAULT_ID_VALID) {
It can't just be removed..
I suspect that IOMMU_HWPT_FAULT_ID_VALID should not be set on the nested domain but on the parent?
By giving another look,
In iommufd_hwpt_paging_alloc(), we reject IOMMU_HWPT_FAULT_ID_VALID: const u32 valid_flags = IOMMU_HWPT_ALLOC_NEST_PARENT | IOMMU_HWPT_ALLOC_DIRTY_TRACKING; ... if (flags & ~valid_flags) return ERR_PTR(-EOPNOTSUPP);
In iommufd_hwpt_nested_alloc(), we mask the flag away: if ((flags & ~IOMMU_HWPT_FAULT_ID_VALID) || !user_data->len || !ops->domain_alloc_user) return ERR_PTR(-EOPNOTSUPP); ... hwpt->domain = ops->domain_alloc_user(idev->dev, flags & ~IOMMU_HWPT_FAULT_ID_VALID, parent->common.domain, user_data);
Then, in the common function it has a section of if (cmd->flags & IOMMU_HWPT_FAULT_ID_VALID) { ...
It seems that this IOMMU_HWPT_FAULT_ID_VALID is for nested domains?
So, aligning with that, here we need: if ((flags & ~IOMMU_HWPT_FAULT_ID_VALID) || !user_data->len) return ERR_PTR(-EOPNOTSUPP);
And looks like we don't have some detailed documentation w.r.t. this flag and Fault Queue. We'd likely need to update the uAPI doc, prior to (or along with) the Part-3 vIRQ series.
Thanks Nicolin