Re: [Qemu RFC 0/7] Early enabling of DCD emulation in Qemu

[Fan Ni <fan.ni@samsung.com>]

On Tue, Jul 25, 2023 at 08:18:08AM -0700, Ira Weiny wrote:

> Fan Ni wrote:
> > On Thu, May 11, 2023 at 05:56:40PM +0000, Fan Ni wrote:
> > 
> > FYI.
> > 
> > I have updated the patch series and sent out again.
> > 
> > I suggested anyone who are interested in DCD and using this patch series to
> > use the new series. Quite a few things has been fixed.
> > 
> > https://urldefense.com/v3/__https://lore.kernel.org/linux-cxl/20230724162313.34196-1-fan.ni@samsung.com/T/*t__;Iw!!EwVzqGoTKBqv-0DWAJBm!V8kDTpT5yLUAyWm3sFm7XIgN0QdNUyQYZd9vYLHjUVkMkhDT14F8avgNBh23KPAtsS_dGm2LZuHJ102mgIg$ 
> > 
> > Also, if you want to use the code repo directly, you can try
> > 
> > https://urldefense.com/v3/__https://protect2.fireeye.com/v1/url?k=0600fd6b-678be820-06017624-74fe485fb305-f529279062b02b73&q=1&e=5df65010-e62f-40a1-9a21-609eb1400921&u=https*3A*2F*2Fgithub.com*2Fmoking*2Fqemu-dcd-preview-latest*2Ftree*2Fdcd-dev__;JSUlJSUlJQ!!EwVzqGoTKBqv-0DWAJBm!V8kDTpT5yLUAyWm3sFm7XIgN0QdNUyQYZd9vYLHjUVkMkhDT14F8avgNBh23KPAtsS_dGm2LZuHJpB0SmNs$ 
> 
> Thanks for the branch!
> 
> I took a quick look and I don't see a resolution to the problem I
> mentioned with non DCD devices being supported.[1]
> 
> [1] https://urldefense.com/v3/__https://lore.kernel.org/all/6483946e8152f_f1132294a2@iweiny-mobl.notmuch/__;!!EwVzqGoTKBqv-0DWAJBm!V8kDTpT5yLUAyWm3sFm7XIgN0QdNUyQYZd9vYLHjUVkMkhDT14F8avgNBh23KPAtsS_dGm2LZuHJ4geSEAg$ 
> 
> Did you fix this in a different way?  If I don't add DC to my mem devices they
> don't get probed properly.  I'm still looking into this with your new branch,
> but I don't think DC commands should be in the CEL if the device does not
> support it.
> 
> Also I get a build warning on this branch I had to fix[3] as my build is
> treating warnings as errors.[2]
> 
> I don't think this fix is technically necessary as 'list' should never be NULL
> that I can see.  But might be nice to check or just use my fix.
> 
> I'll try and get to a review once I get the DCD stuff out on the list again.
> 
> Ira
> 

Oh, I missed your previous comments, let me look into it and fix
accordingly and send out a new version.

Btw, when I did the DCD test with the last DCD kernel code, I found
some issue there.

When I add a DCD extent for the first time, it will be recognized as
system RAM automatically and show up with command lsmem.

However, when I release it and try to re-add the same extent again.
The adding seems normal and the device will show up under /dev/ as
dax0.X. But it will not show up with lsmem command and I have to use
daxctl reconfigure command to turn it to system ram and then it can
show up with lsmem command. I would expect the behavior for the
first add and second add be the same.

Fan.


> 
> [2]
> ../hw/mem/cxl_type3.c: In function ‘qmp_cxl_process_dynamic_capacity_event.constprop’:
> ../hw/mem/cxl_type3.c:2063:28: error: ‘rid’ may be used uninitialized [-Werror=maybe-uninitialized]
>  2063 |     dCap.updated_region_id = rid;
>       |     ~~~~~~~~~~~~~~~~~~~~~~~^~~~~
> ../hw/mem/cxl_type3.c:1987:13: note: ‘rid’ was declared here
>  1987 |     uint8_t rid;
>       |             ^~~
> cc1: all warnings being treated as errors
> 
> [3]
> 
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index e67328780407..d25e6064f6c9 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -1984,7 +1984,7 @@ static void qmp_cxl_process_dynamic_capacity_event(const char *path,
>      CXLDCExtentRecordList *list = records;
>      CXLDCExtent_raw *extents;
>      uint64_t dpa, len;
> -    uint8_t rid;
> +    uint8_t rid = 0;
>      int i;
>  
>      if (!obj) {