Received-SPF: pass (google.com: domain of linux-kernel+bounces-186320-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Wed, 22 May 2024 10:59:44 -0300
From: Jason Gunthorpe <jgg@nvidia.com>
To: Nicolin Chen <nicolinc@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 07/14] iommufd: Add viommu set/unset_dev_id ops
Message-ID: <20240522135944.GZ20229@nvidia.com>
References: <cover.1712978212.git.nicolinc@nvidia.com>
 <6e57d7b5aa1705bdd547b1cd2aca93d3bf70dfa4.1712978212.git.nicolinc@nvidia.com>
 <ZkDWXnPW7CaX5TtA@nvidia.com>
 <ZkGZc5dvLigXcWib@nvidia.com>
 <ZkOI8ztR1mUMJ8oe@nvidia.com>
 <ZkQW6/OAQ8MzN6Go@nvidia.com>
 <20240521182448.GN20229@nvidia.com>
 <Zk0ftlf3f4gBaNgy@nvidia.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Zk0ftlf3f4gBaNgy@nvidia.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?epMLSKaQSdkRMUqA24cKrgICEmn/QNnfY/5jbA/C4W+eEcOfQFSzfemnQ8ZW?=
 =?us-ascii?Q?WSlOTBk6zWhmDLRuoFhTr0YtlYCmv6p1Pu8NFT/7ZNQuqVSjTdM934BcTNJU?=
 =?us-ascii?Q?Llum5G42Jtf1CuBRFvkMEQIvNZlvlMdIszWBap4UDwGSED9tn6ab2RY2WnrV?=
 =?us-ascii?Q?vUC1YRAnk3jp4io74YJPHlaWveIK4miDBO3FCBvh0FgEBBoR42nkaFUwnpeE?=
 =?us-ascii?Q?7ylsxTT0P2uyLIIQOXn5ei3NB6ebYdZDJWB29LWB3uLN7WuPnPvbZawbVsoO?=
 =?us-ascii?Q?Ai9VlwDOjv9Xt10PYPD6gm9CwjDI57sCwUewtXFkxtkAiCagrPSTRefBc1s8?=
 =?us-ascii?Q?XiXRTiXxX33arNGen2rcJhnz8GZLSiujCBNL+csAvLc19/EL7H7DUX4qOiXF?=
 =?us-ascii?Q?CTbFmMPnrKBnQK2KeaVs21qRrI0ZvyArMzG8HSDaMg1PElGIxFMRm4uOMB9l?=
 =?us-ascii?Q?w17ESZZ/mBJO/ssanJRLc6bFTYL49oBgvS43Vi2pmLtdWRDXu0iRxmUMJvkV?=
 =?us-ascii?Q?Lddk0H1r4PCwFMgW+N+Hjp16/GnIf9F7CZfNeIu4dVe6DAp17IWnmH020azv?=
 =?us-ascii?Q?I3knF8KhIFd01fUf3m+tYJXbJ+TQocUKx64G0dhWY0TvgWhRUZC4DXc+DoBB?=
 =?us-ascii?Q?YwCa5a6R3tUf96jc4vyK1I3FnXroCoqpYpwCkPdAxXAwR+d/fvt6pME4+Ip0?=
 =?us-ascii?Q?85hLANf2ymUNMU3ktzl+BTY/LuUApvPkoR/YI3e8FH0pi6hY/TLlMV0MWcBC?=
 =?us-ascii?Q?JcZaI2xHc20U8MV0VztKZ++iLDs22gcgKVq9iEaeC6Ru1/xhGzz89vvJXVAT?=
 =?us-ascii?Q?lBMOc9rknPE1N05I79JzPZ/6WPnyWjuEhiDQ8WN9nkUlMSWCH0/hnhsBTlQb?=
 =?us-ascii?Q?fZnIP6snEDInYkVwQqTqkN2Lc/3g1Gn3LWE0QblKZbOa3KJUq0zjDGBzBMD0?=
 =?us-ascii?Q?PYl5wbEcX5SMGLZUN3PYSNM4loIav7bGmjjZeTKL31pulP4n9Z/cS8tG/E4v?=
 =?us-ascii?Q?dQE1sosrn/GjS3A0FYriQbIXydKBWbDSL2UewT0Q5TMv+FXzpQjS+Js/qaGM?=
 =?us-ascii?Q?nXWECTF3De+A9PpWQ/toYglCJMfjeEKS2KqsP/FyeirjRcjR2devnKt9P7Wu?=
 =?us-ascii?Q?A3MfjCJ7adg2LvQQkt3A/7kX9GcYPrsKvq4uCis/8qlRfGYhKJ7xcsMJ+KEC?=
 =?us-ascii?Q?uzROT8TKmwZn619X85RiF+x0liU/m7XoWg3yxBxzKjiaUqTTRADjs7XIGUth?=
 =?us-ascii?Q?eqfIdY3aPLfVdtK/qDKvNl/UZYjwq6W/lp/Qua6Cuj7vXu1/GyzIDRaPNJ4y?=
 =?us-ascii?Q?g2nzrxGECSr62drpcsg/6LBVAb7Z1nK18IOtbTGyxmKV49hge1cF2Y6EIUVJ?=
 =?us-ascii?Q?4x2O4frHz430bnK11fL0XpKVZyLcM7Vr5J2aZcB2dhCpVRwYuZjfNJu/WnoB?=
 =?us-ascii?Q?o6rQo0r1SN03+CGOjef+jnUGHwJshWLg4uhiLR6gjBWG8zPD15lhviTvwUbm?=
 =?us-ascii?Q?eiigtSh2Hb/UJclHGF49PIosQZc8ZOKHmLimcWatM4EyZs97zopMOfmz627u?=
 =?us-ascii?Q?hv9AaOSSmg3HjqRy9Ks=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2deb7fe2-0a0e-4bad-d011-08dc7a677058
