linux-kernel - Re: [PATCH v8 4/8] PCI/portdrv: Create pcie_is_port

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 12 Nov 2021 13:14:28 -0500
From:   Jim Quinlan <jim2101024@...il.com>
To:     Krzysztof Wilczyński <kw@...ux.com>
Cc:     linux-pci <linux-pci@...r.kernel.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Nicolas Saenz Julienne <nsaenz@...nel.org>,
        Rob Herring <robh@...nel.org>, Mark Brown <broonie@...nel.org>,
        bcm-kernel-feedback-list <bcm-kernel-feedback-list@...adcom.com>,
        Jim Quinlan <james.quinlan@...adcom.com>,
        Sean V Kelley <sean.v.kelley@...el.com>,
        Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        Qiuxu Zhuo <qiuxu.zhuo@...el.com>,
        Keith Busch <kbusch@...nel.org>,
        open list <linux-kernel@...r.kernel.org>,
        Jim Quinlan <jim2101024@...il.com>
Subject: Re: [PATCH v8 4/8] PCI/portdrv: Create pcie_is_port_dev() func from
 existing code

On Thu, Nov 11, 2021 at 6:50 PM Krzysztof Wilczyński <kw@...ux.com> wrote:
>
> Hi Jim,
>
> [...]
> > +bool pcie_is_port_dev(struct pci_dev *dev)
> > +{
> > +     int type;
> > +
> > +     if (!dev)
> > +             return false;
> > +
> > +     type = pci_pcie_type(dev);
> > +
> > +     return pci_is_pcie(dev) &&
> > +             ((type == PCI_EXP_TYPE_ROOT_PORT) ||
> > +              (type == PCI_EXP_TYPE_UPSTREAM) ||
> > +              (type == PCI_EXP_TYPE_DOWNSTREAM) ||
> > +              (type == PCI_EXP_TYPE_RC_EC));
> > +}
> > +EXPORT_SYMBOL_GPL(pcie_is_port_dev);
>
> It would be really nice to document what the above function does (not that
> some of the logic has been extracted from other function).  You know, for
> the future generations of kernel hackers.

Hi Krzysztof and others,

I gave this a second look and realized that the portdrv's
pci_device_id list for the probe is doing filtering that is not
included in the function.  So perhaps the code must be the following
in order to live up to its name:

static inline bool pci_is_port_dev(struct pci_dev *dev)
{
    int type, class;

    if (!dev || !pci_is_pcie(dev))
        return false;

    class = dev->class;

    /* This must be kept in sync with port_pci_ids[] of protdev_pci.c */
    if (!(class == ((PCI_CLASS_BRIDGE_PCI << 8) | 0x00) ||
          class == ((PCI_CLASS_BRIDGE_PCI << 8) | 0x01) ||
          class == ((PCI_CLASS_SYSTEM_RCEC << 8) | 0x00)))
        return false;

    type = pci_pcie_type(dev);

    return ((type == PCI_EXP_TYPE_ROOT_PORT) ||
        (type == PCI_EXP_TYPE_UPSTREAM) ||
        (type == PCI_EXP_TYPE_DOWNSTREAM) ||
        (type == PCI_EXP_TYPE_RC_EC));
}

Kind of large for an inline, plus the code must be kept in sync with
the device list.   Suggestions?

As for a description, my understanding is that the code identifies a
pci_dev that is directly under a host bridge device.  I'm not really
sure about the PCI_CLASS_SYSTEM_RCEC though.

Regards,
Jim Quinlan
Broadcom STB

>
>         Krzysztof