diff mbox series

[2/5] cxl/region: Fix missing probe failure

Message ID 166993220462.1995348.1698008475198427361.stgit@dwillia2-xfh.jf.intel.com (mailing list archive)
State Accepted
Commit bf3e5da8cb43a671b32fc125fa81b8f6a3677192
Headers show
Series cxl, nvdimm: Move CPU cache management to region drivers | expand

Commit Message

Dan Williams Dec. 1, 2022, 10:03 p.m. UTC
cxl_region_probe() allows for regions not in the 'commit' state to be
enabled. Fail probe when the region is not committed otherwise the
kernel may indicate that an address range is active when none of the
decoders are active.

Fixes: 8d48817df6ac ("cxl/region: Add region driver boiler plate")
Cc: <stable@vger.kernel.org>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/cxl/core/region.c |    3 +++
 1 file changed, 3 insertions(+)

Comments

Dave Jiang Dec. 1, 2022, 10:30 p.m. UTC | #1
On 12/1/2022 3:03 PM, Dan Williams wrote:
> cxl_region_probe() allows for regions not in the 'commit' state to be
> enabled. Fail probe when the region is not committed otherwise the
> kernel may indicate that an address range is active when none of the
> decoders are active.
> 
> Fixes: 8d48817df6ac ("cxl/region: Add region driver boiler plate")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.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 f9ae5ad284ff..1bc2ebefa2a5 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1923,6 +1923,9 @@ static int cxl_region_probe(struct device *dev)
>   	 */
>   	up_read(&cxl_region_rwsem);
>   
> +	if (rc)
> +		return rc;
> +
>   	switch (cxlr->mode) {
>   	case CXL_DECODER_PMEM:
>   		return devm_cxl_add_pmem_region(cxlr);
>
Davidlohr Bueso Dec. 2, 2022, 1:45 a.m. UTC | #2
On Thu, 01 Dec 2022, Dan Williams wrote:

>cxl_region_probe() allows for regions not in the 'commit' state to be
>enabled. Fail probe when the region is not committed otherwise the
>kernel may indicate that an address range is active when none of the
>decoders are active.
>
>Fixes: 8d48817df6ac ("cxl/region: Add region driver boiler plate")
>Cc: <stable@vger.kernel.org>
>Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>

>---
> 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 f9ae5ad284ff..1bc2ebefa2a5 100644
>--- a/drivers/cxl/core/region.c
>+++ b/drivers/cxl/core/region.c
>@@ -1923,6 +1923,9 @@ static int cxl_region_probe(struct device *dev)
>	 */
>	up_read(&cxl_region_rwsem);
>
>+	if (rc)
>+		return rc;
>+
>	switch (cxlr->mode) {
>	case CXL_DECODER_PMEM:
>		return devm_cxl_add_pmem_region(cxlr);
>
Jonathan Cameron Dec. 2, 2022, 2:23 p.m. UTC | #3
On Thu, 01 Dec 2022 14:03:24 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> cxl_region_probe() allows for regions not in the 'commit' state to be
> enabled. Fail probe when the region is not committed otherwise the
> kernel may indicate that an address range is active when none of the
> decoders are active.
> 
> Fixes: 8d48817df6ac ("cxl/region: Add region driver boiler plate")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Huh. I wonder why this wasn't triggering a build warning given
rc is assigned but unused.

Ah well, this is clearly the original intent and makes sense.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.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 f9ae5ad284ff..1bc2ebefa2a5 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1923,6 +1923,9 @@ static int cxl_region_probe(struct device *dev)
>  	 */
>  	up_read(&cxl_region_rwsem);
>  
> +	if (rc)
> +		return rc;
> +
>  	switch (cxlr->mode) {
>  	case CXL_DECODER_PMEM:
>  		return devm_cxl_add_pmem_region(cxlr);
>
Dan Williams Dec. 3, 2022, 8:03 a.m. UTC | #4
Jonathan Cameron wrote:
> On Thu, 01 Dec 2022 14:03:24 -0800
> Dan Williams <dan.j.williams@intel.com> wrote:
> 
> > cxl_region_probe() allows for regions not in the 'commit' state to be
> > enabled. Fail probe when the region is not committed otherwise the
> > kernel may indicate that an address range is active when none of the
> > decoders are active.
> > 
> > Fixes: 8d48817df6ac ("cxl/region: Add region driver boiler plate")
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> Huh. I wonder why this wasn't triggering a build warning given
> rc is assigned but unused.

Yes, I thought that was curious too.

> 
> Ah well, this is clearly the original intent and makes sense.
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.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 f9ae5ad284ff..1bc2ebefa2a5 100644
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> > @@ -1923,6 +1923,9 @@ static int cxl_region_probe(struct device *dev)
> >  	 */
> >  	up_read(&cxl_region_rwsem);
> >  
> > +	if (rc)
> > +		return rc;
> > +
> >  	switch (cxlr->mode) {
> >  	case CXL_DECODER_PMEM:
> >  		return devm_cxl_add_pmem_region(cxlr);
> > 
>
diff mbox series

Patch

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index f9ae5ad284ff..1bc2ebefa2a5 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -1923,6 +1923,9 @@  static int cxl_region_probe(struct device *dev)
 	 */
 	up_read(&cxl_region_rwsem);
 
+	if (rc)
+		return rc;
+
 	switch (cxlr->mode) {
 	case CXL_DECODER_PMEM:
 		return devm_cxl_add_pmem_region(cxlr);