RE: [PATCH v8 11/15] iommufd/viommu: Select vIOMMU operations before allocation

[Aneesh Kumar K.V <aneesh.kumar@kernel.org>]

"Tian, Kevin" <kevin.tian@intel.com> writes:

>> From: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>> Sent: Thursday, November 5, 2026 12:16 AM
>> 
>> Physical IOMMU drivers currently provide the vIOMMU size and
>> initialization callbacks through iommu_ops. The initialization callback
>> then selects and installs the corresponding iommufd_viommu_ops.
>> 
>> Add iommu_ops::get_viommu_ops() to select the operations from the
>> physical device and requested vIOMMU type before allocation. Move the
>> size and initialization callbacks into iommufd_viommu_ops. The core can
>> then validate the selected operations, allocate the driver structure,
>> initialize it, and install the operations.
>> 
>> Convert AMD, Arm SMMUv3, Tegra CMDQV, and the selftest backend to the
>> new interface.
>> 
>> Selecting the operations early allows subsequent changes to use
>> implementation-specific vIOMMU properties, including when validating the
>> parent HWPT.
>
> what does "implementation-specific" mean? Isn't every existing vIOMMU
> type already implementation specific?
>

How about I update this to.

The current code requires a valid parent HWPT before calling
viommu_init(), which selects the vIOMMU operations. A later patch adds
an operations flag indicating that a vIOMMU type does not require a parent
HWPT. Selecting the operations earlier allows the core to check that flag
before deciding whether to look up and validate a parent HWPT.

>
>> 
>>  /*
>>   * This function is assigned to struct
>> iommufd_viommu_ops.alloc_domain_nested()
>> - * during the call to struct iommu_ops.viommu_init().
>> + * when the vIOMMU operations are selected.
>>   */
>>  struct iommu_domain *
>>  amd_iommu_alloc_domain_nested(struct iommufd_viommu *viommu, u32
>> flags,
>
> the comment sounds changing the semantics.
>
> ideally it's still assigned at viommu_init(), just from iommufd_viommu_ops
> instead of iommu_ops now.

How about
 /*
  * This function is assigned to struct iommufd_viommu_ops.alloc_domain_nested()
- * during the call to struct iommu_ops.viommu_init().
+ * during the call to struct iommufd_viommu_ops.viommu_init().
  */

-aneesh