[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250829164203.00005381@huawei.com>
Date: Fri, 29 Aug 2025 16:42:03 +0100
From: Jonathan Cameron <jonathan.cameron@...wei.com>
To: Lukas Wunner <lukas@...ner.de>
CC: Terry Bowman <terry.bowman@....com>, <dave@...olabs.net>,
<dave.jiang@...el.com>, <alison.schofield@...el.com>,
<dan.j.williams@...el.com>, <bhelgaas@...gle.com>, <shiju.jose@...wei.com>,
<ming.li@...omail.com>, <Smita.KoralahalliChannabasappa@....com>,
<rrichter@....com>, <dan.carpenter@...aro.org>,
<PradeepVineshReddy.Kodamati@....com>, <Benjamin.Cheatham@....com>,
<sathyanarayanan.kuppuswamy@...ux.intel.com>, <linux-cxl@...r.kernel.org>,
<alucerop@....com>, <ira.weiny@...el.com>, <linux-kernel@...r.kernel.org>,
<linux-pci@...r.kernel.org>
Subject: Re: [PATCH v11 07/23] CXL/PCI: Move CXL DVSEC definitions into
uapi/linux/pci_regs.h
On Wed, 27 Aug 2025 16:51:15 +0200
Lukas Wunner <lukas@...ner.de> wrote:
> On Tue, Aug 26, 2025 at 08:35:22PM -0500, Terry Bowman wrote:
> > The CXL DVSECs are currently defined in cxl/core/cxlpci.h. These are not
> > accessible to other subsystems.
> >
> > Change DVSEC name formatting to follow the existing PCI format in
> > pci_regs.h. The current format uses CXL_DVSEC_XYZ. Change to be PCI_DVSEC_CXL_XYZ.
> [...]
> > +++ b/include/uapi/linux/pci_regs.h
> > @@ -1225,9 +1225,61 @@
> > /* Deprecated old name, replaced with PCI_DOE_DATA_OBJECT_DISC_RSP_3_TYPE */
> > #define PCI_DOE_DATA_OBJECT_DISC_RSP_3_PROTOCOL PCI_DOE_DATA_OBJECT_DISC_RSP_3_TYPE
> >
> > -/* Compute Express Link (CXL r3.1, sec 8.1.5) */
> > -#define PCI_DVSEC_CXL_PORT 3
> > -#define PCI_DVSEC_CXL_PORT_CTL 0x0c
> > -#define PCI_DVSEC_CXL_PORT_CTL_UNMASK_SBR 0x00000001
> > +/* Compute Express Link (CXL r3.2, sec 8.1)
> > + *
> > + * Note that CXL DVSEC id 3 and 7 to be ignored when the CXL link state
> > + * is "disconnected" (CXL r3.2, sec 9.12.3). Re-enumerate these
> > + * registers on downstream link-up events.
> > + */
> > +
> > +#define PCI_DVSEC_HEADER1_LENGTH_MASK GENMASK(31, 20)
> > +
> > +/* CXL 3.2 8.1.3: PCIe DVSEC for CXL Device */
> > +#define PCI_DVSEC_CXL_DEVICE 0
> > +#define PCI_DVSEC_CXL_CAP_OFFSET 0xA
> > +#define PCI_DVSEC_CXL_MEM_CAPABLE BIT(2)
> > +#define PCI_DVSEC_CXL_HDM_COUNT_MASK GENMASK(5, 4)
> > +#define PCI_DVSEC_CXL_CTRL_OFFSET 0xC
> > +#define PCI_DVSEC_CXL_MEM_ENABLE BIT(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 BIT(0)
> > +#define PCI_DVSEC_CXL_MEM_ACTIVE BIT(1)
> > +#define PCI_DVSEC_CXL_MEM_SIZE_LOW_MASK 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_MASK GENMASK(31, 28)
>
> Is it legal to use BIT() in a uapi header?
>
> We've only allowed use of GENMASK() in uapi headers since 2023 with
> commit 3c7a8e190bc5 ("uapi: introduce uapi-friendly macros for GENMASK").
>
> But there is no uapi header in the kernel tree defining BIT().
>
> I note that include/uapi/cxl/features.h has plenty of occurrences of BIT()
> since commit 9b8e73cdb141 ("cxl: Move cxl feature command structs to user
> header"), which went into v6.15.
>
> ndctl contains a bitmap.h header defining BIT(), I guess that's why this
> wasn't perceived as a problem so far:
> https://github.com/pmem/ndctl/raw/main/util/bitmap.h
>
> However existing user space applications including <linux/pci_regs.h>
> may not have a BIT() definition and I suspect your change above will
> break the build of those applications.
Probably not breaking existing code, as it would have to actually use the
define to run into problems. However, it's still a an excellent point
as adding register definitions that can't be used is not helpful.
Jonathan
>
> Thanks,
>
> Lukas
>
Powered by blists - more mailing lists