qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Ani Sinha <ani@anisinha.ca>
To: qemu-devel@nongnu.org
Cc: "Eduardo Habkost" <ehabkost@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	jusual@redhat.com, "Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Aleksandar Markovic" <aleksandar.qemu.devel@gmail.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Ani Sinha" <ani@anisinha.ca>,
	"Igor Mammedov" <imammedo@redhat.com>,
	"Aurelien Jarno" <aurelien@aurel32.net>,
	"Richard Henderson" <rth@twiddle.net>
Subject: [PATCH v4 04/11] Fix a gap where acpi_pcihp_find_hotplug_bus() returns a non-hotpluggable bus
Date: Wed, 16 Sep 2020 11:43:28 +0530	[thread overview]
Message-ID: <20200916061335.14045-5-ani@anisinha.ca> (raw)
In-Reply-To: <20200916061335.14045-1-ani@anisinha.ca>

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(+)

change log:
v1: initial patch
v4: commit log updated as per suggestion.

diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 39b1f74442..32ae8b2c0a 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 (find.bus && !qbus_is_hotpluggable(BUS(find.bus))) {
+        find.bus = NULL;
+    }
     return find.bus;
 }
 
-- 
2.17.1



  parent reply	other threads:[~2020-09-16  6:19 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-16  6:13 [PATCH v4 00/11] i440fx/acpi: addition of feature and bug fixes Ani Sinha
2020-09-16  6:13 ` [PATCH v4 01/11] tests/acpi: document addition of table DSDT.roothp for unit testing root pci hotplug on/off Ani Sinha
2020-09-16  6:13 ` [PATCH v4 02/11] tests/acpi: add a new unit test to test hotplug off/on feature on the root pci bus Ani Sinha
2020-09-16  6:35   ` Igor Mammedov
2020-09-16  6:13 ` [PATCH v4 03/11] tests/acpi: add a new ACPI table in order to test root pci hotplug on/off Ani Sinha
2020-09-16  6:36   ` Igor Mammedov
2020-09-16  6:13 ` Ani Sinha [this message]
2020-09-16  6:41   ` [PATCH v4 04/11] Fix a gap where acpi_pcihp_find_hotplug_bus() returns a non-hotpluggable bus Igor Mammedov
2020-09-16  6:13 ` [PATCH v4 05/11] i440fx/acpi: do not add hotplug related amls for cold plugged bridges Ani Sinha
2020-09-16  6:55   ` Igor Mammedov
2020-09-16  6:13 ` [PATCH v4 06/11] tests/acpi: list added acpi table binary file for pci bridge hotplug test Ani Sinha
2020-09-16  6:13 ` [PATCH v4 07/11] tests/acpi: unit test for 'acpi-pci-hotplug-with-bridge-support' bridge flag Ani Sinha
2020-09-16  7:01   ` Igor Mammedov
2020-09-16  6:13 ` [PATCH v4 08/11] tests/acpi: add newly added acpi DSDT table blob for pci bridge hotplug flag Ani Sinha
2020-09-16  7:02   ` Igor Mammedov
2020-09-16  6:13 ` [PATCH v4 09/11] piix4: don't reserve hw resources when hotplug is off globally Ani Sinha
2020-09-16  6:13 ` [PATCH v4 10/11] tests/acpi: unit test exercizing hotplug off for pci root bus & bridge in i440fx Ani Sinha
2020-09-16  7:11   ` Igor Mammedov
2020-09-16  7:36   ` Igor Mammedov
2020-09-16  7:41   ` Thomas Huth
2020-09-16 10:57     ` Ani Sinha
2020-09-16  6:13 ` [PATCH v4 11/11] tests/acpi: add DSDT.hpbrroot DSDT table blob to test global i440fx hotplug Ani Sinha
2020-09-16  7:15 ` [PATCH v4 00/11] i440fx/acpi: addition of feature and bug fixes Igor Mammedov
2020-09-16  7:18   ` Ani Sinha

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200916061335.14045-5-ani@anisinha.ca \
    --to=ani@anisinha.ca \
    --cc=aleksandar.qemu.devel@gmail.com \
    --cc=aurelien@aurel32.net \
    --cc=ehabkost@redhat.com \
    --cc=f4bug@amsat.org \
    --cc=imammedo@redhat.com \
    --cc=jusual@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).