[Qemu-devel] [PATCH v5 0/2] arm/acpi: simplify aml code and enable SHPC

[Qemu-devel] [PATCH v5 0/2] arm/acpi: simplify aml code and enable SHPC

[Heyi Guo]

After the introduction of generic PCIe root port and PCIe-PCI bridge, we will
also have SHPC controller on ARM, and we don't support ACPI hot plug, so just
enable SHPC native hot plug.

Igor also spotted the store operation outside of bit and/or is not necessary, so
simply the code at first.

v5:
- Refine commit message of patch 1/2

v4:
- Improve the code indention.

Cc: Shannon Zhao <shannon.zhaosl@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>



Heyi Guo (2):
  hw/arm/acpi: simplify AML bit and/or statement
  hw/arm/acpi: enable SHPC native hot plug

 hw/arm/virt-acpi-build.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v5 1/2] hw/arm/acpi: simplify AML bit and/or statement

[Heyi Guo]

The last argument of AML bit and/or statement is the target variable,
so we don't need to use a NULL target and then an additional store
operation; using just aml_and() or aml_or() statement is enough.

Cc: Shannon Zhao <shannon.zhaosl@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Suggested-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Heyi Guo <guoheyi@huawei.com>
---
 hw/arm/virt-acpi-build.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index d7e2e48..cebec4c 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -265,17 +265,17 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
         aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
     aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
     aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL")));
-    aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D), NULL),
-                                aml_name("CTRL")));
+    aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D),
+                              aml_name("CTRL")));
 
     ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1))));
-    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08), NULL),
-                                 aml_name("CDW1")));
+    aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x08),
+                              aml_name("CDW1")));
     aml_append(ifctx, ifctx1);
 
     ifctx1 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"), aml_name("CTRL"))));
-    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10), NULL),
-                                 aml_name("CDW1")));
+    aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x10),
+                              aml_name("CDW1")));
     aml_append(ifctx, ifctx1);
 
     aml_append(ifctx, aml_store(aml_name("CTRL"), aml_name("CDW3")));
@@ -283,8 +283,8 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
     aml_append(method, ifctx);
 
     elsectx = aml_else();
-    aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4), NULL),
-                                  aml_name("CDW1")));
+    aml_append(elsectx, aml_or(aml_name("CDW1"), aml_int(4),
+                               aml_name("CDW1")));
     aml_append(elsectx, aml_return(aml_arg(3)));
     aml_append(method, elsectx);
     aml_append(dev, method);
-- 
1.8.3.1

[Qemu-devel] [PATCH v5 2/2] hw/arm/acpi: enable SHPC native hot plug

[Heyi Guo]

After the introduction of generic PCIe root port and PCIe-PCI bridge,
we will also have SHPC controller on ARM, so just enable SHPC native
hot plug.

Cc: Shannon Zhao <shannon.zhaosl@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Heyi Guo <guoheyi@huawei.com>
---
 hw/arm/virt-acpi-build.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index cebec4c..b6fef28 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -265,7 +265,12 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
         aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
     aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
     aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL")));
-    aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D),
+
+    /*
+     * Allow OS control for all 5 features:
+     * PCIeHotplug SHPCHotplug PME AER PCIeCapability.
+     */
+    aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1F),
                               aml_name("CTRL")));
 
     ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1))));
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v5 0/2] arm/acpi: simplify aml code and enable SHPC

[Michael S. Tsirkin]

On Sat, Mar 09, 2019 at 10:30:47AM +0800, Heyi Guo wrote:
> After the introduction of generic PCIe root port and PCIe-PCI bridge, we will
> also have SHPC controller on ARM, and we don't support ACPI hot plug, so just
> enable SHPC native hot plug.
> 
> Igor also spotted the store operation outside of bit and/or is not necessary, so
> simply the code at first.
> 
> v5:
> - Refine commit message of patch 1/2
> 
> v4:
> - Improve the code indention.
> 
> Cc: Shannon Zhao <shannon.zhaosl@gmail.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>


Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> 
> 
> Heyi Guo (2):
>   hw/arm/acpi: simplify AML bit and/or statement
>   hw/arm/acpi: enable SHPC native hot plug
> 
>  hw/arm/virt-acpi-build.c | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH v5 0/2] arm/acpi: simplify aml code and enable SHPC

[Guoheyi]

Hi Peter, Igor,

I couldn't find these 2 patches in the latest tree. Could you help to 
merge them?

Thanks,

HG


