Am So., 4. Dez. 2022 um 22:22 Uhr schrieb Pierre-Clément Tosi <pt...@google.com>: > > Hi, > > On Fri, Dec 02, 2022 at 08:38:37PM +0100, s...@geanix.com wrote: > > > > Quoting Pierre-Clément Tosi <pt...@google.com>: > > > > > Add a check for calloc() failing to allocate the requested memory. > > > > > > Make decode_regions() return an error code. > > > > > > Cc: Bin Meng <bmeng...@gmail.com> > > > Cc: Simon Glass <s...@chromium.org> > > > Cc: Stefan Roese <s...@denx.de> > > > Signed-off-by: Pierre-Clément Tosi <pt...@google.com> > > > --- > > > > Hi, > > > > We upgraded from v2022.04 -> v2022.10, and this PATCH breaks PCI (scsi/sata) > > support for x86_64. > > I have seen this in qemu, i havn't had the time to test this on real > > hardware. > > > > Steps to reproduce: > > $ make efi-x86_payload64_defconfig && make -j32 && scripts/build-efi.sh > > -sPrp >
Breaks my use case too and is basically a revert of https://source.denx.de/u-boot/u-boot/-/commit/f2825f6ec0bb50e7bd9376828a32212f1961f979 In my use case we are using coreboot for different Intel SoCs with a generic U-Boot payload. > Decompiling the resulting u-boot.dtb shows > > pci { > compatible = "pci-x86"; > u-boot,dm-pre-reloc; > }; > Yes.. that is what can be found here: https://source.denx.de/u-boot/u-boot/-/blob/master/arch/x86/dts/coreboot.dts#L34 > which isn't supported by the patch as we return -EINVAL when missing "ranges". > The rationale here was that the property seemed mandatory (see [1] and [2]); > was > this a misunderstanding of potential corner cases? > At the moment I would like to revert this change. > OTOH, I see that most DTS using "pci-x86" (a pseudo-device intended to act as > a > PCI bus) actually provide "ranges". In particular, a comment added by > 0ced70a0bb7a ("x86: tpl: Add a fake PCI bus") states that > > The BARs are allocated statically in the device tree. > > So I'll let others comment on this but either: > > - arch/x86/dts/efi-x86_payload.dts (and a few other DTS) is missing "ranges"; > or > - pci_uclass_pre_probe() should treat UCLASS_SIMPLE_BUS differently. > > [1]: > https://www.kernel.org/doc/Documentation/devicetree/bindings/pci/host-generic-pci.txt > [2]: IEEE Std 1275-1994 > > > > > Br, > > /Sean > > > > > drivers/pci/pci-uclass.c | 15 ++++++++++----- > > > 1 file changed, 10 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c > > > index 970ee1adf1..2c85e78a13 100644 > > > --- a/drivers/pci/pci-uclass.c > > > +++ b/drivers/pci/pci-uclass.c > > > @@ -954,7 +954,7 @@ int pci_bind_bus_devices(struct udevice *bus) > > > return 0; > > > } > > > > > > -static void decode_regions(struct pci_controller *hose, ofnode > > > parent_node, > > > +static int decode_regions(struct pci_controller *hose, ofnode > > > parent_node, > > > ofnode node) > > > { > > > int pci_addr_cells, addr_cells, size_cells; > > > @@ -968,7 +968,7 @@ static void decode_regions(struct pci_controller > > > *hose, ofnode parent_node, > > > prop = ofnode_get_property(node, "ranges", &len); > > > if (!prop) { > > > debug("%s: Cannot decode regions\n", __func__); > > > - return; > > > + return -EINVAL; > > > } > > > > > > pci_addr_cells = ofnode_read_simple_addr_cells(node); > > > @@ -986,6 +986,8 @@ static void decode_regions(struct pci_controller > > > *hose, ofnode parent_node, > > > max_regions = len / cells_per_record + CONFIG_NR_DRAM_BANKS; > > > hose->regions = (struct pci_region *) > > > calloc(1, max_regions * sizeof(struct pci_region)); > > > + if (!hose->regions) > > > + return -ENOMEM; > > > > > > for (i = 0; i < max_regions; i++, len -= cells_per_record) { > > > u64 pci_addr, addr, size; > > > @@ -1053,7 +1055,7 @@ static void decode_regions(struct pci_controller > > > *hose, ofnode parent_node, > > > /* Add a region for our local memory */ > > > bd = gd->bd; > > > if (!bd) > > > - return; > > > + return 0; > > > > > > for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) { > > > if (bd->bi_dram[i].size) { > > > @@ -1068,7 +1070,7 @@ static void decode_regions(struct pci_controller > > > *hose, ofnode parent_node, > > > } > > > } > > > > > > - return; > > > + return 0; > > > } > > > > > > static int pci_uclass_pre_probe(struct udevice *bus) > > > @@ -1097,7 +1099,10 @@ static int pci_uclass_pre_probe(struct udevice > > > *bus) > > > /* For bridges, use the top-level PCI controller */ > > > if (!device_is_on_pci_bus(bus)) { > > > hose->ctlr = bus; > > > - decode_regions(hose, dev_ofnode(bus->parent), > > > dev_ofnode(bus)); > > > + ret = decode_regions(hose, dev_ofnode(bus->parent), > > > + dev_ofnode(bus)); > > > + if (ret) > > > + return ret; > > > } else { > > > struct pci_controller *parent_hose; > > > > > > -- > > > 2.36.1.124.g0e6072fb45-goog > > > > > > > > -- > Pierre -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
