qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* RFC [PATCH] piix4: don't reserve hw resources when hotplug is off
@ 2020-09-01  9:49 Ani Sinha
  2020-09-01  9:49 ` [PATCH] piix4: don't reserve hw resources when hotplug is off globally Ani Sinha
  0 siblings, 1 reply; 5+ messages in thread
From: Ani Sinha @ 2020-09-01  9:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: Eduardo Habkost, Michael S. Tsirkin, jusual, Aleksandar Markovic,
	Paolo Bonzini, Igor Mammedov, Philippe Mathieu-Daudé,
	Aurelien Jarno, Richard Henderson

This is a RFC patch. This patch attempts to disable all acpi hotplug related hardware
initialization and amls when hotplug for both pci root port and pci bridges are
disabled on i440fx machines.

Thanks for looking at it and please provide comments. I will add unit tests when the final
patch is ready and accepted.

--Ani




^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] piix4: don't reserve hw resources when hotplug is off globally
  2020-09-01  9:49 RFC [PATCH] piix4: don't reserve hw resources when hotplug is off Ani Sinha
@ 2020-09-01  9:49 ` Ani Sinha
  2020-09-02 15:22   ` Ani Sinha
  2020-09-08 14:04   ` Michael S. Tsirkin
  0 siblings, 2 replies; 5+ messages in thread
From: Ani Sinha @ 2020-09-01  9:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: Eduardo Habkost, Michael S. Tsirkin, jusual, Aleksandar Markovic,
	Paolo Bonzini, Ani Sinha, Igor Mammedov,
	Philippe Mathieu-Daudé, Aurelien Jarno, Richard Henderson

When acpi hotplug is turned off for both root pci bus as well as for pci
bridges, we should not generate the related amls 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.

Signed-off-by: Ani Sinha <ani@anisinha.ca>
---
 hw/acpi/piix4.c      |  6 ++++--
 hw/i386/acpi-build.c | 10 ++++++++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index e6163bb6ce..b70b1f98af 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -596,8 +596,10 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
                           "acpi-gpe0", GPE_LEN);
     memory_region_add_subregion(parent, GPE_BASE, &s->io_gpe);
 
-    acpi_pcihp_init(OBJECT(s), &s->acpi_pci_hotplug, bus, parent,
-                    s->use_acpi_hotplug_bridge);
+    if (s->use_acpi_hotplug_bridge || s->use_acpi_root_pci_hotplug) {
+        acpi_pcihp_init(OBJECT(s), &s->acpi_pci_hotplug, bus, parent,
+                        s->use_acpi_hotplug_bridge);
+    }
 
     s->cpu_hotplug_legacy = true;
     object_property_add_bool(OBJECT(s), "cpu-hotplug-legacy",
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b7bcbbbb2a..5365b3d290 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)
@@ -1504,7 +1508,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         build_hpet_aml(dsdt);
         build_piix4_isa_bridge(dsdt);
         build_isa_devices_aml(dsdt);
