[PATCH v6 0/2] arm/acpi: simplify aml code and enable SHPC

[PATCH v6 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.

v6:
- Fix "make check" errors by updating tests/data/acpi/virt/DSDT*.

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 +++++++++++++--------
 tests/data/acpi/virt/DSDT         | Bin 18470 -> 18462 bytes
 tests/data/acpi/virt/DSDT.memhp   | Bin 19807 -> 19799 bytes
 tests/data/acpi/virt/DSDT.numamem | Bin 18470 -> 18462 bytes
 4 files changed, 13 insertions(+), 8 deletions(-)

-- 
2.19.1

[PATCH v6 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.

Also update tests/data/acpi/virt/DSDT* to pass "make check".

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 ++++++++--------
 tests/data/acpi/virt/DSDT         | Bin 18470 -> 18462 bytes
 tests/data/acpi/virt/DSDT.memhp   | Bin 19807 -> 19799 bytes
 tests/data/acpi/virt/DSDT.numamem | Bin 18470 -> 18462 bytes
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 4cd50175e0..51b293e0a1 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -267,17 +267,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")));
@@ -285,8 +285,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);
diff --git a/tests/data/acpi/virt/DSDT b/tests/data/acpi/virt/DSDT
index bce76e3d23e99e6c5ef64c94c770282dd30ecdd0..05bcfc8a912f58f266aa906563ea01c24906717e 100644
GIT binary patch
delta 133
zcmZ2BfpOjhMlP3Nmk>D*1_q|2iCof5o%I{lJ2{y;?{412x!p#<jWgaq*qNm(o59&7
z+;D-%<VrV7_iE>mARjJS5V=5L(&S9WT970c2Uv;Nq{%?q7$gZ1761tsfcPNsCD{x4
MAmS{W8QoPG0j8@bzW@LL

delta 141
zcmbO?fpOUcMlP3Nmk>1%1_q`n6S<_B8XGpMcXBc{-rKy1bGwazA7{LOuro_nHiNTE
zxZwi7$(3%F{sq;}AwfP|vJ4<<fzYJMnT!RsAbBnhh%$*ulYv}gkTg_604z}e5&_99
R$zCV`m0@An{L@X95dZ+BD!u>!

diff --git a/tests/data/acpi/virt/DSDT.memhp b/tests/data/acpi/virt/DSDT.memhp
index b4b153fcdc30d211237fced6be1e99d3500bd276..c041a910fdf272cb89263bb636239ae3a5e1708d 100644
GIT binary patch
delta 132
zcmcaVi}Cs_MlP3NmymE@1_ma@iCof*O&is^IGH-{Zr;SX-A2HTGu}VgnWZb6!PzC;
zaDm6<N;gaQYUhw3A1+xCxj<mj<V?m|kR%reSc%xA$w1l|Bnc4~00|d>_#p8m*$ep~
L;w+mP-Q(B*s{AMU

delta 140
zcmcaUi}C&}MlP3Nmymd01_maViCof*T^rT9IGGynZQjJW-A2HVGu}VgnWZb6!PzC;
zaDm_CN;gaYf@<fGARjJS1`xGCXwu|N#)4XqJQoK<nZ%^YK&~-J8Y&?GmM8#;fMk|r
QFBE{vurO@?=@!QZ00dYn_y7O^

diff --git a/tests/data/acpi/virt/DSDT.numamem b/tests/data/acpi/virt/DSDT.numamem
index bce76e3d23e99e6c5ef64c94c770282dd30ecdd0..05bcfc8a912f58f266aa906563ea01c24906717e 100644
GIT binary patch
delta 133
zcmZ2BfpOjhMlP3Nmk>D*1_q|2iCof5o%I{lJ2{y;?{412x!p#<jWgaq*qNm(o59&7
z+;D-%<VrV7_iE>mARjJS5V=5L(&S9WT970c2Uv;Nq{%?q7$gZ1761tsfcPNsCD{x4
MAmS{W8QoPG0j8@bzW@LL

delta 141
zcmbO?fpOUcMlP3Nmk>1%1_q`n6S<_B8XGpMcXBc{-rKy1bGwazA7{LOuro_nHiNTE
zxZwi7$(3%F{sq;}AwfP|vJ4<<fzYJMnT!RsAbBnhh%$*ulYv}gkTg_604z}e5&_99
R$zCV`m0@An{L@X95dZ+BD!u>!

-- 
2.19.1

[PATCH v6 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.

Also update tests/data/acpi/virt/DSDT* to pass "make check".

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 ++++++-
 tests/data/acpi/virt/DSDT         | Bin 18462 -> 18462 bytes
 tests/data/acpi/virt/DSDT.memhp   | Bin 19799 -> 19799 bytes
 tests/data/acpi/virt/DSDT.numamem | Bin 18462 -> 18462 bytes
 4 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 51b293e0a1..bd5f771e9b 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -267,7 +267,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))));
diff --git a/tests/data/acpi/virt/DSDT b/tests/data/acpi/virt/DSDT
index 05bcfc8a912f58f266aa906563ea01c24906717e..d0f3afeb134fdf1c11f64cd06dbcdd30be603b80 100644
GIT binary patch
delta 28
kcmbO?fpOjhMlP3Nmk>D*1_q{tja=*8809zbbW3Ff0C~9xM*si-

delta 28
kcmbO?fpOjhMlP3Nmk>D*1_q|2ja=*87-cu_bW3Ff0C~j-M*si-

diff --git a/tests/data/acpi/virt/DSDT.memhp b/tests/data/acpi/virt/DSDT.memhp
index c041a910fdf272cb89263bb636239ae3a5e1708d..41ccc6431b917252bcbaac86c33b340c796be5ce 100644
GIT binary patch
delta 28
kcmcaUi}Cs_MlP3NmymE@1_mbija=*8809zbbeqQp0Eq|*2mk;8

delta 28
kcmcaUi}Cs_MlP3NmymE@1_ma@ja=*87-cu_beqQp0ErX{2mk;8

diff --git a/tests/data/acpi/virt/DSDT.numamem b/tests/data/acpi/virt/DSDT.numamem
index 05bcfc8a912f58f266aa906563ea01c24906717e..d0f3afeb134fdf1c11f64cd06dbcdd30be603b80 100644
GIT binary patch
delta 28
kcmbO?fpOjhMlP3Nmk>D*1_q{tja=*8809zbbW3Ff0C~9xM*si-

delta 28
kcmbO?fpOjhMlP3Nmk>D*1_q|2ja=*87-cu_bW3Ff0C~j-M*si-

-- 
2.19.1

Re: [PATCH v6 0/2] arm/acpi: simplify aml code and enable SHPC

[Peter Maydell]

On Mon, 9 Dec 2019 at 06:38, 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.
>
> Igor also spotted the store operation outside of bit and/or is not necessary, so
> simply the code at first.
>
> v6:
> - Fix "make check" errors by updating tests/data/acpi/virt/DSDT*.
>
> 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>

Thanks, applied to target-arm.next. (it's a bit awkward that acpi
table updates require also updating a bunch of binary test files,
but I suppose trying to make the golden-reference be some textual
format would be not very feasible.)

thanks
-- PMM

Re: [PATCH v6 0/2] arm/acpi: simplify aml code and enable SHPC

[Igor Mammedov]

On Mon, 9 Dec 2019 17:51:10 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:

> On Mon, 9 Dec 2019 at 06:38, 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.
> >
> > Igor also spotted the store operation outside of bit and/or is not necessary, so
> > simply the code at first.
> >
> > v6:
> > - Fix "make check" errors by updating tests/data/acpi/virt/DSDT*.
> >
> > 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>  
> 
> Thanks, applied to target-arm.next. (it's a bit awkward that acpi
> table updates require also updating a bunch of binary test files,
> but I suppose trying to make the golden-reference be some textual
> format would be not very feasible.)

Michael tried document it (commit 30c63d4fbd69)
so that binary blobs would not be required (trusted).

Problem is usually in blobs being un-review-able and
unresolvable merge conflicts, that's why Michael
updates tables manually for all changes in pull req.

But in this case it touches only ARM tables and it's
the first change to go in, so it could just work out.


> 
> thanks
> -- PMM
> 

Re: [PATCH v6 0/2] arm/acpi: simplify aml code and enable SHPC

[Peter Maydell]

On Tue, 10 Dec 2019 at 13:49, Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Mon, 9 Dec 2019 17:51:10 +0000
> Peter Maydell <peter.maydell@linaro.org> wrote:
>
> > On Mon, 9 Dec 2019 at 06:38, 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.
> > >
> > > Igor also spotted the store operation outside of bit and/or is not necessary, so
> > > simply the code at first.
> > >
> > > v6:
> > > - Fix "make check" errors by updating tests/data/acpi/virt/DSDT*.
> > >
> > > 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>
> >
> > Thanks, applied to target-arm.next. (it's a bit awkward that acpi
> > table updates require also updating a bunch of binary test files,
> > but I suppose trying to make the golden-reference be some textual
> > format would be not very feasible.)
>
> Michael tried document it (commit 30c63d4fbd69)
> so that binary blobs would not be required (trusted).
>
> Problem is usually in blobs being un-review-able and
> unresolvable merge conflicts, that's why Michael
> updates tables manually for all changes in pull req.

A workflow that requires me to do a blob update when I
apply patches to target-arm.next isn't going to work for me.
If Michael wants the blobs to be handled like that, then all
patches which need to touch them will have to go via his tree.

thanks
-- PMM