| Message ID | 167564537131.847146.9020072654741860107.stgit@dwillia2-xfh.jf.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | CXL RAM and the 'Soft Reserved' => 'System RAM' default | expand |
On Sun, Feb 05, 2023 at 05:02:51PM -0800, Dan Williams wrote: > In preparation for a new region mode, do not, for example, allow > 'ram' decoders to be assigned to 'pmem' regions and vice versa. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > drivers/cxl/core/region.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index c9e7f05caa0f..53d6dbe4de6d 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -1217,6 +1217,12 @@ static int cxl_region_attach(struct cxl_region *cxlr, > struct cxl_dport *dport; > int i, rc = -ENXIO; > > + if (cxled->mode != cxlr->mode) { > + dev_dbg(&cxlr->dev, "%s region mode: %d mismatch: %d\n", > + dev_name(&cxled->cxld.dev), cxlr->mode, cxled->mode); > + return -EINVAL; > + } > + > if (cxled->mode == CXL_DECODER_DEAD) { > dev_dbg(&cxlr->dev, "%s dead\n", dev_name(&cxled->cxld.dev)); > return -ENODEV; > Maybe a stupid question. It will be entirely possible to "convert" pmem to "ram" (i.e. just online it as system memory). Are type-3 devices with pmem expected to carry 2 decoders (one ram, 1 pmem), such that you can create a ram region from pmem, or is it expected that the pmem decoder will be used for pmem, even if it gets onlined as volatile? Just asking because depending on how that flushes out, you might expected a "ram region" to get assigned to a "pmem decoder", unless you're expecting all pmem-carrying-devices to create 2 decoders per pmem region depending on how the end user intends to use it. ~Gregory
On Mon, Feb 06, 2023 at 01:51:18PM -0800, Dan Williams wrote: > Gregory Price wrote: > > On Sun, Feb 05, 2023 at 05:02:51PM -0800, Dan Williams wrote: > > > In preparation for a new region mode, do not, for example, allow > > > 'ram' decoders to be assigned to 'pmem' regions and vice versa. > > > > > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > > > --- > > > drivers/cxl/core/region.c | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > [...] sounds good, thanks for the explanation Reviewed-by: Gregory Price <gregory.price@memverge.com>
Gregory Price wrote: > On Sun, Feb 05, 2023 at 05:02:51PM -0800, Dan Williams wrote: > > In preparation for a new region mode, do not, for example, allow > > 'ram' decoders to be assigned to 'pmem' regions and vice versa. > > > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > > --- > > drivers/cxl/core/region.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > > index c9e7f05caa0f..53d6dbe4de6d 100644 > > --- a/drivers/cxl/core/region.c > > +++ b/drivers/cxl/core/region.c > > @@ -1217,6 +1217,12 @@ static int cxl_region_attach(struct cxl_region *cxlr, > > struct cxl_dport *dport; > > int i, rc = -ENXIO; > > > > + if (cxled->mode != cxlr->mode) { > > + dev_dbg(&cxlr->dev, "%s region mode: %d mismatch: %d\n", > > + dev_name(&cxled->cxld.dev), cxlr->mode, cxled->mode); > > + return -EINVAL; > > + } > > + > > if (cxled->mode == CXL_DECODER_DEAD) { > > dev_dbg(&cxlr->dev, "%s dead\n", dev_name(&cxled->cxld.dev)); > > return -ENODEV; > > > > > Maybe a stupid question. It will be entirely possible to "convert" pmem > to "ram" (i.e. just online it as system memory). That's a good question, there are two mechanisms for this: 1/ Use ndctl to assign the resulting pmem namespace to devdax mode and then use dax_kmem to online that persistent memory. 2/ If the CXL device supports partitioning ram vs pmem capacity: tear down the pmem region ('cxl destroy-region ...'), repartition the pmem ('cxl set-partition -t ram ...'), create a new ram region ('cxl create-region ...'). > Are type-3 devices with pmem expected to carry 2 decoders (one ram, 1 > pmem), such that you can create a ram region from pmem, or is it expected > that the pmem decoder will be used for pmem, even if it gets onlined > as volatile? Not sure there are any formal expectations when it comes to number of decoders. > Just asking because depending on how that flushes out, you might > expected a "ram region" to get assigned to a "pmem decoder", unless > you're expecting all pmem-carrying-devices to create 2 decoders per pmem > region depending on how the end user intends to use it. Per 2/ above even with a 1-decoder-device there is a spec-defined mechanism to assign all of the device's capacity to ram.
On 2/5/23 6:02 PM, Dan Williams wrote: > In preparation for a new region mode, do not, for example, allow > 'ram' decoders to be assigned to 'pmem' regions and vice versa. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> > --- > drivers/cxl/core/region.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index c9e7f05caa0f..53d6dbe4de6d 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -1217,6 +1217,12 @@ static int cxl_region_attach(struct cxl_region *cxlr, > struct cxl_dport *dport; > int i, rc = -ENXIO; > > + if (cxled->mode != cxlr->mode) { > + dev_dbg(&cxlr->dev, "%s region mode: %d mismatch: %d\n", > + dev_name(&cxled->cxld.dev), cxlr->mode, cxled->mode); > + return -EINVAL; > + } > + > if (cxled->mode == CXL_DECODER_DEAD) { > dev_dbg(&cxlr->dev, "%s dead\n", dev_name(&cxled->cxld.dev)); > return -ENODEV; >
On Sun, 2023-02-05 at 17:02 -0800, Dan Williams wrote: > In preparation for a new region mode, do not, for example, allow > 'ram' decoders to be assigned to 'pmem' regions and vice versa. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > drivers/cxl/core/region.c | 6 ++++++ > 1 file changed, 6 insertions(+) Looks good, Reviewed-by: Vishal Verma <vishal.l.verma@intel.com> > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index c9e7f05caa0f..53d6dbe4de6d 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -1217,6 +1217,12 @@ static int cxl_region_attach(struct cxl_region *cxlr, > struct cxl_dport *dport; > int i, rc = -ENXIO; > > + if (cxled->mode != cxlr->mode) { > + dev_dbg(&cxlr->dev, "%s region mode: %d mismatch: %d\n", > + dev_name(&cxled->cxld.dev), cxlr->mode, cxled->mode); > + return -EINVAL; > + } > + > if (cxled->mode == CXL_DECODER_DEAD) { > dev_dbg(&cxlr->dev, "%s dead\n", dev_name(&cxled->cxld.dev)); > return -ENODEV; >
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index c9e7f05caa0f..53d6dbe4de6d 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -1217,6 +1217,12 @@ static int cxl_region_attach(struct cxl_region *cxlr, struct cxl_dport *dport; int i, rc = -ENXIO; + if (cxled->mode != cxlr->mode) { + dev_dbg(&cxlr->dev, "%s region mode: %d mismatch: %d\n", + dev_name(&cxled->cxld.dev), cxlr->mode, cxled->mode); + return -EINVAL; + } + if (cxled->mode == CXL_DECODER_DEAD) { dev_dbg(&cxlr->dev, "%s dead\n", dev_name(&cxled->cxld.dev)); return -ENODEV;
In preparation for a new region mode, do not, for example, allow 'ram' decoders to be assigned to 'pmem' regions and vice versa. Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- drivers/cxl/core/region.c | 6 ++++++ 1 file changed, 6 insertions(+)