Received-SPF: pass (google.com: domain of linux-kernel+bounces-193175-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Tue, 28 May 2024 15:20:30 -0700
From: Alison Schofield <alison.schofield@intel.com>
To: Ilpo =?iso-8859-1?Q?J=E4rvinen?= <ilpo.jarvinen@linux.intel.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>,
	Jonathan Cameron <jonathan.cameron@huawei.com>,
	Dave Jiang <dave.jiang@intel.com>,
	Vishal Verma <vishal.l.verma@intel.com>,
	Ira Weiny <ira.weiny@intel.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Ben Widawsky <bwidawsk@kernel.org>, linux-cxl@vger.kernel.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH 1/1] cxl/pci: Convert PCIBIOS_* return codes to errnos
Message-ID: <ZlZYrsPfzAiFzNLM@aschofie-mobl2>
References: <20240527123403.13098-1-ilpo.jarvinen@linux.intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20240527123403.13098-1-ilpo.jarvinen@linux.intel.com>

On Mon, May 27, 2024 at 03:34:02PM +0300, Ilpo J?rvinen wrote:
> pci_{read,write}_config_*word() and pcie_capability_read_word() return
> PCIBIOS_* codes, not usual errnos.
> 
> Fix return value checks to handle PCIBIOS_* return codes correctly by
> dropping < 0 from the check and convert the PCIBIOS_* return codes into
> errnos using pcibios_err_to_errno() before returning them.


Do we ever make a bad decision based on the wrong rc value or is this
a correction to the emitted dev_*() messaging, or both?

-- Alison

