On Mon, Jul 31, 2023 at 06:31:20AM +0000, Tian, Kevin wrote:
From: Jason Gunthorpe jgg@nvidia.com Sent: Saturday, July 29, 2023 1:56 AM
On Mon, Jul 24, 2023 at 04:03:57AM -0700, Yi Liu wrote:
- switch (pt_obj->type) {
- case IOMMUFD_OBJ_IOAS:
ioas = container_of(pt_obj, struct iommufd_ioas, obj);break;- case IOMMUFD_OBJ_HW_PAGETABLE:
/* pt_id points HWPT only when hwpt_typeis !IOMMU_HWPT_TYPE_DEFAULT */
if (cmd->hwpt_type == IOMMU_HWPT_TYPE_DEFAULT) {rc = -EINVAL;goto out_put_pt;}parent = container_of(pt_obj, struct iommufd_hw_pagetable,obj);
/** Cannot allocate user-managed hwpt linking toauto_created
* hwpt. If the parent hwpt is already a user-managed hwpt,* don't allocate another user-managed hwpt linking to it.*/if (parent->auto_domain || parent->parent) {rc = -EINVAL;goto out_put_pt;}ioas = parent->ioas;Why do we set ioas here? I would think it should be NULL.
I think it is looking like a mistake to try and re-use iommufd_hw_pagetable_alloc() directly for the nested case. It should not have a IOAS argument, it should not do enforce_cc, or iopt_* functions
enforce_cc is still required since it's about memory accesses post page table walking (no matter the walked table is single stage or nested).
enforce_cc only has meaning if the kernel owns the IOPTEs, and if we are creating a nest it does not. The guest has to set any necessary IOPTE bits.
enforce_cc will be done on the parent of the nest as normal.
Ideally expanding uAPI structure size should come with new flag bits.
Flags or some kind of 'zero is the same behavior as a smaller struct' scheme.
This patch is doing the zero option:
__u32 __reserved; + __u32 hwpt_type; + __u32 data_len; + __aligned_u64 data_uptr; };
hwpt_type == 0 means default type data_len == 0 means no data data_uptr is ignored (zero is safe)
So there is no need to change it
Jason