Re: [PATCH v7 19/24] PCI: Add macros for legacy I/O and memory address space sizes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] PCI_LEGACY_IO_SIZE macro is defined with an incorrect size of (SZ_64K - 1).
--

commit d53406577948bbef31acd8d9cf7f8f8d4aed412a
Author: Krzysztof Wilczyński <kwilczynski@kernel.org>

PCI: Add macros for legacy I/O and memory address space sizes

This commit adds defines for the standard PCI legacy address space sizes
to replace raw literals used by legacy sysfs attributes. It then updates
the sysfs code to use these newly added macros.

> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index a6451bbf69a8c..7ba12b2abe486 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1019,7 +1019,7 @@ void pci_create_legacy_files(struct pci_bus *b)
>  
>  	sysfs_bin_attr_init(b->legacy_io);
>  	b->legacy_io->attr.name = "legacy_io";
> -	b->legacy_io->size = 0xffff;
> +	b->legacy_io->size = PCI_LEGACY_IO_SIZE;
>  	b->legacy_io->attr.mode = 0600;

[ ... ]

> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index c56f2cf0d2ab5..e37677a8dd3ca 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h

[ ... ]

> @@ -1169,6 +1170,10 @@ enum {
>  /* These external functions are only available when PCI support is enabled */
>  #ifdef CONFIG_PCI
>  
> +/* PCI legacy I/O port and memory address space sizes. */
> +#define PCI_LEGACY_IO_SIZE	(SZ_64K - 1)

Is this macro value mathematically correct for a size definition?

The legacy PCI I/O port space spans ports 0x0000 through 0xFFFF, meaning
its total size is exactly 64 KiB.

While the original sysfs code hardcoded 0xffff as the file size (which this
patch intentionally maintains), exposing this off-by-one limit globally under
a SIZE naming convention seems potentially confusing.

If other subsystems start using PCI_LEGACY_IO_SIZE for memory allocations
or loop bounds, could this lead to unexpected truncations? Might it make
sense to define the true size as SZ_64K, and handle the off-by-one quirk
directly at the sysfs site instead?

> +#define PCI_LEGACY_MEM_SIZE	SZ_1M
> +
>  extern unsigned int pci_flags;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508043543.217179-1-kwilczynski@kernel.org?part=19