Re: [PATCH 1/2] PCI: Add MMIO Capabilities Register Block definitions

[email protected]
Newsgroups org.kernel.vger.linux-cxl,org.kernel.vger.linux-pci
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The PCI_MCAP_HDR_BASE(n) macro incorrectly calculates the capability header base offset by omitting the 8-byte MCAP Array Header size.
--

--- Patch [1]: [PATCH 1/2] PCI: Add MMIO Capabilities Register Block definitions ---
commit 20bc3890e7eab9f1dcc6c8e26abf81734aa23ac0
Author: pguo <[email protected]>

PCI: Add MMIO Capabilities Register Block definitions

This commit adds definitions for the PCIe Management Message Passthrough
via MMIO Mailbox ECN. It introduces the MMIO Capabilities Register Block
array and capability headers.

> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
[ ... ]
> +/* MMIO Capabilities Register Block (MCAP) */
> +#define PCI_MCAP_ARRAY_1		0x00
> +#define  PCI_MCAP_ARRAY_ID		__GENMASK(15, 0)
> +#define  PCI_MCAP_ARRAY_VERSION		__GENMASK(23, 16)
> +#define  PCI_MCAP_ARRAY_TYPE		__GENMASK(27, 24)
> +#define  PCI_MCAP_ARRAY_TYPE_CLASS_CODE	0x0
> +#define PCI_MCAP_ARRAY_2		0x04
> +#define  PCI_MCAP_ARRAY_COUNT		__GENMASK(15, 0)
> +
> +#define PCI_MCAP_HDR_SIZEOF		0x10
> +#define PCI_MCAP_HDR_BASE(n)		((n) * PCI_MCAP_HDR_SIZEOF)

[Severity: Medium]
Does this macro calculation correctly account for the MCAP Array Header size?

Since PCI_MCAP_ARRAY_1 and PCI_MCAP_ARRAY_2 occupy the first 8 bytes of the
block, calculating PCI_MCAP_HDR_BASE(0) as 0x00 causes the capability
headers to directly overlap with the array header registers instead of
starting at the correct offset.

Should this macro include an 8-byte offset for the array header?

> +#define PCI_MCAP_HDR_REG_1		0x00
> +#define  PCI_MCAP_CAP_ID		__GENMASK(15, 0)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.