linux-kernel - Re: [PATCH 1/3] PCI: Add standard PCI Config Address macros

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]

Message-ID: <20220926182710.GA1613197@bhelgaas>
Date:   Mon, 26 Sep 2022 13:27:10 -0500
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Pali Rohár <pali@...nel.org>
Cc:     Bjorn Helgaas <bhelgaas@...gle.com>,
        Lorenzo Pieralisi <lpieralisi@...nel.org>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Wilczyński <kw@...ux.com>,
        Sergio Paracuellos <sergio.paracuellos@...il.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org
Subject: Re: [PATCH 1/3] PCI: Add standard PCI Config Address macros

On Sat, Sep 24, 2022 at 11:24:02AM +0200, Pali Rohár wrote:
> Lot of PCI and PCIe controllers are using standard Config Address for PCI
> Configuration Mechanism #1 (as defined in PCI Local Bus Specification) or
> its extended version.
> 
> So introduce new macros PCI_CONF1_ADDRESS() and PCI_CONF1_EXT_ADDRESS() in
> include file drivers/pci/pci.h which can be suitable for PCI and PCIe
> controllers which uses this type of access to PCI config space.
> 
> Signed-off-by: Pali Rohár <pali@...nel.org>

Acked-by: Bjorn Helgaas <bhelgaas@...gle.com>

> +#define PCI_CONF1_BUS_SHIFT	16 /* Bus number */
> +#define PCI_CONF1_DEV_SHIFT	11 /* Device number */
> +#define PCI_CONF1_FUNC_SHIFT	8  /* Function number */
> +
> +#define PCI_CONF1_BUS_MASK	0xff
> +#define PCI_CONF1_DEV_MASK	0x1f
> +#define PCI_CONF1_FUNC_MASK	0x7
> +#define PCI_CONF1_REG_MASK	0xfc /* Limit aligned offset to a maximum of 256B */

Since all the above are used only in the macros below, I personally
don't think they're really necessary and I would find it easier to
read if they were just open-coded, e.g.,

  +#define PCI_CONF1_BUS(x)     (((x) & 0xff) << 16)
  +#define PCI_CONF1_DEV(x)	(((x) & 0x1f) << 11)
  +#define PCI_CONF1_FUNC(x)	(((x) & 0x07) <<  8)
  +#define PCI_CONF1_REG(x)     ( (x) & 0xfc)

> +#define PCI_CONF1_ENABLE	BIT(31)
> +#define PCI_CONF1_BUS(x)	(((x) & PCI_CONF1_BUS_MASK) << PCI_CONF1_BUS_SHIFT)
> +#define PCI_CONF1_DEV(x)	(((x) & PCI_CONF1_DEV_MASK) << PCI_CONF1_DEV_SHIFT)
> +#define PCI_CONF1_FUNC(x)	(((x) & PCI_CONF1_FUNC_MASK) << PCI_CONF1_FUNC_SHIFT)
> +#define PCI_CONF1_REG(x)	((x) & PCI_CONF1_REG_MASK)