On Fri, Jan 07, 2022 at 12:45:48PM -0600, Bjorn Helgaas wrote:
> On Thu, Nov 25, 2021 at 01:45:54PM +0100, Pali Rohár wrote:
> > Function mvebu_pcie_hw_rd_conf() does not handle invalid size. So correctly
> > set read value to all-ones and return appropriate error return value
> > PCIBIOS_BAD_REGISTER_NUMBER like in mvebu_pcie_hw_wr_conf() function.
> >
> > Signed-off-by: Pali Rohár <pali@xxxxxxxxxx>
> > Cc: stable@xxxxxxxxxxxxxxx
>
> Is there a bug that this fixes? If not, I would drop the stable tag
> (as I see Lorenzo already did, thanks!).
>
> > ---
> > drivers/pci/controller/pci-mvebu.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c
> > index 08274132cdfb..19c6ee298442 100644
> > --- a/drivers/pci/controller/pci-mvebu.c
> > +++ b/drivers/pci/controller/pci-mvebu.c
> > @@ -261,6 +261,9 @@ static int mvebu_pcie_hw_rd_conf(struct mvebu_pcie_port *port,
> > case 4:
> > *val = readl_relaxed(conf_data);
> > break;
> > + default:
> > + *val = 0xffffffff;
> > + return PCIBIOS_BAD_REGISTER_NUMBER;
>
> Might be the right thing to do, but there are many config accessors
> that do not set *val to ~0 before returning
> PCIBIOS_BAD_REGISTER_NUMBER:
I think a better question would be - how can this function be called
with a size that isn't 1, 2 or 4? I suppose if someone were to add
another PCI_OP_READ/PCI_OP_WRITE. However... they really need to audit
every implementation if they do that.
The generic implementation does this:
if (size == 1)
*val = readb(addr);
else if (size == 2)
*val = readw(addr);
else
*val = readl(addr);
and therefore completely ignores the size if it isn't 1 or 2. So I
don't think this is something that needs fixing.
If we're going to fix this in drivers, shouldn't we fix the generic
implementation too?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