-        build_piix4_pci_hotplug(dsdt);
+        if (pm->pcihp_bridge_en || pm->pcihp_root_en) {
+            build_piix4_pci_hotplug(dsdt);
+        }
         build_piix4_pci0_int(dsdt);
     } else {
         sb_scope = aml_scope("_SB");
@@ -1698,7 +1704,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
     crs_range_set_free(&crs_range_set);
 
     /* reserve PCIHP resources */
-    if (pm->pcihp_io_len) {
+    if (pm->pcihp_io_len && (pm->pcihp_bridge_en || pm->pcihp_root_en)) {
         dev = aml_device("PHPR");
         aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A06")));
         aml_append(dev,
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] piix4: don't reserve hw resources when hotplug is off globally
  2020-09-01  9:49 ` [PATCH] piix4: don't reserve hw resources when hotplug is off globally Ani Sinha
@ 2020-09-02 15:22   ` Ani Sinha
  2020-09-08 14:04   ` Michael S. Tsirkin
  1 sibling, 0 replies; 5+ messages in thread
From: Ani Sinha @ 2020-09-02 15:22 UTC (permalink / raw)
  To: qemu-devel
  Cc: Eduardo Habkost, Michael S. Tsirkin, jusual, Aleksandar Markovic,
	Paolo Bonzini, Igor Mammedov, Philippe Mathieu-Daudé,
	Aurelien Jarno, Richard Henderson

[-- Attachment #1: Type: text/plain, Size: 2708 bytes --]

Please provide comments for this patch.
On Sep 1, 2020, 15:20 +0530, Ani Sinha <ani@anisinha.ca>, wrote:
> When acpi hotplug is turned off for both root pci bus as well as for pci
> bridges, we should not generate the related amls 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.
>
> Signed-off-by: Ani Sinha <ani@anisinha.ca>
> ---
> hw/acpi/piix4.c | 6 ++++--
> hw/i386/acpi-build.c | 10 ++++++++--
> 2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index e6163bb6ce..b70b1f98af 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -596,8 +596,10 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
> "acpi-gpe0", GPE_LEN);
> memory_region_add_subregion(parent, GPE_BASE, &s->io_gpe);
>
> - acpi_pcihp_init(OBJECT(s), &s->acpi_pci_hotplug, bus, parent,
> - s->use_acpi_hotplug_bridge);
> + if (s->use_acpi_hotplug_bridge || s->use_acpi_root_pci_hotplug) {
> + acpi_pcihp_init(OBJECT(s), &s->acpi_pci_hotplug, bus, parent,
> + s->use_acpi_hotplug_bridge);
> + }
>
> s->cpu_hotplug_legacy = true;
> object_property_add_bool(OBJECT(s), "cpu-hotplug-legacy",
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index b7bcbbbb2a..5365b3d290 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)
> @@ -1504,7 +1508,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> build_hpet_aml(dsdt);
> build_piix4_isa_bridge(dsdt);
> build_isa_devices_aml(dsdt);
> - build_piix4_pci_hotplug(dsdt);
> + if (pm->pcihp_bridge_en || pm->pcihp_root_en) {
> + build_piix4_pci_hotplug(dsdt);
> + }
> build_piix4_pci0_int(dsdt);
> } else {
> sb_scope = aml_scope("_SB");
> @@ -1698,7 +1704,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> crs_range_set_free(&crs_range_set);
>
> /* reserve PCIHP resources */
> - if (pm->pcihp_io_len) {
> + if (pm->pcihp_io_len && (pm->pcihp_bridge_en || pm->pcihp_root_en)) {
> dev = aml_device("PHPR");
> aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A06")));
> aml_append(dev,
> --
> 2.17.1
>

[-- Attachment #2: Type: text/html, Size: 3537 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] piix4: don't reserve hw resources when hotplug is off globally
  2020-09-01  9:49 ` [PATCH] piix4: don't reserve hw resources when hotplug is off globally Ani Sinha
  2020-09-02 15:22   ` Ani Sinha
@ 2020-09-08 14:04   ` Michael S. Tsirkin
  2020-09-08 14:53     ` Ani Sinha
  1 sibling, 1 reply; 5+ messages in thread
From: Michael S. Tsirkin @ 2020-09-08 14:04 UTC (permalink / raw)
  To: Ani Sinha
  Cc: Eduardo Habkost, jusual, qemu-devel, Aleksandar Markovic,
	Paolo Bonzini, Igor Mammedov, Philippe Mathieu-Daudé,
	Aurelien Jarno, Richard Henderson

On Tue, Sep 01, 2020 at 03:19:22PM +0530, Ani Sinha wrote:
> When acpi hotplug is turned off for both root pci bus as well as for pci
> bridges, we should not generate the related amls 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.
> 
> Signed-off-by: Ani Sinha <ani@anisinha.ca>

Queued. in the future pls do not reply to patch with a new version.

Thanks!

> ---
>  hw/acpi/piix4.c      |  6 ++++--
>  hw/i386/acpi-build.c | 10 ++++++++--
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index e6163bb6ce..b70b1f98af 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -596,8 +596,10 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
>                            "acpi-gpe0", GPE_LEN);
>      memory_region_add_subregion(parent, GPE_BASE, &s->io_gpe);
>  
> -    acpi_pcihp_init(OBJECT(s), &s->acpi_pci_hotplug, bus, parent,
> -                    s->use_acpi_hotplug_bridge);
> +    if (s->use_acpi_hotplug_bridge || s->use_acpi_root_pci_hotplug) {
> +        acpi_pcihp_init(OBJECT(s), &s->acpi_pci_hotplug, bus, parent,
> +                        s->use_acpi_hotplug_bridge);
> +    }
>  
>      s->cpu_hotplug_legacy = true;
>      object_property_add_bool(OBJECT(s), "cpu-hotplug-legacy",
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index b7bcbbbb2a..5365b3d290 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)
> @@ -1504,7 +1508,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>          build_hpet_aml(dsdt);
>          build_piix4_isa_bridge(dsdt);
>          build_isa_devices_aml(dsdt);
> -        build_piix4_pci_hotplug(dsdt);
> +        if (pm->pcihp_bridge_en || pm->pcihp_root_en) {
> +            build_piix4_pci_hotplug(dsdt);
> +        }
>          build_piix4_pci0_int(dsdt);
>      } else {
>          sb_scope = aml_scope("_SB");
> @@ -1698,7 +1704,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>      crs_range_set_free(&crs_range_set);
>  
>      /* reserve PCIHP resources */
> -    if (pm->pcihp_io_len) {
> +    if (pm->pcihp_io_len && (pm->pcihp_bridge_en || pm->pcihp_root_en)) {
>          dev = aml_device("PHPR");
>          aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A06")));
>          aml_append(dev,
> -- 
> 2.17.1



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] piix4: don't reserve hw resources when hotplug is off globally
  2020-09-08 14:04   ` Michael S. Tsirkin
@ 2020-09-08 14:53     ` Ani Sinha
  0 siblings, 0 replies; 5+ messages in thread
From: Ani Sinha @ 2020-09-08 14:53 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Eduardo Habkost, jusual, qemu-devel, Aleksandar Markovic,
	Paolo Bonzini, Igor Mammedov, Philippe Mathieu-Daudé,
	Aurelien Jarno, Richard Henderson

[-- Attachment #1: Type: text/plain, Size: 3108 bytes --]

On Sep 8, 2020, 19:35 +0530, Michael S. Tsirkin <mst@redhat.com>, wrote:
> On Tue, Sep 01, 2020 at 03:19:22PM +0530, Ani Sinha wrote:
> > When acpi hotplug is turned off for both root pci bus as well as for pci
> > bridges, we should not generate the related amls 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.
> >
> > Signed-off-by: Ani Sinha <ani@anisinha.ca>
>
> Queued. in the future pls do not reply to patch with a new version.

What new version? This is the only version for this patch.

You must be getting confused with some other patch I sent. I sent quite a few.

>
> Thanks!
>
> > ---
> > hw/acpi/piix4.c | 6 ++++--
> > hw/i386/acpi-build.c | 10 ++++++++--
> > 2 files changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> > index e6163bb6ce..b70b1f98af 100644
> > --- a/hw/acpi/piix4.c
> > +++ b/hw/acpi/piix4.c
> > @@ -596,8 +596,10 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
> > "acpi-gpe0", GPE_LEN);
> > memory_region_add_subregion(parent, GPE_BASE, &s->io_gpe);
> >
> > - acpi_pcihp_init(OBJECT(s), &s->acpi_pci_hotplug, bus, parent,
> > - s->use_acpi_hotplug_bridge);
> > + if (s->use_acpi_hotplug_bridge || s->use_acpi_root_pci_hotplug) {
> > + acpi_pcihp_init(OBJECT(s), &s->acpi_pci_hotplug, bus, parent,
> > + s->use_acpi_hotplug_bridge);
> > + }
> >
> > s->cpu_hotplug_legacy = true;
> > object_property_add_bool(OBJECT(s), "cpu-hotplug-legacy",
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index b7bcbbbb2a..5365b3d290 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)
> > @@ -1504,7 +1508,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > build_hpet_aml(dsdt);
> > build_piix4_isa_bridge(dsdt);
> > build_isa_devices_aml(dsdt);
> > - build_piix4_pci_hotplug(dsdt);
> > + if (pm->pcihp_bridge_en || pm->pcihp_root_en) {
> > + build_piix4_pci_hotplug(dsdt);
> > + }
> > build_piix4_pci0_int(dsdt);
> > } else {
> > sb_scope = aml_scope("_SB");
> > @@ -1698,7 +1704,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > crs_range_set_free(&crs_range_set);
> >
> > /* reserve PCIHP resources */
> > - if (pm->pcihp_io_len) {
> > + if (pm->pcihp_io_len && (pm->pcihp_bridge_en || pm->pcihp_root_en)) {
> > dev = aml_device("PHPR");
> > aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A06")));
> > aml_append(dev,
> > --
> > 2.17.1
>

[-- Attachment #2: Type: text/html, Size: 30046 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-09-08 14:54 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-01  9:49 RFC [PATCH] piix4: don't reserve hw resources when hotplug is off Ani Sinha
2020-09-01  9:49 ` [PATCH] piix4: don't reserve hw resources when hotplug is off globally Ani Sinha
2020-09-02 15:22   ` Ani Sinha
2020-09-08 14:04   ` Michael S. Tsirkin
2020-09-08 14:53     ` 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).