Re: [PATCH 2/5] cxl/region: Add dynamic capacity cxl region support.

[Ira Weiny <ira.weiny@intel.com>]

Fan Ni wrote:
> The 06/14/2023 12:16, ira.weiny@intel.com wrote:
> > From: Navneet Singh <navneet.singh@intel.com>
> > 

[snip]

> > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > index 543c4499379e..144232c8305e 100644
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> > @@ -1733,7 +1733,7 @@ static int cxl_region_attach(struct cxl_region *cxlr,
> >  	lockdep_assert_held_write(&cxl_region_rwsem);
> >  	lockdep_assert_held_read(&cxl_dpa_rwsem);
> >  
> > -	if (cxled->mode != cxlr->mode) {
> > +	if (decoder_mode_is_dc(cxlr->mode) && !decoder_mode_is_dc(cxled->mode)) {
> For mode other than dc, no check will be performed, is that what we
> want?
> 

:-/  Yes, looks like I may have screwed up the logic here thanks.  But this
code is changing because after this thread Navneet and I decided to introduce a
new cxl_region_mode enum which should clarify this check.

[snip]

> > +
> > +static ssize_t create_dc_region_store(struct device *dev,
> > +				      struct device_attribute *attr,
> > +				      const char *buf, size_t len)
> > +{
> > +	/*
> > +	 * All DC regions use decoder mode DC0 as the region does not need the
> > +	 * index information
> > +	 */
> > +	return store_dcN_region(to_cxl_root_decoder(dev), buf,
> > +				CXL_DECODER_DC0, len);
> If all DC regions use DC0, what will CXL_DECODER_DC1~7 be used for?

Before sending the patches it did not set well with me that the mode for cxl
region was not longer 1:1 with endpoint decoder mode.  I basically hacked in
the idea that DC0 decoder mode would represent DC region mode.  But this is
really hacky.  So this is why we have introduced cxl_region_mode which
represents ram, pmem, or DC in v2.  I'm still squashing in all the changes and
clean ups and should post something soon.

Ira