Re: [PATCH v14 02/34] PCI: Update CXL DVSEC definitions

[Jonathan Cameron <jonathan.cameron@huawei.com>]

On Wed, 14 Jan 2026 12:20:23 -0600
Terry Bowman <terry.bowman@amd.com> wrote:

> CXL DVSEC definitions were recently moved into uapi/pci_regs.h, but the
> newly added macros do not follow the file's existing naming conventions.
> The current format uses CXL_DVSEC_XYZ, while the new CXL entries must
> instead use the PCI_DVSEC_CXL_XYZ prefix to match the conventions already
> established in pci_regs.h.
> 
> The new CXL DVSEC macros also introduce _MASK and _OFFSET suffixes, which
> are not used anywhere else in the file. These suffixes lengthen the
> identifiers and reduce readability. Remove _MASK and _OFFSET from the
> recently added definitions.
> 
> Additionally, remove PCI_DVSEC_HEADER1_LENGTH, as it duplicates the existing
> PCI_DVSEC_HEADER1_LEN() macro.
> 
> Update all existing references to use the new macro names.
> 
> Finally, update the inline documentation to reference the latest revision
> of the CXL specification.
> 
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> 
> ---
> 
> Changes in v13->v14:
> - New patch. Split from previous patch such that there is now a separate
>   move patch and a format fix patch.
> - Formatting update requested (Bjorn)
> - Remove PCI_DVSEC_HEADER1_LENGTH_MASK because it duplicates
>   PCI_DVSEC_HEADER1_LEN() (Bjorn)
> - Add Dan's review-by

A couple of name choices don't quite work to my reading. 
I care more about the _DEVICE_ one than the other.

If consensus is that it is fine with out then I could probably be
persuaded.

> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index 6c4b6f19b18e..662582bdccf0 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h


> +/* CXL r4.0, 8.1.3: PCIe DVSEC for CXL Device */
> +#define PCI_DVSEC_CXL_DEVICE				0
> +#define  PCI_DVSEC_CXL_CAP				0xA

Why drop the _DEVICE_ bit of these I'd kind of expect
#define  PCI_DVSEC_CXL_DEVICE_CAP
to indicate which DVSEC it is in.



> +#define   PCI_DVSEC_CXL_MEM_CAPABLE			_BITUL(2)
> +#define   PCI_DVSEC_CXL_HDM_COUNT			__GENMASK(5, 4)
> +#define  PCI_DVSEC_CXL_CTRL				0xC
> +#define   PCI_DVSEC_CXL_MEM_ENABLE			_BITUL(2)
> +#define  PCI_DVSEC_CXL_RANGE_SIZE_HIGH(i)		(0x18 + (i * 0x10))
> +#define  PCI_DVSEC_CXL_RANGE_SIZE_LOW(i)		(0x1C + (i * 0x10))
> +#define   PCI_DVSEC_CXL_MEM_INFO_VALID			_BITUL(0)
> +#define   PCI_DVSEC_CXL_MEM_ACTIVE			_BITUL(1)
> +#define   PCI_DVSEC_CXL_MEM_SIZE_LOW			__GENMASK(31, 28)
> +#define  PCI_DVSEC_CXL_RANGE_BASE_HIGH(i)		(0x20 + (i * 0x10))
> +#define  PCI_DVSEC_CXL_RANGE_BASE_LOW(i)		(0x24 + (i * 0x10))
> +#define   PCI_DVSEC_CXL_MEM_BASE_LOW			__GENMASK(31, 28)
>  

> +/* CXL r4.0, 8.1.4: Non-CXL Function Map DVSEC */
> +#define PCI_DVSEC_CXL_FUNCTION_MAP			2
> +
> +/* CXL r4.0, 8.1.5: Extensions DVSEC for Ports */
> +#define PCI_DVSEC_CXL_PORT				3
> +#define  PCI_DVSEC_CXL_PORT_CTL				0x0c
> +#define   PCI_DVSEC_CXL_PORT_CTL_UNMASK_SBR		0x00000001
> +
> +/* CXL r4.0, 8.1.6: GPF DVSEC for CXL Port */
> +#define PCI_DVSEC_CXL_PORT_GPF				4

Nothing like ambiguous naming in the CXL spec as the
following fields sound like they are in the CXL_PORT dvsec
but they aren't.  Well the spec avoids it with GPF_FOR_PORT
but we don't want to go there.  I wonder...
PCI_DVSEC_CXL_PORTGPF maybe to avoid that?

Sigh. It's probably not worth it and does look horrible, so stick
with these.

> +#define  PCI_DVSEC_CXL_PORT_GPF_PHASE_1_CONTROL		0x0C
> +#define   PCI_DVSEC_CXL_PORT_GPF_PHASE_1_TMO_BASE	__GENMASK(3, 0)
> +#define   PCI_DVSEC_CXL_PORT_GPF_PHASE_1_TMO_SCALE	__GENMASK(11, 8)
> +#define  PCI_DVSEC_CXL_PORT_GPF_PHASE_2_CONTROL		0xE
> +#define   PCI_DVSEC_CXL_PORT_GPF_PHASE_2_TMO_BASE	__GENMASK(3, 0)
> +#define   PCI_DVSEC_CXL_PORT_GPF_PHASE_2_TMO_SCALE	__GENMASK(11, 8)
> +
> +/* CXL r4.0, 8.1.7: GPF DVSEC for CXL Device */
> +#define PCI_DVSEC_CXL_DEVICE_GPF			5
> +
> +/* CXL r4.0, 8.1.9: Register Locator DVSEC */
> +#define PCI_DVSEC_CXL_REG_LOCATOR			8
> +#define  PCI_DVSEC_CXL_REG_LOCATOR_BLOCK1		0xC
> +#define   PCI_DVSEC_CXL_REG_LOCATOR_BIR			__GENMASK(2, 0)
> +#define   PCI_DVSEC_CXL_REG_LOCATOR_BLOCK_ID		__GENMASK(15, 8)
> +#define   PCI_DVSEC_CXL_REG_LOCATOR_BLOCK_OFF_LOW	__GENMASK(31, 16)
>  
>  #endif /* LINUX_PCI_REGS_H */