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?
