On Monday 04 July 2022 08:00:23 Stefan Roese wrote:
> On 03.07.22 12:48, Pali Rohár wrote:
> > PCIe config space has address range 0-4095. So do not allow reading from
> > addresses outside of this range. Lot of U-Boot drivers do not expect that
> > passed value is not in this range. PCI DM read function is exetended to
>
> s/exetended/extended
Should I send a new patch version?
> > fill read value to all ones or zeros when it fails as U-Boot callers
> > ignores return value.
> >
> > Calling U-Boot command 'pci display.b 0.0.0 0 0x2000' now stops printing
> > config space at the end (before 0x1000 address).
> >
> > Signed-off-by: Pali Rohár <p...@kernel.org>
> > ---
> > cmd/pci.c | 16 ++++++++++++++--
> > drivers/pci/pci-uclass.c | 10 +++++++++-
> > 2 files changed, 23 insertions(+), 3 deletions(-)
> >
> > diff --git a/cmd/pci.c b/cmd/pci.c
> > index a99e8f8ad6e0..6258699fec81 100644
> > --- a/cmd/pci.c
> > +++ b/cmd/pci.c
> > @@ -358,6 +358,9 @@ static int pci_cfg_display(struct udevice *dev, ulong
> > addr,
> > if (length == 0)
> > length = 0x40 / byte_size; /* Standard PCI config space */
> > + if (addr >= 4096)
> > + return 1;
> > +
> > /* Print the lines.
> > * once, and all accesses are with the specified bus width.
> > */
> > @@ -378,7 +381,10 @@ static int pci_cfg_display(struct udevice *dev, ulong
> > addr,
> > rc = 1;
> > break;
> > }
> > - } while (nbytes > 0);
> > + } while (nbytes > 0 && addr < 4096);
> > +
> > + if (rc == 0 && nbytes > 0)
> > + return 1;
> > return (rc);
> > }
> > @@ -390,6 +396,9 @@ static int pci_cfg_modify(struct udevice *dev, ulong
> > addr, ulong size,
> > int nbytes;
> > ulong val;
> > + if (addr >= 4096)
> > + return 1;
> > +
> > /* Print the address, followed by value. Then accept input for
> > * the next value. A non-converted value exits.
> > */
> > @@ -427,7 +436,10 @@ static int pci_cfg_modify(struct udevice *dev, ulong
> > addr, ulong size,
> > addr += size;
> > }
> > }
> > - } while (nbytes);
> > + } while (nbytes && addr < 4096);
> > +
> > + if (nbytes)
> > + return 1;
> > return 0;
> > }
> > diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> > index 89245a271e16..7402079471c8 100644
> > --- a/drivers/pci/pci-uclass.c
> > +++ b/drivers/pci/pci-uclass.c
> > @@ -288,6 +288,8 @@ int pci_bus_write_config(struct udevice *bus, pci_dev_t
> > bdf, int offset,
> > ops = pci_get_ops(bus);
> > if (!ops->write_config)
> > return -ENOSYS;
> > + if (offset < 0 || offset >= 4096)
> > + return -EINVAL;
> > return ops->write_config(bus, bdf, offset, value, size);
> > }
> > @@ -366,8 +368,14 @@ int pci_bus_read_config(const struct udevice *bus,
> > pci_dev_t bdf, int offset,
> > struct dm_pci_ops *ops;
> > ops = pci_get_ops(bus);
> > - if (!ops->read_config)
> > + if (!ops->read_config) {
> > + *valuep = pci_conv_32_to_size(~0, offset, size);
> > return -ENOSYS;
> > + }
> > + if (offset < 0 || offset >= 4096) {
> > + *valuep = pci_conv_32_to_size(0, offset, size);
> > + return -EINVAL;
> > + }
>
> How about introducing a macro for this 4096 max value instead?
>
> Other than this:
>
> Reviewed-by: Stefan Roese <s...@denx.de>
>
> Thanks,
> Stefan