X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB3849.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 May 2024 13:59:46.5874
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: ATZITZr1KNB/K16WcnzzkHRlGin7lmyFlZp94b38pWItA7g2o8p7HXCFY4ncm/p+
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR12MB6841

On Tue, May 21, 2024 at 03:27:02PM -0700, Nicolin Chen wrote:
> On Tue, May 21, 2024 at 03:24:48PM -0300, Jason Gunthorpe wrote:
> > On Tue, May 14, 2024 at 06:59:07PM -0700, Nicolin Chen wrote:
> > > So, you want a proxy S1 domain for a device to attach, in case
> > > of a stage-2 only setup, because an S2 domain will no longer has
> > > a VMID, since it's shared among viommus. In the SMMU driver case,
> > > an arm_smmu_domain won't have an smmu pointer, so a device can't
> > > attach to an S2 domain but always an nested S1 domain, right?
> > 
> > That seems like a simple solution to the VMID lifetime, but it means
> > the kernel has to decode more types of vSTE.
> 
> Yea. For vSTE=abort, likely we need a nested block domain too?

Sure, it is easy to do
 
> > I don't know if there is merit one way or the other. A more specific
> > API surface is nice, but the two APIs are completely duplicating.
> > 
> > So maybe:
> > 
> > #define IOMMU_VIOMMU_INVALIDATE IOMMU_HWPT_INVALIDATE
> > 
> > As documentation and have the kernel just detect based on the type of
> > the passed ID?
> 
> Yea, the only difference is viommu_id v.s. hwpt_id that we can
> document.
> 
> Then in this case, we have two mostly identical uAPIs for the
> SMMU driver to use. Should we implement both?

I suspect it will turn out nicely naturally, lets try and see
 
> > > > We can add ATS invalidation after either as an enhancement as part of
> > > > adding the VIOMMU either as DEV_INVALIDATE or VIOMMU_INVALIDATE (or
> > > > both)
> > > 
> > > Yea, maybe step by step like this:
> > > 
> > > Part-1 VIOMMU_ALLOC and VIOMMU_ATTACH
> > > Part-2 VIOMMU_SET/UNSET_VDEV_ID
> > > Part-3 VIOMMU_INVALIDATE
> > > Part-4 VQUEUE_ALLOC
> > > ...
> > 
> > So we have this stuff still open:
> >  - Identity STE with PASID (part 2b)
> >  - IOMMU_GET_HW_INFO (part 3)
> >  - IOMMU_HWPT_ALLOC_NEST_PARENT (part 3)
> >  - IOMMU_HWPT_DATA_ARM_SMMUV3 (part 3)
> >  - IOMMU_HWPT_INVALIDATE_DATA_ARM_SMMUV3
> >  - VIOMMU_ALLOC, VIOMMU_ATTACH
> >  - VIOMMU_INVALIDATE
> >  - VIOMMU_SET/UNSET_VDEV_ID
> >  - VQUEUE_ALLOC / vCMDQ
> > 
> > I feel like IOMMU_HWPT_INVALIDATE_DATA_ARM_SMMUV3 is a reasonable fit
> > to part 3. Then part 4 would be VIOMMU_ALLOC -> VIOMMU_SET/UNSET_VDEV_ID
> > which brings ATS support the API.
> 
> There is some conflict at passing in viommu_id/viommu v.s. parent
> hwpt_id/domain for a nested domain allocation. Do you think that
> should be addressed later in VIOMMU series v.s. part3?
>
> More specifically, I have two drafts in my viommu series:
> 87a659e65229 WAR: iommufd: Allow pt_it to carry viommu_id
> 7c5fd8f50bc9 WAR pass in viommu pointer to domain_alloc_user op

It would be good for viommu to come with all the uAPI changes in one
shot, so all the pt_ids should be updated to accept viommu to pass the
S2 HWPT.

Then whatever driver changes are needed to make ATS work should come
together too.

> I know that these two only make sense with VIOMMU_ALOC. Yet, will
> there be a problem, if we establish nested domain allocation with
> parent domain/hwpt by part3, in the uAPI, and then change later?
> Will we end up with supporting two for backward compatibility?

I think this is fairly minor compatability, let's see.

Jason