Re: [Qemu PATCH v2 5/9] hw/mem/cxl_type3: Add host backend and address space handling for DC regions

[Gregory Price]

On Fri, Aug 04, 2023 at 05:36:23PM +0100, Jonathan Cameron wrote:
> On Tue, 25 Jul 2023 18:39:56 +0000
> Fan Ni <fan.ni@samsung.com> wrote:
> 
> > From: Fan Ni <nifan@outlook.com>
> > 
> > Add (file/memory backed) host backend, all the dynamic capacity regions
> > will share a single, large enough host backend. Set up address space for
> > DC regions to support read/write operations to dynamic capacity for DCD.
> > 
> > With the change, following supports are added:
> > 1. add a new property to type3 device "nonvolatile-dc-memdev" to point to 
> > host
> >    memory backend for dynamic capacity;
> > 2. add namespace for dynamic capacity for read/write support;
> > 3. create cdat entries for each dynamic capacity region;
> > 4. fix dvsec range registers to include DC regions.
> > 
> > Signed-off-by: Fan Ni <fan.ni@samsung.com>
> Hi Fan,
> 
> I'm not sure if we want to do all regions backed by one memory backend
> or one backend each.  It will become complex when some are shared
> (e.g. what Gregory is working on).

I thought about this briefly when i implemented the original volatile
support due to the potential for partitioning. We landed on, iirc, 
2 backends (1 for volatile, 1 for non-volatile).

The reality, though, is the driver (presently) does not have a good way
to create more than 1 dax per memdev, and in practice with real devices
we see that this just tends to be the case: 1 dax per device.  So unless
that's going to change, ever having more than 1 backend will just be
unused complexity.

To me, this is a good example of "maybe piling everything into the core
ct3d is going to get ugly fast".  Maybe it would be better to do
something similar to the CCI interface and allow for overriding the
other functions as well.

just a thought.  I apologize for not engaging with the DCD patch set,
conferences have been keeping me busier than expected.  I plan on
putting it through the grinder this month.

> 
> A few questions inline.  In particular there are subtle changes to
> existing handling that are either bug fixes (in which case they need
> to be sent first) or bugs / have no effect and shouldn't be in here.
> 
> 
> > ---
> >  hw/cxl/cxl-mailbox-utils.c  |  19 +++-
> >  hw/mem/cxl_type3.c          | 203 +++++++++++++++++++++++++++++-------
> >  include/hw/cxl/cxl_device.h |   4 +
> >  3 files changed, 185 insertions(+), 41 deletions(-)
> >