Skip to content

Commit

Permalink
14174 pcieadm needs to handle v1 pcie cap better
Browse files Browse the repository at this point in the history 
14175 pcieadm show-devs can do better on missing pcidb entries
14176 pcieadm aer cap compares wrong field
Reviewed by: Patrick Mooney <pmooney@pfmooney.com>
Approved by: Dan McDonald <danmcd@joyent.com>
  • Loading branch information
@rmustacc
rmustacc committed Oct 28, 2021
1 parent aa85903 commit bc729d4
Show file tree
Hide file tree
Showing 2 changed files with 144 additions and 6 deletions.
140 changes: 135 additions & 5 deletions usr/src/cmd/pcieadm/pcieadm_cfgspace.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1749,8 +1749,56 @@ static pcieadm_regdef_t pcieadm_regdef_pcie_slotsts2[] = {
{ -1, -1, NULL }
};

static pcieadm_cfgspace_print_t pcieadm_cap_pcie_v1_dev[] = {
{ PCIE_PCIECAP, 2, "cap", "Capability Register",
pcieadm_cfgspace_print_regdef, pcieadm_regdef_pcie_cap },
{ PCIE_DEVCAP, 4, "devcap", "Device Capabilities",
pcieadm_cfgspace_print_regdef, pcieadm_regdef_pcie_devcap },
{ PCIE_DEVSTS, 2, "devsts", "Device Status",
pcieadm_cfgspace_print_regdef, pcieadm_regdef_pcie_devsts },
{ -1, -1, NULL }
};

static pcieadm_cfgspace_print_t pcieadm_cap_pcie_v1_link[] = {
{ PCIE_PCIECAP, 2, "cap", "Capability Register",
pcieadm_cfgspace_print_regdef, pcieadm_regdef_pcie_cap },
{ PCIE_DEVCAP, 4, "devcap", "Device Capabilities",
pcieadm_cfgspace_print_regdef, pcieadm_regdef_pcie_devcap },
{ PCIE_DEVSTS, 2, "devsts", "Device Status",
pcieadm_cfgspace_print_regdef, pcieadm_regdef_pcie_devsts },
{ PCIE_LINKCAP, 4, "linkcap", "Link Capabilities",
pcieadm_cfgspace_print_regdef, pcieadm_regdef_pcie_linkcap },
{ PCIE_LINKCTL, 2, "linkctl", "Link Control",
pcieadm_cfgspace_print_regdef, pcieadm_regdef_pcie_linkctl },
{ PCIE_LINKSTS, 2, "linksts", "Link Status",
pcieadm_cfgspace_print_regdef, pcieadm_regdef_pcie_linksts },
{ -1, -1, NULL }
};

static pcieadm_cfgspace_print_t pcieadm_cap_pcie_v1_slot[] = {
{ PCIE_PCIECAP, 2, "cap", "Capability Register",
pcieadm_cfgspace_print_regdef, pcieadm_regdef_pcie_cap },
{ PCIE_DEVCAP, 4, "devcap", "Device Capabilities",
pcieadm_cfgspace_print_regdef, pcieadm_regdef_pcie_devcap },
{ PCIE_DEVSTS, 2, "devsts", "Device Status",
pcieadm_cfgspace_print_regdef, pcieadm_regdef_pcie_devsts },
{ PCIE_LINKCAP, 4, "linkcap", "Link Capabilities",
pcieadm_cfgspace_print_regdef, pcieadm_regdef_pcie_linkcap },
{ PCIE_LINKCTL, 2, "linkctl", "Link Control",
pcieadm_cfgspace_print_regdef, pcieadm_regdef_pcie_linkctl },
{ PCIE_LINKSTS, 2, "linksts", "Link Status",
pcieadm_cfgspace_print_regdef, pcieadm_regdef_pcie_linksts },
{ PCIE_SLOTCAP, 4, "slotcap", "Slot Capabilities",
pcieadm_cfgspace_print_regdef, pcieadm_regdef_pcie_slotcap },
{ PCIE_SLOTCTL, 2, "slotctl", "Slot Control",
pcieadm_cfgspace_print_regdef, pcieadm_regdef_pcie_slotctl },
{ PCIE_SLOTSTS, 2, "slotsts", "Slot Status",
pcieadm_cfgspace_print_regdef, pcieadm_regdef_pcie_slotsts },
{ -1, -1, NULL }
};

