* [PATCH 1/1] cxl/pci: Convert PCIBIOS_* return codes to errnos
@ 2024-05-27 12:34 Ilpo Järvinen
2024-05-28 22:20 ` Alison Schofield
2024-05-29 13:30 ` Ira Weiny
0 siblings, 2 replies; 5+ messages in thread
From: Ilpo Järvinen @ 2024-05-27 12:34 UTC (permalink / raw)
To: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield,
Vishal Verma, Ira Weiny, Dan Williams, Ben Widawsky, linux-cxl,
linux-kernel
Cc: Ilpo Järvinen, stable
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.
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] cxl/pci: Convert PCIBIOS_* return codes to errnos 2024-05-27 12:34 [PATCH 1/1] cxl/pci: Convert PCIBIOS_* return codes to errnos Ilpo Järvinen @ 2024-05-28 22:20 ` Alison Schofield 2024-05-29 12:19 ` Ilpo Järvinen 2024-05-29 13:30 ` Ira Weiny 1 sibling, 1 reply; 5+ messages in thread From: Alison Schofield @ 2024-05-28 22:20 UTC (permalink / raw) To: Ilpo Järvinen Cc: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Vishal Verma, Ira Weiny, Dan Williams, Ben Widawsky, linux-cxl, linux-kernel, stable 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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] cxl/pci: Convert PCIBIOS_* return codes to errnos 2024-05-28 22:20 ` Alison Schofield @ 2024-05-29 12:19 ` Ilpo Järvinen 2024-05-29 16:43 ` Alison Schofield 0 siblings, 1 reply; 5+ messages in thread From: Ilpo Järvinen @ 2024-05-29 12:19 UTC (permalink / raw) To: Alison Schofield Cc: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Vishal Verma, Ira Weiny, Dan Williams, Ben Widawsky, linux-cxl, LKML, stable [-- Attachment #1: Type: text/plain, Size: 6324 bytes --] On Tue, 28 May 2024, Alison Schofield wrote: > 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? There is potential for bad decision. Eg. cxl_set_mem_enable() it can return 0, 1 and rc that is currently returning PCIBIOS_* return codes that are > 0). devm_cxl_enable_mem() then tries to check for >0 and <0 but the <0 condition won't match correctly because PCIBIOS_* is not <0 but >0, devm_cxl_enable_mem() then return 0 where it should have returned an error. The positive "error code" from wait_for_valid(), cxl_dvsec_rr_decode(), and cxl_pci_ras_unmask leaks out of .probe(). -- i. > > 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 > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] cxl/pci: Convert PCIBIOS_* return codes to errnos 2024-05-29 12:19 ` Ilpo Järvinen @ 2024-05-29 16:43 ` Alison Schofield 0 siblings, 0 replies; 5+ messages in thread From: Alison Schofield @ 2024-05-29 16:43 UTC (permalink / raw) To: Ilpo Järvinen Cc: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Vishal Verma, Ira Weiny, Dan Williams, Ben Widawsky, linux-cxl, LKML, stable On Wed, May 29, 2024 at 03:19:19PM +0300, Ilpo Järvinen wrote: > On Tue, 28 May 2024, Alison Schofield wrote: > > > 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? > > There is potential for bad decision. > > Eg. cxl_set_mem_enable() it can return 0, 1 and rc that is currently > returning PCIBIOS_* return codes that are > 0). devm_cxl_enable_mem() > then tries to check for >0 and <0 but the <0 condition won't match > correctly because PCIBIOS_* is not <0 but >0, devm_cxl_enable_mem() then > return 0 where it should have returned an error. > > The positive "error code" from wait_for_valid(), cxl_dvsec_rr_decode(), > and cxl_pci_ras_unmask leaks out of .probe(). > Ah, I see the fix is quite tidy but the impact is more complex. Please update the commit log to explain user visible impacts of this fix. > -- > i. > > > > 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 > > > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] cxl/pci: Convert PCIBIOS_* return codes to errnos 2024-05-27 12:34 [PATCH 1/1] cxl/pci: Convert PCIBIOS_* return codes to errnos Ilpo Järvinen 2024-05-28 22:20 ` Alison Schofield @ 2024-05-29 13:30 ` Ira Weiny 1 sibling, 0 replies; 5+ messages in thread From: Ira Weiny @ 2024-05-29 13:30 UTC (permalink / raw) To: Ilpo Järvinen, Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield, Vishal Verma, Ira Weiny, Dan Williams, Ben Widawsky, linux-cxl, linux-kernel Cc: Ilpo Järvinen, stable 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. > > 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> Reviewed-by: Ira Weiny <ira.weiny@intel.com> [snip] ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-05-29 16:43 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-05-27 12:34 [PATCH 1/1] cxl/pci: Convert PCIBIOS_* return codes to errnos Ilpo Järvinen 2024-05-28 22:20 ` Alison Schofield 2024-05-29 12:19 ` Ilpo Järvinen 2024-05-29 16:43 ` Alison Schofield 2024-05-29 13:30 ` Ira Weiny
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox