Date:   Fri, 10 Feb 2023 20:44:40 -0400
From:   Jason Gunthorpe <jgg@nvidia.com>
To:     Alex Williamson <alex.williamson@redhat.com>
Cc:     Nicolin Chen <nicolinc@nvidia.com>, kevin.tian@intel.com,
        joro@8bytes.org, will@kernel.org, robin.murphy@arm.com,
        shuah@kernel.org, yi.l.liu@intel.com, linux-kernel@vger.kernel.org,
        iommu@lists.linux.dev, kvm@vger.kernel.org,
        linux-kselftest@vger.kernel.org, baolu.lu@linux.intel.com,
        "Raj, Ashok" <ashok.raj@intel.com>
Subject: Re: [PATCH v2 02/10] iommu: Introduce a new
 iommu_group_replace_domain() API
Message-ID: <Y+bk+GSCPKOJfr1f@nvidia.com>
References: <cover.1675802050.git.nicolinc@nvidia.com>
 <fa81379dca611c1d9f50f9d8cd2bca0d4ec7f965.1675802050.git.nicolinc@nvidia.com>
 <20230210165110.4e89ce55.alex.williamson@redhat.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230210165110.4e89ce55.alex.williamson@redhat.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?drZ8aNO3mFW7znSRkjLQrHGI0eKPXMaGy/6Zm5KjOxmbiH1Hz4tllynDlaMX?=
 =?us-ascii?Q?QzZJ8Qb/N72pB8mPXkcoIOJCdVl5zQMrKeq5P0rosoCHMl5dSnjLW2urD4jC?=
 =?us-ascii?Q?MOOyW7sw+YPF8xEW3uFo71cWBIFoEL4EROoWyl6So7kP6ILj0p5pwwxdr2HC?=
 =?us-ascii?Q?F0iyQ7Bje/kV1NZi/Mv5ZSrNZKrvJ2CYVCKKzTeUICjx5HzAS7lKiTInIFcw?=
 =?us-ascii?Q?4KKuz8d8nSu4TJfpNskgwGClDjDFG3K1B8q+CUy7E66jo0+Y+jxGAJFJvpRi?=
 =?us-ascii?Q?i6RMmi82SfLU3Idysoj+DMEnsBlu4W/mJc1+OXKxnQzRnmXzvFIrFZ5WK0y6?=
 =?us-ascii?Q?LUaDrPOW9umE17ff3k2LqCB7LlcEBZ2zHYcjB5QDjO9eba8IbQYEU09TgPII?=
 =?us-ascii?Q?NlsyC/JROE3NHRMPv1F/neeClRYQ1OOH4PJuca53T6z7CAC8nGNtZ+j2lezG?=
 =?us-ascii?Q?Bd2tPcQn4z8bIJxsjCmkTkiRCPzaClN0pVI6YKAt/jCGr3Um90n/WrVzOTP0?=
 =?us-ascii?Q?qeqGIdFA4S/cZMEZnkLkPke8jpMcUZaPikcukPEf38hdS+2m2abXDPhv7vAg?=
 =?us-ascii?Q?2qhqbRpQ3n8o3acDs/SbF0paWx9tVnNCIk1j4BEsYve9PIU9RDNgJoX4ns32?=
 =?us-ascii?Q?P/WSkm4rAxi0MCo/+rqZkqrnDw4pfu+4ptfUQSc4Gt4/NUoFjIyQmOAXefkY?=
 =?us-ascii?Q?z5SamF3vKrIo7JeGgQdGbMobXStvJDDpd/2xPZ6GqXtDxsYoZT6tZXOHmbmo?=
 =?us-ascii?Q?OqblEK+ZvauYM4J9/8INb9JXIXJh68ba73FqJAedcF8eOVQTKxQO/YQk2Gck?=
 =?us-ascii?Q?nV3p8ZdxF1V7oFeR1/7+shIE37YUJilLDn4hLuNkG2mnSrugqkf3auDUAYQc?=
 =?us-ascii?Q?MzFP1EydTl1KDRzfQh9wjs4wdzliafhQLdBiJHzpBmp03hD0rn1Y6AEEQJE+?=
 =?us-ascii?Q?xr6eP2Y3FofCTHWDNGLAR2nNE/weCJDLP/k3JLyn9Z0QhAFqamx6vZ8rqdcl?=
 =?us-ascii?Q?spBt3DkVRSX1yWv6vHuOsDP34uENuGoxLCveziMOJqrto8EZoMSgwgBsVqCw?=
 =?us-ascii?Q?5o2vx5yriiucuMr5ea0SVWuutEn5sITI+JnzTKaFhUIzUxXnoTXcoBF3ZbAZ?=
 =?us-ascii?Q?ikD/NKaACSyYVWRntg+nbUIwFG/cNTIDU310LlVivYBrFHAJe5dr8RzNtrpO?=
 =?us-ascii?Q?UV90Jbc4UbSSi4VbTlawysB4UUZwfu2axaJjsGJNQZcF7u4OUWNQdpbMr9N9?=
 =?us-ascii?Q?Uh3aGX9siTAfDIANEfeD0i7lU991d7np+9+9lu2nZ3kFsSOv5EO95EbDVPeX?=
 =?us-ascii?Q?Ai0t5t83439EEeGmIWMeSxSoYYQ2V/41WnGQu8nc2s/+8EPvR8QVcfgIjW9P?=
 =?us-ascii?Q?qBRHVNY8PiI+nTbbXQX7EiTtsiWeTGaOgfq+Nxn0ugtf7ZoRPvMxxCXKVc8Y?=
 =?us-ascii?Q?1wWpGtDOiUOTVZ4wG2C4v0LjCUSW9UBFgwpTY5KgfRcWUsStrDIjcGIpiWW7?=
 =?us-ascii?Q?pC7+cwHSd/KYwKkYOuMbwCIshAVNwyuGA5xlX3wR0QI7IRenwobJzA1SurlC?=
 =?us-ascii?Q?itsTpRcZRjCc0toWUDxvonnMzHzEEcMTvc/FC96Z?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 03a7c498-612b-4360-886c-08db0bc92916
