> > - -object
> > memory-backend-file,id=cxl-mem1,share=on,mem-path=/tmp/cxltest.raw,size=256M
> > \
> > - -object
> > memory-backend-file,id=cxl-lsa1,share=on,mem-path=/tmp/lsa.raw,size=256M \
> > + -object
> > memory-backend-file,pmem=true,id=pmem0,share=on,mem-path=/tmp/cxltest.raw,size=256M
> > \
> > + -object
> > memory-backend-file,pmem=true,id=cxl-lsa0,share=on,mem-path=/tmp/lsa.raw,size=256M
> > \
>
> Why make the pmem=true change in here? If we want to do that I think it
> should be in a
> separate patch with explanation.
>
this is mostly an observation that memory-backend's have a pmem option.
It was unclear to me that using this backend for a pmem region without
setting pmem=true was "correct", but i also am not sure it has a real
effect. I'll drop this from the changeset.
> > +error_cleanup:
> > + int i;
> > + for (i = 0; i < cur_ent; i++) {
> > + g_free(*cdat_table[i]);
>
> Until the steal pointer above, *cdata_table not set to anything.
> Possibly gfree(table[i])?
>
>
good catch
> Hmm. I wonder if this is simpler done as below. Both look fine
> to me though so up to you for next version. Trade off between
> slightly ugly nested logic, and the readability always lost when
> a ternary operator puts in an appearance.
>
> if (ct3d->hostvmem) {
this seems reasonable, pulled in
>
> If we have both volatile and persistent and yet still only have our one HDM
> decoder, then I think a write into the persistent range will have the wrong
> offset.
>
> dpa_offset == address space offset when we only had one region. Not so much
> any more.
> For persistent i think we'll need to subtract the size of the volatile region
> (possibly taking care with alignment - I need to check that).
>
I had originally done this, but it wasn't clear to me what was correct
here, I'll make the adjustment
> > diff --git a/tests/qtest/cxl-test.c b/tests/qtest/cxl-test.c
> > index 6baed747fa..a05ddc0c7b 100644
> > --- a/tests/qtest/cxl-test.c
> > +++ b/tests/qtest/cxl-test.c
> > @@ -34,29 +34,46 @@
> > - "-object
> > memory-backend-file,id=cxl-mem3,mem-path=%s,size=256M " \
> > - "-object
> > memory-backend-file,id=lsa3,mem-path=%s,size=256M " \
> > - "-device
> > cxl-type3,bus=rp3,memdev=cxl-mem3,lsa=lsa3,id=cxl-pmem3 "
>
> If re-indenting makes sense (I'm really convinced it is worth the noise) then
> do it
> in a precusor no-op patch so we can more easily see what is new here.)
>
can do
