[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] Fix a gap where acpi_pcihp_find_hotplug_bus() returns a n
From: |
Ani Sinha |
Subject: |
Re: [PATCH v2] Fix a gap where acpi_pcihp_find_hotplug_bus() returns a non-hotpluggable bus |
Date: |
Thu, 3 Sep 2020 19:55:32 +0530 |
Please do not pull this patch. This patch has a bug and it will crash here:
Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
0x000055c2805339e9 in acpi_pcihp_find_hotplug_bus (bsel=bsel@entry=1,
s=<optimized out>)
at /home/ani/workspace/qemu/hw/acpi/pcihp.c:162
162 if (!qbus_is_hotpluggable(BUS(find.bus))) {
(gdb) p find.bus
$1 = (PCIBus *) 0x0
I will submit a rework soon.
On Thu, Sep 3, 2020 at 3:55 PM Ani Sinha <ani@anisinha.ca> wrote:
>
> When ACPI hotplug for the root bus is disabled, the bsel property for that
> bus is not set. Please see the following commit:
>
> 3d7e78aa7777f ("Introduce a new flag for i440fx to disable PCI hotplug on the
> root bus").
>
> As a result, when acpi_pcihp_find_hotplug_bus() is called
> with bsel set to 0, it may return the root bus. This can cause devices
> attached to
> the root bus to get hot-unplugged if the user issues the following set of
> commmands:
>
> outl 0xae10 0
> outl 0xae08 your_slot
>
> Thanks to Julia for pointing this out here:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg734548.html
>
> In this patch, we fix the issue in this function by checking if the bus which
> is
> returned by the function is actually hotpluggable. If not, we simply return
> NULL.
> This avoids the scenario where we were returning a non-hotpluggable bus.
>
> Signed-off-by: Ani Sinha <ani@anisinha.ca>
> ---
> hw/acpi/pcihp.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index 39b1f74442..f148e73c89 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -147,6 +147,21 @@ static PCIBus
> *acpi_pcihp_find_hotplug_bus(AcpiPciHpState *s, int bsel)
> if (!bsel && !find.bus) {
> find.bus = s->root;
> }
> +
> + /*
> + * Check if find.bus is actually hotpluggable. If bsel is set to
> + * NULL for example on the root bus in order to make it
> + * non-hotpluggable, find.bus will match the root bus when bsel
> + * is 0. See acpi_pcihp_test_hotplug_bus() above. Since the
> + * bus is not hotpluggable however, we should not select the bus.
> + * Instead, we should set find.bus to NULL in that case. In the check
> + * below, we generalize this case for all buses, not just the root bus.
> + * The callers of this function check for a null return value and
> + * handle them appropriately.
> + */
> + if (!qbus_is_hotpluggable(BUS(find.bus))) {
> + find.bus = NULL;
> + }
> return find.bus;
> }
>
> --
> 2.17.1
>