X-MS-Exchange-CrossTenant-AuthSource: LV2PR12MB5869.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Feb 2023 00:44:41.0450
 (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: GRxw1X9VpO1SbKV1gAG7XALT9EMEk0dzKlP9mnVhETTo3ozmrx5qWGIed9sgbPQ4
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB8594
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, Feb 10, 2023 at 04:51:10PM -0700, Alex Williamson wrote:
> On Tue, 7 Feb 2023 13:17:54 -0800
> Nicolin Chen <nicolinc@nvidia.com> wrote:
> 
> > qemu has a need to replace the translations associated with a domain
> > when the guest does large-scale operations like switching between an
> > IDENTITY domain and, say, dma-iommu.c.
> > 
> > Currently, it does this by replacing all the mappings in a single
> > domain, but this is very inefficient and means that domains have to be
> > per-device rather than per-translation.
> > 
> > Provide a high-level API to allow replacements of one domain with
> > another. This is similar to a detach/attach cycle except it doesn't
> > force the group to go to the blocking domain in-between.
> > 
> > By removing this forced blocking domain the iommu driver has the
> > opportunity to implement an atomic replacement of the domains to the
> > greatest extent its hardware allows.
> > 
> > It could be possible to adderss this by simply removing the protection
> > from the iommu_attach_group(), but it is not so clear if that is safe
> > for the few users. Thus, add a new API to serve this new purpose.
> > 
> > Atomic replacement allows the qemu emulation of the viommu to be more
> > complete, as real hardware has this ability.
> 
> I was under the impression that we could not atomically switch a
> device's domain relative to in-flight DMA.  

Certainly all the HW can be proper atomic but not necessarily easily -
the usual issue is a SW complication to manage the software controlled
cache tags in a way that doesn't corrupt the cache.

This is because the cache tag and the io page table top are in
different 64 bit words so atomic writes don't cover both, and thus the
IOMMU HW could tear the two stores and mismatch the cache tag to the
table top. This would corrupt the cache.

The easiest way to avoid this is for SW to use the same DID for the
new and old tables. This is possible if this translation entry is the
only user of the DID. A more complex way would use a temporary DID
that can be safely corrupted. But realistically I'd expect VT-d
drivers to simply make the PASID invalid for the duration of the
update.

However something like AMD has a single cache tag for every entry in
the PASID table so you could do an atomic replace trivially. Just
update the PASID and invalidate the caches.

ARM has a flexible PASID table and atomic replace would be part of
resizing it. eg you can atomically update the top of the PASID table
with a single 64 bit store.

So replace lets the drivers implement those special behaviors if it
makes sense for them.

> Or maybe atomic is the wrong word here since we expect no in-flight DMA
> during the sort of mode transitions referred to here, and we're really
> just trying to convey that we can do this via a single operation with
> reduced latency?  Thanks,

atomic means DMA will either translate with the old domain or the new
domain but never a blocking domain. Keep in mind that with nesting
"domain" can mean a full PASID table in guest memory.

I should reiterate that replace is not an API that is required to be
atomic.

Jason