> 
> Fixes: ce17ad0d5498 ("cxl: Wait Memory_Info_Valid before access memory related info")
> Fixes: 34e37b4c432c ("cxl/port: Enable HDM Capability after validating DVSEC Ranges")
> Fixes: 14d788740774 ("cxl/mem: Consolidate CXL DVSEC Range enumeration in the core")
> Fixes: 560f78559006 ("cxl/pci: Retrieve CXL DVSEC memory info")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ilpo J?rvinen <ilpo.jarvinen@linux.intel.com>
> ---
>  drivers/cxl/core/pci.c | 30 +++++++++++++++---------------
>  drivers/cxl/pci.c      |  2 +-
>  2 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 8567dd11eaac..9ca67d4e0a89 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -121,7 +121,7 @@ static int cxl_dvsec_mem_range_valid(struct cxl_dev_state *cxlds, int id)
>  					   d + CXL_DVSEC_RANGE_SIZE_LOW(id),
>  					   &temp);
>  		if (rc)
> -			return rc;
> +			return pcibios_err_to_errno(rc);
>  
>  		valid = FIELD_GET(CXL_DVSEC_MEM_INFO_VALID, temp);
>  		if (valid)
> @@ -155,7 +155,7 @@ static int cxl_dvsec_mem_range_active(struct cxl_dev_state *cxlds, int id)
>  		rc = pci_read_config_dword(
>  			pdev, d + CXL_DVSEC_RANGE_SIZE_LOW(id), &temp);
>  		if (rc)
> -			return rc;
> +			return pcibios_err_to_errno(rc);
>  
>  		active = FIELD_GET(CXL_DVSEC_MEM_ACTIVE, temp);
>  		if (active)
> @@ -188,7 +188,7 @@ int cxl_await_media_ready(struct cxl_dev_state *cxlds)
>  	rc = pci_read_config_word(pdev,
>  				  d + CXL_DVSEC_CAP_OFFSET, &cap);
>  	if (rc)
> -		return rc;
> +		return pcibios_err_to_errno(rc);
>  
>  	hdm_count = FIELD_GET(CXL_DVSEC_HDM_COUNT_MASK, cap);
>  	for (i = 0; i < hdm_count; i++) {
> @@ -225,7 +225,7 @@ static int wait_for_valid(struct pci_dev *pdev, int d)
>  	 */
>  	rc = pci_read_config_dword(pdev, d + CXL_DVSEC_RANGE_SIZE_LOW(0), &val);
>  	if (rc)
> -		return rc;
> +		return pcibios_err_to_errno(rc);
>  
>  	if (val & CXL_DVSEC_MEM_INFO_VALID)
>  		return 0;
> @@ -234,7 +234,7 @@ static int wait_for_valid(struct pci_dev *pdev, int d)
>  
>  	rc = pci_read_config_dword(pdev, d + CXL_DVSEC_RANGE_SIZE_LOW(0), &val);
>  	if (rc)
> -		return rc;
> +		return pcibios_err_to_errno(rc);
>  
>  	if (val & CXL_DVSEC_MEM_INFO_VALID)
>  		return 0;
> @@ -250,8 +250,8 @@ static int cxl_set_mem_enable(struct cxl_dev_state *cxlds, u16 val)
>  	int rc;
>  
>  	rc = pci_read_config_word(pdev, d + CXL_DVSEC_CTRL_OFFSET, &ctrl);
> -	if (rc < 0)
> -		return rc;
> +	if (rc)
> +		return pcibios_err_to_errno(rc);
>  
>  	if ((ctrl & CXL_DVSEC_MEM_ENABLE) == val)
>  		return 1;
> @@ -259,8 +259,8 @@ static int cxl_set_mem_enable(struct cxl_dev_state *cxlds, u16 val)
>  	ctrl |= val;
>  
>  	rc = pci_write_config_word(pdev, d + CXL_DVSEC_CTRL_OFFSET, ctrl);
> -	if (rc < 0)
> -		return rc;
> +	if (rc)
> +		return pcibios_err_to_errno(rc);
>  
>  	return 0;
>  }
> @@ -336,11 +336,11 @@ int cxl_dvsec_rr_decode(struct device *dev, int d,
>  
>  	rc = pci_read_config_word(pdev, d + CXL_DVSEC_CAP_OFFSET, &cap);
>  	if (rc)
> -		return rc;
> +		return pcibios_err_to_errno(rc);
>  
>  	rc = pci_read_config_word(pdev, d + CXL_DVSEC_CTRL_OFFSET, &ctrl);
>  	if (rc)
> -		return rc;
> +		return pcibios_err_to_errno(rc);
>  
>  	if (!(cap & CXL_DVSEC_MEM_CAPABLE)) {
>  		dev_dbg(dev, "Not MEM Capable\n");
> @@ -379,14 +379,14 @@ int cxl_dvsec_rr_decode(struct device *dev, int d,
>  		rc = pci_read_config_dword(
>  			pdev, d + CXL_DVSEC_RANGE_SIZE_HIGH(i), &temp);
>  		if (rc)
> -			return rc;
> +			return pcibios_err_to_errno(rc);
>  
>  		size = (u64)temp << 32;
>  
>  		rc = pci_read_config_dword(
>  			pdev, d + CXL_DVSEC_RANGE_SIZE_LOW(i), &temp);
>  		if (rc)
> -			return rc;
> +			return pcibios_err_to_errno(rc);
>  
>  		size |= temp & CXL_DVSEC_MEM_SIZE_LOW_MASK;
>  		if (!size) {
> @@ -400,14 +400,14 @@ int cxl_dvsec_rr_decode(struct device *dev, int d,
>  		rc = pci_read_config_dword(
>  			pdev, d + CXL_DVSEC_RANGE_BASE_HIGH(i), &temp);
>  		if (rc)
> -			return rc;
> +			return pcibios_err_to_errno(rc);
>  
>  		base = (u64)temp << 32;
>  
>  		rc = pci_read_config_dword(
>  			pdev, d + CXL_DVSEC_RANGE_BASE_LOW(i), &temp);
>  		if (rc)
> -			return rc;
> +			return pcibios_err_to_errno(rc);
>  
>  		base |= temp & CXL_DVSEC_MEM_BASE_LOW_MASK;
>  
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index e53646e9f2fb..0ec9cbc64896 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -540,7 +540,7 @@ static int cxl_pci_ras_unmask(struct pci_dev *pdev)
>  
>  	rc = pcie_capability_read_word(pdev, PCI_EXP_DEVCTL, &cap);
>  	if (rc)
> -		return rc;
> +		return pcibios_err_to_errno(rc);
>  
>  	if (cap & PCI_EXP_DEVCTL_URRE) {
>  		addr = cxlds->regs.ras + CXL_RAS_UNCORRECTABLE_MASK_OFFSET;
> -- 
> 2.39.2
>