Linux has some basic, but incomplete, definition for the TPH Requester
capability registers. Also the definitions of TPH Requester control
register and TPH Completer capability, as well as the ST fields of
MSI-X entry, are missing. Add all required definitions to support TPH
without changing the existing Linux UAPI.
Co-developed-by: Eric Van Tassell <Eric.VanTassell@amd.com>
Signed-off-by: Eric Van Tassell <Eric.VanTassell@amd.com>
Signed-off-by: Wei Huang <wei.huang2@amd.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
---
include/uapi/linux/pci_regs.h | 38 +++++++++++++++++++++++++++--------
1 file changed, 30 insertions(+), 8 deletions(-)
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 94c00996e633..643236f43f4d 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -340,7 +340,9 @@
#define PCI_MSIX_ENTRY_UPPER_ADDR 0x4 /* Message Upper Address */
#define PCI_MSIX_ENTRY_DATA 0x8 /* Message Data */
#define PCI_MSIX_ENTRY_VECTOR_CTRL 0xc /* Vector Control */
-#define PCI_MSIX_ENTRY_CTRL_MASKBIT 0x00000001
+#define PCI_MSIX_ENTRY_CTRL_MASKBIT 0x00000001 /* Mask Bit */
+#define PCI_MSIX_ENTRY_CTRL_ST_LOWER 0x00ff0000 /* ST Lower */
+#define PCI_MSIX_ENTRY_CTRL_ST_UPPER 0xff000000 /* ST Upper */
/* CompactPCI Hotswap Register */
@@ -657,6 +659,7 @@
#define PCI_EXP_DEVCAP2_ATOMIC_COMP64 0x00000100 /* 64b AtomicOp completion */
#define PCI_EXP_DEVCAP2_ATOMIC_COMP128 0x00000200 /* 128b AtomicOp completion */
#define PCI_EXP_DEVCAP2_LTR 0x00000800 /* Latency tolerance reporting */
+#define PCI_EXP_DEVCAP2_TPH_COMP_MASK 0x00003000 /* TPH completer support */
#define PCI_EXP_DEVCAP2_OBFF_MASK 0x000c0000 /* OBFF support mechanism */
#define PCI_EXP_DEVCAP2_OBFF_MSG 0x00040000 /* New message signaling */
#define PCI_EXP_DEVCAP2_OBFF_WAKE 0x00080000 /* Re-use WAKE# for OBFF */
@@ -1020,15 +1023,34 @@
#define PCI_DPA_CAP_SUBSTATE_MASK 0x1F /* # substates - 1 */
#define PCI_DPA_BASE_SIZEOF 16 /* size with 0 substates */
+/* TPH Completer Support */
+#define PCI_EXP_DEVCAP2_TPH_COMP_NONE 0x0 /* None */
+#define PCI_EXP_DEVCAP2_TPH_COMP_TPH_ONLY 0x1 /* TPH only */
+#define PCI_EXP_DEVCAP2_TPH_COMP_TPH_AND_EXT 0x3 /* TPH and Extended TPH */
+
/* TPH Requester */
#define PCI_TPH_CAP 4 /* capability register */
-#define PCI_TPH_CAP_LOC_MASK 0x600 /* location mask */
-#define PCI_TPH_LOC_NONE 0x000 /* no location */
-#define PCI_TPH_LOC_CAP 0x200 /* in capability */
-#define PCI_TPH_LOC_MSIX 0x400 /* in MSI-X */
-#define PCI_TPH_CAP_ST_MASK 0x07FF0000 /* ST table mask */
-#define PCI_TPH_CAP_ST_SHIFT 16 /* ST table shift */
-#define PCI_TPH_BASE_SIZEOF 0xc /* size with no ST table */
+#define PCI_TPH_CAP_NO_ST 0x00000001 /* No ST Mode Supported */
+#define PCI_TPH_CAP_INT_VEC 0x00000002 /* Interrupt Vector Mode Supported */
+#define PCI_TPH_CAP_DEV_SPEC 0x00000004 /* Device Specific Mode Supported */
+#define PCI_TPH_CAP_EXT_TPH 0x00000100 /* Ext TPH Requester Supported */
+#define PCI_TPH_CAP_LOC_MASK 0x00000600 /* ST Table Location */
+#define PCI_TPH_LOC_NONE 0x00000000 /* Not present */
+#define PCI_TPH_LOC_CAP 0x00000200 /* In capability */
+#define PCI_TPH_LOC_MSIX 0x00000400 /* In MSI-X */
+#define PCI_TPH_CAP_ST_MASK 0x07FF0000 /* ST Table Size */
+#define PCI_TPH_CAP_ST_SHIFT 16 /* ST Table Size shift */
+#define PCI_TPH_BASE_SIZEOF 0xc /* Size with no ST table */
+
+#define PCI_TPH_CTRL 8 /* control register */
+#define PCI_TPH_CTRL_MODE_SEL_MASK 0x00000007 /* ST Mode Select */
+#define PCI_TPH_NO_ST_MODE 0x0 /* No ST Mode */
+#define PCI_TPH_INT_VEC_MODE 0x1 /* Interrupt Vector Mode */
+#define PCI_TPH_DEV_SPEC_MODE 0x2 /* Device Specific Mode */
+#define PCI_TPH_CTRL_REQ_EN_MASK 0x00000300 /* TPH Requester Enable */
+#define PCI_TPH_REQ_DISABLE 0x0 /* No TPH requests allowed */
+#define PCI_TPH_REQ_TPH_ONLY 0x1 /* TPH only requests allowed */
+#define PCI_TPH_REQ_EXT_TPH 0x3 /* Extended TPH requests allowed */
/* Downstream Port Containment */
#define PCI_EXP_DPC_CAP 0x04 /* DPC Capability */
--
2.45.1On Thu, Aug 22, 2024 at 03:41:10PM -0500, Wei Huang wrote: > Linux has some basic, but incomplete, definition for the TPH Requester > capability registers. Also the definitions of TPH Requester control > register and TPH Completer capability, as well as the ST fields of > MSI-X entry, are missing. Add all required definitions to support TPH > without changing the existing Linux UAPI. > /* TPH Requester */ > #define PCI_TPH_CAP 4 /* capability register */ > -#define PCI_TPH_CAP_LOC_MASK 0x600 /* location mask */ > -#define PCI_TPH_LOC_NONE 0x000 /* no location */ > -#define PCI_TPH_LOC_CAP 0x200 /* in capability */ > -#define PCI_TPH_LOC_MSIX 0x400 /* in MSI-X */ > -#define PCI_TPH_CAP_ST_MASK 0x07FF0000 /* ST table mask */ > -#define PCI_TPH_CAP_ST_SHIFT 16 /* ST table shift */ > -#define PCI_TPH_BASE_SIZEOF 0xc /* size with no ST table */ > +#define PCI_TPH_CAP_NO_ST 0x00000001 /* No ST Mode Supported */ > +#define PCI_TPH_CAP_INT_VEC 0x00000002 /* Interrupt Vector Mode Supported */ > +#define PCI_TPH_CAP_DEV_SPEC 0x00000004 /* Device Specific Mode Supported */ I think these modes should all include "ST" to clearly delineate Steering Tags from the Processing Hints. E.g., PCI_TPH_CAP_ST_NO_ST or maybe PCI_TPH_CAP_ST_NONE PCI_TPH_CAP_ST_INT_VEC PCI_TPH_CAP_ST_DEV_SPEC > +#define PCI_TPH_CAP_EXT_TPH 0x00000100 /* Ext TPH Requester Supported */ > +#define PCI_TPH_CAP_LOC_MASK 0x00000600 /* ST Table Location */ > +#define PCI_TPH_LOC_NONE 0x00000000 /* Not present */ > +#define PCI_TPH_LOC_CAP 0x00000200 /* In capability */ > +#define PCI_TPH_LOC_MSIX 0x00000400 /* In MSI-X */ These are existing symbols just being tidied, so can't really add "ST" here unless we just add aliases. Since they're just used internally, not by drivers, I think they're fine as-is. > +#define PCI_TPH_CAP_ST_MASK 0x07FF0000 /* ST Table Size */ > +#define PCI_TPH_CAP_ST_SHIFT 16 /* ST Table Size shift */ > +#define PCI_TPH_BASE_SIZEOF 0xc /* Size with no ST table */ > + > +#define PCI_TPH_CTRL 8 /* control register */ > +#define PCI_TPH_CTRL_MODE_SEL_MASK 0x00000007 /* ST Mode Select */ > +#define PCI_TPH_NO_ST_MODE 0x0 /* No ST Mode */ > +#define PCI_TPH_INT_VEC_MODE 0x1 /* Interrupt Vector Mode */ > +#define PCI_TPH_DEV_SPEC_MODE 0x2 /* Device Specific Mode */ These are also internal, but they're new and I think they should also include "ST" to match the CAP #defines. Even better, maybe we only add these and use them for both CAP and CTRL since they're defined with identical values.
On 9/4/24 14:52, Bjorn Helgaas wrote: >> -#define PCI_TPH_CAP_ST_MASK 0x07FF0000 /* ST table mask */ >> -#define PCI_TPH_CAP_ST_SHIFT 16 /* ST table shift */ >> -#define PCI_TPH_BASE_SIZEOF 0xc /* size with no ST table */ >> +#define PCI_TPH_CAP_NO_ST 0x00000001 /* No ST Mode Supported */ >> +#define PCI_TPH_CAP_INT_VEC 0x00000002 /* Interrupt Vector Mode Supported */ >> +#define PCI_TPH_CAP_DEV_SPEC 0x00000004 /* Device Specific Mode Supported */ > > I think these modes should all include "ST" to clearly delineate > Steering Tags from the Processing Hints. E.g., > > PCI_TPH_CAP_ST_NO_ST or maybe PCI_TPH_CAP_ST_NONE Can I keep "NO_ST" instead of switching over to "ST_NONE"? First, it matches with PCIe spec. Secondly, IMO "ST_NONE" implies no ST support at all. > PCI_TPH_CAP_ST_INT_VEC > PCI_TPH_CAP_ST_DEV_SPEC Will change > >> +#define PCI_TPH_CAP_EXT_TPH 0x00000100 /* Ext TPH Requester Supported */ >> +#define PCI_TPH_CAP_LOC_MASK 0x00000600 /* ST Table Location */ >> +#define PCI_TPH_LOC_NONE 0x00000000 /* Not present */ >> +#define PCI_TPH_LOC_CAP 0x00000200 /* In capability */ >> +#define PCI_TPH_LOC_MSIX 0x00000400 /* In MSI-X */ > > These are existing symbols just being tidied, so can't really add "ST" > here unless we just add aliases. Since they're just used internally, > not by drivers, I think they're fine as-is. > Yes. In V1 review, Alex Williamson and Jonathan Cameron asked not to change these definitions as they might be used by existing software. >> +#define PCI_TPH_CAP_ST_MASK 0x07FF0000 /* ST Table Size */ >> +#define PCI_TPH_CAP_ST_SHIFT 16 /* ST Table Size shift */ >> +#define PCI_TPH_BASE_SIZEOF 0xc /* Size with no ST table */ >> + >> +#define PCI_TPH_CTRL 8 /* control register */ >> +#define PCI_TPH_CTRL_MODE_SEL_MASK 0x00000007 /* ST Mode Select */ >> +#define PCI_TPH_NO_ST_MODE 0x0 /* No ST Mode */ >> +#define PCI_TPH_INT_VEC_MODE 0x1 /* Interrupt Vector Mode */ >> +#define PCI_TPH_DEV_SPEC_MODE 0x2 /* Device Specific Mode */ > > These are also internal, but they're new and I think they should also > include "ST" to match the CAP #defines. > > Even better, maybe we only add these and use them for both CAP and > CTRL since they're defined with identical values. Can you elaborate here? In CTRL register, "ST Mode Select" is defined as a 2-bit field. The possible values are 0, 1, 2. But in CAP register, the modes are individual bit masked. So I cannot use CTRL definitions in CAP register directly unless I do shifting.
On Thu, Sep 05, 2024 at 10:08:33AM -0500, Wei Huang wrote: > On 9/4/24 14:52, Bjorn Helgaas wrote: > >> -#define PCI_TPH_CAP_ST_MASK 0x07FF0000 /* ST table mask */ > >> -#define PCI_TPH_CAP_ST_SHIFT 16 /* ST table shift */ > >> -#define PCI_TPH_BASE_SIZEOF 0xc /* size with no ST table */ > >> +#define PCI_TPH_CAP_NO_ST 0x00000001 /* No ST Mode Supported */ > >> +#define PCI_TPH_CAP_INT_VEC 0x00000002 /* Interrupt Vector Mode Supported */ > >> +#define PCI_TPH_CAP_DEV_SPEC 0x00000004 /* Device Specific Mode Supported */ > > > > I think these modes should all include "ST" to clearly delineate > > Steering Tags from the Processing Hints. E.g., > > > > PCI_TPH_CAP_ST_NO_ST or maybe PCI_TPH_CAP_ST_NONE > > Can I keep "NO_ST" instead of switching over to "ST_NONE"? First, it > matches with PCIe spec. Secondly, IMO "ST_NONE" implies no ST support at > all. Sure. Does PCI_TPH_CAP_ST_NO_ST work for you? That follows the same PCI_TPH_CAP_ST_* pattern as below. > > PCI_TPH_CAP_ST_INT_VEC > > PCI_TPH_CAP_ST_DEV_SPEC > > Will change > >> +#define PCI_TPH_CAP_ST_MASK 0x07FF0000 /* ST Table Size */ > >> +#define PCI_TPH_CAP_ST_SHIFT 16 /* ST Table Size shift */ > >> +#define PCI_TPH_BASE_SIZEOF 0xc /* Size with no ST table */ > >> + > >> +#define PCI_TPH_CTRL 8 /* control register */ > >> +#define PCI_TPH_CTRL_MODE_SEL_MASK 0x00000007 /* ST Mode Select */ > >> +#define PCI_TPH_NO_ST_MODE 0x0 /* No ST Mode */ > >> +#define PCI_TPH_INT_VEC_MODE 0x1 /* Interrupt Vector Mode */ > >> +#define PCI_TPH_DEV_SPEC_MODE 0x2 /* Device Specific Mode */ > > > > These are also internal, but they're new and I think they should also > > include "ST" to match the CAP #defines. > > > > Even better, maybe we only add these and use them for both CAP and > > CTRL since they're defined with identical values. > > Can you elaborate here? In CTRL register, "ST Mode Select" is defined as > a 2-bit field. The possible values are 0, 1, 2. But in CAP register, the > modes are individual bit masked. So I cannot use CTRL definitions in CAP > register directly unless I do shifting. Oops, sorry, I thought they were the same values, but they're not, so ignore this comment.
On 9/5/24 11:41 AM, Bjorn Helgaas wrote: > On Thu, Sep 05, 2024 at 10:08:33AM -0500, Wei Huang wrote: >> On 9/4/24 14:52, Bjorn Helgaas wrote: >>> I think these modes should all include "ST" to clearly delineate >>> Steering Tags from the Processing Hints. E.g., >>> >>> PCI_TPH_CAP_ST_NO_ST or maybe PCI_TPH_CAP_ST_NONE >> >> Can I keep "NO_ST" instead of switching over to "ST_NONE"? First, it >> matches with PCIe spec. Secondly, IMO "ST_NONE" implies no ST support at >> all. > > Sure. Does PCI_TPH_CAP_ST_NO_ST work for you? That follows the same > PCI_TPH_CAP_ST_* pattern as below. So I tweaked a bit in V5 to make them look cleaner: instead of ST_NO_ST, V5 has ST_NS. Similar pattern applies to other modes: PCI_TPH_CAP_NO_ST => PCI_TPH_CAP_ST_NS PCI_TPH_CAP_INT_VEC => PCI_TPH_CAP_ST_IV PCI_TPH_CAP_DEV_SPEC => PCI_TPH_CAP_ST_DS Ctrl register has similar changes.
© 2016 - 2026 Red Hat, Inc.