[PATCH v2 0/3] cxl/pci: Fix the GPF DVSEC setup path

[PATCH v2 0/3] cxl/pci: Fix the GPF DVSEC setup path

[Guixin Liu]

Three small fixes for the GPF setup path in cxl/core/pci.c, found
while reworking the same area for a downstream backport.

Patch 1 is the one with user-visible impact: on an RCD the GPF DVSEC
lookup takes the port path, so dirty shutdown tracking never arms and
the count stays invalid. The other two deal with error handling and
a full-register rewrite of the phase control register.

Testing on a QEMU CXL topology, with the patched cxl_core hot-swapped
in and the topology re-enumerated. With the phase 1/2 timeout
registers of all four dports (two root ports, two switch downstream
ports) zeroed beforehand, every dport came back at 0x0702 — base 2,
scale 7, the same value the unpatched code writes. So the timeout
write ran on each of them: the read-modify-write of patch 3, through
the error-checked setup of patch 2. With the registers left at their
programmed value instead, the setup takes the "already at max" early
return and writes nothing. Re-enumeration comes up clean, all three
memdevs back.

What QEMU cannot exercise: the topology has no RCD, so the RC_END
branch of patch 1 never ran — though the emulated type-3 devices do
expose the Device GPF DVSEC that the fix selects for a restricted
endpoint. A config write to a live DVSEC does not fail, so the error
paths of patch 2 stayed cold. And the phase control register defines
no non-timeout bits today, so patch 3 lands on the same value the old
full rewrite did.

Changes since v1 [1]:

- Collect the Reviewed-by tags from Jonathan Cameron and Dave Jiang
  for patches 1 and 2 (code unchanged).

- Rework patch 3 to use a pair of FIELD_MODIFY() calls instead of
  clearing and setting the timeout masks by hand, as suggested by
  Jonathan Cameron. The generated code for update_gpf_port_dvsec()
  is identical to the manual version.

[1] https://lore.kernel.org/all/20260922100026.3742401-1-kanie@linux.alibaba.com/

Guixin Liu (3):
  cxl/pci: Use the Device GPF DVSEC for restricted endpoints
  cxl/pci: Program the Port GPF timeouts before caching the DVSEC
  cxl/pci: Update only the Port GPF timeout fields

 drivers/cxl/core/pci.c | 37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

-- 
2.43.7

[PATCH v2 1/3] cxl/pci: Use the Device GPF DVSEC for restricted endpoints

[Guixin Liu]

cxl_gpf_get_dvsec() picks between the Port and the Device GPF DVSEC
from the PCIe type of the given device, and only a plain endpoint
counts as a device. A restricted CXL device is an endpoint too, but
it falls into the port case and is probed for the Port GPF DVSEC
instead of the Device GPF DVSEC.

On an RCD, dirty shutdown tracking therefore never arms: the DVSEC
lookup in cxl_nvdimm_arm_dirty_shutdown_tracking() fails, the dirty
shutdown count stays invalid, and the warning complains about a
missing Port GPF DVSEC on a device.

Treat restricted endpoints as devices.

Fixes: 36aace15d9bd ("cxl/pci: Drop the parameter is_port of cxl_gpf_get_dvsec()")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/core/pci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 9d807c1a002c..e31aad7a525a 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -804,7 +804,8 @@ u16 cxl_gpf_get_dvsec(struct device *dev)
 		return 0;
 
 	pdev = to_pci_dev(dev);
-	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ENDPOINT)
+	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ENDPOINT ||
+	    is_cxl_restricted(pdev))
 		is_port = false;
 
 	dvsec = pci_find_dvsec_capability(pdev, PCI_VENDOR_ID_CXL,
-- 
2.43.7

[PATCH v2 2/3] cxl/pci: Program the Port GPF timeouts before caching the DVSEC

[Guixin Liu]

cxl_gpf_port_setup() caches the Port GPF DVSEC offset in the dport
before programming the phase timeouts, and ignores the return value
of both update_gpf_port_dvsec() calls: the function always reports
success.

A failing config write is therefore not only silent, it is also
permanent: the timeouts stay at the hardware defaults instead of the
maximum flush window the kernel is asking for, and the cached offset
keeps any later endpoint attach from retrying the update.

Propagate the errors and cache the offset only after both phases are
programmed, so the next endpoint attach retries the setup.

Fixes: 6af941db6a60 ("cxl/pci: Update Port GPF timeout only when the first EP attaching")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/core/pci.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index e31aad7a525a..1f470ab0df3b 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -840,7 +840,7 @@ static int update_gpf_port_dvsec(struct pci_dev *pdev, int dvsec, int phase)
 
 	rc = pci_read_config_word(pdev, dvsec + offset, &ctrl);
 	if (rc)
-		return rc;
+		return pcibios_err_to_errno(rc);
 
 	if (FIELD_GET(base, ctrl) == GPF_TIMEOUT_BASE_MAX &&
 	    FIELD_GET(scale, ctrl) == GPF_TIMEOUT_SCALE_MAX)
@@ -850,11 +850,17 @@ static int update_gpf_port_dvsec(struct pci_dev *pdev, int dvsec, int phase)
 	ctrl |= FIELD_PREP(scale, GPF_TIMEOUT_SCALE_MAX);
 
 	rc = pci_write_config_word(pdev, dvsec + offset, ctrl);
-	if (!rc)
-		pci_dbg(pdev, "Port GPF phase %d timeout: %d0 secs\n",
-			phase, GPF_TIMEOUT_BASE_MAX);
+	if (rc) {
+		rc = pcibios_err_to_errno(rc);
+		pci_warn(pdev, "Port GPF phase %d timeout write failed: %d\n",
+			 phase, rc);
+		return rc;
+	}
 
-	return rc;
+	pci_dbg(pdev, "Port GPF phase %d timeout: %d0 secs\n",
+		phase, GPF_TIMEOUT_BASE_MAX);
+
+	return 0;
 }
 
 int cxl_gpf_port_setup(struct cxl_dport *dport)
