Re: [PATCH v4 22/23] iommu/tegra241-cmdqv: Add user-space use support

Nicolin Chen
2025-05-16T04:10:44Z

On Fri, May 16, 2025 at 04:00:58AM +0000, Tian, Kevin wrote:
> > From: Nicolin Chen <nicolinc@nvidia.com>
> > Sent: Friday, May 16, 2025 1:14 AM
> >
> > On Thu, May 15, 2025 at 08:27:17AM +0000, Tian, Kevin wrote:
> > > > From: Nicolin Chen <nicolinc@nvidia.com>
> > > > Sent: Friday, May 9, 2025 11:03 AM
> > > >
> > > > /**
> > > > * struct iommu_hw_info_arm_smmuv3 - ARM SMMUv3 hardware
> > > > information
> > > > * (IOMMU_HW_INFO_TYPE_ARM_SMMUV3)
> > > > *
> > > > - * @flags: Must be set to 0
> > > > - * @impl: Must be 0
> > > > + * @flags: Combination of enum iommu_hw_info_arm_smmuv3_flags
> > > > + * @impl: Implementation-defined bits when the following flags are set:
> > > > + * - IOMMU_HW_INFO_ARM_SMMUV3_HAS_TEGRA241_CMDQV
> > > > + * Bits[15:12] - Log2 of the total number of SID replacements
> > > > + * Bits[11:08] - Log2 of the total number of VINTFs per vIOMMU
> > > > + * Bits[07:04] - Log2 of the total number of VCMDQs per vIOMMU
> > > > + * Bits[03:00] - Version number for the CMDQ-V HW
> > >
> > > hmm throughout this series I drew an equation between VINTF
> > > and vIOMMU. Not sure how multiple VINTFs can be represented
> > > w/o introducing more objects. Do we want to keep such info here?
> >
> > You are right that VINTF=vIOMMU. This is a per SMMU instance ioctl.
> > So, each VM should only have one VTINF/vIOMMU per SMMU instance.
> >
> > For multi-VINTF (multi-vIOMMU) case, there needs to be more SMMUs
> > backing passthrough devices being assigned to the VM.
> >
> > What exactly the concern of keeping this info here?
>
> First, you agreed that VINTF=vIOMMU, then "total number of VINTFs
> per vIOMMU" doesn't make sense as it's fixed to 1 in concept.
>
> Then, each VM can only get one VINTF/vIOMMU per SMMU instance,
> and this ioctl is per SMMU instance. This also implies that only one
> VINTF can be reported in the ioctl.
>
> In multi-VINTF case, the VM should get 1VINTF per ioctl from each
> SMMU backing passthrough devices.
>
> Then what is the point of " Bits[11:08] - Log2 of the total number
> of VINTFs per vIOMMU "?

It was not there in the previous version. Pranjal pointed out that
it was missing a field.

You are right that this field must set to 0, indicating only single
VINTF is allowed.

Given Jason's comments about this impl rework (to a data structure),
I think I will just drop this number of VTINFs. Instead will add a
line of comments say that VMM should set this field to 0, i.e. only
provide VM one VINTF per SMMU.

Thanks
Nicolin