| Message ID | 20240907081836.5801-19-alejandro.lucero-palau@amd.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl: add Type2 device support | expand |
On 9/7/24 1:18 AM, alejandro.lucero-palau@amd.com wrote: > From: Alejandro Lucero <alucerop@amd.com> > > By definition a type2 cxl device will use the host managed memory for > specific functionality, therefore it should not be available to other > uses. > > Signed-off-by: Alejandro Lucero <alucerop@amd.com> > --- > drivers/cxl/core/region.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index d8c29e28e60c..45b4891035a6 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -3699,6 +3699,9 @@ static int cxl_region_probe(struct device *dev) > case CXL_DECODER_PMEM: > return devm_cxl_add_pmem_region(cxlr); > case CXL_DECODER_RAM: > + if (cxlr->type != CXL_DECODER_HOSTONLYMEM) > + return 0; > + While it's highly unlikely that someone puts pmem on a type2 device, the spec does not forbid it. Maybe it makes sense to just return 0 above this switch block entirely and skip this code block? > /* > * The region can not be manged by CXL if any portion of > * it is already online as 'System RAM'
On 9/13/24 18:26, Dave Jiang wrote: > > On 9/7/24 1:18 AM, alejandro.lucero-palau@amd.com wrote: >> From: Alejandro Lucero <alucerop@amd.com> >> >> By definition a type2 cxl device will use the host managed memory for >> specific functionality, therefore it should not be available to other >> uses. >> >> Signed-off-by: Alejandro Lucero <alucerop@amd.com> >> --- >> drivers/cxl/core/region.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c >> index d8c29e28e60c..45b4891035a6 100644 >> --- a/drivers/cxl/core/region.c >> +++ b/drivers/cxl/core/region.c >> @@ -3699,6 +3699,9 @@ static int cxl_region_probe(struct device *dev) >> case CXL_DECODER_PMEM: >> return devm_cxl_add_pmem_region(cxlr); >> case CXL_DECODER_RAM: >> + if (cxlr->type != CXL_DECODER_HOSTONLYMEM) >> + return 0; >> + > While it's highly unlikely that someone puts pmem on a type2 device, the spec does not forbid it. Maybe it makes sense to just return 0 above this switch block entirely and skip this code block? Yes, it makes sense. I'll do it. Thanks > >> /* >> * The region can not be manged by CXL if any portion of >> * it is already online as 'System RAM'
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index d8c29e28e60c..45b4891035a6 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -3699,6 +3699,9 @@ static int cxl_region_probe(struct device *dev) case CXL_DECODER_PMEM: return devm_cxl_add_pmem_region(cxlr); case CXL_DECODER_RAM: + if (cxlr->type != CXL_DECODER_HOSTONLYMEM) + return 0; + /* * The region can not be manged by CXL if any portion of * it is already online as 'System RAM'