@@ -864,16 +870,22 @@ int cxl_gpf_port_setup(struct cxl_dport *dport)
 
 	if (!dport->gpf_dvsec) {
 		struct pci_dev *pdev;
-		int dvsec;
+		int dvsec, rc;
 
 		dvsec = cxl_gpf_get_dvsec(dport->dport_dev);
 		if (!dvsec)
 			return -EINVAL;
 
-		dport->gpf_dvsec = dvsec;
 		pdev = to_pci_dev(dport->dport_dev);
-		update_gpf_port_dvsec(pdev, dport->gpf_dvsec, 1);
-		update_gpf_port_dvsec(pdev, dport->gpf_dvsec, 2);
+		rc = update_gpf_port_dvsec(pdev, dvsec, 1);
+		if (rc)
+			return rc;
+
+		rc = update_gpf_port_dvsec(pdev, dvsec, 2);
+		if (rc)
+			return rc;
+
+		dport->gpf_dvsec = dvsec;
 	}
 
 	return 0;
-- 
2.43.7

[PATCH v2 3/3] cxl/pci: Update only the Port GPF timeout fields

[Guixin Liu]

update_gpf_port_dvsec() rebuilds the phase control register from just
the timeout base and scale, so the rest of the register is cleared by
the write. The other bits are reserved today and nothing is lost, but
the full-register rewrite would clobber any field the spec defines
there later.

Read-modify-write the register and change only the two timeout
fields.

Fixes: a52b6a2c1c99 ("cxl/pci: Support Global Persistent Flush (GPF)")
Assisted-by: LLM
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/cxl/core/pci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 1f470ab0df3b..edf0b47f39db 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -846,8 +846,8 @@ static int update_gpf_port_dvsec(struct pci_dev *pdev, int dvsec, int phase)
 	    FIELD_GET(scale, ctrl) == GPF_TIMEOUT_SCALE_MAX)
 		return 0;
 
-	ctrl = FIELD_PREP(base, GPF_TIMEOUT_BASE_MAX);
-	ctrl |= FIELD_PREP(scale, GPF_TIMEOUT_SCALE_MAX);
+	FIELD_MODIFY(base, &ctrl, GPF_TIMEOUT_BASE_MAX);
+	FIELD_MODIFY(scale, &ctrl, GPF_TIMEOUT_SCALE_MAX);
 
 	rc = pci_write_config_word(pdev, dvsec + offset, ctrl);
 	if (rc) {
-- 
2.43.7

Re: [PATCH v2 2/3] cxl/pci: Program the Port GPF timeouts before caching the DVSEC

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] cxl: build fragility due to FIELD_PREP/FIELD_GET non-constant mask usage
--

commit 502048ecbaa85e66fc08a6f9dd92c92082d18d7f
Author: Guixin Liu <kanie@linux.alibaba.com>

cxl/pci: Program the Port GPF timeouts before caching the DVSEC

This commit delays caching the Port GPF DVSEC offset until both configuration
phases are successfully programmed. This ensures that any endpoint attach
attempts will retry the setup if a configuration write fails.

> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -849,7 +849,7 @@ static int update_gpf_port_dvsec(struct pci_dev *pdev, int dvsec, int phase)
>  
>  	rc = pci_read_config_word(pdev, dvsec + offset, &ctrl);
>  	if (rc)
> -		return rc;
> +		return pcibios_err_to_errno(rc);
>  
>  	if (FIELD_GET(base, ctrl) == GPF_TIMEOUT_BASE_MAX &&
>  	    FIELD_GET(scale, ctrl) == GPF_TIMEOUT_SCALE_MAX)
> @@ -859,11 +859,17 @@ static int update_gpf_port_dvsec(struct pci_dev *pdev, int dvsec, int phase)
>  	ctrl |= FIELD_PREP(scale, GPF_TIMEOUT_SCALE_MAX);
>  
>  	rc = pci_write_config_word(pdev, dvsec + offset, ctrl);
> -	if (!rc)
> -		pci_dbg(pdev, "Port GPF phase %d timeout: %d0 secs\n",
> -			phase, GPF_TIMEOUT_BASE_MAX);
> +	if (rc) {
> +		rc = pcibios_err_to_errno(rc);
> +		pci_warn(pdev, "Port GPF phase %d timeout write failed: %d\n",
> +			 phase, rc);
> +		return rc;
> +	}

