Received-SPF: pass (google.com: domain of linux-kernel+bounces-204208-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Thu, 6 Jun 2024 08:55:03 -0300
From: Jason Gunthorpe <jgg@nvidia.com>
To: Yan Zhao <yan.y.zhao@intel.com>
Cc: Christoph Hellwig <hch@infradead.org>, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, x86@kernel.org,
	alex.williamson@redhat.com, kevin.tian@intel.com,
	iommu@lists.linux.dev, pbonzini@redhat.com, seanjc@google.com,
	dave.hansen@linux.intel.com, luto@kernel.org, peterz@infradead.org,
	tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, hpa@zytor.com,
	corbet@lwn.net, joro@8bytes.org, will@kernel.org,
	robin.murphy@arm.com, baolu.lu@linux.intel.com, yi.l.liu@intel.com,
	Russell King <linux@armlinux.org.uk>
Subject: Re: [PATCH 3/5] x86/mm: Introduce and export interface
 arch_clean_nonsnoop_dma()
Message-ID: <20240606115503.GZ19897@nvidia.com>
References: <20240507061802.20184-1-yan.y.zhao@intel.com>
 <20240507062044.20399-1-yan.y.zhao@intel.com>
 <ZktZDmcNnsHhp4Tm@infradead.org>
 <20240521154939.GH20229@nvidia.com>
 <20240521160016.GA2513156@nvidia.com>
 <ZlV7rlmWdU7dJZKo@infradead.org>
 <Zlt6huNJeW8ekJlE@nvidia.com>
 <ZmEjavnYePBLDbrS@yzhao56-desk.sh.intel.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZmEjavnYePBLDbrS@yzhao56-desk.sh.intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?7IG1awX2rOpgLEOvkba20ylMW851rxc+DmVaBEj1x2fNQc1V6S/ZwEKvnqm2?=
 =?us-ascii?Q?QaKjjuPDOAFHelZqsvFRp4nwhA/qKSJiXvhjkPeJ7WNYjHXUJKpNu3ljO9Ur?=
 =?us-ascii?Q?WcG6fPTLVo/QRixEfh6/1z/9xTVBrVoI2yYKgBV1gbDDxkAou3Jnt7i/yeO4?=
 =?us-ascii?Q?6OkDNs/K5f9tH22/2TlIwBvjKVqnxAcHQc0OPZwIlV1SKBqnZHHGL3CEIdbh?=
 =?us-ascii?Q?SQmcVJRjA1fh7CDaJeYe0u4LjpNSac2l4c2iHNujnz4XGRNA1Ztdw/ZMQ8Il?=
 =?us-ascii?Q?+6zATncyHHvFgr+1r2CW6OfPBzw+8iOJS3aJU4rCwVY/3FGR0rdfgNyKSazu?=
 =?us-ascii?Q?JQgLWZWNWoRNT/x6tMMrPf3YZ1SoCX/zCLec/onXACiGJZMvSGJybFcHNuUd?=
 =?us-ascii?Q?pcdNcyfqL0+fswUmrYmXPkeF5JDo3x8cmra6+CZYqYVID1dYrmiSIoGNhwH7?=
 =?us-ascii?Q?pAZp0m686CPk3MUshX74WxZNfqgxeQXKVYdwcElwDaAaM87akenAOHJ6+Eh6?=
 =?us-ascii?Q?4uPkofpdrv3EmpiV8RpiQtkGKQTmtSP+rslGJTZ1qsItOayqGZVyTyJ5Kssz?=
 =?us-ascii?Q?QPgX2U6POdgje/kwUEBi1rxx8uw1uV6QJ4Lqj6seHHTRBPG1HeTjJgCV4hCO?=
 =?us-ascii?Q?BA/S2YjoP34v7eBfW5LBFOX2rcwdQH2jqLE3eOzD6jwlpaLtW2wznu6Tj3sZ?=
 =?us-ascii?Q?KHCfnRPNq1Wzvr46HMlf06YX9FZm3DQ9An9pkzPcpmHdKTMkwaomcNbt7q/K?=
 =?us-ascii?Q?Hcvl3feWXO5mC7qSH3WEHlQSXCNtEN0FJWj4dDEuNMytDjRK7VEjajQ7YN6g?=
 =?us-ascii?Q?+CguNkr1ZEz3DOSExcc42r2so/6DD2yojRAHFjRK1ZJs2qlvwHvDgq5XOnyy?=
 =?us-ascii?Q?qc+SJuWEIGOq4rFKXOa/dSKMpdSzikahkS51DW47FMfTMJvI6Qg6dfJyuY3D?=
 =?us-ascii?Q?4tFZOQ/gb1pyVkzqKwj2nchT5E8YUb5HHnym7SL5sd+FM+jfal/4bmTd7cr6?=
 =?us-ascii?Q?TAX6riMByxoKTIYlwpIqoupBv89ADSR+wiLSk2LBgG3+3TuSY+IEeWJ/v0KK?=
 =?us-ascii?Q?F/eK5vRS6N6JCm5gyJv2Ifp8s+f3Ir3LPI1337kw+yT/47cC6ayT9nny3c12?=
 =?us-ascii?Q?8jwLXitR25K4x4dcEdQj4qf/4sC2Ste44mkdIyOe9WLrBRtv3Is0hKxmmjDI?=
 =?us-ascii?Q?dr5I3A00BB9l4bhG/2X9dVt0m/klpDy3xD8jVI0kMWHLGcgtCxeQjVan8pYJ?=
 =?us-ascii?Q?LP++gZk3KttgY8O70GcEl5cQ/SqHb8ExugUSE+VBCUmmFxN1qG2CBxXAW4Yo?=
 =?us-ascii?Q?qlcSHLssBvG2F/bJfNRExw1fWi4ISyA424k0ze5rDrgMPTcdoJjgXok5BRaH?=
 =?us-ascii?Q?q7oD6dDZ+uIrOZ1T85D6ersP1P22xKvWR59DCFNum4MmmdVAyRNaCEHDv1L1?=
 =?us-ascii?Q?79BW0f+O/0FNGzaYjpIb1MkjKmI0UImo/vbWKq2Y1X076qx7dt3l3m3lThyF?=
 =?us-ascii?Q?DLct25uCgFMWog71YJ0WpmGBxP2uTR0r3Zvj11FpPAgJEJls6eUw0pcuUgP+?=
 =?us-ascii?Q?Y8QfsY7dOIFJkwn7RBw/KiArZoOYi7fN7QxMm/ps?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: b8dbd145-c0e1-4eba-cd13-08dc861f8116
X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB3849.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Jun 2024 11:55:04.9681
 (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: sbM8tDNfm0TocNK8sDMbKl7/wnpE4eV9Rrpt5CCEEZTC0VCB1zkI4gxEOqzh/vdK
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4371

On Thu, Jun 06, 2024 at 10:48:10AM +0800, Yan Zhao wrote:
> On Sat, Jun 01, 2024 at 04:46:14PM -0300, Jason Gunthorpe wrote:
> > On Mon, May 27, 2024 at 11:37:34PM -0700, Christoph Hellwig wrote:
> > > On Tue, May 21, 2024 at 01:00:16PM -0300, Jason Gunthorpe wrote:
> > > > > > Err, no.  There should really be no exported cache manipulation macros,
> > > > > > as drivers are almost guaranteed to get this wrong.  I've added
> > > > > > Russell to the Cc list who has been extremtly vocal about this at least
> > > > > > for arm.
> > > > > 
> > > > > We could possibly move this under some IOMMU core API (ie flush and
> > > > > map, unmap and flush), the iommu APIs are non-modular so this could
> > > > > avoid the exported symbol.
> > > > 
> > > > Though this would be pretty difficult for unmap as we don't have the
> > > > pfns in the core code to flush. I don't think we have alot of good
> > > > options but to make iommufd & VFIO handle this directly as they have
> > > > the list of pages to flush on the unmap side. Use a namespace?
> > > 
> > > Just have a unmap version that also takes a list of PFNs that you'd
> > > need for non-coherent mappings?
> > 
> > VFIO has never supported that so nothing like that exists yet.. This
> > is sort of the first steps to some very basic support for a
> > non-coherent cache flush in a limited case of a VM that can do its own
> > cache flushing through kvm.
> > 
> > The pfn list is needed for unpin_user_pages() and it has an ugly
> > design where vfio/iommufd read back the pfns seperately from unmap,
> > and they both do it differently without a common range list
> > datastructure here.
> > 
> > So, we'd need to build some new unmap function that returns a pfn list
> > that it internally fetches via the read ops. Then it can do the read,
> > unmap, flush iotlb, flush cache in core code.
> Would the core code flush CPU caches by providing page physical address?

Physical address is all we will have in the core code..

> If yes, do you think it's still necessary to export arch_flush_cache_phys()
> (as what's implemented in this patch)?

Christoph is asking not to export it, that would mean relying on the
iommu core to be non-modulare and putting the arch calls there with a
more restricted exported API - ie based on unmap.

> > I've been working towards this very slowly as I want to push this
> > stuff down into the io page table walk and remove the significant
> > inefficiency, so it is not throw away work, but it is certainly some
> > notable amount of work to do.
> Will VFIO also be switched to this new unmap interface? Do we need to care
> about backporting?

I don't know :)
 
> And is it possible for VFIO alone to implement in the current proposed way
> in this series as the first step for easier backport?

I think this series is the best option we have right now, but make the
EXPORT a NS export to try to discourage abuse of it while we continue
working

Jason