On Tue, Aug 27, 2013 at 10:44 AM, Jiang Liu <liuj97@xxxxxxxxx> wrote:
> From: Jiang Liu <jiang.liu@xxxxxxxxxx>
>
> PCIe Base Spec 1.0 Section 5.8 states that "The PCI Express Capabilities,
> Device Capabilities, Device Status/Control, Link Capabilities and
> Link Status/Control registers are required for all PCI Express devices."
And spec 1.1 amended that (in sec 7.8, not 5.8) to say that the link
registers are not required for Root Complex Integrated Endpoints.
Integrated endpoints and event collectors don't have links, so even if
some RC-integrated devices do implement link registers per spec 1.0, I
don't think there's any point in allowing access to them.
> And PCIe Base Spec 3.0 Section 7.8 states that "The Link Capabilities,
> Link Status, and Link Control registers are required for all Root Ports,
> Switch Ports, Bridges, and Endpoints that are not Root Complex Integrated
> Endpoints."
>
> Changeset 1b6b8ce2ac372e "PCI: only save/restore existent registers in the
> PCIe capability" introduces pcie_cap_has_lnkctl(), which assumes:
> 1) PCIe root port, endpoint and legacy endpoint with 1.0 PCIe Capability
> implement Link Cap/Status/Control registers.
> 2) All PCIe devices with 2.0 PCIe Capability implemnet Link Cap/Status/
> Control registers.
>
> The above assumptions don't conform to PCIe base specifications, so change
> pcie_cap_has_lnkctl() to follow PCIe specifications.
>
> Signed-off-by: Jiang Liu <jiang.liu@xxxxxxxxxx>
> ---
> Hi Bjorn,
> Yuval has a PCIe 1.0 swither, so triggers the bug. I'm not sure
> why Yu Zhao has implemented pcie_cap_has_linctl() in that way, any special
> consideration here?
>
> Hi Yuval,
> Could you please help to test this patch? Due to hardware resource
> constraints, I have just compiled the patch on my laptop without testing.
>
> Thanks!
>
> ---
> drivers/pci/access.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/access.c b/drivers/pci/access.c
> index 1cc2366..23ff366 100644
> --- a/drivers/pci/access.c
> +++ b/drivers/pci/access.c
> @@ -484,10 +484,14 @@ static inline bool pcie_cap_has_lnkctl(const struct pci_dev *dev)
> {
> int type = pci_pcie_type(dev);
>
> - return pcie_cap_version(dev) > 1 ||
> - type == PCI_EXP_TYPE_ROOT_PORT ||
> - type == PCI_EXP_TYPE_ENDPOINT ||
> - type == PCI_EXP_TYPE_LEG_END;
> + return pcie_cap_version(dev) == 1 ||
> + type == PCI_EXP_TYPE_ENDPOINT ||
> + type == PCI_EXP_TYPE_LEG_END ||
> + type == PCI_EXP_TYPE_ROOT_PORT ||
> + type == PCI_EXP_TYPE_UPSTREAM ||
> + type == PCI_EXP_TYPE_DOWNSTREAM ||
> + type == PCI_EXP_TYPE_PCI_BRIDGE ||
> + type == PCI_EXP_TYPE_PCIE_BRIDGE;
> }
>
> static inline bool pcie_cap_has_sltctl(const struct pci_dev *dev)
> --
> 1.8.1.2
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