[Severity: Low]
Does adding this error handling and logging run the risk of breaking the
build on some compiler configurations?

Both FIELD_GET() and FIELD_PREP() internally use a BUILD_BUG_ON_MSG() macro
that strictly requires their mask arguments to be compile-time constants.
In update_gpf_port_dvsec(), base and scale are dynamically assigned
variables rather than constants.

This code currently builds only because the compiler heuristically inlines
the function or performs interprocedural constant propagation.

Because this patch increases the size of update_gpf_port_dvsec(), is there a
risk that compiler heuristics might decide against inlining it (such as when
compiling with -Os), which would expose the BUILD_BUG_ON_MSG() failure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008034619.28059-1-kanie@linux.alibaba.com?part=2

Re: [PATCH v2 3/3] cxl/pci: Update only the Port GPF timeout fields

[Dave Jiang]

On 10/7/26 8:46 PM, Guixin Liu wrote:
> update_gpf_port_dvsec() rebuilds the phase control register from just
> the timeout base and scale, so the rest of the register is cleared by
> the write. The other bits are reserved today and nothing is lost, but
> the full-register rewrite would clobber any field the spec defines
> there later.
> 
> Read-modify-write the register and change only the two timeout
> fields.
> 
> Fixes: a52b6a2c1c99 ("cxl/pci: Support Global Persistent Flush (GPF)")
> Assisted-by: LLM
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/cxl/core/pci.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 1f470ab0df3b..edf0b47f39db 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -846,8 +846,8 @@ static int update_gpf_port_dvsec(struct pci_dev *pdev, int dvsec, int phase)
>  	    FIELD_GET(scale, ctrl) == GPF_TIMEOUT_SCALE_MAX)
>  		return 0;
>  
> -	ctrl = FIELD_PREP(base, GPF_TIMEOUT_BASE_MAX);
> -	ctrl |= FIELD_PREP(scale, GPF_TIMEOUT_SCALE_MAX);
> +	FIELD_MODIFY(base, &ctrl, GPF_TIMEOUT_BASE_MAX);
> +	FIELD_MODIFY(scale, &ctrl, GPF_TIMEOUT_SCALE_MAX);
>  
>  	rc = pci_write_config_word(pdev, dvsec + offset, ctrl);
>  	if (rc) {

Re: [PATCH v2 1/3] cxl/pci: Use the Device GPF DVSEC for restricted endpoints

[Alison Schofield]

On Thu, Oct 08, 2026 at 11:46:12AM +0800, Guixin Liu wrote:
> cxl_gpf_get_dvsec() picks between the Port and the Device GPF DVSEC
> from the PCIe type of the given device, and only a plain endpoint
> counts as a device. A restricted CXL device is an endpoint too, but
> it falls into the port case and is probed for the Port GPF DVSEC
> instead of the Device GPF DVSEC.
> 
> On an RCD, dirty shutdown tracking therefore never arms: the DVSEC
> lookup in cxl_nvdimm_arm_dirty_shutdown_tracking() fails, the dirty
> shutdown count stays invalid, and the warning complains about a
> missing Port GPF DVSEC on a device.
> 
> Treat restricted endpoints as devices.

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

Re: [PATCH v2 2/3] cxl/pci: Program the Port GPF timeouts before caching the DVSEC

[Alison Schofield]

On Thu, Oct 08, 2026 at 11:46:13AM +0800, Guixin Liu wrote:
> cxl_gpf_port_setup() caches the Port GPF DVSEC offset in the dport
> before programming the phase timeouts, and ignores the return value
> of both update_gpf_port_dvsec() calls: the function always reports
> success.
> 
> A failing config write is therefore not only silent, it is also
> permanent: the timeouts stay at the hardware defaults instead of the
> maximum flush window the kernel is asking for, and the cached offset
> keeps any later endpoint attach from retrying the update.
> 
> Propagate the errors and cache the offset only after both phases are
> programmed, so the next endpoint attach retries the setup.

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

Re: [PATCH v2 3/3] cxl/pci: Update only the Port GPF timeout fields

[Alison Schofield]

On Thu, Oct 08, 2026 at 11:46:14AM +0800, Guixin Liu wrote:
> update_gpf_port_dvsec() rebuilds the phase control register from just
> the timeout base and scale, so the rest of the register is cleared by
> the write. The other bits are reserved today and nothing is lost, but
> the full-register rewrite would clobber any field the spec defines
> there later.
> 
> Read-modify-write the register and change only the two timeout
> fields.

Reviewed-by: Alison Schofield <alison.schofield@intel.com>