RE: Questions about CXL device (type 3 memory) hotplug

From: Vikram Sethi <vsethi@nvidia.com>
To: Dan Williams <dan.j.williams@intel.com>,
	"Yasunori Gotou (Fujitsu)" <y-goto@fujitsu.com>,
	"linux-cxl@vger.kernel.org" <linux-cxl@vger.kernel.org>,
	"catalin.marinas@arm.com" <Catalin.Marinas@arm.com>,
	James Morse <james.morse@arm.com>
Cc: "Natu, Mahesh" <mahesh.natu@intel.com>
Subject: RE: Questions about CXL device (type 3 memory) hotplug
Date: Wed, 24 May 2023 00:02:31 +0000	[thread overview]
Message-ID: <BYAPR12MB3336AB6A519C709DBEA75CB5BD419@BYAPR12MB3336.namprd12.prod.outlook.com> (raw)
In-Reply-To: <646d0892eadc3_afb77294cb@dwillia2-xfh.jf.intel.com.notmuch>

> From: Dan Williams <dan.j.williams@intel.com>
> Sent: Tuesday, May 23, 2023 1:40 PM
> To: Vikram Sethi <vsethi@nvidia.com>; Dan Williams
> <dan.j.williams@intel.com>; Yasunori Gotou (Fujitsu) <y-goto@fujitsu.com>;
> linux-cxl@vger.kernel.org; catalin.marinas@arm.com; James Morse
> <james.morse@arm.com>
> Cc: Natu, Mahesh <mahesh.natu@intel.com>
> Subject: RE: Questions about CXL device (type 3 memory) hotplug
> 
> Vikram Sethi wrote:
> > Hi Dan,
> >
> > > From: Dan Williams <dan.j.williams@intel.com>
> > > Sent: Monday, May 22, 2023 7:12 PM
> > > To: Yasunori Gotou (Fujitsu) <y-goto@fujitsu.com>; linux-
> > > cxl@vger.kernel.org
> > > Cc: 'Dan Williams' <dan.j.williams@intel.com>
> > > Subject: RE: Questions about CXL device (type 3 memory) hotplug
> > >
> > > > Q4) Current CXL drivers/tools support Hot-removal request from PCIe?
> > > >
> > > >     CXL specification says "In a managed Hot-Remove flow, software is
> > > >     notified of a hot removal request."
> > >
> > > Currently there is a requirement that:
> > >
> > > cxl disable-memdev
> > >
> > > ...is run before the device can be removed. There is no warning from
> > > the PCI hotplug driver. Which means that if end user does the wrong
> > > sequence they can crash the kernel / remove memory that may still be in
> active use.
> > >
> > Is there any notion of a cache flush when memory is removed (or in future
> CXL reset)?
> 
> No.
> 
> > Generally, CPU caches must be flushed when memory is removed because
> > any evictions when the memory isn't present can cause async errors
> > which can be fatal to the system or at least to VMs, depending on ISA.
> 
> This seems incompatible with memory hotplug. The cache flushing is only
> done on the subsequent reuse of physical address range to make sure that
> any pending evictions are complete before the newly constituted address
> range is put into service, or that any prior clean cache lines of old content are
> dropped. See cxl_region_invalidate_memregion() for where this is called.
> 
> > If the kernel does the cache flush, it must be done with only
> > uncacheable mappings present to prevent speculative fetches after the
> cache flush.
> 
> This is why the invalidation is done after physical address range is populated
> by new devices. To flush any speculative fetches to the old composition of
> the address range.
> 
I don't think invalidate on the probe path will always work for devices with snoop filters, including HDM-DB memory devices or CXL type2 accelerators. 
After CXL reset or hot plug insertion, a HDM-DB device's snoop filter isn't tracking any lines checked out by the host. Even if those were just clean lines in CPU caches, hosts can send drop notifications in CXL in response to the cache flush (MemClnEvict).
A device that isn't expecting this evict notification can go into error state and optionally raise a device internal error interrupt. So you could end up with a non functional device.

> > Even so, kernel VA based cache flushes will likely be slow, so may be
> > better to have the notion of an arch callback that can invoke firmware to do
> the cache flush.
> > Perhaps arch_remove_memory is the right place to invoke such a cache
> flush/FW call?
> > I think the CXL specification should also address the need for cache
> > flush when removing memory or doing CXL reset.
> 
> Seems out of scope for the CXL specification, this is up to each arch to
> handle.
> 
> Here is some discussion of what ARM is thinking about in this space:
> 
> https://lore.kernel.org/all/40cd479b-f0f8-5dba-0e41-
> 4cef73693927@arm.com/