| Message ID | 20240324-dcd-type2-upstream-v1-4-b7b00d623625@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | DCD: Add support for Dynamic Capacity Devices (DCD) | expand |
On Sun, 24 Mar 2024 16:18:07 -0700 ira.weiny@intel.com wrote: > From: Navneet Singh <navneet.singh@intel.com> > > Region mode must reflect a general dynamic capacity type which is > associated with a specific Dynamic Capacity (DC) partitions in each > device decoder within the region. DC partitions are also know as DC > regions per CXL 3.1. > > Decoder mode reflects a specific DC partition. > > Define the new modes to use in subsequent patches and the helper > functions required to make the association between these new modes. > > Signed-off-by: Navneet Singh <navneet.singh@intel.com> > Co-developed-by: Ira Weiny <ira.weiny@intel.com> > Signed-off-by: Ira Weiny <ira.weiny@intel.com> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
On Sun, Mar 24, 2024 at 04:18:07PM -0700, ira.weiny@intel.com wrote: > From: Navneet Singh <navneet.singh@intel.com> > > Region mode must reflect a general dynamic capacity type which is > associated with a specific Dynamic Capacity (DC) partitions in each s/partitions/partition/ Otherwise, Reviewed-by: Fan Ni <fan.ni@samsung.com> > device decoder within the region. DC partitions are also know as DC > regions per CXL 3.1. > > Decoder mode reflects a specific DC partition. > > Define the new modes to use in subsequent patches and the helper > functions required to make the association between these new modes. > > Signed-off-by: Navneet Singh <navneet.singh@intel.com> > Co-developed-by: Ira Weiny <ira.weiny@intel.com> > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > --- > Changes for v1 > [iweiny: split out from: Add dynamic capacity cxl region support.] > --- > drivers/cxl/core/region.c | 4 ++++ > drivers/cxl/cxl.h | 23 +++++++++++++++++++++++ > 2 files changed, 27 insertions(+) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 1723d17f121e..ec3b8c6948e9 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -1690,6 +1690,8 @@ static bool cxl_modes_compatible(enum cxl_region_mode rmode, > return true; > if (rmode == CXL_REGION_PMEM && dmode == CXL_DECODER_PMEM) > return true; > + if (rmode == CXL_REGION_DC && cxl_decoder_mode_is_dc(dmode)) > + return true; > > return false; > } > @@ -2824,6 +2826,8 @@ cxl_decoder_to_region_mode(enum cxl_decoder_mode mode) > return CXL_REGION_RAM; > case CXL_DECODER_PMEM: > return CXL_REGION_PMEM; > + case CXL_DECODER_DC0 ... CXL_DECODER_DC7: > + return CXL_REGION_DC; > case CXL_DECODER_MIXED: > default: > return CXL_REGION_MIXED; > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h > index 9a0cce1e6fca..3b8935089c0c 100644 > --- a/drivers/cxl/cxl.h > +++ b/drivers/cxl/cxl.h > @@ -365,6 +365,14 @@ enum cxl_decoder_mode { > CXL_DECODER_NONE, > CXL_DECODER_RAM, > CXL_DECODER_PMEM, > + CXL_DECODER_DC0, > + CXL_DECODER_DC1, > + CXL_DECODER_DC2, > + CXL_DECODER_DC3, > + CXL_DECODER_DC4, > + CXL_DECODER_DC5, > + CXL_DECODER_DC6, > + CXL_DECODER_DC7, > CXL_DECODER_MIXED, > CXL_DECODER_DEAD, > }; > @@ -375,6 +383,14 @@ static inline const char *cxl_decoder_mode_name(enum cxl_decoder_mode mode) > [CXL_DECODER_NONE] = "none", > [CXL_DECODER_RAM] = "ram", > [CXL_DECODER_PMEM] = "pmem", > + [CXL_DECODER_DC0] = "dc0", > + [CXL_DECODER_DC1] = "dc1", > + [CXL_DECODER_DC2] = "dc2", > + [CXL_DECODER_DC3] = "dc3", > + [CXL_DECODER_DC4] = "dc4", > + [CXL_DECODER_DC5] = "dc5", > + [CXL_DECODER_DC6] = "dc6", > + [CXL_DECODER_DC7] = "dc7", > [CXL_DECODER_MIXED] = "mixed", > }; > > @@ -383,10 +399,16 @@ static inline const char *cxl_decoder_mode_name(enum cxl_decoder_mode mode) > return "mixed"; > } > > +static inline bool cxl_decoder_mode_is_dc(enum cxl_decoder_mode mode) > +{ > + return (mode >= CXL_DECODER_DC0 && mode <= CXL_DECODER_DC7); > +} > + > enum cxl_region_mode { > CXL_REGION_NONE, > CXL_REGION_RAM, > CXL_REGION_PMEM, > + CXL_REGION_DC, > CXL_REGION_MIXED, > }; > > @@ -396,6 +418,7 @@ static inline const char *cxl_region_mode_name(enum cxl_region_mode mode) > [CXL_REGION_NONE] = "none", > [CXL_REGION_RAM] = "ram", > [CXL_REGION_PMEM] = "pmem", > + [CXL_REGION_DC] = "dc", > [CXL_REGION_MIXED] = "mixed", > }; > > > -- > 2.44.0 >
On 3/24/24 4:18 PM, ira.weiny@intel.com wrote: > From: Navneet Singh <navneet.singh@intel.com> > > Region mode must reflect a general dynamic capacity type which is > associated with a specific Dynamic Capacity (DC) partitions in each > device decoder within the region. DC partitions are also know as DC > regions per CXL 3.1. This section reads somewhat awkward to me. Does this read any better? One or more Dynamic Capacity (DC) partitions (and decoders) form a CXL software region. The region mode reflects composition of that entire software region. Decoder mode reflects a specific DC partition. DC partitions are also known as DC regions per CXL specification r3.1 but is not the same entity as CXL software regions. DJ > > Decoder mode reflects a specific DC partition. > > Define the new modes to use in subsequent patches and the helper > functions required to make the association between these new modes. > > Signed-off-by: Navneet Singh <navneet.singh@intel.com> > Co-developed-by: Ira Weiny <ira.weiny@intel.com> > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > --- > Changes for v1 > [iweiny: split out from: Add dynamic capacity cxl region support.] > --- > drivers/cxl/core/region.c | 4 ++++ > drivers/cxl/cxl.h | 23 +++++++++++++++++++++++ > 2 files changed, 27 insertions(+) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 1723d17f121e..ec3b8c6948e9 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -1690,6 +1690,8 @@ static bool cxl_modes_compatible(enum cxl_region_mode rmode, > return true; > if (rmode == CXL_REGION_PMEM && dmode == CXL_DECODER_PMEM) > return true; > + if (rmode == CXL_REGION_DC && cxl_decoder_mode_is_dc(dmode)) > + return true; > > return false; > } > @@ -2824,6 +2826,8 @@ cxl_decoder_to_region_mode(enum cxl_decoder_mode mode) > return CXL_REGION_RAM; > case CXL_DECODER_PMEM: > return CXL_REGION_PMEM; > + case CXL_DECODER_DC0 ... CXL_DECODER_DC7: > + return CXL_REGION_DC; > case CXL_DECODER_MIXED: > default: > return CXL_REGION_MIXED; > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h > index 9a0cce1e6fca..3b8935089c0c 100644 > --- a/drivers/cxl/cxl.h > +++ b/drivers/cxl/cxl.h > @@ -365,6 +365,14 @@ enum cxl_decoder_mode { > CXL_DECODER_NONE, > CXL_DECODER_RAM, > CXL_DECODER_PMEM, > + CXL_DECODER_DC0, > + CXL_DECODER_DC1, > + CXL_DECODER_DC2, > + CXL_DECODER_DC3, > + CXL_DECODER_DC4, > + CXL_DECODER_DC5, > + CXL_DECODER_DC6, > + CXL_DECODER_DC7, > CXL_DECODER_MIXED, > CXL_DECODER_DEAD, > }; > @@ -375,6 +383,14 @@ static inline const char *cxl_decoder_mode_name(enum cxl_decoder_mode mode) > [CXL_DECODER_NONE] = "none", > [CXL_DECODER_RAM] = "ram", > [CXL_DECODER_PMEM] = "pmem", > + [CXL_DECODER_DC0] = "dc0", > + [CXL_DECODER_DC1] = "dc1", > + [CXL_DECODER_DC2] = "dc2", > + [CXL_DECODER_DC3] = "dc3", > + [CXL_DECODER_DC4] = "dc4", > + [CXL_DECODER_DC5] = "dc5", > + [CXL_DECODER_DC6] = "dc6", > + [CXL_DECODER_DC7] = "dc7", > [CXL_DECODER_MIXED] = "mixed", > }; > > @@ -383,10 +399,16 @@ static inline const char *cxl_decoder_mode_name(enum cxl_decoder_mode mode) > return "mixed"; > } > > +static inline bool cxl_decoder_mode_is_dc(enum cxl_decoder_mode mode) > +{ > + return (mode >= CXL_DECODER_DC0 && mode <= CXL_DECODER_DC7); > +} > + > enum cxl_region_mode { > CXL_REGION_NONE, > CXL_REGION_RAM, > CXL_REGION_PMEM, > + CXL_REGION_DC, > CXL_REGION_MIXED, > }; > > @@ -396,6 +418,7 @@ static inline const char *cxl_region_mode_name(enum cxl_region_mode mode) > [CXL_REGION_NONE] = "none", > [CXL_REGION_RAM] = "ram", > [CXL_REGION_PMEM] = "pmem", > + [CXL_REGION_DC] = "dc", > [CXL_REGION_MIXED] = "mixed", > }; > >
Dave Jiang wrote: > > > On 3/24/24 4:18 PM, ira.weiny@intel.com wrote: > > From: Navneet Singh <navneet.singh@intel.com> > > > > Region mode must reflect a general dynamic capacity type which is > > associated with a specific Dynamic Capacity (DC) partitions in each > > device decoder within the region. DC partitions are also know as DC > > regions per CXL 3.1. > > This section reads somewhat awkward to me. Does this read any better? > > One or more Dynamic Capacity (DC) partitions (and decoders) form a CXL > software region. The region mode reflects composition of that entire software > region. Decoder mode reflects a specific DC partition. DC partitions are also > known as DC regions per CXL specification r3.1 but is not the same entity as > CXL software regions. Yea that does sound better but I think this builds on your text and is even more clear. <commit> cxl/region: Add dynamic capacity decoder and region modes One or more decoders each pointing to a Dynamic Capacity (DC) partition form a CXL software region. The region mode reflects composition of that entire software region. Decoder mode reflects a specific DC partition. DC partitions are also known as DC regions per CXL specification r3.1 but they are not the same entity as CXL software regions. Define the new modes and helper functions required to make the association between these new modes. </commit> Ira
On 4/5/24 11:19 AM, Ira Weiny wrote: > Dave Jiang wrote: >> >> >> On 3/24/24 4:18 PM, ira.weiny@intel.com wrote: >>> From: Navneet Singh <navneet.singh@intel.com> >>> >>> Region mode must reflect a general dynamic capacity type which is >>> associated with a specific Dynamic Capacity (DC) partitions in each >>> device decoder within the region. DC partitions are also know as DC >>> regions per CXL 3.1. >> >> This section reads somewhat awkward to me. Does this read any better? >> >> One or more Dynamic Capacity (DC) partitions (and decoders) form a CXL >> software region. The region mode reflects composition of that entire software >> region. Decoder mode reflects a specific DC partition. DC partitions are also >> known as DC regions per CXL specification r3.1 but is not the same entity as >> CXL software regions. > > Yea that does sound better but I think this builds on your text and is even > more clear. > > <commit> > cxl/region: Add dynamic capacity decoder and region modes > > One or more decoders each pointing to a Dynamic Capacity (DC) partition form a > CXL software region. The region mode reflects composition of that entire > software region. Decoder mode reflects a specific DC partition. DC partitions > are also known as DC regions per CXL specification r3.1 but they are not the > same entity as CXL software regions. > > Define the new modes and helper functions required to make the association > between these new modes. > > </commit> > LGTM > > Ira
The following change is preferred to show the correct mode name.
diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index 35ee565e27c9..729006ca4997 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -684,7 +684,7 @@ int cxl_dpa_alloc(struct cxl_endpoint_decoder *cxled, unsigned long long size)
if (size > avail) {
dev_dbg(dev, "%pa exceeds available %s capacity: %pa\n", &size,
- cxled->mode == CXL_DECODER_RAM ? "ram" : "pmem",
+ cxl_decoder_mode_name(cxled->mode),
&avail);
rc = -ENOSPC;
goto out;
On 25/03/2024 07:18, ira.weiny@intel.com wrote:
> From: Navneet Singh <navneet.singh@intel.com>
>
> Region mode must reflect a general dynamic capacity type which is
> associated with a specific Dynamic Capacity (DC) partitions in each
> device decoder within the region. DC partitions are also know as DC
> regions per CXL 3.1.
>
> Decoder mode reflects a specific DC partition.
>
> Define the new modes to use in subsequent patches and the helper
> functions required to make the association between these new modes.
>
> Signed-off-by: Navneet Singh <navneet.singh@intel.com>
> Co-developed-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> ---
> Changes for v1
> [iweiny: split out from: Add dynamic capacity cxl region support.]
> ---
> drivers/cxl/core/region.c | 4 ++++
> drivers/cxl/cxl.h | 23 +++++++++++++++++++++++
> 2 files changed, 27 insertions(+)
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 1723d17f121e..ec3b8c6948e9 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1690,6 +1690,8 @@ static bool cxl_modes_compatible(enum cxl_region_mode rmode,
> return true;
> if (rmode == CXL_REGION_PMEM && dmode == CXL_DECODER_PMEM)
> return true;
> + if (rmode == CXL_REGION_DC && cxl_decoder_mode_is_dc(dmode))
> + return true;
>
> return false;
> }
> @@ -2824,6 +2826,8 @@ cxl_decoder_to_region_mode(enum cxl_decoder_mode mode)
> return CXL_REGION_RAM;
> case CXL_DECODER_PMEM:
> return CXL_REGION_PMEM;
> + case CXL_DECODER_DC0 ... CXL_DECODER_DC7:
> + return CXL_REGION_DC;
> case CXL_DECODER_MIXED:
> default:
> return CXL_REGION_MIXED;
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 9a0cce1e6fca..3b8935089c0c 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -365,6 +365,14 @@ enum cxl_decoder_mode {
> CXL_DECODER_NONE,
> CXL_DECODER_RAM,
> CXL_DECODER_PMEM,
> + CXL_DECODER_DC0,
> + CXL_DECODER_DC1,
> + CXL_DECODER_DC2,
> + CXL_DECODER_DC3,
> + CXL_DECODER_DC4,
> + CXL_DECODER_DC5,
> + CXL_DECODER_DC6,
> + CXL_DECODER_DC7,
> CXL_DECODER_MIXED,
> CXL_DECODER_DEAD,
> };
> @@ -375,6 +383,14 @@ static inline const char *cxl_decoder_mode_name(enum cxl_decoder_mode mode)
> [CXL_DECODER_NONE] = "none",
> [CXL_DECODER_RAM] = "ram",
> [CXL_DECODER_PMEM] = "pmem",
> + [CXL_DECODER_DC0] = "dc0",
> + [CXL_DECODER_DC1] = "dc1",
> + [CXL_DECODER_DC2] = "dc2",
> + [CXL_DECODER_DC3] = "dc3",
> + [CXL_DECODER_DC4] = "dc4",
> + [CXL_DECODER_DC5] = "dc5",
> + [CXL_DECODER_DC6] = "dc6",
> + [CXL_DECODER_DC7] = "dc7",
> [CXL_DECODER_MIXED] = "mixed",
> };
>
> @@ -383,10 +399,16 @@ static inline const char *cxl_decoder_mode_name(enum cxl_decoder_mode mode)
> return "mixed";
> }
>
> +static inline bool cxl_decoder_mode_is_dc(enum cxl_decoder_mode mode)
> +{
> + return (mode >= CXL_DECODER_DC0 && mode <= CXL_DECODER_DC7);
> +}
> +
> enum cxl_region_mode {
> CXL_REGION_NONE,
> CXL_REGION_RAM,
> CXL_REGION_PMEM,
> + CXL_REGION_DC,
> CXL_REGION_MIXED,
> };
>
> @@ -396,6 +418,7 @@ static inline const char *cxl_region_mode_name(enum cxl_region_mode mode)
> [CXL_REGION_NONE] = "none",
> [CXL_REGION_RAM] = "ram",
> [CXL_REGION_PMEM] = "pmem",
> + [CXL_REGION_DC] = "dc",
> [CXL_REGION_MIXED] = "mixed",
> };
>
>
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index 1723d17f121e..ec3b8c6948e9 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -1690,6 +1690,8 @@ static bool cxl_modes_compatible(enum cxl_region_mode rmode, return true; if (rmode == CXL_REGION_PMEM && dmode == CXL_DECODER_PMEM) return true; + if (rmode == CXL_REGION_DC && cxl_decoder_mode_is_dc(dmode)) + return true; return false; } @@ -2824,6 +2826,8 @@ cxl_decoder_to_region_mode(enum cxl_decoder_mode mode) return CXL_REGION_RAM; case CXL_DECODER_PMEM: return CXL_REGION_PMEM; + case CXL_DECODER_DC0 ... CXL_DECODER_DC7: + return CXL_REGION_DC; case CXL_DECODER_MIXED: default: return CXL_REGION_MIXED; diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index 9a0cce1e6fca..3b8935089c0c 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -365,6 +365,14 @@ enum cxl_decoder_mode { CXL_DECODER_NONE, CXL_DECODER_RAM, CXL_DECODER_PMEM, + CXL_DECODER_DC0, + CXL_DECODER_DC1, + CXL_DECODER_DC2, + CXL_DECODER_DC3, + CXL_DECODER_DC4, + CXL_DECODER_DC5, + CXL_DECODER_DC6, + CXL_DECODER_DC7, CXL_DECODER_MIXED, CXL_DECODER_DEAD, }; @@ -375,6 +383,14 @@ static inline const char *cxl_decoder_mode_name(enum cxl_decoder_mode mode) [CXL_DECODER_NONE] = "none", [CXL_DECODER_RAM] = "ram", [CXL_DECODER_PMEM] = "pmem", + [CXL_DECODER_DC0] = "dc0", + [CXL_DECODER_DC1] = "dc1", + [CXL_DECODER_DC2] = "dc2", + [CXL_DECODER_DC3] = "dc3", + [CXL_DECODER_DC4] = "dc4", + [CXL_DECODER_DC5] = "dc5", + [CXL_DECODER_DC6] = "dc6", + [CXL_DECODER_DC7] = "dc7", [CXL_DECODER_MIXED] = "mixed", }; @@ -383,10 +399,16 @@ static inline const char *cxl_decoder_mode_name(enum cxl_decoder_mode mode) return "mixed"; } +static inline bool cxl_decoder_mode_is_dc(enum cxl_decoder_mode mode) +{ + return (mode >= CXL_DECODER_DC0 && mode <= CXL_DECODER_DC7); +} + enum cxl_region_mode { CXL_REGION_NONE, CXL_REGION_RAM, CXL_REGION_PMEM, + CXL_REGION_DC, CXL_REGION_MIXED, }; @@ -396,6 +418,7 @@ static inline const char *cxl_region_mode_name(enum cxl_region_mode mode) [CXL_REGION_NONE] = "none", [CXL_REGION_RAM] = "ram", [CXL_REGION_PMEM] = "pmem", + [CXL_REGION_DC] = "dc", [CXL_REGION_MIXED] = "mixed", };