| Message ID | 20240531213841.3246055-3-wei.huang2@amd.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | PCIe TPH and cache direct injection support | expand |
On Fri, 31 May 2024 16:38:34 -0500 Wei Huang <wei.huang2@amd.com> wrote: > Linux has some basic, but incomplete, definition for the TPH Requester > capability registers. Also the control registers of TPH Requester and > the TPH Completer are missing. This patch adds all required definitions > to support TPH enablement. > > 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> As below, you can't modify uapi headers because we have no idea what userspace code is already using them. Also, (annoyingly) the field contents in this header tend (or maybe always are) in the shifted form. > --- > drivers/vfio/pci/vfio_pci_config.c | 7 +++--- > include/uapi/linux/pci_regs.h | 35 ++++++++++++++++++++++++++---- > 2 files changed, 35 insertions(+), 7 deletions(-) > > diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c > index 97422aafaa7b..de622cdfc2a4 100644 > --- a/drivers/vfio/pci/vfio_pci_config.c > +++ b/drivers/vfio/pci/vfio_pci_config.c > @@ -1434,14 +1434,15 @@ static int vfio_ext_cap_len(struct vfio_pci_core_device *vdev, u16 ecap, u16 epo > if (ret) > return pcibios_err_to_errno(ret); > > - if ((dword & PCI_TPH_CAP_LOC_MASK) == PCI_TPH_LOC_CAP) { > + if (((dword & PCI_TPH_CAP_LOC_MASK) >> PCI_TPH_CAP_LOC_SHIFT) > + == PCI_TPH_LOC_CAP) { > int sts; > > sts = dword & PCI_TPH_CAP_ST_MASK; > sts >>= PCI_TPH_CAP_ST_SHIFT; > - return PCI_TPH_BASE_SIZEOF + (sts * 2) + 2; > + return PCI_TPH_ST_TABLE + (sts * 2) + 2; > } > - return PCI_TPH_BASE_SIZEOF; > + return PCI_TPH_ST_TABLE; > case PCI_EXT_CAP_ID_DVSEC: > ret = pci_read_config_dword(pdev, epos + PCI_DVSEC_HEADER1, &dword); > if (ret) > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h > index 94c00996e633..ae1cf048b04a 100644 > --- a/include/uapi/linux/pci_regs.h > +++ b/include/uapi/linux/pci_regs.h > @@ -657,6 +657,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 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 +1021,41 @@ > #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_SHIFT 12 > +#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_NO_ST 0x1 /* no ST mode supported */ > +#define PCI_TPH_CAP_NO_ST_SHIFT 0x0 /* no ST mode supported shift */ > +#define PCI_TPH_CAP_INT_VEC 0x2 /* interrupt vector mode supported */ > +#define PCI_TPH_CAP_INT_VEC_SHIFT 0x1 /* interrupt vector mode supported shift */ > +#define PCI_TPH_CAP_DS 0x4 /* device specific mode supported */ > +#define PCI_TPH_CAP_DS_SHIFT 0x4 /* device specific mode supported shift */ > #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 */ It's a userspace header, relatively unlikely to be safe to change it... This would also be inconsistent with how some other registers are defined in here. I'd love it if we could tidy this up, but we are stuck by this being in uapi. > +#define PCI_TPH_CAP_LOC_SHIFT 9 /* location shift */ > +#define PCI_TPH_LOC_NONE 0x0 /* no ST Table */ > +#define PCI_TPH_LOC_CAP 0x1 /* ST Table in extended capability */ > +#define PCI_TPH_LOC_MSIX 0x2 /* ST table in MSI-X table */ > #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_CTRL 0x8 /* control register */ > +#define PCI_TPH_CTRL_MODE_SEL_MASK 0x7 /* ST Model Select mask */ > +#define PCI_TPH_CTRL_MODE_SEL_SHIFT 0x0 /* ST Model Select shift */ > +#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 0x300 /* TPH Requester mask */ > +#define PCI_TPH_CTRL_REQ_EN_SHIFT 8 /* TPH Requester shift */ > +#define PCI_TPH_REQ_DISABLE 0x0 /* No TPH request allowed */ > +#define PCI_TPH_REQ_TPH_ONLY 0x1 /* 8-bit TPH tags allowed */ > +#define PCI_TPH_REQ_EXT_TPH 0x3 /* 16-bit TPH tags allowed */ > + > +#define PCI_TPH_ST_TABLE 0xc /* base of ST table */ > > /* Downstream Port Containment */ > #define PCI_EXP_DPC_CAP 0x04 /* DPC Capability */
On Fri, May 31, 2024 at 04:38:34PM -0500, Wei Huang wrote: > Linux has some basic, but incomplete, definition for the TPH Requester > capability registers. Also the control registers of TPH Requester and > the TPH Completer are missing. This patch adds all required definitions > to support TPH enablement. s/This patch adds/Add/ > +#define PCI_EXP_DEVCAP2_TPH_COMP_SHIFT 12 > +#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 */ Drop the _SHIFT definitions and use FIELD_GET() and FIELD_PREP() instead. > /* TPH Requester */ > #define PCI_TPH_CAP 4 /* capability register */ > +#define PCI_TPH_CAP_NO_ST 0x1 /* no ST mode supported */ > +#define PCI_TPH_CAP_NO_ST_SHIFT 0x0 /* no ST mode supported shift */ Drop _SHIFT and show full register width for PCI_TPH_CAP_NO_ST, e.g., #define PCI_TPH_CAP_NO_ST 0x00000001 The existing PCI_TPH_CAP_* definitions don't follow that convention, but the rest of the file does, and this should match.
On 6/7/24 11:17, Jonathan Cameron wrote: > On Fri, 31 May 2024 16:38:34 -0500 > Wei Huang <wei.huang2@amd.com> wrote: >> /* TPH Requester */ >> #define PCI_TPH_CAP 4 /* capability register */ >> +#define PCI_TPH_CAP_NO_ST 0x1 /* no ST mode supported */ >> +#define PCI_TPH_CAP_NO_ST_SHIFT 0x0 /* no ST mode supported shift */ >> +#define PCI_TPH_CAP_INT_VEC 0x2 /* interrupt vector mode supported */ >> +#define PCI_TPH_CAP_INT_VEC_SHIFT 0x1 /* interrupt vector mode supported shift */ >> +#define PCI_TPH_CAP_DS 0x4 /* device specific mode supported */ >> +#define PCI_TPH_CAP_DS_SHIFT 0x4 /* device specific mode supported shift */ >> #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 */ > > It's a userspace header, relatively unlikely to be safe to change it... > This would also be inconsistent with how some other registers are defined in here. > > I'd love it if we could tidy this up, but we are stuck by this being > in uapi. Alex Williamson had a similar comment in another email. In V3, I will only add (necessary) new definitions and refrain from touching the existing ones.
On 6/7/24 11:42, Bjorn Helgaas wrote: > On Fri, May 31, 2024 at 04:38:34PM -0500, Wei Huang wrote: >> Linux has some basic, but incomplete, definition for the TPH Requester >> capability registers. Also the control registers of TPH Requester and >> the TPH Completer are missing. This patch adds all required definitions >> to support TPH enablement. > > s/This patch adds/Add/ > >> +#define PCI_EXP_DEVCAP2_TPH_COMP_SHIFT 12 >> +#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 */ > > Drop the _SHIFT definitions and use FIELD_GET() and FIELD_PREP() > instead. > >> /* TPH Requester */ >> #define PCI_TPH_CAP 4 /* capability register */ >> +#define PCI_TPH_CAP_NO_ST 0x1 /* no ST mode supported */ >> +#define PCI_TPH_CAP_NO_ST_SHIFT 0x0 /* no ST mode supported shift */ > > Drop _SHIFT and show full register width for PCI_TPH_CAP_NO_ST, e.g., > > #define PCI_TPH_CAP_NO_ST 0x00000001 > > The existing PCI_TPH_CAP_* definitions don't follow that convention, > but the rest of the file does, and this should match. Will fix all of the above in V3.
diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c index 97422aafaa7b..de622cdfc2a4 100644 --- a/drivers/vfio/pci/vfio_pci_config.c +++ b/drivers/vfio/pci/vfio_pci_config.c @@ -1434,14 +1434,15 @@ static int vfio_ext_cap_len(struct vfio_pci_core_device *vdev, u16 ecap, u16 epo if (ret) return pcibios_err_to_errno(ret); - if ((dword & PCI_TPH_CAP_LOC_MASK) == PCI_TPH_LOC_CAP) { + if (((dword & PCI_TPH_CAP_LOC_MASK) >> PCI_TPH_CAP_LOC_SHIFT) + == PCI_TPH_LOC_CAP) { int sts; sts = dword & PCI_TPH_CAP_ST_MASK; sts >>= PCI_TPH_CAP_ST_SHIFT; - return PCI_TPH_BASE_SIZEOF + (sts * 2) + 2; + return PCI_TPH_ST_TABLE + (sts * 2) + 2; } - return PCI_TPH_BASE_SIZEOF; + return PCI_TPH_ST_TABLE; case PCI_EXT_CAP_ID_DVSEC: ret = pci_read_config_dword(pdev, epos + PCI_DVSEC_HEADER1, &dword); if (ret) diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h index 94c00996e633..ae1cf048b04a 100644 --- a/include/uapi/linux/pci_regs.h +++ b/include/uapi/linux/pci_regs.h @@ -657,6 +657,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 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 +1021,41 @@ #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_SHIFT 12 +#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_NO_ST 0x1 /* no ST mode supported */ +#define PCI_TPH_CAP_NO_ST_SHIFT 0x0 /* no ST mode supported shift */ +#define PCI_TPH_CAP_INT_VEC 0x2 /* interrupt vector mode supported */ +#define PCI_TPH_CAP_INT_VEC_SHIFT 0x1 /* interrupt vector mode supported shift */ +#define PCI_TPH_CAP_DS 0x4 /* device specific mode supported */ +#define PCI_TPH_CAP_DS_SHIFT 0x4 /* device specific mode supported shift */ #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_LOC_SHIFT 9 /* location shift */ +#define PCI_TPH_LOC_NONE 0x0 /* no ST Table */ +#define PCI_TPH_LOC_CAP 0x1 /* ST Table in extended capability */ +#define PCI_TPH_LOC_MSIX 0x2 /* ST table in MSI-X table */ #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_CTRL 0x8 /* control register */ +#define PCI_TPH_CTRL_MODE_SEL_MASK 0x7 /* ST Model Select mask */ +#define PCI_TPH_CTRL_MODE_SEL_SHIFT 0x0 /* ST Model Select shift */ +#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 0x300 /* TPH Requester mask */ +#define PCI_TPH_CTRL_REQ_EN_SHIFT 8 /* TPH Requester shift */ +#define PCI_TPH_REQ_DISABLE 0x0 /* No TPH request allowed */ +#define PCI_TPH_REQ_TPH_ONLY 0x1 /* 8-bit TPH tags allowed */ +#define PCI_TPH_REQ_EXT_TPH 0x3 /* 16-bit TPH tags allowed */ + +#define PCI_TPH_ST_TABLE 0xc /* base of ST table */ /* Downstream Port Containment */ #define PCI_EXP_DPC_CAP 0x04 /* DPC Capability */