| Message ID | 20200101111428.11392-1-sigmaris@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | drivers: pci: initialise class to 0 before reading | expand |
On Wed, Jan 1, 2020 at 7:15 PM Hugh Cole-Baker <sigmaris at gmail.com> wrote: > > Otherwise, uninitialised memory from the upper 32 bits can end up in > find_id.class, and this causes bugs later when looking for a driver for > the class. > > Signed-off-by: Hugh Cole-Baker <sigmaris at gmail.com> > --- > drivers/pci/pci-uclass.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c > index fab20fc60e..c28a1cc363 100644 > --- a/drivers/pci/pci-uclass.c > +++ b/drivers/pci/pci-uclass.c > @@ -773,7 +773,7 @@ int pci_bind_bus_devices(struct udevice *bus) > bdf += PCI_BDF(0, 0, 1)) { > struct pci_child_platdata *pplat; > struct udevice *dev; > - ulong class; > + ulong class = 0; > > if (!PCI_FUNC(bdf)) > found_multi = false; > -- I see class is initialized in the call to: pci_bus_read_config(bus, bdf, PCI_CLASS_REVISION, &class, PCI_SIZE_32); class >>= 8; Regards, Bin
Hi Bin, > On 1 Jan 2020, at 13:25, Bin Meng <bmeng.cn at gmail.com> wrote: > > On Wed, Jan 1, 2020 at 7:15 PM Hugh Cole-Baker <sigmaris at gmail.com> wrote: >> >> Otherwise, uninitialised memory from the upper 32 bits can end up in >> find_id.class, and this causes bugs later when looking for a driver for >> the class. >> >> Signed-off-by: Hugh Cole-Baker <sigmaris at gmail.com> >> --- >> drivers/pci/pci-uclass.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c >> index fab20fc60e..c28a1cc363 100644 >> --- a/drivers/pci/pci-uclass.c >> +++ b/drivers/pci/pci-uclass.c >> @@ -773,7 +773,7 @@ int pci_bind_bus_devices(struct udevice *bus) >> bdf += PCI_BDF(0, 0, 1)) { >> struct pci_child_platdata *pplat; >> struct udevice *dev; >> - ulong class; >> + ulong class = 0; >> >> if (!PCI_FUNC(bdf)) >> found_multi = false; >> -- > > I see class is initialized in the call to: > > pci_bus_read_config(bus, bdf, PCI_CLASS_REVISION, &class, > PCI_SIZE_32); > class >>= 8; > Maybe the driver I'm working with is doing its read_config wrongly? I'm trying to use this Rockchip PCIe driver [1] with upstream u-boot. The driver casts &class to u32*, uses readl() and only sets the lower 32 bits. I was assuming that this was normal for a read of PCI_SIZE_32. However, looking at some of the other drivers, it seems like it should be using pci_conv_32_to_size() instead? [1] https://github.com/radxa/u-boot/blob/rk3399-pie-gms-express-baseline/drivers/pci/pcie_rockchip.c#L435 Thanks, Hugh > Regards, > Bin
Hi Hugh, On Wed, Jan 1, 2020 at 9:50 PM Hugh Cole-Baker <sigmaris at gmail.com> wrote: > > Hi Bin, > > > On 1 Jan 2020, at 13:25, Bin Meng <bmeng.cn at gmail.com> wrote: > > > > On Wed, Jan 1, 2020 at 7:15 PM Hugh Cole-Baker <sigmaris at gmail.com> wrote: > >> > >> Otherwise, uninitialised memory from the upper 32 bits can end up in > >> find_id.class, and this causes bugs later when looking for a driver for > >> the class. > >> > >> Signed-off-by: Hugh Cole-Baker <sigmaris at gmail.com> > >> --- > >> drivers/pci/pci-uclass.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c > >> index fab20fc60e..c28a1cc363 100644 > >> --- a/drivers/pci/pci-uclass.c > >> +++ b/drivers/pci/pci-uclass.c > >> @@ -773,7 +773,7 @@ int pci_bind_bus_devices(struct udevice *bus) > >> bdf += PCI_BDF(0, 0, 1)) { > >> struct pci_child_platdata *pplat; > >> struct udevice *dev; > >> - ulong class; > >> + ulong class = 0; > >> > >> if (!PCI_FUNC(bdf)) > >> found_multi = false; > >> -- > > > > I see class is initialized in the call to: > > > > pci_bus_read_config(bus, bdf, PCI_CLASS_REVISION, &class, > > PCI_SIZE_32); > > class >>= 8; > > > > Maybe the driver I'm working with is doing its read_config wrongly? I'm > trying to use this Rockchip PCIe driver [1] with upstream u-boot. The > driver casts &class to u32*, uses readl() and only sets the lower 32 > bits. I was assuming that this was normal for a read of PCI_SIZE_32. > However, looking at some of the other drivers, it seems like it should > be using pci_conv_32_to_size() instead? > The bug should be that https://github.com/radxa/u-boot/blob/rk3399-pie-gms-express-baseline/drivers/pci/pcie_rockchip.c#L318 u32 *val should be declared as ulong *val. > [1] https://github.com/radxa/u-boot/blob/rk3399-pie-gms-express-baseline/drivers/pci/pcie_rockchip.c#L435 > Regards, Bin
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index fab20fc60e..c28a1cc363 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -773,7 +773,7 @@ int pci_bind_bus_devices(struct udevice *bus) bdf += PCI_BDF(0, 0, 1)) { struct pci_child_platdata *pplat; struct udevice *dev; - ulong class; + ulong class = 0; if (!PCI_FUNC(bdf)) found_multi = false;
Otherwise, uninitialised memory from the upper 32 bits can end up in find_id.class, and this causes bugs later when looking for a driver for the class. Signed-off-by: Hugh Cole-Baker <sigmaris at gmail.com> --- drivers/pci/pci-uclass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)