Re: [PULL v3 05/19] hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35

[Philippe Mathieu-Daudé]

On 7/21/21 4:59 PM, Igor Mammedov wrote:
> On Tue, 20 Jul 2021 14:56:06 +0200
> Laurent Vivier <lvivier@redhat.com> wrote:
>> On 20/07/2021 13:38, Laurent Vivier wrote:
>>> On 16/07/2021 17:15, Michael S. Tsirkin wrote:  
>>>> From: Julia Suvorova <jusual@redhat.com>
>>>>
>>>> Q35 has three different types of PCI devices hot-plug: PCIe Native,
>>>> SHPC Native and ACPI hot-plug. This patch changes the default choice
>>>> for cold-plugged bridges from PCIe Native to ACPI Hot-plug with
>>>> ability to use SHPC and PCIe Native for hot-plugged bridges.
>>>>
>>>> This is a list of the PCIe Native hot-plug issues that led to this
>>>> change:
>>>>     * no racy behavior during boot (see 110c477c2ed)
>>>>     * no delay during deleting - after the actual power off software
>>>>       must wait at least 1 second before indicating about it. This case
>>>>       is quite important for users, it even has its own bug:
>>>>           https://bugzilla.redhat.com/show_bug.cgi?id=1594168
>>>>     * no timer-based behavior - in addition to the previous example,
>>>>       the attention button has a 5-second waiting period, during which
>>>>       the operation can be canceled with a second press. While this
>>>>       looks fine for manual button control, automation will result in
>>>>       the need to queue or drop events, and the software receiving
>>>>       events in all sort of unspecified combinations of attention/power
>>>>       indicator states, which is racy and uppredictable.
>>>>     * fixes:
>>>>         * https://bugzilla.redhat.com/show_bug.cgi?id=1752465
>>>>         * https://bugzilla.redhat.com/show_bug.cgi?id=1690256
>>>>
>>>> To return to PCIe Native hot-plug:
>>>>     -global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off
>>>>
>>>> Known issue: older linux guests need the following flag
>>>> to allow hotplugged pci express devices to use io:
>>>>         -device pcie-root-port,io-reserve=4096.
>>>> io is unusual for pci express so this seems minor.
>>>> We'll fix this by a follow up patch.
>>>>
>>>> Signed-off-by: Julia Suvorova <jusual@redhat.com>
>>>> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>>>> Message-Id: <20210713004205.775386-6-jusual@redhat.com>
>>>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>>>> ---

>> It also happens with non-VFIO device like e1000e:
>>
>> ...
>> -device e1000e,bus=pcie-root-port-1,addr=0x0,id=hostdev0 \
>                      ^^^^^^^^^^^^^
> ACPI hotplug operates on slot level, so functions greater than 0 are not 
> considered,
> hence unexpected ACPI error. For above CLI, setting 'addr' on root-ports to 
> dedicated slots
> should fix issue.
> 
> The same will happen on PC machine if you assign bridge to any function other 
> than 0.
> 
> Following should fix ACPI error:
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 17836149fe..e2345bd7d0 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -527,7 +527,7 @@ static void build_append_pci_bus_devices(Aml 
> *parent_scope, PCIBus *bus,
>              QLIST_FOREACH(sec, &bus->child, sibling) {
>                  int32_t devfn = sec->parent_dev->devfn;
>  
> -                if (pci_bus_is_root(sec)) {
> +                if (pci_bus_is_root(sec) || PCI_FUNC(devfn)) {
>                      continue;
>                  }
> 
> but unplug request will stay ignored if root port/bridge is not on function 0.

Shouldn't we emit a warning/error if a such config is used?