Re: [PATCH RFC] hw/cxl: type 3 devices can now present volatile or persistent memory

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
To: Gregory Price <gregory.price@memverge.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>,
	Gregory Price <gourry.memverge@gmail.com>,
	<qemu-devel@nongnu.org>, <linux-cxl@vger.kernel.org>,
	Alison Schofield <alison.schofield@intel.com>,
	"a.manzanares@samsung.com" <a.manzanares@samsung.com>,
	Ben Widawsky <bwidawsk@kernel.org>
Subject: Re: [PATCH RFC] hw/cxl: type 3 devices can now present volatile or persistent memory
Date: Tue, 11 Oct 2022 18:28:58 +0100	[thread overview]
Message-ID: <20221011182858.00005215@huawei.com> (raw)
In-Reply-To: <Y0WmYNcVHhVHeyH+@memverge.com>

On Tue, 11 Oct 2022 13:22:40 -0400
Gregory Price <gregory.price@memverge.com> wrote:

> I'll push the patches to qemu-cxl and linux-cxl today or tomorrow, I
> wanted to get them into a state on gitlab for Jonathan to rebase and
> merge into his work.  He'll likely end up pushing the entire series at
> the end of the day.
> 
> Will update the tests/docs accordingly.
> 
> re: changelog - i'm new to mailing list contributions, where would these
> go exactly?

Two places - in the cover letter, and under the ---
in the individual patches.  

At this stage I'm not super fussed by tests because there won't be
anything visible beyond 'does it crash'.  Don't think we will be affecting
the ACPI table test results for example.  Good to change to the proposed
new command line for persistent memdev though.

Definitely good to get some testing in place for this longer term but
our whole strategy for that is a bit adhoc for now because tooling isn't
yet commonly available in distros etc (and I for one haven't looked
at what is is necessary yet!)

Was pointed out to me last week that the docs currently give arm64 examples
and we don't have upstream arm64 support yet :( 

The wonders of partial pickup of patch sets!

Thanks,

Jonathan

> 
> 
> On Tue, Oct 11, 2022 at 10:14:38AM -0700, Davidlohr Bueso wrote:
> > On Mon, 10 Oct 2022, Gregory Price wrote:
> >   
> > > I've pushed 5 new commits to this branch here (@Jonathan I've also made
> > > a merge request to pull them into your branch).
> > > 
> > > https://gitlab.com/gourry.memverge/qemu/-/commits/cxl-2022-10-09  
> > 
> > This series could perhaps be posted as a reply to the CDAT extensions
> > cover letter. But regardless, at some point it should be in linux-cxl@.
> >   
> > > 
> > > They're built on top of Jonathan's extensions for the CDAT since the
> > > CDAT has memory region relevant entries and trying to do this separate
> > > would be unwise.
> > > 
> > > 1/5: PCI_CLASS_MEMORY_CXL patch
> > > 2/5: CXL_CAPACITY_MULTIPLIER pullout patch (@Davidlohr request)  
> > 
> > Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
> > 
> > However this needs a changelog; for however redundant it may be.
> >   
> > > 3/5: Generalizes CDATDsmas intialization ahead of multi-region  
> >   
> > > 4/5: Multi-region support w/ backward compatibility
> > >     * Requires extra eyes for CDAT and Read/Write Change Validation*  
> > 
> > I'm still eyeballing this but it certainly looks much more complete now -
> > at least with the minimal support I was hoping for.
> >   
> > > 5/5: Test and documentation update  
> > 
> > I think that there should two examples here with volatile and LSA usage.
> > The first is without as it is quite unintuitive otherwise, then a
> > second example with specifying the lba. Also in these cases you want
> > id=cxl-vmem0. And the documentation should be updated to mention that
> > memdev is deprecated and {persistent/volatile}-memdev should be used.
> > 
> > Thanks,
> > Davidlohr  