Re: [RFC PATCH 05/13] cxl: fix check about pmem resource

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Thu, 16 May 2024 09:11:54 +0100
<alucerop@amd.com> wrote:

> From: Alejandro Lucero <alucerop@amd.com>
> 
> Current check is using resource_size which counts on a range bigger than
> 0. For a resource with start and end being 0, resource_size returns 1
> and implying a false positive. Use the end not being zero as the new check.
> 
> Note: If I´m not missing anything here, this should be extended to the
> whole linux kernel where resource_size is being used in conditionals,
> and where the likely right fix is to modify resource_size itself
> checking for the range not being 0.

The start and end being 0 is a valid resource of length 1 so that
should not need fixing.

These should have been initialized with DEFINE_RES_MEM()
/ DEFINE_RES_NAMED()
That will happily set the .end to -1 resulting in a wrap around
so that you get all bits set.

Sometimes this gives slightly confusing range prints but otherwise
I think it is fine.

> 
> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> ---
>  drivers/cxl/core/hdm.c    | 4 ++--
>  drivers/cxl/core/memdev.c | 8 ++++----
>  drivers/cxl/mem.c         | 2 +-
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 47d9faf5897f..c5f70741d70a 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -432,12 +432,12 @@ int cxl_dpa_set_mode(struct cxl_endpoint_decoder *cxled,
>  	 * Only allow modes that are supported by the current partition
>  	 * configuration
>  	 */
> -	if (mode == CXL_DECODER_PMEM && !resource_size(&cxlds->pmem_res)) {
> +	if (mode == CXL_DECODER_PMEM && !cxlds->pmem_res.end) {
>  		dev_dbg(dev, "no available pmem capacity\n");
>  		rc = -ENXIO;
>  		goto out;
>  	}
> -	if (mode == CXL_DECODER_RAM && !resource_size(&cxlds->ram_res)) {
> +	if (mode == CXL_DECODER_RAM && !cxlds->ram_res.end) {
>  		dev_dbg(dev, "no available ram capacity\n");
>  		rc = -ENXIO;
>  		goto out;
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 0336b3f14f4a..b61d57d0d4f4 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -197,14 +197,14 @@ static int cxl_get_poison_by_memdev(struct cxl_memdev *cxlmd)
>  	int rc = 0;
>  
>  	/* CXL 3.0 Spec 8.2.9.8.4.1 Separate pmem and ram poison requests */
> -	if (resource_size(&cxlds->pmem_res)) {
> +	if (cxlds->pmem_res.end) {
>  		offset = cxlds->pmem_res.start;
>  		length = resource_size(&cxlds->pmem_res);
>  		rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
>  		if (rc)
>  			return rc;
>  	}
> -	if (resource_size(&cxlds->ram_res)) {
> +	if (cxlds->ram_res.end) {
>  		offset = cxlds->ram_res.start;
>  		length = resource_size(&cxlds->ram_res);
>  		rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
> @@ -266,7 +266,7 @@ static int __cxl_dpa_to_region(struct device *dev, void *arg)
>  		return 0;
>  
>  	cxled = to_cxl_endpoint_decoder(dev);
> -	if (!cxled->dpa_res || !resource_size(cxled->dpa_res))
> +	if (!cxled->dpa_res || !cxled->dpa_res->end)
>  		return 0;
>  
>  	if (dpa > cxled->dpa_res->end || dpa < cxled->dpa_res->start)
> @@ -302,7 +302,7 @@ static int cxl_validate_poison_dpa(struct cxl_memdev *cxlmd, u64 dpa)
>  	if (!IS_ENABLED(CONFIG_DEBUG_FS))
>  		return 0;
>  
> -	if (!resource_size(&cxlds->dpa_res)) {
> +	if (!cxlds->dpa_res.end) {
>  		dev_dbg(cxlds->dev, "device has no dpa resource\n");
>  		return -EINVAL;
>  	}
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index 6dc2bf1e2b1a..a168343d2d4d 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -174,7 +174,7 @@ static int cxl_mem_probe(struct device *dev)
>  	if (rc)
>  		return rc;
>  
> -	if (resource_size(&cxlds->pmem_res) && IS_ENABLED(CONFIG_CXL_PMEM)) {
> +	if (cxlds->pmem_res.end && IS_ENABLED(CONFIG_CXL_PMEM)) {
>  		rc = devm_cxl_add_nvdimm(cxlmd);
>  		if (rc == -ENODEV)
>  			dev_info(dev, "PMEM disabled by platform\n");