Received-SPF: pass (google.com: domain of linux-kernel+bounces-178888-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Tue, 14 May 2024 12:53:23 -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: <ZkOI8ztR1mUMJ8oe@nvidia.com>
References: <cover.1712978212.git.nicolinc@nvidia.com>
 <6e57d7b5aa1705bdd547b1cd2aca93d3bf70dfa4.1712978212.git.nicolinc@nvidia.com>
 <ZkDWXnPW7CaX5TtA@nvidia.com>
 <ZkGZc5dvLigXcWib@nvidia.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZkGZc5dvLigXcWib@nvidia.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?spytAs+1nl+sRPb1dXgSinUmeHbCHqMAFKzGv9nXa/tNrdaTxh2kcG1ByT0V?=
 =?us-ascii?Q?qCHbsfZTzJ4Zba8HsAQACoTPQU0Th/B/9cybYOy57S74F+0lwtP2QEqcbnuB?=
 =?us-ascii?Q?VvrOPJ/AfQcNjj5g5XvKLBhzRWO2DOriDPJ6/S9iP3UnJfCE/EkHVxhS+cDV?=
 =?us-ascii?Q?J0x42bnTjcCvcnBgGxzVVK2NNFz5aRk3mLJ4pORaY4IjtB1k1ZSjlUdAjyuD?=
 =?us-ascii?Q?3OGWLp5NTRSkOmnqsBFY6N25robejsjNbLKpCfOp6zOfnnHgAKyXKUJ1GbHu?=
 =?us-ascii?Q?o3NkQ2TZYdYjwy0+afWQ4uhUIJca8u93wf15w/1hYtvJX+BSQ8ZeIZ2XPeWD?=
 =?us-ascii?Q?+J32RdM2SyZ5C8ajbs0CNvOeEuj3MG7fvN/sTt6Z3UqG06mYDa7wkPEKbDp0?=
 =?us-ascii?Q?h8zJ7IEEKTDuFPS9qXAyzxIL6nRyT3sQwg+dtqpjZ/Bya5hiJJIYuYF94ovf?=
 =?us-ascii?Q?sv8YNhuYTHGbO5MjIUeXxtcQUdzJY35UP8C+mtEuijyvIm21fhTTsBUtGAfI?=
 =?us-ascii?Q?H+2abxmA9J/TjAO8dmlco7k6geQvOveoeED3WiqmPezMRgFVOvM9DSTIlrTS?=
 =?us-ascii?Q?igVhju2wRgSMP4LzCYNW7C1jjWUVf3anKe8TdN08TfsxDchJEJGyul+DYkdy?=
 =?us-ascii?Q?y9J3mC20Fpfbh5RFALOk8MNdew6LuIOa/azpd6QN9ZlSA156/7VFD7Ymkk3S?=
 =?us-ascii?Q?3d+/NXDcnEUqw2cE7QEl07vl3D3wWw55n1Gd7SxSlmzbqKnweUU4uLbrjU+W?=
 =?us-ascii?Q?p1l/U3mmyvwysprOyFD5GgooUiWmxOBLxh1RKCXSQ3R5/bRy0t5Mw93bBg/x?=
 =?us-ascii?Q?PtfpH6n5W1GXK6kBCVIdID7DrDGspa4jSXnWE3HHZcvYOXaWn3OXwZYOwdWm?=
 =?us-ascii?Q?4jV9pxGp4Cl6Yoz+9l45Yw6FZknnv8C68RiLWzyiSfvGoeYqJLCLMcXSzdzL?=
 =?us-ascii?Q?jNiIdbzgh6iFdIUYertblfcqqWseueEVouafXtMAPgaRjtd8IVBzM0yVs8ot?=
 =?us-ascii?Q?dGNBXr3g8JBw73JQsXrf9Q7Hz4xmByizoP+cPP8YMI+h+tlitfNDT5fnqmju?=
 =?us-ascii?Q?xx1pxaOJzgW7dXyyo8AJAK8XPvj4uZVnwPcVqbeV7XJ/0qZ9af93FLCyR1Ij?=
 =?us-ascii?Q?6wasSPMryxHNMHIdCyvozyc8cHQLDjynR5xk3ChEF84qhx2mRwl0Rr4x8wVO?=
 =?us-ascii?Q?gTOhLOnGQEIZ3iqYIKwH6151sJb5A0F6bWtWis03WWCsRmezU/fiL9y3mhkW?=
 =?us-ascii?Q?GfgLbWogllXs/1byWgEgU7JfgQpGn/mk4mrVsg5lcPa4FH+UiEwrlZUZXDG8?=
 =?us-ascii?Q?arEfsbaOFW2ckdJxH7wBvn2R0n9Px6KsbvJbb3EoTEalbc8mN3YYNpCEPpLs?=
 =?us-ascii?Q?c0JKpGmXM7YSlRGy9beG5rjn7kZDvxsZ6EdIL7Ky5UNz/9E3U0yyxj/v5q+T?=
 =?us-ascii?Q?DnuqJrQrEPR5EuCh2xI/+pGcOAsQaqMwUqGhTH7ejscVRWMevoIC0N0QpQXE?=
 =?us-ascii?Q?+UK2/dFOMtWaG23NJV7m2l5ZxqDLKNIfuClESOvRRkUkHL7XnVaO1Sl6G0PZ?=
 =?us-ascii?Q?rc7vdzMGfTw0G2VqmfM=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 8cf6ad9f-b3fa-4c9e-b8f8-08dc742dfdab
X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB3849.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 May 2024 15:53:25.9178
 (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: pdLP+UKgXsLYfizOIA01NiCq4giQ9XUoxAkKzzPRSsZeaqDXOSAJn36OCK6SssT9
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4369

On Sun, May 12, 2024 at 09:39:15PM -0700, Nicolin Chen wrote:
> On Sun, May 12, 2024 at 11:46:54AM -0300, Jason Gunthorpe wrote:
> > On Fri, Apr 12, 2024 at 08:47:04PM -0700, Nicolin Chen wrote:
> > > Add a pair of ops to set and unet device's virtual ID that belongs to
> > > a viommu object. They will be used, in the following patch, by iommufd
> > > to support some HW-acceleration feature from the host level.
> > > 
> > > For instance, every device behind an ARM SMMU has a Stream ID. The ID
> > > is used by ATC invalidation commands so SMMU HW can direct invalidation
> > > requests to the corresponding PCI device where the ID belongs to. In a
> > > virtualization use case, a passthroughed device in the VM will have a
> > > virtuail Stream ID, used by the ATC invalidation commands in the guest
> > > system. NVIDIA's CMDQV extension for SMMUv3 provides a v-interface to
> > > execute the guest-level ATC invalidation commands directly, yet needs
> > > the HW to be aware of its virtual Stream ID so it can replace with its
> > > physical Stream ID.
> > 
> > I imagine using this as well for the ATC invalidation commands. It
> > would be very easy and simplifying if the command fixup just extracted
> > the vSID from the ATC invalidation and used an xarray to turn it into
> > a pSID and then pushed the resulting command.
> 
> You mean the nested SMMU series right? Actually the set_dev_id
> ioctl was a part of that until we wanted to try DEV_INVALIDATE.

Yes, there is nothing inherently wrong with DEV_INVALIDATE, we could
continue to use that as the API and automatically pick up the VIOMMU
instance from the nesting domain to process the ATS.

The VMM needs a reliable place to send the CMDQ data, on ARM/AMD this
needs to be an always available global-to-the-viommu thing. Intel
needs to associate the virtual invalidation with the correct nesting
domain as well.

So I original thought it would nice and simple to have a
VIOMMU_INVALIDATE as well.

But.. If we need a nesting domain that is indentity (ie the S2) then
when the VIOMMU is created then we'd also create an identity nesting
domain as well. Functionally we could use that global nesting domain
to deliver the DEV_INVALIDATE too.

It is a bit quirky, but it would be OK.

> So again, yes, it makes sense to me that we move viommu and the
> set_dev_id to the nested series, and then drop DEV_INVALIDATE.

I would like to do this bit by bit. viommu is a big series on its own.

DEV_INVALIDATE is fine, it just can't do ATS invalidation.

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)

Jason