On 2019/3/13 0:09, Igor Mammedov wrote:
> On Sat, 9 Mar 2019 10:30:47 +0800
> Heyi Guo <guoheyi@huawei.com> wrote:
>
>> After the introduction of generic PCIe root port and PCIe-PCI bridge, we will
>> also have SHPC controller on ARM, and we don't support ACPI hot plug, so just
>> enable SHPC native hot plug.
> Peter,
>   could you queue it via your tree?
>
>> Igor also spotted the store operation outside of bit and/or is not necessary, so
>> simply the code at first.
>>
>> v5:
>> - Refine commit message of patch 1/2
>>
>> v4:
>> - Improve the code indention.
>>
>> Cc: Shannon Zhao <shannon.zhaosl@gmail.com>
>> Cc: Peter Maydell <peter.maydell@linaro.org>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Igor Mammedov <imammedo@redhat.com>
>>
>>
>>
>> Heyi Guo (2):
>>    hw/arm/acpi: simplify AML bit and/or statement
>>    hw/arm/acpi: enable SHPC native hot plug
>>
>>   hw/arm/virt-acpi-build.c | 21 +++++++++++++--------
>>   1 file changed, 13 insertions(+), 8 deletions(-)
>>
>
> .
>

Re: [Qemu-devel] [PATCH v5 0/2] arm/acpi: simplify aml code and enable SHPC

[Peter Maydell]

On Sat, 30 Nov 2019 at 03:47, Guoheyi <guoheyi@huawei.com> wrote:
>
> Hi Peter, Igor,
>
> I couldn't find these 2 patches in the latest tree. Could you help to
> merge them?

In future I recommend pinging unapplied patches with a shorter
delay than nine months :-)  In QEMU's process, unless somebody
has specifically said they've picked up the patch, it still
"belongs" to the submitter to chase if it hasn't been
applied. In this case I simply didn't see Igor's request
that I take it -- the chances of me actually reading any
particular list email even if it's cc'd to me are not good.

I tried applying them to target-arm.next but unfortunately
they break 'make check':

  TEST    check-qtest-aarch64: tests/bios-tables-test
acpi-test: Warning! DSDT binary file mismatch. Actual
[aml:/tmp/aml-4IELC0], Expected [aml:tests/data/acpi/virt/DSDT].
acpi-test: Warning! DSDT mismatch. Actual [asl:/tmp/asl-AOELC0.dsl,
aml:/tmp/aml-4IELC0], Expected [asl:/tmp/asl-XL7KC0.dsl,
aml:tests/data/acpi/virt/DSDT].
**
ERROR:/home/petmay01/linaro/qemu-from-laptop/qemu/tests/bios-tables-test.c:477:test_acpi_asl:
assertion failed: (all_tables_match)
ERROR - Bail out!
ERROR:/home/petmay01/linaro/qemu-from-laptop/qemu/tests/bios-tables-test.c:477:test_acpi_asl:
assertion failed: (all_tables_match)
Aborted (core dumped)
/home/petmay01/linaro/qemu-from-laptop/qemu/tests/Makefile.include:918:
recipe for target 'check-qtest-aarch64' failed

Could you fix and resubmit, please?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v5 0/2] arm/acpi: simplify aml code and enable SHPC

[Guoheyi]

在 2019/12/6 21:50, Peter Maydell 写道:
> On Sat, 30 Nov 2019 at 03:47, Guoheyi <guoheyi@huawei.com> wrote:
>> Hi Peter, Igor,
>>
>> I couldn't find these 2 patches in the latest tree. Could you help to
>> merge them?
> In future I recommend pinging unapplied patches with a shorter
> delay than nine months :-)

Tha's really a long time...


> In QEMU's process, unless somebody
> has specifically said they've picked up the patch, it still
> "belongs" to the submitter to chase if it hasn't been
> applied. In this case I simply didn't see Igor's request
> that I take it -- the chances of me actually reading any
> particular list email even if it's cc'd to me are not good.

One of the reasons that I didn't check it earlier is that we don't 
really use PCI SHPC in our production version, for Linux ITS driver can 
only allocate a fixed range of MSI interrupts for a PCI-bridge during 
initialization, so a later plugged-in PCI device may not be able to get 
enough MSI interrupts and then fall back to legacy INTx. However, I 
think it is still better to let guest OS make the decision.

>
> I tried applying them to target-arm.next but unfortunately
> they break 'make check':
>
>    TEST    check-qtest-aarch64: tests/bios-tables-test
> acpi-test: Warning! DSDT binary file mismatch. Actual
> [aml:/tmp/aml-4IELC0], Expected [aml:tests/data/acpi/virt/DSDT].
> acpi-test: Warning! DSDT mismatch. Actual [asl:/tmp/asl-AOELC0.dsl,
> aml:/tmp/aml-4IELC0], Expected [asl:/tmp/asl-XL7KC0.dsl,
> aml:tests/data/acpi/virt/DSDT].
> **
> ERROR:/home/petmay01/linaro/qemu-from-laptop/qemu/tests/bios-tables-test.c:477:test_acpi_asl:
> assertion failed: (all_tables_match)
> ERROR - Bail out!
> ERROR:/home/petmay01/linaro/qemu-from-laptop/qemu/tests/bios-tables-test.c:477:test_acpi_asl:
> assertion failed: (all_tables_match)
> Aborted (core dumped)
> /home/petmay01/linaro/qemu-from-laptop/qemu/tests/Makefile.include:918:
> recipe for target 'check-qtest-aarch64' failed
>
> Could you fix and resubmit, please?

Sure.


Thanks,

Heyi

>
> thanks
> -- PMM
>
> .