[Qemu-devel] [PATCH V4 0/2] hw/pxb-pcie: fix PCI Express hotplug support

[Qemu-devel] [PATCH V4 0/2] hw/pxb-pcie: fix PCI Express hotplug support

[Marcel Apfelbaum]

Note to maintainer:
 Please update ACPI test files.

V3 -> V4:
      - Added patch to simplify _OSC (Michael)

V2 -> V3:
      - Modified comment (Igor)

V1 -> V2:
    Addressed Michael S. Tsirkin's comments:
      - Added documentation to q35 osc function
      - Made _osc serialized. I did not add compat property
        since it seems guest OSs do not care anyway about the OSC
        being serialized.
      - Kept the SUPP field even if is not used and also
        left both SUPP and CTRL out of the _osc because all
        systems I checked and the ACPI spec keep them that way,
        maybe is some kind of documentation contract.

Add the missing osc method for pxb-pcie devices as APCI spec recommends

Thanks,
Marcel

Marcel Apfelbaum (1):
  hw/pxb-pcie: fix PCI Express hotplug support

Michael S. Tsirkin (1):
  acpi: simplify _OSC

 dtc                  | 2 +-
 hw/i386/acpi-build.c | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

-- 
2.5.5

[Qemu-devel] [PATCH V4 1/2] acpi: simplify _OSC

[Marcel Apfelbaum]

From: "Michael S. Tsirkin" <mst@redhat.com>

Our _OSC method has a bunch of unused code loading data
into external CTRL and SUPP fields which are then never
used. Drop this in favor of a single local variable.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/acpi-build.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 1c928ab..1da6d9f 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1803,7 +1803,7 @@ static Aml *build_q35_osc_method(void)
     Aml *else_ctx;
     Aml *method;
     Aml *a_cwd1 = aml_name("CDW1");
-    Aml *a_ctrl = aml_name("CTRL");
+    Aml *a_ctrl = aml_local(0);
 
     method = aml_method("_OSC", 4, AML_NOTSERIALIZED);
     aml_append(method, aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1"));
@@ -1813,7 +1813,6 @@ static Aml *build_q35_osc_method(void)
     aml_append(if_ctx, aml_create_dword_field(aml_arg(3), aml_int(4), "CDW2"));
     aml_append(if_ctx, aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
 
-    aml_append(if_ctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
     aml_append(if_ctx, aml_store(aml_name("CDW3"), a_ctrl));
 
     /*
@@ -1898,8 +1897,6 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
         aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
         aml_append(dev, aml_name_decl("_UID", aml_int(1)));
-        aml_append(dev, aml_name_decl("SUPP", aml_int(0)));
-        aml_append(dev, aml_name_decl("CTRL", aml_int(0)));
         aml_append(dev, build_q35_osc_method());
         aml_append(sb_scope, dev);
         aml_append(dsdt, sb_scope);
-- 
2.5.5

[Qemu-devel] [PATCH V4 2/2] hw/pxb-pcie: fix PCI Express hotplug support

[Marcel Apfelbaum]

Add the missing osc method for pxb-pcie devices as APCI spec recommends,
see 6.2.9.1 OSC Implementation Example for PCI Host Bridge Devices, ACPI 3.0a:

    It is recommended that a machine with multiple host bridge devices
    should report the same capabilities for all host bridges, and also
    negotiate control of the features described in the Control Field in
    the same way for all host bridges.

Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
 dtc                  | 2 +-
 hw/i386/acpi-build.c | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/dtc b/dtc
index ec02b34..65cc4d2 160000
--- a/dtc
+++ b/dtc
@@ -1 +1 @@
-Subproject commit ec02b34c05be04f249ffaaca4b666f5246877dea
+Subproject commit 65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 1da6d9f..04029f6 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1961,6 +1961,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
             aml_append(dev, aml_name_decl("_UID", aml_int(bus_num)));
             aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
             aml_append(dev, aml_name_decl("_BBN", aml_int(bus_num)));
+            if (pci_bus_is_express(bus)) {
+                aml_append(dev, build_q35_osc_method());
+            }
 
             if (numa_node != NUMA_NODE_UNASSIGNED) {
                 aml_append(dev, aml_name_decl("_PXM", aml_int(numa_node)));
-- 
2.5.5

Re: [Qemu-devel] [PATCH V4 1/2] acpi: simplify _OSC

[Igor Mammedov]

On Tue, 28 Feb 2017 16:13:28 +0200
Marcel Apfelbaum <marcel@redhat.com> wrote:

> From: "Michael S. Tsirkin" <mst@redhat.com>
> 
> Our _OSC method has a bunch of unused code loading data
> into external CTRL and SUPP fields which are then never
> used. Drop this in favor of a single local variable.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/i386/acpi-build.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 1c928ab..1da6d9f 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1803,7 +1803,7 @@ static Aml *build_q35_osc_method(void)
>      Aml *else_ctx;
>      Aml *method;
>      Aml *a_cwd1 = aml_name("CDW1");
> -    Aml *a_ctrl = aml_name("CTRL");
> +    Aml *a_ctrl = aml_local(0);
>  
>      method = aml_method("_OSC", 4, AML_NOTSERIALIZED);
>      aml_append(method, aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1"));
> @@ -1813,7 +1813,6 @@ static Aml *build_q35_osc_method(void)
>      aml_append(if_ctx, aml_create_dword_field(aml_arg(3), aml_int(4), "CDW2"));
>      aml_append(if_ctx, aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
>  
> -    aml_append(if_ctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
>      aml_append(if_ctx, aml_store(aml_name("CDW3"), a_ctrl));
>  
>      /*
> @@ -1898,8 +1897,6 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>          aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
>          aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
>          aml_append(dev, aml_name_decl("_UID", aml_int(1)));
> -        aml_append(dev, aml_name_decl("SUPP", aml_int(0)));
> -        aml_append(dev, aml_name_decl("CTRL", aml_int(0)));
>          aml_append(dev, build_q35_osc_method());
>          aml_append(sb_scope, dev);
>          aml_append(dsdt, sb_scope);