On Sat, Nov 07, 2020 at 03:18:24PM +0100, Philippe Mathieu-Daudé wrote: > On 11/7/20 1:22 PM, Ani Sinha wrote: > > On Sat, Nov 7, 2020 at 3:40 PM Philippe Mathieu-Daudé <phi...@redhat.com> > > wrote: > >> > >> Hi, > >> > >> On 9/29/20 9:22 AM, Michael S. Tsirkin wrote: > >>> From: Ani Sinha <a...@anisinha.ca> > >>> > >>> When acpi hotplug is turned off for both root pci bus as well as for pci > >>> bridges, we should not generate the related ACPI code for DSDT table or > >>> initialize related hw ports or reserve hw resources. This change makes > >>> sure all those operations are turned off in the case ACPI pci hotplug is > >>> off globally. > >>> > >>> In this change, we also make sure ACPI code for the PCNT method are only > >>> added when bsel is enabled for the corresponding pci bus or bridge hotplug > >>> is turned on. > >> > >> I'm trying to understand the following build failure using gcc 9.3.0 > >> on Ubuntu: > >> > >> [2567/3684] Compiling C object > >> libqemu-x86_64-softmmu.fa.p/hw_i386_acpi-build.c.o > >> FAILED: libqemu-x86_64-softmmu.fa.p/hw_i386_acpi-build.c.o > >> ../hw/i386/acpi-build.c: In function 'build_append_pci_bus_devices': > >> ../hw/i386/acpi-build.c:496:9: error: 'method' may be used uninitialized > >> in this function [-Werror=maybe-uninitialized] > >> 496 | aml_append(parent_scope, method); > >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > >> cc1: all warnings being treated as errors > >> > ... > >>> static void acpi_get_misc_info(AcpiMiscInfo *info) > >>> @@ -456,10 +460,12 @@ static void build_append_pci_bus_devices(Aml > >>> *parent_scope, PCIBus *bus, > >>> } > >>> > >>> /* Append PCNT method to notify about events on local and child > >>> buses. > >>> - * Add unconditionally for root since DSDT expects it. > >>> + * Add this method for root bus only when hotplug is enabled since > >>> DSDT > >>> + * expects it. > >>> */ > >>> - method = aml_method("PCNT", 0, AML_NOTSERIALIZED); > >>> - > >>> + if (bsel || pcihp_bridge_en) { > >>> + method = aml_method("PCNT", 0, AML_NOTSERIALIZED); > >>> + } > >> > >> build_append_pci_bus_devices() is not easy to follow and could certainly > >> benefit from a refactor. > > > > Hmm, ok will do that in my spare time. > > > >> > >> So here, before 'method' was always reinitialized. Now not always, > >> so it can be any value set in the big for() loop before... > > > > In line 467 above, method is initialized when bsel is available or > > pcihp is enabled. In line 496, it is appended to the parent scope only > > under those conditions as well. Basically, in hunks > > > > + if (bsel || pcihp_bridge_en) { > > + method = aml_method("PCNT", 0, AML_NOTSERIALIZED); > > + } > > > > and > > + > > + if (bsel || pcihp_bridge_en) { > > + aml_append(parent_scope, method); > > + } > > > > the conditions are exactly the same. > > The problem is in the (!bsel && !pcihp_bridge_en) case, > what 'method' is used there?
Um ... where exactly?