Received-SPF: pass (google.com: domain of linux-kernel+bounces-179314-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Received-SPF: Pass (protection.outlook.com: domain of nvidia.com designates
 216.228.118.233 as permitted sender) receiver=protection.outlook.com;
 client-ip=216.228.118.233; helo=mail.nvidia.com; pr=C
Date: Tue, 14 May 2024 18:20:06 -0700
From: Nicolin Chen <nicolinc@nvidia.com>
To: Jason Gunthorpe <jgg@nvidia.com>
CC: <will@kernel.org>, <robin.murphy@arm.com>, <kevin.tian@intel.com>,
	<suravee.suthikulpanit@amd.com>, <joro@8bytes.org>,
	<linux-kernel@vger.kernel.org>, <iommu@lists.linux.dev>,
	<linux-arm-kernel@lists.infradead.org>, <linux-tegra@vger.kernel.org>,
	<yi.l.liu@intel.com>, <eric.auger@redhat.com>, <vasant.hegde@amd.com>,
	<jon.grimm@amd.com>, <santosh.shukla@amd.com>, <Dhaval.Giani@amd.com>,
	<shameerali.kolothum.thodi@huawei.com>
Subject: Re: [PATCH RFCv1 05/14] iommufd: Add IOMMUFD_OBJ_VIOMMU and
 IOMMUFD_CMD_VIOMMU_ALLOC
Message-ID: <ZkQNxkYv23z7i6e0@nvidia.com>
References: <cover.1712978212.git.nicolinc@nvidia.com>
 <3aa9bc1df6a2ee58a03c6ea6ededbc210a2d23a8.1712978212.git.nicolinc@nvidia.com>
 <ZkDR4Rp57cy9qSqP@nvidia.com>
 <ZkGYN36N7sh2vUmv@nvidia.com>
 <ZkOFkfHhG2h2fv/c@nvidia.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <ZkOFkfHhG2h2fv/c@nvidia.com>
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 May 2024 01:20:23.3180
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 859b7dd6-078a-4930-8b71-08dc747d31cd
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=43083d15-7273-40c1-b7db-39efd9ccc17a;Ip=[216.228.118.233];Helo=[mail.nvidia.com]
X-MS-Exchange-CrossTenant-AuthSource:
	BN1PEPF00004683.namprd03.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV3PR12MB9439

On Tue, May 14, 2024 at 12:38:57PM -0300, Jason Gunthorpe wrote:
> > > > +
> > > > +/**
> > > > + * enum iommu_viommu_type - VIOMMU Type
> > > > + * @IOMMU_VIOMMU_TEGRA241_CMDQV: NVIDIA Tegra241 CMDQV Extension for SMMUv3
> > > > + */
> > > > +enum iommu_viommu_type {
> > > > +	IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV,
> > > > +};
> > > 
> > > At least the 241 line should be in a following patch
> > 
> > It's for the "enum iommu_viommu_type" mentioned in the following
> > structure. Yi told me that you don't like an empty enum, and he
> > did something like this in HWPT_INVALIDATE series:
> > https://lore.kernel.org/linux-iommu/20240111041015.47920-3-yi.l.liu@intel.com/
> 
> I suspect 0 should be reserved as a non-set value for some
> basic sanity in all these driver type enums.

We have an IOMMU_HWPT_DATA_NONE for HWPT_ALLOC to compatible
with an S2 hwpt, since it doesn't need a data.

Maybe we can have an IOMMU_VIOMMU_TYPE_DEFAULT to be 0, for
an IOMMU driver (e.g. VT-d) that doesn't need to handle nor
be aware of any viommu object?

So, VMM can have a unified "attach-to-viommu" practice with
different IOMMUs, v.s. some still doing "attach-to-s2"?

> > > So, to make this all work perfectly we need approx the following
> > >  - S2 sharing across instances in ARM - meaning the VMID is allocated
> > >    at attach not domain alloc
> > >  - S2 hwpt is refcounted by the VIOMMU in the iommufd layer
> > >  - VIOMMU is refcounted by every nesting child in the iommufd layer
> > >  - The nesting child holds a pointer to both the S2 and the VIOMMU
> > >    (viommu optional)
> > >  - When the nesting child attaches to a device the STE will source the
> > >    VMID from the VIOMMU if present otherwise from the S2
> > >  - "RID" attach (ie naked S2) will have to be done with a Nesting
> > >    Child using a vSTE that indicates Identity. Then the attach logic
> > >    will have enough information to get the VMID from the VIOMMU
> > 
> > What is this RID attach (naked S2) case? S1DSS_BYPASS + SVA?
> 
> No, when the guest installs a vSTE that simply says bypass with no CD
> table pointer. That should result in a pSTE that is the S2 with on CD
> pointer.
> 
> I was originally thinking that the VMM would simply directly attach
> the S2 HWPT in this caes, but given the above issue with the VMID lifetime
> it makes more sense to 'attach' the viommu which holds the correct
> VMID. 
> 
> The issue with direct attach the S2 HWPT is the VMID lifetime, as it
> would have to borrow the VMID from the viommu but then the lifetime
> becomes more complex as it has to live beyond VIOMMU destruction. Not
> unsolvable but it seems easier to just avoid it entirely.

That makes a lot sense. I'd need to go through QEMU code and
see how we will accommodate these two more naturally: likely
the QEMU core should allocate an S2 HWPT for a VM, while the
viommu code should allocate a VIOMMU for each instance.

Thanks
Nicolin