* [PATCH V2] Introduce a new flag for piix to disable root bus PCI hotplug
@ 2020-08-11 13:12 Ani Sinha
2020-08-18 9:54 ` Ani Sinha
2020-08-19 10:00 ` Igor Mammedov
0 siblings, 2 replies; 7+ messages in thread
From: Ani Sinha @ 2020-08-11 13:12 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Aleksandar Markovic,
Paolo Bonzini, ani, Igor Mammedov, Philippe Mathieu-Daudé,
Aurelien Jarno, Richard Henderson
We introduce a new global flag for PIIX with which we can turn on or off PCI
device hotplug on the root bus. This flag can be used to prevent all PCI
devices from getting hotplugged or unplugged from the root PCI bus.
Signed-off-by: Ani Sinha <ani@anisinha.ca>
---
hw/acpi/piix4.c | 3 +++
hw/i386/acpi-build.c | 20 ++++++++++++++++----
2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 26bac4f..94ec35a 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -78,6 +78,7 @@ typedef struct PIIX4PMState {
AcpiPciHpState acpi_pci_hotplug;
bool use_acpi_hotplug_bridge;
+ bool use_acpi_root_pci_hotplug;
uint8_t disable_s3;
uint8_t disable_s4;
@@ -635,6 +636,8 @@ static Property piix4_pm_properties[] = {
DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_VAL, PIIX4PMState, s4_val, 2),
DEFINE_PROP_BOOL("acpi-pci-hotplug-with-bridge-support", PIIX4PMState,
use_acpi_hotplug_bridge, true),
+ DEFINE_PROP_BOOL("acpi-root-pci-hotplug", PIIX4PMState,
+ use_acpi_root_pci_hotplug, true),
DEFINE_PROP_BOOL("memory-hotplug-support", PIIX4PMState,
acpi_memory_hotplug.is_enabled, true),
DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b7bcbbb..a82e5c1 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -95,6 +95,7 @@ typedef struct AcpiPmInfo {
bool s3_disabled;
bool s4_disabled;
bool pcihp_bridge_en;
+ bool pcihp_root_en;
uint8_t s4_val;
AcpiFadtData fadt;
uint16_t cpu_hp_io_base;
@@ -245,6 +246,9 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
pm->pcihp_bridge_en =
object_property_get_bool(obj, "acpi-pci-hotplug-with-bridge-support",
NULL);
+ pm->pcihp_root_en =
+ object_property_get_bool(obj, "acpi-root-pci-hotplug", NULL);
+
}
static void acpi_get_misc_info(AcpiMiscInfo *info)
@@ -337,12 +341,15 @@ static void build_append_pcihp_notify_entry(Aml *method, int slot)
}
static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
- bool pcihp_bridge_en)
+ bool pcihp_bridge_en,
+ bool pcihp_root_en)
{
Aml *dev, *notify_method = NULL, *method;
QObject *bsel;
PCIBus *sec;
int i;
+ bool root_bus = pci_bus_is_root(bus);
+ bool root_pcihp_disabled = (root_bus && !pcihp_root_en);
bsel = object_property_get_qobject(OBJECT(bus), ACPI_PCIHP_PROP_BSEL, NULL);
if (bsel) {
@@ -361,6 +368,9 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
bool bridge_in_acpi;
if (!pdev) {
+ /* skip if pci hotplug for the root bus is disabled */
+ if (root_pcihp_disabled)
+ continue;
if (bsel) { /* add hotplug slots for non present devices */
dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
@@ -419,7 +429,7 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
method = aml_method("_S3D", 0, AML_NOTSERIALIZED);
aml_append(method, aml_return(aml_int(s3d)));
aml_append(dev, method);
- } else if (hotplug_enabled_dev) {
+ } else if (hotplug_enabled_dev && !root_pcihp_disabled) {
/* add _SUN/_EJ0 to make slot hotpluggable */
aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
@@ -439,7 +449,8 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
*/
PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
- build_append_pci_bus_devices(dev, sec_bus, pcihp_bridge_en);
+ build_append_pci_bus_devices(dev, sec_bus, pcihp_bridge_en,
+ pcihp_root_en);
}
/* slot descriptor has been composed, add it into parent context */
aml_append(parent_scope, dev);
@@ -1818,7 +1829,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
if (bus) {
Aml *scope = aml_scope("PCI0");
/* Scan all PCI buses. Generate tables to support hotplug. */
- build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en);
+ build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en,
+ pm->pcihp_root_en);
if (TPM_IS_TIS_ISA(tpm)) {
if (misc->tpm_version == TPM_VERSION_2_0) {
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V2] Introduce a new flag for piix to disable root bus PCI hotplug
2020-08-11 13:12 [PATCH V2] Introduce a new flag for piix to disable root bus PCI hotplug Ani Sinha
@ 2020-08-18 9:54 ` Ani Sinha
2020-08-18 10:16 ` Philippe Mathieu-Daudé
2020-08-19 10:00 ` Igor Mammedov
1 sibling, 1 reply; 7+ messages in thread
From: Ani Sinha @ 2020-08-18 9:54 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Aleksandar Markovic,
Paolo Bonzini, Igor Mammedov, Philippe Mathieu-Daudé,
Aurelien Jarno, Richard Henderson
Igor etc, I just tested this patch using a Windows 2012 R2 image and
it seems to be working. Any feedbacks on this patch?
On Tue, Aug 11, 2020 at 6:42 PM Ani Sinha <ani@anisinha.ca> wrote:
>
> We introduce a new global flag for PIIX with which we can turn on or off PCI
> device hotplug on the root bus. This flag can be used to prevent all PCI
> devices from getting hotplugged or unplugged from the root PCI bus.
>
> Signed-off-by: Ani Sinha <ani@anisinha.ca>
> ---
> hw/acpi/piix4.c | 3 +++
> hw/i386/acpi-build.c | 20 ++++++++++++++++----
> 2 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index 26bac4f..94ec35a 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -78,6 +78,7 @@ typedef struct PIIX4PMState {
>
> AcpiPciHpState acpi_pci_hotplug;
> bool use_acpi_hotplug_bridge;
> + bool use_acpi_root_pci_hotplug;
>
> uint8_t disable_s3;
> uint8_t disable_s4;
> @@ -635,6 +636,8 @@ static Property piix4_pm_properties[] = {
> DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_VAL, PIIX4PMState, s4_val, 2),
> DEFINE_PROP_BOOL("acpi-pci-hotplug-with-bridge-support", PIIX4PMState,
> use_acpi_hotplug_bridge, true),
> + DEFINE_PROP_BOOL("acpi-root-pci-hotplug", PIIX4PMState,
> + use_acpi_root_pci_hotplug, true),
> DEFINE_PROP_BOOL("memory-hotplug-support", PIIX4PMState,
> acpi_memory_hotplug.is_enabled, true),
> DEFINE_PROP_END_OF_LIST(),
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index b7bcbbb..a82e5c1 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -95,6 +95,7 @@ typedef struct AcpiPmInfo {
> bool s3_disabled;
> bool s4_disabled;
> bool pcihp_bridge_en;
> + bool pcihp_root_en;
> uint8_t s4_val;
> AcpiFadtData fadt;
> uint16_t cpu_hp_io_base;
> @@ -245,6 +246,9 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
> pm->pcihp_bridge_en =
> object_property_get_bool(obj, "acpi-pci-hotplug-with-bridge-support",
> NULL);
> + pm->pcihp_root_en =
> + object_property_get_bool(obj, "acpi-root-pci-hotplug", NULL);
> +
> }
>
> static void acpi_get_misc_info(AcpiMiscInfo *info)
> @@ -337,12 +341,15 @@ static void build_append_pcihp_notify_entry(Aml *method, int slot)
> }
>
> static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> - bool pcihp_bridge_en)
> + bool pcihp_bridge_en,
> + bool pcihp_root_en)
> {
> Aml *dev, *notify_method = NULL, *method;
> QObject *bsel;
> PCIBus *sec;
> int i;
> + bool root_bus = pci_bus_is_root(bus);
> + bool root_pcihp_disabled = (root_bus && !pcihp_root_en);
>
> bsel = object_property_get_qobject(OBJECT(bus), ACPI_PCIHP_PROP_BSEL, NULL);
> if (bsel) {
> @@ -361,6 +368,9 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> bool bridge_in_acpi;
>
> if (!pdev) {
> + /* skip if pci hotplug for the root bus is disabled */
> + if (root_pcihp_disabled)
> + continue;
> if (bsel) { /* add hotplug slots for non present devices */
> dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
> aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
> @@ -419,7 +429,7 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> method = aml_method("_S3D", 0, AML_NOTSERIALIZED);
> aml_append(method, aml_return(aml_int(s3d)));
> aml_append(dev, method);
> - } else if (hotplug_enabled_dev) {
> + } else if (hotplug_enabled_dev && !root_pcihp_disabled) {
> /* add _SUN/_EJ0 to make slot hotpluggable */
> aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
>
> @@ -439,7 +449,8 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> */
> PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
>
> - build_append_pci_bus_devices(dev, sec_bus, pcihp_bridge_en);
> + build_append_pci_bus_devices(dev, sec_bus, pcihp_bridge_en,
> + pcihp_root_en);
> }
> /* slot descriptor has been composed, add it into parent context */
> aml_append(parent_scope, dev);
> @@ -1818,7 +1829,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> if (bus) {
> Aml *scope = aml_scope("PCI0");
> /* Scan all PCI buses. Generate tables to support hotplug. */
> - build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en);
> + build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en,
> + pm->pcihp_root_en);
>
> if (TPM_IS_TIS_ISA(tpm)) {
> if (misc->tpm_version == TPM_VERSION_2_0) {
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2] Introduce a new flag for piix to disable root bus PCI hotplug
2020-08-18 9:54 ` Ani Sinha
@ 2020-08-18 10:16 ` Philippe Mathieu-Daudé
2020-08-18 10:19 ` Ani Sinha
0 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-08-18 10:16 UTC (permalink / raw)
To: Ani Sinha, qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Aleksandar Markovic,
Paolo Bonzini, Igor Mammedov, Aurelien Jarno, Richard Henderson
Hi,
On 8/18/20 11:54 AM, Ani Sinha wrote:
> Igor etc, I just tested this patch using a Windows 2012 R2 image and
> it seems to be working. Any feedbacks on this patch?
>
> On Tue, Aug 11, 2020 at 6:42 PM Ani Sinha <ani@anisinha.ca> wrote:
>>
>> We introduce a new global flag for PIIX with which we can turn on or off PCI
>> device hotplug on the root bus. This flag can be used to prevent all PCI
>> devices from getting hotplugged or unplugged from the root PCI bus.
>>
>> Signed-off-by: Ani Sinha <ani@anisinha.ca>
>> ---
>> hw/acpi/piix4.c | 3 +++
>> hw/i386/acpi-build.c | 20 ++++++++++++++++----
>> 2 files changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
>> index 26bac4f..94ec35a 100644
>> --- a/hw/acpi/piix4.c
>> +++ b/hw/acpi/piix4.c
>> @@ -78,6 +78,7 @@ typedef struct PIIX4PMState {
>>
>> AcpiPciHpState acpi_pci_hotplug;
>> bool use_acpi_hotplug_bridge;
>> + bool use_acpi_root_pci_hotplug;
>>
>> uint8_t disable_s3;
>> uint8_t disable_s4;
>> @@ -635,6 +636,8 @@ static Property piix4_pm_properties[] = {
>> DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_VAL, PIIX4PMState, s4_val, 2),
>> DEFINE_PROP_BOOL("acpi-pci-hotplug-with-bridge-support", PIIX4PMState,
>> use_acpi_hotplug_bridge, true),
>> + DEFINE_PROP_BOOL("acpi-root-pci-hotplug", PIIX4PMState,
>> + use_acpi_root_pci_hotplug, true),
From what I understood here, this file is already pretty twisted
and Igor doesn't want more workarounds:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg690564.html
¯\_(ツ)_/¯
>> DEFINE_PROP_BOOL("memory-hotplug-support", PIIX4PMState,
>> acpi_memory_hotplug.is_enabled, true),
>> DEFINE_PROP_END_OF_LIST(),
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index b7bcbbb..a82e5c1 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -95,6 +95,7 @@ typedef struct AcpiPmInfo {
>> bool s3_disabled;
>> bool s4_disabled;
>> bool pcihp_bridge_en;
>> + bool pcihp_root_en;
>> uint8_t s4_val;
>> AcpiFadtData fadt;
>> uint16_t cpu_hp_io_base;
>> @@ -245,6 +246,9 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
>> pm->pcihp_bridge_en =
>> object_property_get_bool(obj, "acpi-pci-hotplug-with-bridge-support",
>> NULL);
>> + pm->pcihp_root_en =
>> + object_property_get_bool(obj, "acpi-root-pci-hotplug", NULL);
>> +
>> }
>>
>> static void acpi_get_misc_info(AcpiMiscInfo *info)
>> @@ -337,12 +341,15 @@ static void build_append_pcihp_notify_entry(Aml *method, int slot)
>> }
>>
>> static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
>> - bool pcihp_bridge_en)
>> + bool pcihp_bridge_en,
>> + bool pcihp_root_en)
>> {
>> Aml *dev, *notify_method = NULL, *method;
>> QObject *bsel;
>> PCIBus *sec;
>> int i;
>> + bool root_bus = pci_bus_is_root(bus);
>> + bool root_pcihp_disabled = (root_bus && !pcihp_root_en);
>>
>> bsel = object_property_get_qobject(OBJECT(bus), ACPI_PCIHP_PROP_BSEL, NULL);
>> if (bsel) {
>> @@ -361,6 +368,9 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
>> bool bridge_in_acpi;
>>
>> if (!pdev) {
>> + /* skip if pci hotplug for the root bus is disabled */
>> + if (root_pcihp_disabled)
>> + continue;
>> if (bsel) { /* add hotplug slots for non present devices */
>> dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
>> aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
>> @@ -419,7 +429,7 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
>> method = aml_method("_S3D", 0, AML_NOTSERIALIZED);
>> aml_append(method, aml_return(aml_int(s3d)));
>> aml_append(dev, method);
>> - } else if (hotplug_enabled_dev) {
>> + } else if (hotplug_enabled_dev && !root_pcihp_disabled) {
>> /* add _SUN/_EJ0 to make slot hotpluggable */
>> aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
>>
>> @@ -439,7 +449,8 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
>> */
>> PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
>>
>> - build_append_pci_bus_devices(dev, sec_bus, pcihp_bridge_en);
>> + build_append_pci_bus_devices(dev, sec_bus, pcihp_bridge_en,
>> + pcihp_root_en);
>> }
>> /* slot descriptor has been composed, add it into parent context */
>> aml_append(parent_scope, dev);
>> @@ -1818,7 +1829,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>> if (bus) {
>> Aml *scope = aml_scope("PCI0");
>> /* Scan all PCI buses. Generate tables to support hotplug. */
>> - build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en);
>> + build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en,
>> + pm->pcihp_root_en);
>>
>> if (TPM_IS_TIS_ISA(tpm)) {
>> if (misc->tpm_version == TPM_VERSION_2_0) {
>> --
>> 2.7.4
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2] Introduce a new flag for piix to disable root bus PCI hotplug
2020-08-18 10:16 ` Philippe Mathieu-Daudé
@ 2020-08-18 10:19 ` Ani Sinha
2020-08-18 10:46 ` Ani Sinha
0 siblings, 1 reply; 7+ messages in thread
From: Ani Sinha @ 2020-08-18 10:19 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Eduardo Habkost, Michael S. Tsirkin, qemu-devel,
Aleksandar Markovic, Paolo Bonzini, Igor Mammedov, Aurelien Jarno,
Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 6759 bytes --]
This is not a workaround, this is a feature we need badly. We have
discussed this at length. Igor suggested we have one additional global flag
to disable all pci hotplug. But then that leads to strange case when pci
hotplug on the bridges are enabled (we have a separate flag for that).
Hence, I think we can simply add another flag to disable hotplug just for
the pci root bus.
On Tue, Aug 18, 2020 at 3:46 PM Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:
> Hi,
>
>
>
> On 8/18/20 11:54 AM, Ani Sinha wrote:
>
> > Igor etc, I just tested this patch using a Windows 2012 R2 image and
>
> > it seems to be working. Any feedbacks on this patch?
>
> >
>
> > On Tue, Aug 11, 2020 at 6:42 PM Ani Sinha <ani@anisinha.ca> wrote:
>
> >>
>
> >> We introduce a new global flag for PIIX with which we can turn on or
> off PCI
>
> >> device hotplug on the root bus. This flag can be used to prevent all PCI
>
> >> devices from getting hotplugged or unplugged from the root PCI bus.
>
> >>
>
> >> Signed-off-by: Ani Sinha <ani@anisinha.ca>
>
> >> ---
>
> >> hw/acpi/piix4.c | 3 +++
>
> >> hw/i386/acpi-build.c | 20 ++++++++++++++++----
>
> >> 2 files changed, 19 insertions(+), 4 deletions(-)
>
> >>
>
> >> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
>
> >> index 26bac4f..94ec35a 100644
>
> >> --- a/hw/acpi/piix4.c
>
> >> +++ b/hw/acpi/piix4.c
>
> >> @@ -78,6 +78,7 @@ typedef struct PIIX4PMState {
>
> >>
>
> >> AcpiPciHpState acpi_pci_hotplug;
>
> >> bool use_acpi_hotplug_bridge;
>
> >> + bool use_acpi_root_pci_hotplug;
>
> >>
>
> >> uint8_t disable_s3;
>
> >> uint8_t disable_s4;
>
> >> @@ -635,6 +636,8 @@ static Property piix4_pm_properties[] = {
>
> >> DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_VAL, PIIX4PMState, s4_val, 2),
>
> >> DEFINE_PROP_BOOL("acpi-pci-hotplug-with-bridge-support",
> PIIX4PMState,
>
> >> use_acpi_hotplug_bridge, true),
>
> >> + DEFINE_PROP_BOOL("acpi-root-pci-hotplug", PIIX4PMState,
>
> >> + use_acpi_root_pci_hotplug, true),
>
>
>
> From what I understood here, this file is already pretty twisted
>
> and Igor doesn't want more workarounds:
>
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg690564.html
>
>
>
> ¯\_(ツ)_/¯
>
>
>
> >> DEFINE_PROP_BOOL("memory-hotplug-support", PIIX4PMState,
>
> >> acpi_memory_hotplug.is_enabled, true),
>
> >> DEFINE_PROP_END_OF_LIST(),
>
> >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>
> >> index b7bcbbb..a82e5c1 100644
>
> >> --- a/hw/i386/acpi-build.c
>
> >> +++ b/hw/i386/acpi-build.c
>
> >> @@ -95,6 +95,7 @@ typedef struct AcpiPmInfo {
>
> >> bool s3_disabled;
>
> >> bool s4_disabled;
>
> >> bool pcihp_bridge_en;
>
> >> + bool pcihp_root_en;
>
> >> uint8_t s4_val;
>
> >> AcpiFadtData fadt;
>
> >> uint16_t cpu_hp_io_base;
>
> >> @@ -245,6 +246,9 @@ static void acpi_get_pm_info(MachineState *machine,
> AcpiPmInfo *pm)
>
> >> pm->pcihp_bridge_en =
>
> >> object_property_get_bool(obj,
> "acpi-pci-hotplug-with-bridge-support",
>
> >> NULL);
>
> >> + pm->pcihp_root_en =
>
> >> + object_property_get_bool(obj, "acpi-root-pci-hotplug", NULL);
>
> >> +
>
> >> }
>
> >>
>
> >> static void acpi_get_misc_info(AcpiMiscInfo *info)
>
> >> @@ -337,12 +341,15 @@ static void build_append_pcihp_notify_entry(Aml
> *method, int slot)
>
> >> }
>
> >>
>
> >> static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus
> *bus,
>
> >> - bool pcihp_bridge_en)
>
> >> + bool pcihp_bridge_en,
>
> >> + bool pcihp_root_en)
>
> >> {
>
> >> Aml *dev, *notify_method = NULL, *method;
>
> >> QObject *bsel;
>
> >> PCIBus *sec;
>
> >> int i;
>
> >> + bool root_bus = pci_bus_is_root(bus);
>
> >> + bool root_pcihp_disabled = (root_bus && !pcihp_root_en);
>
> >>
>
> >> bsel = object_property_get_qobject(OBJECT(bus),
> ACPI_PCIHP_PROP_BSEL, NULL);
>
> >> if (bsel) {
>
> >> @@ -361,6 +368,9 @@ static void build_append_pci_bus_devices(Aml
> *parent_scope, PCIBus *bus,
>
> >> bool bridge_in_acpi;
>
> >>
>
> >> if (!pdev) {
>
> >> + /* skip if pci hotplug for the root bus is disabled */
>
> >> + if (root_pcihp_disabled)
>
> >> + continue;
>
> >> if (bsel) { /* add hotplug slots for non present devices */
>
> >> dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
>
> >> aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
>
> >> @@ -419,7 +429,7 @@ static void build_append_pci_bus_devices(Aml
> *parent_scope, PCIBus *bus,
>
> >> method = aml_method("_S3D", 0, AML_NOTSERIALIZED);
>
> >> aml_append(method, aml_return(aml_int(s3d)));
>
> >> aml_append(dev, method);
>
> >> - } else if (hotplug_enabled_dev) {
>
> >> + } else if (hotplug_enabled_dev && !root_pcihp_disabled) {
>
> >> /* add _SUN/_EJ0 to make slot hotpluggable */
>
> >> aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
>
> >>
>
> >> @@ -439,7 +449,8 @@ static void build_append_pci_bus_devices(Aml
> *parent_scope, PCIBus *bus,
>
> >> */
>
> >> PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
>
> >>
>
> >> - build_append_pci_bus_devices(dev, sec_bus,
> pcihp_bridge_en);
>
> >> + build_append_pci_bus_devices(dev, sec_bus, pcihp_bridge_en,
>
> >> + pcihp_root_en);
>
> >> }
>
> >> /* slot descriptor has been composed, add it into parent
> context */
>
> >> aml_append(parent_scope, dev);
>
> >> @@ -1818,7 +1829,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>
> >> if (bus) {
>
> >> Aml *scope = aml_scope("PCI0");
>
> >> /* Scan all PCI buses. Generate tables to support hotplug.
> */
>
> >> - build_append_pci_bus_devices(scope, bus,
> pm->pcihp_bridge_en);
>
> >> + build_append_pci_bus_devices(scope, bus,
> pm->pcihp_bridge_en,
>
> >> + pm->pcihp_root_en);
>
> >>
>
> >> if (TPM_IS_TIS_ISA(tpm)) {
>
> >> if (misc->tpm_version == TPM_VERSION_2_0) {
>
> >> --
>
> >> 2.7.4
>
> >>
>
> >
>
>
>
>
[-- Attachment #2: Type: text/html, Size: 8685 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2] Introduce a new flag for piix to disable root bus PCI hotplug
2020-08-18 10:19 ` Ani Sinha
@ 2020-08-18 10:46 ` Ani Sinha
0 siblings, 0 replies; 7+ messages in thread
From: Ani Sinha @ 2020-08-18 10:46 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Eduardo Habkost, Michael S. Tsirkin, qemu-devel,
Aleksandar Markovic, Paolo Bonzini, Igor Mammedov, Aurelien Jarno,
Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 7273 bytes --]
> On Aug 18, 2020, at 3:49 PM, Ani Sinha <ani@anisinha.ca> wrote:
>
>
> This is not a workaround, this is a feature we need badly. We have discussed this at length. Igor suggested we have one additional global flag to disable all pci hotplug. But then that leads to strange case when pci hotplug on the bridges are enabled (we have a separate flag for that). Hence, I think we can simply add another flag to disable hotplug just for the pci root bus.
Relevant thread: https://www.mail-archive.com/qemu-devel@nongnu.org/msg729072.html
>> On Tue, Aug 18, 2020 at 3:46 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>> Hi,
>>
>>
>>
>> On 8/18/20 11:54 AM, Ani Sinha wrote:
>>
>> > Igor etc, I just tested this patch using a Windows 2012 R2 image and
>>
>> > it seems to be working. Any feedbacks on this patch?
>>
>> >
>>
>> > On Tue, Aug 11, 2020 at 6:42 PM Ani Sinha <ani@anisinha.ca> wrote:
>>
>> >>
>>
>> >> We introduce a new global flag for PIIX with which we can turn on or off PCI
>>
>> >> device hotplug on the root bus. This flag can be used to prevent all PCI
>>
>> >> devices from getting hotplugged or unplugged from the root PCI bus.
>>
>> >>
>>
>> >> Signed-off-by: Ani Sinha <ani@anisinha.ca>
>>
>> >> ---
>>
>> >> hw/acpi/piix4.c | 3 +++
>>
>> >> hw/i386/acpi-build.c | 20 ++++++++++++++++----
>>
>> >> 2 files changed, 19 insertions(+), 4 deletions(-)
>>
>> >>
>>
>> >> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
>>
>> >> index 26bac4f..94ec35a 100644
>>
>> >> --- a/hw/acpi/piix4.c
>>
>> >> +++ b/hw/acpi/piix4.c
>>
>> >> @@ -78,6 +78,7 @@ typedef struct PIIX4PMState {
>>
>> >>
>>
>> >> AcpiPciHpState acpi_pci_hotplug;
>>
>> >> bool use_acpi_hotplug_bridge;
>>
>> >> + bool use_acpi_root_pci_hotplug;
>>
>> >>
>>
>> >> uint8_t disable_s3;
>>
>> >> uint8_t disable_s4;
>>
>> >> @@ -635,6 +636,8 @@ static Property piix4_pm_properties[] = {
>>
>> >> DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_VAL, PIIX4PMState, s4_val, 2),
>>
>> >> DEFINE_PROP_BOOL("acpi-pci-hotplug-with-bridge-support", PIIX4PMState,
>>
>> >> use_acpi_hotplug_bridge, true),
>>
>> >> + DEFINE_PROP_BOOL("acpi-root-pci-hotplug", PIIX4PMState,
>>
>> >> + use_acpi_root_pci_hotplug, true),
>>
>>
>>
>> From what I understood here, this file is already pretty twisted
>>
>> and Igor doesn't want more workarounds:
>>
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg690564.html
>>
>>
>>
>> ¯\_(ツ)_/¯
>>
>>
>>
>> >> DEFINE_PROP_BOOL("memory-hotplug-support", PIIX4PMState,
>>
>> >> acpi_memory_hotplug.is_enabled, true),
>>
>> >> DEFINE_PROP_END_OF_LIST(),
>>
>> >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>>
>> >> index b7bcbbb..a82e5c1 100644
>>
>> >> --- a/hw/i386/acpi-build.c
>>
>> >> +++ b/hw/i386/acpi-build.c
>>
>> >> @@ -95,6 +95,7 @@ typedef struct AcpiPmInfo {
>>
>> >> bool s3_disabled;
>>
>> >> bool s4_disabled;
>>
>> >> bool pcihp_bridge_en;
>>
>> >> + bool pcihp_root_en;
>>
>> >> uint8_t s4_val;
>>
>> >> AcpiFadtData fadt;
>>
>> >> uint16_t cpu_hp_io_base;
>>
>> >> @@ -245,6 +246,9 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
>>
>> >> pm->pcihp_bridge_en =
>>
>> >> object_property_get_bool(obj, "acpi-pci-hotplug-with-bridge-support",
>>
>> >> NULL);
>>
>> >> + pm->pcihp_root_en =
>>
>> >> + object_property_get_bool(obj, "acpi-root-pci-hotplug", NULL);
>>
>> >> +
>>
>> >> }
>>
>> >>
>>
>> >> static void acpi_get_misc_info(AcpiMiscInfo *info)
>>
>> >> @@ -337,12 +341,15 @@ static void build_append_pcihp_notify_entry(Aml *method, int slot)
>>
>> >> }
>>
>> >>
>>
>> >> static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
>>
>> >> - bool pcihp_bridge_en)
>>
>> >> + bool pcihp_bridge_en,
>>
>> >> + bool pcihp_root_en)
>>
>> >> {
>>
>> >> Aml *dev, *notify_method = NULL, *method;
>>
>> >> QObject *bsel;
>>
>> >> PCIBus *sec;
>>
>> >> int i;
>>
>> >> + bool root_bus = pci_bus_is_root(bus);
>>
>> >> + bool root_pcihp_disabled = (root_bus && !pcihp_root_en);
>>
>> >>
>>
>> >> bsel = object_property_get_qobject(OBJECT(bus), ACPI_PCIHP_PROP_BSEL, NULL);
>>
>> >> if (bsel) {
>>
>> >> @@ -361,6 +368,9 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
>>
>> >> bool bridge_in_acpi;
>>
>> >>
>>
>> >> if (!pdev) {
>>
>> >> + /* skip if pci hotplug for the root bus is disabled */
>>
>> >> + if (root_pcihp_disabled)
>>
>> >> + continue;
>>
>> >> if (bsel) { /* add hotplug slots for non present devices */
>>
>> >> dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
>>
>> >> aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
>>
>> >> @@ -419,7 +429,7 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
>>
>> >> method = aml_method("_S3D", 0, AML_NOTSERIALIZED);
>>
>> >> aml_append(method, aml_return(aml_int(s3d)));
>>
>> >> aml_append(dev, method);
>>
>> >> - } else if (hotplug_enabled_dev) {
>>
>> >> + } else if (hotplug_enabled_dev && !root_pcihp_disabled) {
>>
>> >> /* add _SUN/_EJ0 to make slot hotpluggable */
>>
>> >> aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
>>
>> >>
>>
>> >> @@ -439,7 +449,8 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
>>
>> >> */
>>
>> >> PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
>>
>> >>
>>
>> >> - build_append_pci_bus_devices(dev, sec_bus, pcihp_bridge_en);
>>
>> >> + build_append_pci_bus_devices(dev, sec_bus, pcihp_bridge_en,
>>
>> >> + pcihp_root_en);
>>
>> >> }
>>
>> >> /* slot descriptor has been composed, add it into parent context */
>>
>> >> aml_append(parent_scope, dev);
>>
>> >> @@ -1818,7 +1829,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>>
>> >> if (bus) {
>>
>> >> Aml *scope = aml_scope("PCI0");
>>
>> >> /* Scan all PCI buses. Generate tables to support hotplug. */
>>
>> >> - build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en);
>>
>> >> + build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en,
>>
>> >> + pm->pcihp_root_en);
>>
>> >>
>>
>> >> if (TPM_IS_TIS_ISA(tpm)) {
>>
>> >> if (misc->tpm_version == TPM_VERSION_2_0) {
>>
>> >> --
>>
>> >> 2.7.4
>>
>> >>
>>
>> >
>>
>>
>>
[-- Attachment #2: Type: text/html, Size: 10961 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2] Introduce a new flag for piix to disable root bus PCI hotplug
2020-08-11 13:12 [PATCH V2] Introduce a new flag for piix to disable root bus PCI hotplug Ani Sinha
2020-08-18 9:54 ` Ani Sinha
@ 2020-08-19 10:00 ` Igor Mammedov
2020-08-19 10:36 ` Ani Sinha
1 sibling, 1 reply; 7+ messages in thread
From: Igor Mammedov @ 2020-08-19 10:00 UTC (permalink / raw)
To: Ani Sinha
Cc: Eduardo Habkost, Michael S. Tsirkin, qemu-devel,
Aleksandar Markovic, Paolo Bonzini, Philippe Mathieu-Daudé,
Aurelien Jarno, Richard Henderson
On Tue, 11 Aug 2020 18:42:08 +0530
Ani Sinha <ani@anisinha.ca> wrote:
> We introduce a new global flag for PIIX with which we can turn on or off PCI
> device hotplug on the root bus. This flag can be used to prevent all PCI
> devices from getting hotplugged or unplugged from the root PCI bus.
Tested-by: Igor Mammedov <imammedo@redhat.com>
somewhere in intial versions there were mentionig why we are doing it
(i.e for Windows sake because ...)
I suggest to add it to commit message so reason for this won't be lost.
Also giving example how to use option in commit message would be good.
with this changes:
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>
> Signed-off-by: Ani Sinha <ani@anisinha.ca>
> ---
> hw/acpi/piix4.c | 3 +++
> hw/i386/acpi-build.c | 20 ++++++++++++++++----
> 2 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index 26bac4f..94ec35a 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -78,6 +78,7 @@ typedef struct PIIX4PMState {
>
> AcpiPciHpState acpi_pci_hotplug;
> bool use_acpi_hotplug_bridge;
> + bool use_acpi_root_pci_hotplug;
>
> uint8_t disable_s3;
> uint8_t disable_s4;
> @@ -635,6 +636,8 @@ static Property piix4_pm_properties[] = {
> DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_VAL, PIIX4PMState, s4_val, 2),
> DEFINE_PROP_BOOL("acpi-pci-hotplug-with-bridge-support", PIIX4PMState,
> use_acpi_hotplug_bridge, true),
> + DEFINE_PROP_BOOL("acpi-root-pci-hotplug", PIIX4PMState,
> + use_acpi_root_pci_hotplug, true),
> DEFINE_PROP_BOOL("memory-hotplug-support", PIIX4PMState,
> acpi_memory_hotplug.is_enabled, true),
> DEFINE_PROP_END_OF_LIST(),
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index b7bcbbb..a82e5c1 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -95,6 +95,7 @@ typedef struct AcpiPmInfo {
> bool s3_disabled;
> bool s4_disabled;
> bool pcihp_bridge_en;
> + bool pcihp_root_en;
> uint8_t s4_val;
> AcpiFadtData fadt;
> uint16_t cpu_hp_io_base;
> @@ -245,6 +246,9 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
> pm->pcihp_bridge_en =
> object_property_get_bool(obj, "acpi-pci-hotplug-with-bridge-support",
> NULL);
> + pm->pcihp_root_en =
> + object_property_get_bool(obj, "acpi-root-pci-hotplug", NULL);
> +
> }
>
> static void acpi_get_misc_info(AcpiMiscInfo *info)
> @@ -337,12 +341,15 @@ static void build_append_pcihp_notify_entry(Aml *method, int slot)
> }
>
> static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> - bool pcihp_bridge_en)
> + bool pcihp_bridge_en,
> + bool pcihp_root_en)
> {
> Aml *dev, *notify_method = NULL, *method;
> QObject *bsel;
> PCIBus *sec;
> int i;
> + bool root_bus = pci_bus_is_root(bus);
> + bool root_pcihp_disabled = (root_bus && !pcihp_root_en);
>
> bsel = object_property_get_qobject(OBJECT(bus), ACPI_PCIHP_PROP_BSEL, NULL);
> if (bsel) {
> @@ -361,6 +368,9 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> bool bridge_in_acpi;
>
> if (!pdev) {
> + /* skip if pci hotplug for the root bus is disabled */
> + if (root_pcihp_disabled)
> + continue;
> if (bsel) { /* add hotplug slots for non present devices */
> dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
> aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
> @@ -419,7 +429,7 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> method = aml_method("_S3D", 0, AML_NOTSERIALIZED);
> aml_append(method, aml_return(aml_int(s3d)));
> aml_append(dev, method);
> - } else if (hotplug_enabled_dev) {
> + } else if (hotplug_enabled_dev && !root_pcihp_disabled) {
> /* add _SUN/_EJ0 to make slot hotpluggable */
> aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
>
> @@ -439,7 +449,8 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> */
> PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
>
> - build_append_pci_bus_devices(dev, sec_bus, pcihp_bridge_en);
> + build_append_pci_bus_devices(dev, sec_bus, pcihp_bridge_en,
> + pcihp_root_en);
> }
> /* slot descriptor has been composed, add it into parent context */
> aml_append(parent_scope, dev);
> @@ -1818,7 +1829,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> if (bus) {
> Aml *scope = aml_scope("PCI0");
> /* Scan all PCI buses. Generate tables to support hotplug. */
> - build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en);
> + build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en,
> + pm->pcihp_root_en);
>
> if (TPM_IS_TIS_ISA(tpm)) {
> if (misc->tpm_version == TPM_VERSION_2_0) {
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2] Introduce a new flag for piix to disable root bus PCI hotplug
2020-08-19 10:00 ` Igor Mammedov
@ 2020-08-19 10:36 ` Ani Sinha
0 siblings, 0 replies; 7+ messages in thread
From: Ani Sinha @ 2020-08-19 10:36 UTC (permalink / raw)
To: Igor Mammedov
Cc: Eduardo Habkost, Michael S. Tsirkin, qemu-devel,
Aleksandar Markovic, Paolo Bonzini, Philippe Mathieu-Daudé,
Aurelien Jarno, Richard Henderson
On Wed, Aug 19, 2020 at 3:30 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Tue, 11 Aug 2020 18:42:08 +0530
> Ani Sinha <ani@anisinha.ca> wrote:
>
> > We introduce a new global flag for PIIX with which we can turn on or off PCI
> > device hotplug on the root bus. This flag can be used to prevent all PCI
> > devices from getting hotplugged or unplugged from the root PCI bus.
>
> Tested-by: Igor Mammedov <imammedo@redhat.com>
>
> somewhere in intial versions there were mentionig why we are doing it
> (i.e for Windows sake because ...)
> I suggest to add it to commit message so reason for this won't be lost.
> Also giving example how to use option in commit message would be good.
>
> with this changes:
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Thanks Igor, I have sent a V4 with your suggestions to the commit
message and some more
rework on the patch. The V4 has again been tested by me in the same
setup. Looks good so far.
>
>
>
> >
> > Signed-off-by: Ani Sinha <ani@anisinha.ca>
> > ---
> > hw/acpi/piix4.c | 3 +++
> > hw/i386/acpi-build.c | 20 ++++++++++++++++----
> > 2 files changed, 19 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> > index 26bac4f..94ec35a 100644
> > --- a/hw/acpi/piix4.c
> > +++ b/hw/acpi/piix4.c
> > @@ -78,6 +78,7 @@ typedef struct PIIX4PMState {
> >
> > AcpiPciHpState acpi_pci_hotplug;
> > bool use_acpi_hotplug_bridge;
> > + bool use_acpi_root_pci_hotplug;
> >
> > uint8_t disable_s3;
> > uint8_t disable_s4;
> > @@ -635,6 +636,8 @@ static Property piix4_pm_properties[] = {
> > DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_VAL, PIIX4PMState, s4_val, 2),
> > DEFINE_PROP_BOOL("acpi-pci-hotplug-with-bridge-support", PIIX4PMState,
> > use_acpi_hotplug_bridge, true),
> > + DEFINE_PROP_BOOL("acpi-root-pci-hotplug", PIIX4PMState,
> > + use_acpi_root_pci_hotplug, true),
> > DEFINE_PROP_BOOL("memory-hotplug-support", PIIX4PMState,
> > acpi_memory_hotplug.is_enabled, true),
> > DEFINE_PROP_END_OF_LIST(),
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index b7bcbbb..a82e5c1 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -95,6 +95,7 @@ typedef struct AcpiPmInfo {
> > bool s3_disabled;
> > bool s4_disabled;
> > bool pcihp_bridge_en;
> > + bool pcihp_root_en;
> > uint8_t s4_val;
> > AcpiFadtData fadt;
> > uint16_t cpu_hp_io_base;
> > @@ -245,6 +246,9 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
> > pm->pcihp_bridge_en =
> > object_property_get_bool(obj, "acpi-pci-hotplug-with-bridge-support",
> > NULL);
> > + pm->pcihp_root_en =
> > + object_property_get_bool(obj, "acpi-root-pci-hotplug", NULL);
> > +
> > }
> >
> > static void acpi_get_misc_info(AcpiMiscInfo *info)
> > @@ -337,12 +341,15 @@ static void build_append_pcihp_notify_entry(Aml *method, int slot)
> > }
> >
> > static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> > - bool pcihp_bridge_en)
> > + bool pcihp_bridge_en,
> > + bool pcihp_root_en)
> > {
> > Aml *dev, *notify_method = NULL, *method;
> > QObject *bsel;
> > PCIBus *sec;
> > int i;
> > + bool root_bus = pci_bus_is_root(bus);
> > + bool root_pcihp_disabled = (root_bus && !pcihp_root_en);
> >
> > bsel = object_property_get_qobject(OBJECT(bus), ACPI_PCIHP_PROP_BSEL, NULL);
> > if (bsel) {
> > @@ -361,6 +368,9 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> > bool bridge_in_acpi;
> >
> > if (!pdev) {
> > + /* skip if pci hotplug for the root bus is disabled */
> > + if (root_pcihp_disabled)
> > + continue;
> > if (bsel) { /* add hotplug slots for non present devices */
> > dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
> > aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
> > @@ -419,7 +429,7 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> > method = aml_method("_S3D", 0, AML_NOTSERIALIZED);
> > aml_append(method, aml_return(aml_int(s3d)));
> > aml_append(dev, method);
> > - } else if (hotplug_enabled_dev) {
> > + } else if (hotplug_enabled_dev && !root_pcihp_disabled) {
> > /* add _SUN/_EJ0 to make slot hotpluggable */
> > aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
> >
> > @@ -439,7 +449,8 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> > */
> > PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
> >
> > - build_append_pci_bus_devices(dev, sec_bus, pcihp_bridge_en);
> > + build_append_pci_bus_devices(dev, sec_bus, pcihp_bridge_en,
> > + pcihp_root_en);
> > }
> > /* slot descriptor has been composed, add it into parent context */
> > aml_append(parent_scope, dev);
> > @@ -1818,7 +1829,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > if (bus) {
> > Aml *scope = aml_scope("PCI0");
> > /* Scan all PCI buses. Generate tables to support hotplug. */
> > - build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en);
> > + build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en,
> > + pm->pcihp_root_en);
> >
> > if (TPM_IS_TIS_ISA(tpm)) {
> > if (misc->tpm_version == TPM_VERSION_2_0) {
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-08-19 10:37 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-11 13:12 [PATCH V2] Introduce a new flag for piix to disable root bus PCI hotplug Ani Sinha
2020-08-18 9:54 ` Ani Sinha
2020-08-18 10:16 ` Philippe Mathieu-Daudé
2020-08-18 10:19 ` Ani Sinha
2020-08-18 10:46 ` Ani Sinha
2020-08-19 10:00 ` Igor Mammedov
2020-08-19 10:36 ` Ani Sinha
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).