static pcieadm_cfgspace_print_t pcieadm_cap_pcie_v1[] = {

static pcieadm_cfgspace_print_t pcieadm_cap_pcie_v1_all[] = {
{ PCIE_PCIECAP, 2, "cap", "Capability Register",
pcieadm_cfgspace_print_regdef, pcieadm_regdef_pcie_cap },
{ PCIE_DEVCAP, 4, "devcap", "Device Capabilities",
Expand Down Expand Up @@ -3907,6 +3955,90 @@ pcieadm_cap_info_pcipm(pcieadm_cfgspace_walk_t *walkp,
*lenp = 0;
}

/*
* The PCIe capability underwent a few changes. In version 1 of the capability,
* devices were not required to implement the entire capability. In particular,
* endpoints did not need to implement anything more than the link status
* register. In the v2 capability, this was changed such that all devices had to
* implement the entire capbility, but otherwise hardcode registers to zero. As
* such we get to play guess the length based on the device type.
*/
static pcieadm_cap_vers_t pcieadm_cap_vers_pcie_v1_dev = {
1, 0x0c, pcieadm_cap_pcie_v1_dev
};

static pcieadm_cap_vers_t pcieadm_cap_vers_pcie_v1_link = {
1, 0x14, pcieadm_cap_pcie_v1_link
};

static pcieadm_cap_vers_t pcieadm_cap_vers_pcie_v1_slot = {
1, 0x1c, pcieadm_cap_pcie_v1_slot
};

static pcieadm_cap_vers_t pcieadm_cap_vers_pcie_v1_all = {
1, 0x24, pcieadm_cap_pcie_v1_all
};

static pcieadm_cap_vers_t pcieadm_cap_vers_pcie_v2 = {
2, 0x4c, pcieadm_cap_pcie_v2
};

static void
pcieadm_cap_info_pcie(pcieadm_cfgspace_walk_t *walkp,
const pcieadm_pci_cap_t *cap, uint32_t off,
const pcieadm_cap_vers_t **versp, uint32_t *lenp,
const pcieadm_subcap_t **subcap)
{
uint8_t vers = walkp->pcw_data->pcb_u8[off + 2] & 0xf;
uint16_t pcie = walkp->pcw_data->pcb_u8[off + 2] |
(walkp->pcw_data->pcb_u8[off + 3] << 8);

/*
* Version 2 is simple. There's only one thing to do, so we do it. For
* version 1 we need to look at the device type.
*/
*subcap = NULL;
if (vers == 2) {
*versp = &pcieadm_cap_vers_pcie_v2;
*lenp = (*versp)->ppr_len;
return;
} else if (vers != 1) {
*versp = NULL;
*lenp = 0;
return;
}

switch (pcie & PCIE_PCIECAP_DEV_TYPE_MASK) {
case PCIE_PCIECAP_DEV_TYPE_PCIE_DEV:
case PCIE_PCIECAP_DEV_TYPE_PCI_DEV:
*versp = &pcieadm_cap_vers_pcie_v1_link;
break;
case PCIE_PCIECAP_DEV_TYPE_RC_IEP:
*versp = &pcieadm_cap_vers_pcie_v1_dev;
break;
case PCIE_PCIECAP_DEV_TYPE_UP:
case PCIE_PCIECAP_DEV_TYPE_DOWN:
case PCIE_PCIECAP_DEV_TYPE_PCIE2PCI:
case PCIE_PCIECAP_DEV_TYPE_PCI2PCIE:
if ((pcie & PCIE_PCIECAP_SLOT_IMPL) != 0) {
*versp = &pcieadm_cap_vers_pcie_v1_slot;
} else {
*versp = &pcieadm_cap_vers_pcie_v1_link;
}
break;
case PCIE_PCIECAP_DEV_TYPE_ROOT:
case PCIE_PCIECAP_DEV_TYPE_RC_EC:
*versp = &pcieadm_cap_vers_pcie_v1_all;
break;
default:
*versp = NULL;
*lenp = 0;
return;
}

*lenp = (*versp)->ppr_len;
}

/*
* The length of the MSI capability depends on bits in its control field. As
* such we use a custom function to extract the length and treat each of these
Expand Down Expand Up @@ -3998,7 +4130,7 @@ pcieadm_cap_info_aer(pcieadm_cfgspace_walk_t *walkp,
const pcieadm_cap_vers_t **versp, uint32_t *lenp,
const pcieadm_subcap_t **subcap)
{
if (walkp->pcw_dtype == PCIE_PCIECAP_DEV_TYPE_PCIE2PCI) {
if (walkp->pcw_pcietype == PCIE_PCIECAP_DEV_TYPE_PCIE2PCI) {
uint8_t vers;

*subcap = NULL;
Expand Down Expand Up @@ -4156,9 +4288,7 @@ pcieadm_pci_cap_t pcieadm_pci_caps[] = {
pcieadm_cap_info_fixed, { 0, 8, pcieadm_cap_bridge_subsys } },
{ PCI_CAP_ID_AGP_8X, "agp8x", "AGP 8x" },
{ PCI_CAP_ID_SECURE_DEV, "secdev", "Secure Device" },
{ PCI_CAP_ID_PCI_E, "pcie", "PCI Express", pcieadm_cap_info_vers,
{ { 1, 0x24, pcieadm_cap_pcie_v1 },
{ 2, 0x3c, pcieadm_cap_pcie_v2 } } },
{ PCI_CAP_ID_PCI_E, "pcie", "PCI Express", pcieadm_cap_info_pcie },
{ PCI_CAP_ID_MSI_X, "msix", "MSI-X", pcieadm_cap_info_fixed,
{ { 0, 12, pcieadm_cap_msix } } },
{ PCI_CAP_ID_SATA, "sata", "Serial ATA Configuration",
Expand Down
10 changes: 9 additions & 1 deletion usr/src/cmd/pcieadm/pcieadm_devs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ pcieadm_show_devs_walk_cb(di_node_t node, void *arg)
char *path = NULL;
pcieadm_show_devs_t *psd = arg;
int ret = DI_WALK_CONTINUE;
char venstr[64], devstr[64];
pcieadm_show_devs_ofmt_t oarg;
pcidb_hdl_t *pcidb = psd->psd_pia->pia_pcidb;

Expand Down Expand Up @@ -376,6 +377,10 @@ pcieadm_show_devs_walk_cb(di_node_t node, void *arg)
oarg.psdo_vid);
if (vend != NULL) {
oarg.psdo_vendor = pcidb_vendor_name(vend);
} else {
(void) snprintf(venstr, sizeof (venstr),
"Unknown vendor: 0x%x", oarg.psdo_vid);
oarg.psdo_vendor = venstr;
}
}

Expand All @@ -385,7 +390,10 @@ pcieadm_show_devs_walk_cb(di_node_t node, void *arg)
oarg.psdo_vid, oarg.psdo_did);
if (dev != NULL) {
oarg.psdo_device = pcidb_device_name(dev);

} else {
(void) snprintf(devstr, sizeof (devstr),
"Unknown device: 0x%x", oarg.psdo_did);
oarg.psdo_device = devstr;
}
}

Expand Down

0 comments on commit bc729d4

Please sign in to comment.