* [Qemu-devel] [PATCH 0/9] generalize build_fadt()
@ 2018-02-22 12:42 Igor Mammedov
2018-02-22 12:42 ` [Qemu-devel] [PATCH 1/9] acpi: remove unused acpi-dsdt.aml Igor Mammedov
` (8 more replies)
0 siblings, 9 replies; 24+ messages in thread
From: Igor Mammedov @ 2018-02-22 12:42 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Shannon Zhao, Peter Maydell, Andrew Jones, qemu-arm
Series first cleanups ACPI code around build_fadt() and then
converts current packed structure approach to a build_append_FOO()
API, getting rid of error prone explicit endianness conversions
in code and making build_fadt() look more like APCI table declaration
from spec, which should be easier to read/maintain. After that
build_fadt() becomes generic enough that we could drop ARM specific
implementation and reuse generic build_fadt(), reducing code
duplication.
PS: tested only x86 which has make check coverage,
ARM was only slightly tested.
git tree for testing:
https://github.com/imammedo/qemu.git fadt_refactoring_v1
CC: "Michael S. Tsirkin" <mst@redhat.com>
CC: Shannon Zhao <zhaoshenglong@huawei.com>
CC: Peter Maydell <peter.maydell@linaro.org>
CC: Andrew Jones <drjones@redhat.com>
CC: qemu-devel@nongnu.org
CC: qemu-arm@nongnu.org
Igor Mammedov (9):
acpi: remove unused acpi-dsdt.aml
pc: replace pm object initialization with one-liner in
acpi_get_pm_info()
acpi: reuse AcpiGenericAddress instead of Acpi20GenericAddress
acpi: add build_append_gas() helper for Generic Address Structure
pc: acpi: isolate FADT specific data into AcpiFadtData structure
pc: acpi: use build_append_foo() API to construct FADT
acpi: move build_fadt() from i386 specific to generic ACPI source
virt_arm: acpi: reuse common build_fadt()
tests: acpi: don't read all fields in test_acpi_fadt_table()
include/hw/acpi/acpi-defs.h | 136 +++++++-----------------------
include/hw/acpi/aml-build.h | 23 ++++++
Makefile | 1 -
hw/acpi/aml-build.c | 140 +++++++++++++++++++++++++++++++
hw/arm/virt-acpi-build.c | 39 ++++-----
hw/i386/acpi-build.c | 197 ++++++++++++++------------------------------
pc-bios/acpi-dsdt.aml | Bin 4405 -> 0 bytes
tests/bios-tables-test.c | 82 ++++--------------
8 files changed, 292 insertions(+), 326 deletions(-)
delete mode 100644 pc-bios/acpi-dsdt.aml
--
2.7.4
^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH 1/9] acpi: remove unused acpi-dsdt.aml
2018-02-22 12:42 [Qemu-devel] [PATCH 0/9] generalize build_fadt() Igor Mammedov
@ 2018-02-22 12:42 ` Igor Mammedov
2018-02-22 14:25 ` Gerd Hoffmann
2018-02-27 12:42 ` Auger Eric
2018-02-22 12:42 ` [Qemu-devel] [PATCH 2/9] pc: replace pm object initialization with one-liner in acpi_get_pm_info() Igor Mammedov
` (7 subsequent siblings)
8 siblings, 2 replies; 24+ messages in thread
From: Igor Mammedov @ 2018-02-22 12:42 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Shannon Zhao, Peter Maydell, Andrew Jones,
qemu-arm, Gerd Hoffmann
SeaBIOS blob which is currently shipped with QEMU
doesn't need acpi-dsdt.aml nor is able to use it
and code that loaded it QEMU was removed by
(commit 9fb7aaaf4c "pc: drop external DSDT loading")
in 2013.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
CC: Gerd Hoffmann <kraxel@redhat.com>
---
Makefile | 1 -
pc-bios/acpi-dsdt.aml | Bin 4405 -> 0 bytes
2 files changed, 1 deletion(-)
delete mode 100644 pc-bios/acpi-dsdt.aml
diff --git a/Makefile b/Makefile
index 90e05ac..f3a4cf9 100644
--- a/Makefile
+++ b/Makefile
@@ -648,7 +648,6 @@ bepo cz
ifdef INSTALL_BLOBS
BLOBS=bios.bin bios-256k.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \
vgabios-stdvga.bin vgabios-vmware.bin vgabios-qxl.bin vgabios-virtio.bin \
-acpi-dsdt.aml \
ppc_rom.bin openbios-sparc32 openbios-sparc64 openbios-ppc QEMU,tcx.bin QEMU,cgthree.bin \
pxe-e1000.rom pxe-eepro100.rom pxe-ne2k_pci.rom \
pxe-pcnet.rom pxe-rtl8139.rom pxe-virtio.rom \
diff --git a/pc-bios/acpi-dsdt.aml b/pc-bios/acpi-dsdt.aml
deleted file mode 100644
index 558c10f51ccbbf9ec2f47a4a998a7055059a8963..0000000000000000000000000000000000000000
GIT binary patch
literal 0
HcmV?d00001
literal 4405
zcmb7{O>f)C8OI;KNTx=#Ov$uk$9XY?b_=xIjb0q@_EJP5WkrsxFrpHqP*76NE}&CE
zqzSOzpn&5RO*WTe*G<u*ve0?^5!z#q`3UV-*rJF}QJ-hVlxK_$u;qa>|Kam8znLL9
z<A?s>dJ#bTx_LkF0GjuGY(WhGo!+3koGWcQ9rFPU5B+94((<~g4WH$C9dAv`{m^gT
zZEJrW$A5|A$IoMJl)(Ns&a3@V^7|L@K9JFq{e&^9IOQm8M#H0x!0S}3=w`>a8*i9l
zMGe0XR&=-HYff+FBQoL^UcVI<n+xnWFBU<!u}c6mx@m3g#6Gb#3)?l@pr*I@_{5&;
z#Tgm?=lKNy@f?7`Y?dceyma7Ch?1^<&1QdpC#wIrasZas-`*<LS>@%=&fLZ0Lo8-=
z2|2%0`vJHO7J2;;UQ)-|gCMNeL^TdtX>}ZQ>$N1Pgb_W)N-Ls=$)m@-itRY~WHYgk
zgX+BqrWEW?)FoC3!tE_lT@6}k^@E_hy_E!2ipVPzkypAAJ^BL$Apdw8JA0Oxf|hkN
zXbsXi&~OfL^l_GN27^7ov3&C`59aWhLwfmMtLJY9eLvcCx1(^-fP`A&gqlWQ#LS5&
z_E*O-9LM?DYzmXYSH~mx^T>vO|2H#*DO<8=Sc*kf_+yTS?9DqcX}p{p+4*D-kG#yi
zb|d180Xv{$XK)dCIxy@<o~j1#hl>PNAErQ+8n3KJVco~H$7En{Z-6#s#%p5=&X1+|
z>%skMU4%Dqj4^z*PT^<HPDV28n4R#f8{BTI;^{1=ethujk0=Ux0^Ga?3*DgA)8G>@
zyarVauZe}V<Kx}wZd^0cwM;RGM?dcmJR}qgKaWeEhGmVdw6z2haP%@Q?MLtk^y~o)
zk3PQD^ylV=;pX_@&&QKH#t?&sUZ29JSeA7h*5T1l_HN&uJ1#AsceGfh3=SFYnmfKX
ze-#(NT@&+50P!S?b2^3B<~-vDTWf3I8Q&RTwzap$TO7yo4fv_alm4<B4CYDAc_<p8
z?+N9w#kTgj@ws7H<wNe@GQHb-)pT?+n)o23J)-e_Uzii)!~m=8@Gv_Rrgkn2)8}z;
zg5DcPKhZIcg>jsYb+#sOA%+7j58pBiUkMT(uE)EZc=I=hhhb|Mzl_$me4&!?nw8e>
zrn?k)tz9iS(8fRw?snkyc5t5KZ$5k#v#U?yN%1MgInZJNd^U)+Nr_tgvleDJyAxVO
zPWv%F!Kn)RgVL?v9+vVZzHHF#-SR=yHLN$FWK%oSQ8ZIwpzxryXxg(Ge)CX;b46Zg
zSP;*+ADX6;JTX4^)VU|xo+|Q8P4P9NjA+U|QIaS2hTGy7TG*Z{@=Q$);fbc)6D4`3
zS@1I<Y`Lcir;Oax6rO44QOcYd?wR%=!#z{ejPOi5k5cB$Dx6vFnVM!*PLwj|g2K7L
zJyXsFl@q1RX(^nR!fC0TC}mFkpyCLnoH>Ovr*fi{Ihn%A6i%jcqLexDH;OrNO!%zi
z70$fMiBjgo54$v<w!&$voG4|^MTHap`xqyk&qb9JrOa7SI137ALFGg#b1o^IOA6<b
z%863ubQDfU;dE3^lrkr7#-#ZyDx5`?6Q#^qQaDQrXG!HmDRV9>oXZO5vdW24=5!TK
zSK)M3PLwj|io&^~aIUDFC}qx7g>zNmTva(y%AB|xl-BJ9h4X^SiBjfVQ#jWY&NY=2
zrOdgmaIPzy>nbNone(E;c~RlKsB)r|IX4te+zyF%j(;^bR8EvK=Ou;nlEQgO<wPlS
zURF3SE1Z{APLwj|6@~MP!g)pIL@9G#RXDFIoL5y&l!9~k>_^uO`jgU*EWn+e7WD5_
zEWB0eR-;?pa+f=I@RvWyJ!OYu+`;CiEbnf2?s)wi8uTm00?U7yg&aRY9KcIzV;Q`6
zCiz!mc9@K*KBea2QFnpf=yXS7<8GMt+Vm$6i>qw;%L3#K{9WM*1%OT{c#v2UJ3Z<I
zb<ZtEekX+AQJo#~mL-1Dm{OOxz7U1|P<uHRy}+$`zeDY(*_-FG<L2rIXRk`xt2}!Z
z`$y-TG<((k{_NG^(H^mT=dv^X|43hx(${$U+PU<#_oT0#ruWaM$J5Rarmsus>pXq^
zT>AQZ($|Maw@suE&!;y<`g94=kqD<e-Q4HhET3#QFUFX<icK`TPP;%`LHD{B>@_qz
zV0*#s-WcMfm}eH?9)hk>GJY{)I`G1PBt~VzbmU(20$kE(UXx6W8+$q?xy%b%yZW%q
z{)wleKHuy9jcpE}*<9c)y5YFHS*&<~ODl{%!&5$OWOp*J;^)+h)37m&ChTd<7T}A0
zZU426&7a``5jTMQ$<uue9!|<%ACDd;4|&&Pn6TrAT5quPt5|z&@sb%&VyBmj+Chtt
z+hW5DI+aRg8*mW1l?u2kQL9pg2WMw1+E%*`w$|W**tBCmxpiGQZHeN#C{81NEYv5W
U_=PAMqG*c36NN8|mMC`Mf01wzJpcdz
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH 2/9] pc: replace pm object initialization with one-liner in acpi_get_pm_info()
2018-02-22 12:42 [Qemu-devel] [PATCH 0/9] generalize build_fadt() Igor Mammedov
2018-02-22 12:42 ` [Qemu-devel] [PATCH 1/9] acpi: remove unused acpi-dsdt.aml Igor Mammedov
@ 2018-02-22 12:42 ` Igor Mammedov
2018-02-27 12:42 ` Auger Eric
2018-02-27 23:41 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2018-02-22 12:42 ` [Qemu-devel] [PATCH 3/9] acpi: reuse AcpiGenericAddress instead of Acpi20GenericAddress Igor Mammedov
` (6 subsequent siblings)
8 siblings, 2 replies; 24+ messages in thread
From: Igor Mammedov @ 2018-02-22 12:42 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Shannon Zhao, Peter Maydell, Andrew Jones, qemu-arm
next patch will need it before it gets to piix4/lpc branches
that initializes 'obj' now.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/i386/acpi-build.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index deb440f..b85fefe 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -128,7 +128,7 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
{
Object *piix = piix4_pm_find();
Object *lpc = ich9_lpc_find();
- Object *obj = NULL;
+ Object *obj = piix ? piix : lpc;
QObject *o;
pm->force_rev1_fadt = false;
@@ -138,7 +138,6 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
if (piix) {
/* w2k requires FADT(rev1) or it won't boot, keep PC compatible */
pm->force_rev1_fadt = true;
- obj = piix;
pm->cpu_hp_io_base = PIIX4_CPU_HOTPLUG_IO_BASE;
pm->pcihp_io_base =
object_property_get_uint(obj, ACPI_PCIHP_IO_BASE_PROP, NULL);
@@ -146,7 +145,6 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
object_property_get_uint(obj, ACPI_PCIHP_IO_LEN_PROP, NULL);
}
if (lpc) {
- obj = lpc;
pm->cpu_hp_io_base = ICH9_CPU_HOTPLUG_IO_BASE;
}
assert(obj);
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH 3/9] acpi: reuse AcpiGenericAddress instead of Acpi20GenericAddress
2018-02-22 12:42 [Qemu-devel] [PATCH 0/9] generalize build_fadt() Igor Mammedov
2018-02-22 12:42 ` [Qemu-devel] [PATCH 1/9] acpi: remove unused acpi-dsdt.aml Igor Mammedov
2018-02-22 12:42 ` [Qemu-devel] [PATCH 2/9] pc: replace pm object initialization with one-liner in acpi_get_pm_info() Igor Mammedov
@ 2018-02-22 12:42 ` Igor Mammedov
2018-02-27 12:42 ` Auger Eric
2018-02-22 12:42 ` [Qemu-devel] [PATCH 4/9] acpi: add build_append_gas() helper for Generic Address Structure Igor Mammedov
` (5 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Igor Mammedov @ 2018-02-22 12:42 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Shannon Zhao, Peter Maydell, Andrew Jones, qemu-arm
Drop duplicate in form of Acpi20GenericAddress and reuse
AcpiGenericAddress.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
include/hw/acpi/acpi-defs.h | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index 80c8099..9942bc5 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -40,18 +40,6 @@ enum {
ACPI_FADT_F_LOW_POWER_S0_IDLE_CAPABLE,
};
-/*
- * ACPI 2.0 Generic Address Space definition.
- */
-struct Acpi20GenericAddress {
- uint8_t address_space_id;
- uint8_t register_bit_width;
- uint8_t register_bit_offset;
- uint8_t reserved;
- uint64_t address;
-} QEMU_PACKED;
-typedef struct Acpi20GenericAddress Acpi20GenericAddress;
-
struct AcpiRsdpDescriptor { /* Root System Descriptor Pointer */
uint64_t signature; /* ACPI signature, contains "RSD PTR " */
uint8_t checksum; /* To make sum of struct == 0 */
@@ -167,7 +155,8 @@ struct AcpiGenericAddress {
uint8_t space_id; /* Address space where struct or register exists */
uint8_t bit_width; /* Size in bits of given register */
uint8_t bit_offset; /* Bit offset within the register */
- uint8_t access_width; /* Minimum Access size (ACPI 3.0) */
+ uint8_t access_width; /* ACPI 3.0: Minimum Access size (ACPI 3.0),
+ ACPI 2.0: Reserved, Table 5-1 */
uint64_t address; /* 64-bit address of struct or register */
} QEMU_PACKED;
@@ -456,7 +445,7 @@ typedef struct AcpiGenericTimerTable AcpiGenericTimerTable;
struct Acpi20Hpet {
ACPI_TABLE_HEADER_DEF /* ACPI common table header */
uint32_t timer_block_id;
- Acpi20GenericAddress addr;
+ struct AcpiGenericAddress addr;
uint8_t hpet_number;
uint16_t min_tick;
uint8_t page_protect;
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH 4/9] acpi: add build_append_gas() helper for Generic Address Structure
2018-02-22 12:42 [Qemu-devel] [PATCH 0/9] generalize build_fadt() Igor Mammedov
` (2 preceding siblings ...)
2018-02-22 12:42 ` [Qemu-devel] [PATCH 3/9] acpi: reuse AcpiGenericAddress instead of Acpi20GenericAddress Igor Mammedov
@ 2018-02-22 12:42 ` Igor Mammedov
2018-02-27 12:48 ` Auger Eric
2018-02-22 12:42 ` [Qemu-devel] [PATCH 5/9] pc: acpi: isolate FADT specific data into AcpiFadtData structure Igor Mammedov
` (4 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Igor Mammedov @ 2018-02-22 12:42 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Shannon Zhao, Peter Maydell, Andrew Jones, qemu-arm
it will help to add Generic Address Structure to ACPI tables
without using packed C structures and avoid endianness
issues as API doesn't need an explicit conversion.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
include/hw/acpi/aml-build.h | 20 ++++++++++++++++++++
hw/acpi/aml-build.c | 16 ++++++++++++++++
2 files changed, 36 insertions(+)
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 88d0738..8692ccc 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -78,6 +78,15 @@ typedef enum {
} AmlUpdateRule;
typedef enum {
+ AML_AS_SYSTEM_MEMORY = 0X00,
+ AML_AS_SYSTEM_IO = 0X01,
+ AML_AS_PCI_CONFIG = 0X02,
+ AML_AS_EMBEDDED_CTRL = 0X03,
+ AML_AS_SMBUS = 0X04,
+ AML_AS_FFH = 0X7F,
+} AmlAddressSpace;
+
+typedef enum {
AML_SYSTEM_MEMORY = 0X00,
AML_SYSTEM_IO = 0X01,
AML_PCI_CONFIG = 0X02,
@@ -389,6 +398,17 @@ int
build_append_named_dword(GArray *array, const char *name_format, ...)
GCC_FMT_ATTR(2, 3);
+void build_append_gas(GArray *table, AmlAddressSpace as,
+ uint8_t bit_width, uint8_t bit_offset,
+ uint8_t access_width, uint64_t address);
+
+static inline void
+build_append_gas_from_struct(GArray *table, const struct AcpiGenericAddress *s)
+{
+ build_append_gas(table, s->space_id, s->bit_width, s->bit_offset,
+ s->access_width, s->address);
+}
+
void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
uint64_t len, int node, MemoryAffinityFlags flags);
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 36a6cc4..3fef5f6 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -258,6 +258,22 @@ static void build_append_int(GArray *table, uint64_t value)
}
}
+/* Generic Address Structure (GAS)
+ * ACPI 2.0/3.0: 5.2.3.1 Generic Address Structure
+ * 2.0 compat note:
+ * @access_width must be 0, see ACPI 2.0:Table 5-1
+ */
+void build_append_gas(GArray *table, AmlAddressSpace as,
+ uint8_t bit_width, uint8_t bit_offset,
+ uint8_t access_width, uint64_t address)
+{
+ build_append_int_noprefix(table, as, 1);
+ build_append_int_noprefix(table, bit_width, 1);
+ build_append_int_noprefix(table, bit_offset, 1);
+ build_append_int_noprefix(table, access_width, 1);
+ build_append_int_noprefix(table, address, 8);
+}
+
/*
* Build NAME(XXXX, 0x00000000) where 0x00000000 is encoded as a dword,
* and return the offset to 0x00000000 for runtime patching.
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH 5/9] pc: acpi: isolate FADT specific data into AcpiFadtData structure
2018-02-22 12:42 [Qemu-devel] [PATCH 0/9] generalize build_fadt() Igor Mammedov
` (3 preceding siblings ...)
2018-02-22 12:42 ` [Qemu-devel] [PATCH 4/9] acpi: add build_append_gas() helper for Generic Address Structure Igor Mammedov
@ 2018-02-22 12:42 ` Igor Mammedov
2018-02-27 13:47 ` Auger Eric
2018-02-22 12:42 ` [Qemu-devel] [PATCH 6/9] pc: acpi: use build_append_foo() API to construct FADT Igor Mammedov
` (3 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Igor Mammedov @ 2018-02-22 12:42 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Shannon Zhao, Peter Maydell, Andrew Jones, qemu-arm
move FADT data initialization out of fadt_setup() into dedicated
init_fadt_data() that will set common for pc/q35 values in
AcpiFadtData structure and acpi_get_pm_info() will complement
it with pc/q35 specific values initialization.
That will allow to get rid of fadt_setup() and generalize
build_fadt() so it could be easily extended for rev5 and
reused by ARM target.
While at it also move facs/dsdt/xdsdt offsets from build_fadt()
arg list into AcpiFadtData, as they belong to the same dataset.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
include/hw/acpi/acpi-defs.h | 28 ++++++
hw/i386/acpi-build.c | 204 ++++++++++++++++++++++++--------------------
2 files changed, 138 insertions(+), 94 deletions(-)
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index 9942bc5..e0accc4 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -175,6 +175,34 @@ struct AcpiFadtDescriptorRev5_1 {
typedef struct AcpiFadtDescriptorRev5_1 AcpiFadtDescriptorRev5_1;
+typedef struct AcpiFadtData {
+ struct AcpiGenericAddress pm1_cnt; /* PM1a_CNT_BLK */
+ struct AcpiGenericAddress pm1_evt; /* PM1a_EVT_BLK */
+ struct AcpiGenericAddress pm_tmr; /* PM_TMR_BLK */
+ struct AcpiGenericAddress gpe0_blk; /* GPE0_BLK */
+ struct AcpiGenericAddress reset_reg; /* RESET_REG */
+ uint8_t reset_val; /* RESET_VALUE */
+ uint8_t rev; /* Revision */
+ uint32_t flags; /* Flags */
+ uint32_t smi_cmd; /* SMI_CMD */
+ uint16_t sci_int; /* SCI_INT */
+ uint8_t int_model; /* INT_MODEL */
+ uint8_t acpi_enable_cmd; /* ACPI_ENABLE */
+ uint8_t acpi_disable_cmd; /* ACPI_DISABLE */
+ uint8_t rtc_century; /* CENTURY */
+ uint16_t c2_latency; /* P_LVL2_LAT */
+ uint16_t c3_latency; /* P_LVL3_LAT */
+
+ /*
+ * respective tables offsets within ACPI_BUILD_TABLE_FILE,
+ * NULL if table doesn't exist (in that case field's value
+ * won't be patched by linker and will be kept set to 0)
+ */
+ unsigned *facs_tbl_offset; /* FACS offset in */
+ unsigned *dsdt_tbl_offset;
+ unsigned *xdsdt_tbl_offset;
+} AcpiFadtData;
+
#define ACPI_FADT_ARM_PSCI_COMPLIANT (1 << 0)
#define ACPI_FADT_ARM_PSCI_USE_HVC (1 << 1)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b85fefe..706ba35 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -91,17 +91,11 @@ typedef struct AcpiMcfgInfo {
} AcpiMcfgInfo;
typedef struct AcpiPmInfo {
- bool force_rev1_fadt;
bool s3_disabled;
bool s4_disabled;
bool pcihp_bridge_en;
uint8_t s4_val;
- uint16_t sci_int;
- uint8_t acpi_enable_cmd;
- uint8_t acpi_disable_cmd;
- uint32_t gpe0_blk;
- uint32_t gpe0_blk_len;
- uint32_t io_base;
+ AcpiFadtData fadt;
uint16_t cpu_hp_io_base;
uint16_t pcihp_io_base;
uint16_t pcihp_io_len;
@@ -124,20 +118,60 @@ typedef struct AcpiBuildPciBusHotplugState {
bool pcihp_bridge_en;
} AcpiBuildPciBusHotplugState;
+#define ACPI_PORT_SMI_CMD 0x00b2 /* TODO: this is APM_CNT_IOPORT */
+
+static void init_fadt_data(Object *o, AcpiFadtData *data)
+{
+ uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL);
+ AmlAddressSpace as = AML_AS_SYSTEM_IO;
+ AcpiFadtData fadt = {
+ .rev = 3,
+ .flags =
+ (1 << ACPI_FADT_F_WBINVD) |
+ (1 << ACPI_FADT_F_PROC_C1) |
+ (1 << ACPI_FADT_F_SLP_BUTTON) |
+ (1 << ACPI_FADT_F_RTC_S4) |
+ (1 << ACPI_FADT_F_USE_PLATFORM_CLOCK) |
+ /* APIC destination mode ("Flat Logical") has an upper limit of 8
+ * CPUs for more than 8 CPUs, "Clustered Logical" mode has to be
+ * used
+ */
+ ((max_cpus > 8) ? (1 << ACPI_FADT_F_FORCE_APIC_CLUSTER_MODEL) : 0),
+ .int_model = 1 /* Multiple APIC */,
+ .rtc_century = RTC_CENTURY,
+ .c2_latency = 0xfff /* C2 state not supported */,
+ .c3_latency = 0xfff /* C3 state not supported */,
+ .smi_cmd = ACPI_PORT_SMI_CMD,
+ .sci_int = object_property_get_uint(o, ACPI_PM_PROP_SCI_INT, NULL),
+ .acpi_enable_cmd =
+ object_property_get_uint(o, ACPI_PM_PROP_ACPI_ENABLE_CMD, NULL),
+ .acpi_disable_cmd =
+ object_property_get_uint(o, ACPI_PM_PROP_ACPI_DISABLE_CMD, NULL),
+ .pm1_evt = { .space_id = as, .bit_width = 4 * 8, .address = io },
+ .pm1_cnt = { .space_id = as, .bit_width = 2 * 8, .address = io + 0x04 },
+ .pm_tmr = { .space_id = as, .bit_width = 4 * 8, .address = io + 0x08 },
+ .gpe0_blk = { .space_id = as, .bit_width =
+ object_property_get_uint(o, ACPI_PM_PROP_GPE0_BLK_LEN, NULL) * 8,
+ .address = object_property_get_uint(o, ACPI_PM_PROP_GPE0_BLK, NULL)
+ },
+ };
+ *data = fadt;
+}
+
static void acpi_get_pm_info(AcpiPmInfo *pm)
{
Object *piix = piix4_pm_find();
Object *lpc = ich9_lpc_find();
Object *obj = piix ? piix : lpc;
QObject *o;
-
- pm->force_rev1_fadt = false;
pm->cpu_hp_io_base = 0;
pm->pcihp_io_base = 0;
pm->pcihp_io_len = 0;
+
+ init_fadt_data(obj, &pm->fadt);
if (piix) {
/* w2k requires FADT(rev1) or it won't boot, keep PC compatible */
- pm->force_rev1_fadt = true;
+ pm->fadt.rev = 1;
pm->cpu_hp_io_base = PIIX4_CPU_HOTPLUG_IO_BASE;
pm->pcihp_io_base =
object_property_get_uint(obj, ACPI_PCIHP_IO_BASE_PROP, NULL);
@@ -145,10 +179,19 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
object_property_get_uint(obj, ACPI_PCIHP_IO_LEN_PROP, NULL);
}
if (lpc) {
+ struct AcpiGenericAddress r = { .space_id = AML_AS_SYSTEM_IO,
+ .bit_width = 8, .address = ICH9_RST_CNT_IOPORT };
+ pm->fadt.reset_reg = r;
+ pm->fadt.reset_val = 0xf;
+ pm->fadt.flags |= 1 << ACPI_FADT_F_RESET_REG_SUP;
pm->cpu_hp_io_base = ICH9_CPU_HOTPLUG_IO_BASE;
}
assert(obj);
+ /* The above need not be conditional on machine type because the reset port
+ * happens to be the same on PIIX (pc) and ICH9 (q35). */
+ QEMU_BUILD_BUG_ON(ICH9_RST_CNT_IOPORT != RCR_IOPORT);
+
/* Fill in optional s3/s4 related properties */
o = object_property_get_qobject(obj, ACPI_PM_PROP_S3_DISABLED, NULL);
if (o) {
@@ -172,22 +215,6 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
}
qobject_decref(o);
- /* Fill in mandatory properties */
- pm->sci_int = object_property_get_uint(obj, ACPI_PM_PROP_SCI_INT, NULL);
-
- pm->acpi_enable_cmd = object_property_get_uint(obj,
- ACPI_PM_PROP_ACPI_ENABLE_CMD,
- NULL);
- pm->acpi_disable_cmd =
- object_property_get_uint(obj,
- ACPI_PM_PROP_ACPI_DISABLE_CMD,
- NULL);
- pm->io_base = object_property_get_uint(obj, ACPI_PM_PROP_PM_IO_BASE,
- NULL);
- pm->gpe0_blk = object_property_get_uint(obj, ACPI_PM_PROP_GPE0_BLK,
- NULL);
- pm->gpe0_blk_len = object_property_get_uint(obj, ACPI_PM_PROP_GPE0_BLK_LEN,
- NULL);
pm->pcihp_bridge_en =
object_property_get_bool(obj, "acpi-pci-hotplug-with-bridge-support",
NULL);
@@ -255,8 +282,6 @@ static void acpi_get_pci_holes(Range *hole, Range *hole64)
NULL));
}
-#define ACPI_PORT_SMI_CMD 0x00b2 /* TODO: this is APM_CNT_IOPORT */
-
static void acpi_align_size(GArray *blob, unsigned align)
{
/* Align size to multiple of given size. This reduces the chance
@@ -275,73 +300,53 @@ build_facs(GArray *table_data, BIOSLinker *linker)
}
/* Load chipset information in FADT */
-static void fadt_setup(AcpiFadtDescriptorRev3 *fadt, AcpiPmInfo *pm)
+static void fadt_setup(AcpiFadtDescriptorRev3 *fadt, AcpiFadtData f)
{
- fadt->model = 1;
+ fadt->model = f.int_model;
fadt->reserved1 = 0;
- fadt->sci_int = cpu_to_le16(pm->sci_int);
- fadt->smi_cmd = cpu_to_le32(ACPI_PORT_SMI_CMD);
- fadt->acpi_enable = pm->acpi_enable_cmd;
- fadt->acpi_disable = pm->acpi_disable_cmd;
+ fadt->sci_int = cpu_to_le16(f.sci_int);
+ fadt->smi_cmd = cpu_to_le32(f.smi_cmd);
+ fadt->acpi_enable = f.acpi_enable_cmd;
+ fadt->acpi_disable = f.acpi_disable_cmd;
/* EVT, CNT, TMR offset matches hw/acpi/core.c */
- fadt->pm1a_evt_blk = cpu_to_le32(pm->io_base);
- fadt->pm1a_cnt_blk = cpu_to_le32(pm->io_base + 0x04);
- fadt->pm_tmr_blk = cpu_to_le32(pm->io_base + 0x08);
- fadt->gpe0_blk = cpu_to_le32(pm->gpe0_blk);
+ fadt->pm1a_evt_blk = cpu_to_le32(f.pm1_evt.address);
+ fadt->pm1a_cnt_blk = cpu_to_le32(f.pm1_cnt.address);
+ fadt->pm_tmr_blk = cpu_to_le32(f.pm_tmr.address);
+ fadt->gpe0_blk = cpu_to_le32(f.gpe0_blk.address);
/* EVT, CNT, TMR length matches hw/acpi/core.c */
- fadt->pm1_evt_len = 4;
- fadt->pm1_cnt_len = 2;
- fadt->pm_tmr_len = 4;
- fadt->gpe0_blk_len = pm->gpe0_blk_len;
- fadt->plvl2_lat = cpu_to_le16(0xfff); /* C2 state not supported */
- fadt->plvl3_lat = cpu_to_le16(0xfff); /* C3 state not supported */
- fadt->flags = cpu_to_le32((1 << ACPI_FADT_F_WBINVD) |
- (1 << ACPI_FADT_F_PROC_C1) |
- (1 << ACPI_FADT_F_SLP_BUTTON) |
- (1 << ACPI_FADT_F_RTC_S4));
- fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_USE_PLATFORM_CLOCK);
- /* APIC destination mode ("Flat Logical") has an upper limit of 8 CPUs
- * For more than 8 CPUs, "Clustered Logical" mode has to be used
- */
- if (max_cpus > 8) {
- fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_FORCE_APIC_CLUSTER_MODEL);
- }
- fadt->century = RTC_CENTURY;
- if (pm->force_rev1_fadt) {
+ fadt->pm1_evt_len = f.pm1_evt.bit_width / 8;
+ fadt->pm1_cnt_len = f.pm1_cnt.bit_width / 8;
+ fadt->pm_tmr_len = f.pm_tmr.bit_width / 8;
+ fadt->gpe0_blk_len = f.gpe0_blk.bit_width / 8;
+ fadt->plvl2_lat = cpu_to_le16(f.c2_latency);
+ fadt->plvl3_lat = cpu_to_le16(f.c3_latency);
+ fadt->flags = cpu_to_le32(f.flags);
+ fadt->century = f.rtc_century;
+ if (f.rev == 1) {
return;
}
- fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_RESET_REG_SUP);
- fadt->reset_value = 0xf;
- fadt->reset_register.space_id = AML_SYSTEM_IO;
- fadt->reset_register.bit_width = 8;
- fadt->reset_register.address = cpu_to_le64(ICH9_RST_CNT_IOPORT);
- /* The above need not be conditional on machine type because the reset port
- * happens to be the same on PIIX (pc) and ICH9 (q35). */
- QEMU_BUILD_BUG_ON(ICH9_RST_CNT_IOPORT != RCR_IOPORT);
+ fadt->reset_value = f.reset_val;
+ fadt->reset_register = f.reset_reg;
+ fadt->reset_register.address = cpu_to_le64(f.reset_reg.address);
- fadt->xpm1a_event_block.space_id = AML_SYSTEM_IO;
- fadt->xpm1a_event_block.bit_width = fadt->pm1_evt_len * 8;
- fadt->xpm1a_event_block.address = cpu_to_le64(pm->io_base);
+ fadt->xpm1a_event_block = f.pm1_evt;
+ fadt->xpm1a_event_block.address = cpu_to_le64(f.pm1_evt.address);
- fadt->xpm1a_control_block.space_id = AML_SYSTEM_IO;
- fadt->xpm1a_control_block.bit_width = fadt->pm1_cnt_len * 8;
- fadt->xpm1a_control_block.address = cpu_to_le64(pm->io_base + 0x4);
+ fadt->xpm1a_control_block = f.pm1_cnt;
+ fadt->xpm1a_control_block.address = cpu_to_le64(f.pm1_cnt.address);
- fadt->xpm_timer_block.space_id = AML_SYSTEM_IO;
- fadt->xpm_timer_block.bit_width = fadt->pm_tmr_len * 8;
- fadt->xpm_timer_block.address = cpu_to_le64(pm->io_base + 0x8);
+ fadt->xpm_timer_block = f.pm_tmr;
+ fadt->xpm_timer_block.address = cpu_to_le64(f.pm_tmr.address);
- fadt->xgpe0_block.space_id = AML_SYSTEM_IO;
- fadt->xgpe0_block.bit_width = pm->gpe0_blk_len * 8;
- fadt->xgpe0_block.address = cpu_to_le64(pm->gpe0_blk);
+ fadt->xgpe0_block = f.gpe0_blk;
+ fadt->xgpe0_block.address = cpu_to_le64(f.gpe0_blk.address);
}
/* FADT */
static void
-build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,
- unsigned facs_tbl_offset, unsigned dsdt_tbl_offset,
+build_fadt(GArray *table_data, BIOSLinker *linker, AcpiFadtData *f,
const char *oem_id, const char *oem_table_id)
{
AcpiFadtDescriptorRev3 *fadt = acpi_data_push(table_data, sizeof(*fadt));
@@ -349,29 +354,32 @@ build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,
unsigned dsdt_entry_offset = (char *)&fadt->dsdt - table_data->data;
unsigned xdsdt_entry_offset = (char *)&fadt->x_dsdt - table_data->data;
int fadt_size = sizeof(*fadt);
- int rev = 3;
/* FACS address to be filled by Guest linker */
- bios_linker_loader_add_pointer(linker,
- ACPI_BUILD_TABLE_FILE, fw_ctrl_offset, sizeof(fadt->firmware_ctrl),
- ACPI_BUILD_TABLE_FILE, facs_tbl_offset);
+ if (f->facs_tbl_offset) {
+ bios_linker_loader_add_pointer(linker,
+ ACPI_BUILD_TABLE_FILE, fw_ctrl_offset, sizeof(fadt->firmware_ctrl),
+ ACPI_BUILD_TABLE_FILE, *f->facs_tbl_offset);
+ }
/* DSDT address to be filled by Guest linker */
- fadt_setup(fadt, pm);
- bios_linker_loader_add_pointer(linker,
- ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->dsdt),
- ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
- if (pm->force_rev1_fadt) {
- rev = 1;
+ fadt_setup(fadt, *f);
+ if (f->dsdt_tbl_offset) {
+ bios_linker_loader_add_pointer(linker,
+ ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->dsdt),
+ ACPI_BUILD_TABLE_FILE, *f->dsdt_tbl_offset);
+ }
+ if (f->rev == 1) {
fadt_size = offsetof(typeof(*fadt), reset_register);
- } else {
+ } else if (f->xdsdt_tbl_offset) {
bios_linker_loader_add_pointer(linker,
ACPI_BUILD_TABLE_FILE, xdsdt_entry_offset, sizeof(fadt->x_dsdt),
- ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
+ ACPI_BUILD_TABLE_FILE, *f->xdsdt_tbl_offset);
}
build_header(linker, table_data,
- (void *)fadt, "FACP", fadt_size, rev, oem_id, oem_table_id);
+ (void *)fadt, "FACP", fadt_size, f->rev,
+ oem_id, oem_table_id);
}
void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
@@ -2051,7 +2059,12 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
crs = aml_resource_template();
aml_append(crs,
- aml_io(AML_DECODE16, pm->gpe0_blk, pm->gpe0_blk, 1, pm->gpe0_blk_len)
+ aml_io(
+ AML_DECODE16,
+ pm->fadt.gpe0_blk.address,
+ pm->fadt.gpe0_blk.address,
+ 1,
+ pm->fadt.gpe0_blk.bit_width / 8)
);
aml_append(dev, aml_name_decl("_CRS", crs));
aml_append(scope, dev);
@@ -2698,7 +2711,10 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
/* ACPI tables pointed to by RSDT */
fadt = tables_blob->len;
acpi_add_table(table_offsets, tables_blob);
- build_fadt(tables_blob, tables->linker, &pm, facs, dsdt,
+ pm.fadt.facs_tbl_offset = &facs;
+ pm.fadt.dsdt_tbl_offset = &dsdt;
+ pm.fadt.xdsdt_tbl_offset = &dsdt;
+ build_fadt(tables_blob, tables->linker, &pm.fadt,
slic_oem.id, slic_oem.table_id);
aml_len += tables_blob->len - fadt;
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH 6/9] pc: acpi: use build_append_foo() API to construct FADT
2018-02-22 12:42 [Qemu-devel] [PATCH 0/9] generalize build_fadt() Igor Mammedov
` (4 preceding siblings ...)
2018-02-22 12:42 ` [Qemu-devel] [PATCH 5/9] pc: acpi: isolate FADT specific data into AcpiFadtData structure Igor Mammedov
@ 2018-02-22 12:42 ` Igor Mammedov
2018-02-27 14:10 ` Auger Eric
2018-02-22 12:42 ` [Qemu-devel] [PATCH 7/9] acpi: move build_fadt() from i386 specific to generic ACPI source Igor Mammedov
` (2 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Igor Mammedov @ 2018-02-22 12:42 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Shannon Zhao, Peter Maydell, Andrew Jones, qemu-arm
build_append_foo() API doesn't need explicit endianness
conversions which eliminates a source of errors and
it makes build_fadt() look like declarative definition of
FADT table in ACPI spec, which makes it easy to review.
Also it allows easily extending FADT to support other
revisions which will be used by follow up patches
where build_fadt() will be reused for ARM target.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/i386/acpi-build.c | 147 +++++++++++++++++++++++++++++----------------------
1 file changed, 85 insertions(+), 62 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 706ba35..544a4bc 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -299,87 +299,110 @@ build_facs(GArray *table_data, BIOSLinker *linker)
facs->length = cpu_to_le32(sizeof(*facs));
}
-/* Load chipset information in FADT */
-static void fadt_setup(AcpiFadtDescriptorRev3 *fadt, AcpiFadtData f)
-{
- fadt->model = f.int_model;
- fadt->reserved1 = 0;
- fadt->sci_int = cpu_to_le16(f.sci_int);
- fadt->smi_cmd = cpu_to_le32(f.smi_cmd);
- fadt->acpi_enable = f.acpi_enable_cmd;
- fadt->acpi_disable = f.acpi_disable_cmd;
- /* EVT, CNT, TMR offset matches hw/acpi/core.c */
- fadt->pm1a_evt_blk = cpu_to_le32(f.pm1_evt.address);
- fadt->pm1a_cnt_blk = cpu_to_le32(f.pm1_cnt.address);
- fadt->pm_tmr_blk = cpu_to_le32(f.pm_tmr.address);
- fadt->gpe0_blk = cpu_to_le32(f.gpe0_blk.address);
- /* EVT, CNT, TMR length matches hw/acpi/core.c */
- fadt->pm1_evt_len = f.pm1_evt.bit_width / 8;
- fadt->pm1_cnt_len = f.pm1_cnt.bit_width / 8;
- fadt->pm_tmr_len = f.pm_tmr.bit_width / 8;
- fadt->gpe0_blk_len = f.gpe0_blk.bit_width / 8;
- fadt->plvl2_lat = cpu_to_le16(f.c2_latency);
- fadt->plvl3_lat = cpu_to_le16(f.c3_latency);
- fadt->flags = cpu_to_le32(f.flags);
- fadt->century = f.rtc_century;
- if (f.rev == 1) {
- return;
- }
-
- fadt->reset_value = f.reset_val;
- fadt->reset_register = f.reset_reg;
- fadt->reset_register.address = cpu_to_le64(f.reset_reg.address);
-
- fadt->xpm1a_event_block = f.pm1_evt;
- fadt->xpm1a_event_block.address = cpu_to_le64(f.pm1_evt.address);
-
- fadt->xpm1a_control_block = f.pm1_cnt;
- fadt->xpm1a_control_block.address = cpu_to_le64(f.pm1_cnt.address);
-
- fadt->xpm_timer_block = f.pm_tmr;
- fadt->xpm_timer_block.address = cpu_to_le64(f.pm_tmr.address);
-
- fadt->xgpe0_block = f.gpe0_blk;
- fadt->xgpe0_block.address = cpu_to_le64(f.gpe0_blk.address);
-}
-
-
/* FADT */
static void
-build_fadt(GArray *table_data, BIOSLinker *linker, AcpiFadtData *f,
+build_fadt(GArray *tbl, BIOSLinker *linker, AcpiFadtData *f,
const char *oem_id, const char *oem_table_id)
{
- AcpiFadtDescriptorRev3 *fadt = acpi_data_push(table_data, sizeof(*fadt));
- unsigned fw_ctrl_offset = (char *)&fadt->firmware_ctrl - table_data->data;
- unsigned dsdt_entry_offset = (char *)&fadt->dsdt - table_data->data;
- unsigned xdsdt_entry_offset = (char *)&fadt->x_dsdt - table_data->data;
- int fadt_size = sizeof(*fadt);
+ int off;
+ int fadt_start = tbl->len;
+
+ acpi_data_push(tbl, sizeof(AcpiTableHeader));
- /* FACS address to be filled by Guest linker */
+ /* FACS address to be filled by Guest linker at runtime */
+ off = tbl->len;
+ build_append_int_noprefix(tbl, 0, 4); /* FIRMWARE_CTRL */
if (f->facs_tbl_offset) {
bios_linker_loader_add_pointer(linker,
- ACPI_BUILD_TABLE_FILE, fw_ctrl_offset, sizeof(fadt->firmware_ctrl),
+ ACPI_BUILD_TABLE_FILE, off, 4,
ACPI_BUILD_TABLE_FILE, *f->facs_tbl_offset);
}
- /* DSDT address to be filled by Guest linker */
- fadt_setup(fadt, *f);
+ /* DSDT address to be filled by Guest linker at runtime */
+ off = tbl->len;
+ build_append_int_noprefix(tbl, 0, 4); /* DSDT */
if (f->dsdt_tbl_offset) {
bios_linker_loader_add_pointer(linker,
- ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->dsdt),
+ ACPI_BUILD_TABLE_FILE, off, 4,
ACPI_BUILD_TABLE_FILE, *f->dsdt_tbl_offset);
}
+
+ /* ACPI1.0: INT_MODEL, ACPI2.0+: Reserved */
+ build_append_int_noprefix(tbl, f->int_model /* Multiple APIC */, 1);
+ /* Preferred_PM_Profile */
+ build_append_int_noprefix(tbl, 0 /* Unspecified */, 1);
+ build_append_int_noprefix(tbl, f->sci_int, 2); /* SCI_INT */
+ build_append_int_noprefix(tbl, f->smi_cmd, 4); /* SMI_CMD */
+ build_append_int_noprefix(tbl, f->acpi_enable_cmd, 1); /* ACPI_ENABLE */
+ build_append_int_noprefix(tbl, f->acpi_disable_cmd, 1); /* ACPI_DISABLE */
+ build_append_int_noprefix(tbl, 0 /* not supported */, 1); /* S4BIOS_REQ */
+ /* ACPI1.0: Reserved, ACPI2.0+: PSTATE_CNT */
+ build_append_int_noprefix(tbl, 0, 1);
+ build_append_int_noprefix(tbl, f->pm1_evt.address, 4); /* PM1a_EVT_BLK */
+ build_append_int_noprefix(tbl, 0, 4); /* PM1b_EVT_BLK */
+ build_append_int_noprefix(tbl, f->pm1_cnt.address, 4); /* PM1a_CNT_BLK */
+ build_append_int_noprefix(tbl, 0, 4); /* PM1b_CNT_BLK */
+ build_append_int_noprefix(tbl, 0, 4); /* PM2_CNT_BLK */
+ build_append_int_noprefix(tbl, f->pm_tmr.address, 4); /* PM_TMR_BLK */
+ build_append_int_noprefix(tbl, f->gpe0_blk.address, 4); /* GPE0_BLK */
+ build_append_int_noprefix(tbl, 0, 4); /* GPE1_BLK */
+ /* PM1_EVT_LEN */
+ build_append_int_noprefix(tbl, f->pm1_evt.bit_width / 8, 1);
+ /* PM1_CNT_LEN */
+ build_append_int_noprefix(tbl, f->pm1_cnt.bit_width / 8, 1);
+ build_append_int_noprefix(tbl, 0, 1); /* PM2_CNT_LEN */
+ build_append_int_noprefix(tbl, f->pm_tmr.bit_width / 8, 1); /* PM_TMR_LEN */
+ /* GPE0_BLK_LEN */
+ build_append_int_noprefix(tbl, f->gpe0_blk.bit_width / 8, 1);
+ build_append_int_noprefix(tbl, 0, 1); /* GPE1_BLK_LEN */
+ build_append_int_noprefix(tbl, 0, 1); /* GPE1_BASE */
+ build_append_int_noprefix(tbl, 0, 1); /* CST_CNT */
+ build_append_int_noprefix(tbl, f->c2_latency, 2); /* P_LVL2_LAT */
+ build_append_int_noprefix(tbl, f->c3_latency, 2); /* P_LVL3_LAT */
+ build_append_int_noprefix(tbl, 0, 2); /* FLUSH_SIZE */
+ build_append_int_noprefix(tbl, 0, 2); /* FLUSH_STRIDE */
+ build_append_int_noprefix(tbl, 0, 1); /* DUTY_OFFSET */
+ build_append_int_noprefix(tbl, 0, 1); /* DUTY_WIDTH */
+ build_append_int_noprefix(tbl, 0, 1); /* DAY_ALRM */
+ build_append_int_noprefix(tbl, 0, 1); /* MON_ALRM */
+ build_append_int_noprefix(tbl, f->rtc_century, 1); /* CENTURY */
+ build_append_int_noprefix(tbl, 0, 2); /* IAPC_BOOT_ARCH */
+ build_append_int_noprefix(tbl, 0, 1); /* Reserved */
+ build_append_int_noprefix(tbl, f->flags, 4); /* Flags */
+
if (f->rev == 1) {
- fadt_size = offsetof(typeof(*fadt), reset_register);
- } else if (f->xdsdt_tbl_offset) {
+ goto build_hdr;
+ }
+
+ build_append_gas_from_struct(tbl, &f->reset_reg); /* RESET_REG */
+ build_append_int_noprefix(tbl, f->reset_val, 1); /* RESET_VALUE */
+ build_append_int_noprefix(tbl, 0, 3); /* Reserved, ACPI 3.0 */
+ build_append_int_noprefix(tbl, 0, 8); /* X_FIRMWARE_CTRL */
+
+ /* XDSDT address to be filled by Guest linker at runtime */
+ off = tbl->len;
+ build_append_int_noprefix(tbl, 0, 8); /* X_DSDT */
+ if (f->xdsdt_tbl_offset) {
bios_linker_loader_add_pointer(linker,
- ACPI_BUILD_TABLE_FILE, xdsdt_entry_offset, sizeof(fadt->x_dsdt),
+ ACPI_BUILD_TABLE_FILE, off, 8,
ACPI_BUILD_TABLE_FILE, *f->xdsdt_tbl_offset);
}
- build_header(linker, table_data,
- (void *)fadt, "FACP", fadt_size, f->rev,
- oem_id, oem_table_id);
+ build_append_gas_from_struct(tbl, &f->pm1_evt); /* X_PM1a_EVT_BLK */
+ /* X_PM1b_EVT_BLK */
+ build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
+ build_append_gas_from_struct(tbl, &f->pm1_cnt); /* X_PM1a_CNT_BLK */
+ /* X_PM1b_CNT_BLK */
+ build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
+ /* X_PM2_CNT_BLK */
+ build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
+ build_append_gas_from_struct(tbl, &f->pm_tmr); /* X_PM_TMR_BLK */
+ build_append_gas_from_struct(tbl, &f->gpe0_blk); /* X_GPE0_BLK */
+ build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0); /* X_GPE1_BLK */
+
+build_hdr:
+ build_header(linker, tbl, (void *)(tbl->data + fadt_start),
+ "FACP", tbl->len - fadt_start, f->rev, oem_id, oem_table_id);
}
void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH 7/9] acpi: move build_fadt() from i386 specific to generic ACPI source
2018-02-22 12:42 [Qemu-devel] [PATCH 0/9] generalize build_fadt() Igor Mammedov
` (5 preceding siblings ...)
2018-02-22 12:42 ` [Qemu-devel] [PATCH 6/9] pc: acpi: use build_append_foo() API to construct FADT Igor Mammedov
@ 2018-02-22 12:42 ` Igor Mammedov
2018-02-27 14:15 ` Auger Eric
2018-02-22 12:42 ` [Qemu-devel] [PATCH 8/9] virt_arm: acpi: reuse common build_fadt() Igor Mammedov
2018-02-22 12:42 ` [Qemu-devel] [PATCH 9/9] tests: acpi: don't read all fields in test_acpi_fadt_table() Igor Mammedov
8 siblings, 1 reply; 24+ messages in thread
From: Igor Mammedov @ 2018-02-22 12:42 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Shannon Zhao, Peter Maydell, Andrew Jones, qemu-arm
it will be extended and reused in follow up patch by ARM target
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
include/hw/acpi/aml-build.h | 3 ++
hw/acpi/aml-build.c | 105 +++++++++++++++++++++++++++++++++++++++++++
hw/arm/virt-acpi-build.c | 6 +--
hw/i386/acpi-build.c | 106 --------------------------------------------
4 files changed, 111 insertions(+), 109 deletions(-)
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 8692ccc..6c36903 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -413,4 +413,7 @@ void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
uint64_t len, int node, MemoryAffinityFlags flags);
void build_slit(GArray *table_data, BIOSLinker *linker);
+
+void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
+ const char *oem_id, const char *oem_table_id);
#endif
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 3fef5f6..17fb71b 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1678,3 +1678,108 @@ void build_slit(GArray *table_data, BIOSLinker *linker)
"SLIT",
table_data->len - slit_start, 1, NULL, NULL);
}
+
+/* build rev1/rev3 FADT */
+void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
+ const char *oem_id, const char *oem_table_id)
+{
+ int off;
+ int fadt_start = tbl->len;
+
+ acpi_data_push(tbl, sizeof(AcpiTableHeader));
+
+ /* FACS address to be filled by Guest linker at runtime */
+ off = tbl->len;
+ build_append_int_noprefix(tbl, 0, 4); /* FIRMWARE_CTRL */
+ if (f->facs_tbl_offset) {
+ bios_linker_loader_add_pointer(linker,
+ ACPI_BUILD_TABLE_FILE, off, 4,
+ ACPI_BUILD_TABLE_FILE, *f->facs_tbl_offset);
+ }
+
+ /* DSDT address to be filled by Guest linker at runtime */
+ off = tbl->len;
+ build_append_int_noprefix(tbl, 0, 4); /* DSDT */
+ if (f->dsdt_tbl_offset) {
+ bios_linker_loader_add_pointer(linker,
+ ACPI_BUILD_TABLE_FILE, off, 4,
+ ACPI_BUILD_TABLE_FILE, *f->dsdt_tbl_offset);
+ }
+
+ /* ACPI1.0: INT_MODEL, ACPI2.0+: Reserved */
+ build_append_int_noprefix(tbl, f->int_model /* Multiple APIC */, 1);
+ /* Preferred_PM_Profile */
+ build_append_int_noprefix(tbl, 0 /* Unspecified */, 1);
+ build_append_int_noprefix(tbl, f->sci_int, 2); /* SCI_INT */
+ build_append_int_noprefix(tbl, f->smi_cmd, 4); /* SMI_CMD */
+ build_append_int_noprefix(tbl, f->acpi_enable_cmd, 1); /* ACPI_ENABLE */
+ build_append_int_noprefix(tbl, f->acpi_disable_cmd, 1); /* ACPI_DISABLE */
+ build_append_int_noprefix(tbl, 0 /* not supported */, 1); /* S4BIOS_REQ */
+ /* ACPI1.0: Reserved, ACPI2.0+: PSTATE_CNT */
+ build_append_int_noprefix(tbl, 0, 1);
+ build_append_int_noprefix(tbl, f->pm1_evt.address, 4); /* PM1a_EVT_BLK */
+ build_append_int_noprefix(tbl, 0, 4); /* PM1b_EVT_BLK */
+ build_append_int_noprefix(tbl, f->pm1_cnt.address, 4); /* PM1a_CNT_BLK */
+ build_append_int_noprefix(tbl, 0, 4); /* PM1b_CNT_BLK */
+ build_append_int_noprefix(tbl, 0, 4); /* PM2_CNT_BLK */
+ build_append_int_noprefix(tbl, f->pm_tmr.address, 4); /* PM_TMR_BLK */
+ build_append_int_noprefix(tbl, f->gpe0_blk.address, 4); /* GPE0_BLK */
+ build_append_int_noprefix(tbl, 0, 4); /* GPE1_BLK */
+ /* PM1_EVT_LEN */
+ build_append_int_noprefix(tbl, f->pm1_evt.bit_width / 8, 1);
+ /* PM1_CNT_LEN */
+ build_append_int_noprefix(tbl, f->pm1_cnt.bit_width / 8, 1);
+ build_append_int_noprefix(tbl, 0, 1); /* PM2_CNT_LEN */
+ build_append_int_noprefix(tbl, f->pm_tmr.bit_width / 8, 1); /* PM_TMR_LEN */
+ /* GPE0_BLK_LEN */
+ build_append_int_noprefix(tbl, f->gpe0_blk.bit_width / 8, 1);
+ build_append_int_noprefix(tbl, 0, 1); /* GPE1_BLK_LEN */
+ build_append_int_noprefix(tbl, 0, 1); /* GPE1_BASE */
+ build_append_int_noprefix(tbl, 0, 1); /* CST_CNT */
+ build_append_int_noprefix(tbl, f->c2_latency, 2); /* P_LVL2_LAT */
+ build_append_int_noprefix(tbl, f->c3_latency, 2); /* P_LVL3_LAT */
+ build_append_int_noprefix(tbl, 0, 2); /* FLUSH_SIZE */
+ build_append_int_noprefix(tbl, 0, 2); /* FLUSH_STRIDE */
+ build_append_int_noprefix(tbl, 0, 1); /* DUTY_OFFSET */
+ build_append_int_noprefix(tbl, 0, 1); /* DUTY_WIDTH */
+ build_append_int_noprefix(tbl, 0, 1); /* DAY_ALRM */
+ build_append_int_noprefix(tbl, 0, 1); /* MON_ALRM */
+ build_append_int_noprefix(tbl, f->rtc_century, 1); /* CENTURY */
+ build_append_int_noprefix(tbl, 0, 2); /* IAPC_BOOT_ARCH */
+ build_append_int_noprefix(tbl, 0, 1); /* Reserved */
+ build_append_int_noprefix(tbl, f->flags, 4); /* Flags */
+
+ if (f->rev == 1) {
+ goto build_hdr;
+ }
+
+ build_append_gas_from_struct(tbl, &f->reset_reg); /* RESET_REG */
+ build_append_int_noprefix(tbl, f->reset_val, 1); /* RESET_VALUE */
+ build_append_int_noprefix(tbl, 0, 3); /* Reserved, ACPI 3.0 */
+ build_append_int_noprefix(tbl, 0, 8); /* X_FIRMWARE_CTRL */
+
+ /* XDSDT address to be filled by Guest linker at runtime */
+ off = tbl->len;
+ build_append_int_noprefix(tbl, 0, 8); /* X_DSDT */
+ if (f->xdsdt_tbl_offset) {
+ bios_linker_loader_add_pointer(linker,
+ ACPI_BUILD_TABLE_FILE, off, 8,
+ ACPI_BUILD_TABLE_FILE, *f->xdsdt_tbl_offset);
+ }
+
+ build_append_gas_from_struct(tbl, &f->pm1_evt); /* X_PM1a_EVT_BLK */
+ /* X_PM1b_EVT_BLK */
+ build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
+ build_append_gas_from_struct(tbl, &f->pm1_cnt); /* X_PM1a_CNT_BLK */
+ /* X_PM1b_CNT_BLK */
+ build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
+ /* X_PM2_CNT_BLK */
+ build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
+ build_append_gas_from_struct(tbl, &f->pm_tmr); /* X_PM_TMR_BLK */
+ build_append_gas_from_struct(tbl, &f->gpe0_blk); /* X_GPE0_BLK */
+ build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0); /* X_GPE1_BLK */
+
+build_hdr:
+ build_header(linker, tbl, (void *)(tbl->data + fadt_start),
+ "FACP", tbl->len - fadt_start, f->rev, oem_id, oem_table_id);
+}
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index f7fa795..b644da9 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -651,8 +651,8 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
}
/* FADT */
-static void build_fadt(GArray *table_data, BIOSLinker *linker,
- VirtMachineState *vms, unsigned dsdt_tbl_offset)
+static void build_fadt_rev5(GArray *table_data, BIOSLinker *linker,
+ VirtMachineState *vms, unsigned dsdt_tbl_offset)
{
int fadt_start = table_data->len;
AcpiFadtDescriptorRev5_1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
@@ -761,7 +761,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
/* FADT MADT GTDT MCFG SPCR pointed to by RSDT */
acpi_add_table(table_offsets, tables_blob);
- build_fadt(tables_blob, tables->linker, vms, dsdt);
+ build_fadt_rev5(tables_blob, tables->linker, vms, dsdt);
acpi_add_table(table_offsets, tables_blob);
build_madt(tables_blob, tables->linker, vms);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 544a4bc..898a7e8 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -299,112 +299,6 @@ build_facs(GArray *table_data, BIOSLinker *linker)
facs->length = cpu_to_le32(sizeof(*facs));
}
-/* FADT */
-static void
-build_fadt(GArray *tbl, BIOSLinker *linker, AcpiFadtData *f,
- const char *oem_id, const char *oem_table_id)
-{
- int off;
- int fadt_start = tbl->len;
-
- acpi_data_push(tbl, sizeof(AcpiTableHeader));
-
- /* FACS address to be filled by Guest linker at runtime */
- off = tbl->len;
- build_append_int_noprefix(tbl, 0, 4); /* FIRMWARE_CTRL */
- if (f->facs_tbl_offset) {
- bios_linker_loader_add_pointer(linker,
- ACPI_BUILD_TABLE_FILE, off, 4,
- ACPI_BUILD_TABLE_FILE, *f->facs_tbl_offset);
- }
-
- /* DSDT address to be filled by Guest linker at runtime */
- off = tbl->len;
- build_append_int_noprefix(tbl, 0, 4); /* DSDT */
- if (f->dsdt_tbl_offset) {
- bios_linker_loader_add_pointer(linker,
- ACPI_BUILD_TABLE_FILE, off, 4,
- ACPI_BUILD_TABLE_FILE, *f->dsdt_tbl_offset);
- }
-
- /* ACPI1.0: INT_MODEL, ACPI2.0+: Reserved */
- build_append_int_noprefix(tbl, f->int_model /* Multiple APIC */, 1);
- /* Preferred_PM_Profile */
- build_append_int_noprefix(tbl, 0 /* Unspecified */, 1);
- build_append_int_noprefix(tbl, f->sci_int, 2); /* SCI_INT */
- build_append_int_noprefix(tbl, f->smi_cmd, 4); /* SMI_CMD */
- build_append_int_noprefix(tbl, f->acpi_enable_cmd, 1); /* ACPI_ENABLE */
- build_append_int_noprefix(tbl, f->acpi_disable_cmd, 1); /* ACPI_DISABLE */
- build_append_int_noprefix(tbl, 0 /* not supported */, 1); /* S4BIOS_REQ */
- /* ACPI1.0: Reserved, ACPI2.0+: PSTATE_CNT */
- build_append_int_noprefix(tbl, 0, 1);
- build_append_int_noprefix(tbl, f->pm1_evt.address, 4); /* PM1a_EVT_BLK */
- build_append_int_noprefix(tbl, 0, 4); /* PM1b_EVT_BLK */
- build_append_int_noprefix(tbl, f->pm1_cnt.address, 4); /* PM1a_CNT_BLK */
- build_append_int_noprefix(tbl, 0, 4); /* PM1b_CNT_BLK */
- build_append_int_noprefix(tbl, 0, 4); /* PM2_CNT_BLK */
- build_append_int_noprefix(tbl, f->pm_tmr.address, 4); /* PM_TMR_BLK */
- build_append_int_noprefix(tbl, f->gpe0_blk.address, 4); /* GPE0_BLK */
- build_append_int_noprefix(tbl, 0, 4); /* GPE1_BLK */
- /* PM1_EVT_LEN */
- build_append_int_noprefix(tbl, f->pm1_evt.bit_width / 8, 1);
- /* PM1_CNT_LEN */
- build_append_int_noprefix(tbl, f->pm1_cnt.bit_width / 8, 1);
- build_append_int_noprefix(tbl, 0, 1); /* PM2_CNT_LEN */
- build_append_int_noprefix(tbl, f->pm_tmr.bit_width / 8, 1); /* PM_TMR_LEN */
- /* GPE0_BLK_LEN */
- build_append_int_noprefix(tbl, f->gpe0_blk.bit_width / 8, 1);
- build_append_int_noprefix(tbl, 0, 1); /* GPE1_BLK_LEN */
- build_append_int_noprefix(tbl, 0, 1); /* GPE1_BASE */
- build_append_int_noprefix(tbl, 0, 1); /* CST_CNT */
- build_append_int_noprefix(tbl, f->c2_latency, 2); /* P_LVL2_LAT */
- build_append_int_noprefix(tbl, f->c3_latency, 2); /* P_LVL3_LAT */
- build_append_int_noprefix(tbl, 0, 2); /* FLUSH_SIZE */
- build_append_int_noprefix(tbl, 0, 2); /* FLUSH_STRIDE */
- build_append_int_noprefix(tbl, 0, 1); /* DUTY_OFFSET */
- build_append_int_noprefix(tbl, 0, 1); /* DUTY_WIDTH */
- build_append_int_noprefix(tbl, 0, 1); /* DAY_ALRM */
- build_append_int_noprefix(tbl, 0, 1); /* MON_ALRM */
- build_append_int_noprefix(tbl, f->rtc_century, 1); /* CENTURY */
- build_append_int_noprefix(tbl, 0, 2); /* IAPC_BOOT_ARCH */
- build_append_int_noprefix(tbl, 0, 1); /* Reserved */
- build_append_int_noprefix(tbl, f->flags, 4); /* Flags */
-
- if (f->rev == 1) {
- goto build_hdr;
- }
-
- build_append_gas_from_struct(tbl, &f->reset_reg); /* RESET_REG */
- build_append_int_noprefix(tbl, f->reset_val, 1); /* RESET_VALUE */
- build_append_int_noprefix(tbl, 0, 3); /* Reserved, ACPI 3.0 */
- build_append_int_noprefix(tbl, 0, 8); /* X_FIRMWARE_CTRL */
-
- /* XDSDT address to be filled by Guest linker at runtime */
- off = tbl->len;
- build_append_int_noprefix(tbl, 0, 8); /* X_DSDT */
- if (f->xdsdt_tbl_offset) {
- bios_linker_loader_add_pointer(linker,
- ACPI_BUILD_TABLE_FILE, off, 8,
- ACPI_BUILD_TABLE_FILE, *f->xdsdt_tbl_offset);
- }
-
- build_append_gas_from_struct(tbl, &f->pm1_evt); /* X_PM1a_EVT_BLK */
- /* X_PM1b_EVT_BLK */
- build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
- build_append_gas_from_struct(tbl, &f->pm1_cnt); /* X_PM1a_CNT_BLK */
- /* X_PM1b_CNT_BLK */
- build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
- /* X_PM2_CNT_BLK */
- build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
- build_append_gas_from_struct(tbl, &f->pm_tmr); /* X_PM_TMR_BLK */
- build_append_gas_from_struct(tbl, &f->gpe0_blk); /* X_GPE0_BLK */
- build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0); /* X_GPE1_BLK */
-
-build_hdr:
- build_header(linker, tbl, (void *)(tbl->data + fadt_start),
- "FACP", tbl->len - fadt_start, f->rev, oem_id, oem_table_id);
-}
-
void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
const CPUArchIdList *apic_ids, GArray *entry)
{
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH 8/9] virt_arm: acpi: reuse common build_fadt()
2018-02-22 12:42 [Qemu-devel] [PATCH 0/9] generalize build_fadt() Igor Mammedov
` (6 preceding siblings ...)
2018-02-22 12:42 ` [Qemu-devel] [PATCH 7/9] acpi: move build_fadt() from i386 specific to generic ACPI source Igor Mammedov
@ 2018-02-22 12:42 ` Igor Mammedov
2018-02-27 14:42 ` Auger Eric
2018-02-27 15:07 ` Auger Eric
2018-02-22 12:42 ` [Qemu-devel] [PATCH 9/9] tests: acpi: don't read all fields in test_acpi_fadt_table() Igor Mammedov
8 siblings, 2 replies; 24+ messages in thread
From: Igor Mammedov @ 2018-02-22 12:42 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Shannon Zhao, Peter Maydell, Andrew Jones, qemu-arm
Extend generic build_fadt() to support rev5.1 FADT
and reuse it for 'virt' board, it would allow to
phase out usage of AcpiFadtDescriptorRev5_1 and
later ACPI_FADT_COMMON_DEF.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
include/hw/acpi/acpi-defs.h | 12 ++----------
hw/acpi/aml-build.c | 21 ++++++++++++++++++++-
hw/arm/virt-acpi-build.c | 33 ++++++++++++---------------------
3 files changed, 34 insertions(+), 32 deletions(-)
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index e0accc4..34e49dc 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -165,16 +165,6 @@ struct AcpiFadtDescriptorRev3 {
} QEMU_PACKED;
typedef struct AcpiFadtDescriptorRev3 AcpiFadtDescriptorRev3;
-struct AcpiFadtDescriptorRev5_1 {
- ACPI_FADT_COMMON_DEF
- /* 64-bit Sleep Control register (ACPI 5.0) */
- struct AcpiGenericAddress sleep_control;
- /* 64-bit Sleep Status register (ACPI 5.0) */
- struct AcpiGenericAddress sleep_status;
-} QEMU_PACKED;
-
-typedef struct AcpiFadtDescriptorRev5_1 AcpiFadtDescriptorRev5_1;
-
typedef struct AcpiFadtData {
struct AcpiGenericAddress pm1_cnt; /* PM1a_CNT_BLK */
struct AcpiGenericAddress pm1_evt; /* PM1a_EVT_BLK */
@@ -192,6 +182,8 @@ typedef struct AcpiFadtData {
uint8_t rtc_century; /* CENTURY */
uint16_t c2_latency; /* P_LVL2_LAT */
uint16_t c3_latency; /* P_LVL3_LAT */
+ uint16_t arm_boot_arch; /* ARM_BOOT_ARCH */
+ uint8_t minor_ver; /* FADT Minor Version */
/*
* respective tables offsets within ACPI_BUILD_TABLE_FILE,
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 17fb71b..5a33cd0 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1755,7 +1755,14 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
build_append_gas_from_struct(tbl, &f->reset_reg); /* RESET_REG */
build_append_int_noprefix(tbl, f->reset_val, 1); /* RESET_VALUE */
- build_append_int_noprefix(tbl, 0, 3); /* Reserved, ACPI 3.0 */
+ /* Since ACPI 5.1 */
+ if ((f->rev >= 6) || ((f->rev == 5) && f->minor_ver > 0)) {
+ build_append_int_noprefix(tbl, f->arm_boot_arch, 2); /* ARM_BOOT_ARCH */
+ /* FADT Minor Version */
+ build_append_int_noprefix(tbl, f->minor_ver, 1);
+ } else {
+ build_append_int_noprefix(tbl, 0, 3); /* Reserved upto ACPI 5.0 */
+ }
build_append_int_noprefix(tbl, 0, 8); /* X_FIRMWARE_CTRL */
/* XDSDT address to be filled by Guest linker at runtime */
@@ -1779,6 +1786,18 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
build_append_gas_from_struct(tbl, &f->gpe0_blk); /* X_GPE0_BLK */
build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0); /* X_GPE1_BLK */
+ if (f->rev <= 4) {
+ goto build_hdr;
+ }
+
+ /* SLEEP_CONTROL_REG */
+ build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
+ /* SLEEP_STATUS_REG */
+ build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
+
+ /* TODO: extra fields need to be added to support revisions above rev5 */
+ assert(f->rev == 5);
+
build_hdr:
build_header(linker, tbl, (void *)(tbl->data + fadt_start),
"FACP", tbl->len - fadt_start, f->rev, oem_id, oem_table_id);
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index b644da9..c7c6a57 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -654,39 +654,30 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
static void build_fadt_rev5(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms, unsigned dsdt_tbl_offset)
{
- int fadt_start = table_data->len;
- AcpiFadtDescriptorRev5_1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
- unsigned xdsdt_entry_offset = (char *)&fadt->x_dsdt - table_data->data;
- uint16_t bootflags;
+ /* ACPI v5.1 */
+ AcpiFadtData fadt = {
+ .rev = 5,
+ .minor_ver = 1,
+ .flags = 1 << ACPI_FADT_F_HW_REDUCED_ACPI,
+ .xdsdt_tbl_offset = &dsdt_tbl_offset,
+ };
switch (vms->psci_conduit) {
case QEMU_PSCI_CONDUIT_DISABLED:
- bootflags = 0;
+ fadt.arm_boot_arch = 0;
break;
case QEMU_PSCI_CONDUIT_HVC:
- bootflags = ACPI_FADT_ARM_PSCI_COMPLIANT | ACPI_FADT_ARM_PSCI_USE_HVC;
+ fadt.arm_boot_arch = ACPI_FADT_ARM_PSCI_COMPLIANT |
+ ACPI_FADT_ARM_PSCI_USE_HVC;
break;
case QEMU_PSCI_CONDUIT_SMC:
- bootflags = ACPI_FADT_ARM_PSCI_COMPLIANT;
+ fadt.arm_boot_arch = ACPI_FADT_ARM_PSCI_COMPLIANT;
break;
default:
g_assert_not_reached();
}
- /* Hardware Reduced = 1 and use PSCI 0.2+ */
- fadt->flags = cpu_to_le32(1 << ACPI_FADT_F_HW_REDUCED_ACPI);
- fadt->arm_boot_flags = cpu_to_le16(bootflags);
-
- /* ACPI v5.1 (fadt->revision.fadt->minor_revision) */
- fadt->minor_revision = 0x1;
-
- /* DSDT address to be filled by Guest linker */
- bios_linker_loader_add_pointer(linker,
- ACPI_BUILD_TABLE_FILE, xdsdt_entry_offset, sizeof(fadt->x_dsdt),
- ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
-
- build_header(linker, table_data, (void *)(table_data->data + fadt_start),
- "FACP", table_data->len - fadt_start, 5, NULL, NULL);
+ build_fadt(table_data, linker, &fadt, NULL, NULL);
}
/* DSDT */
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH 9/9] tests: acpi: don't read all fields in test_acpi_fadt_table()
2018-02-22 12:42 [Qemu-devel] [PATCH 0/9] generalize build_fadt() Igor Mammedov
` (7 preceding siblings ...)
2018-02-22 12:42 ` [Qemu-devel] [PATCH 8/9] virt_arm: acpi: reuse common build_fadt() Igor Mammedov
@ 2018-02-22 12:42 ` Igor Mammedov
8 siblings, 0 replies; 24+ messages in thread
From: Igor Mammedov @ 2018-02-22 12:42 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Shannon Zhao, Peter Maydell, Andrew Jones, qemu-arm
there is no point to read fields here but not actually
checking them so drop it and read only header + dsdt/facs
addresses since it's needed later to fetch that tables.
With this cleanup we can get rid of AcpiFadtDescriptorRev3/
ACPI_FADT_COMMON_DEF which have no users left.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
include/hw/acpi/acpi-defs.h | 81 --------------------------------------------
tests/bios-tables-test.c | 82 ++++++++++-----------------------------------
2 files changed, 18 insertions(+), 145 deletions(-)
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index 34e49dc..1f872ee 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -75,82 +75,6 @@ struct AcpiTableHeader {
} QEMU_PACKED;
typedef struct AcpiTableHeader AcpiTableHeader;
-/*
- * ACPI Fixed ACPI Description Table (FADT)
- */
-#define ACPI_FADT_COMMON_DEF /* FADT common definition */ \
- ACPI_TABLE_HEADER_DEF /* ACPI common table header */ \
- uint32_t firmware_ctrl; /* Physical address of FACS */ \
- uint32_t dsdt; /* Physical address of DSDT */ \
- uint8_t model; /* System Interrupt Model */ \
- uint8_t reserved1; /* Reserved */ \
- uint16_t sci_int; /* System vector of SCI interrupt */ \
- uint32_t smi_cmd; /* Port address of SMI command port */ \
- uint8_t acpi_enable; /* Value to write to smi_cmd to enable ACPI */ \
- uint8_t acpi_disable; /* Value to write to smi_cmd to disable ACPI */ \
- /* Value to write to SMI CMD to enter S4BIOS state */ \
- uint8_t S4bios_req; \
- uint8_t reserved2; /* Reserved - must be zero */ \
- /* Port address of Power Mgt 1a acpi_event Reg Blk */ \
- uint32_t pm1a_evt_blk; \
- /* Port address of Power Mgt 1b acpi_event Reg Blk */ \
- uint32_t pm1b_evt_blk; \
- uint32_t pm1a_cnt_blk; /* Port address of Power Mgt 1a Control Reg Blk */ \
- uint32_t pm1b_cnt_blk; /* Port address of Power Mgt 1b Control Reg Blk */ \
- uint32_t pm2_cnt_blk; /* Port address of Power Mgt 2 Control Reg Blk */ \
- uint32_t pm_tmr_blk; /* Port address of Power Mgt Timer Ctrl Reg Blk */ \
- /* Port addr of General Purpose acpi_event 0 Reg Blk */ \
- uint32_t gpe0_blk; \
- /* Port addr of General Purpose acpi_event 1 Reg Blk */ \
- uint32_t gpe1_blk; \
- uint8_t pm1_evt_len; /* Byte length of ports at pm1_x_evt_blk */ \
- uint8_t pm1_cnt_len; /* Byte length of ports at pm1_x_cnt_blk */ \
- uint8_t pm2_cnt_len; /* Byte Length of ports at pm2_cnt_blk */ \
- uint8_t pm_tmr_len; /* Byte Length of ports at pm_tm_blk */ \
- uint8_t gpe0_blk_len; /* Byte Length of ports at gpe0_blk */ \
- uint8_t gpe1_blk_len; /* Byte Length of ports at gpe1_blk */ \
- uint8_t gpe1_base; /* Offset in gpe model where gpe1 events start */ \
- uint8_t reserved3; /* Reserved */ \
- uint16_t plvl2_lat; /* Worst case HW latency to enter/exit C2 state */ \
- uint16_t plvl3_lat; /* Worst case HW latency to enter/exit C3 state */ \
- uint16_t flush_size; /* Size of area read to flush caches */ \
- uint16_t flush_stride; /* Stride used in flushing caches */ \
- uint8_t duty_offset; /* Bit location of duty cycle field in p_cnt reg */ \
- uint8_t duty_width; /* Bit width of duty cycle field in p_cnt reg */ \
- uint8_t day_alrm; /* Index to day-of-month alarm in RTC CMOS RAM */ \
- uint8_t mon_alrm; /* Index to month-of-year alarm in RTC CMOS RAM */ \
- uint8_t century; /* Index to century in RTC CMOS RAM */ \
- /* IA-PC Boot Architecture Flags (see below for individual flags) */ \
- uint16_t boot_flags; \
- uint8_t reserved; /* Reserved, must be zero */ \
- /* Miscellaneous flag bits (see below for individual flags) */ \
- uint32_t flags; \
- /* 64-bit address of the Reset register */ \
- struct AcpiGenericAddress reset_register; \
- /* Value to write to the reset_register port to reset the system */ \
- uint8_t reset_value; \
- /* ARM-Specific Boot Flags (see below for individual flags) (ACPI 5.1) */ \
- uint16_t arm_boot_flags; \
- uint8_t minor_revision; /* FADT Minor Revision (ACPI 5.1) */ \
- uint64_t x_facs; /* 64-bit physical address of FACS */ \
- uint64_t x_dsdt; /* 64-bit physical address of DSDT */ \
- /* 64-bit Extended Power Mgt 1a Event Reg Blk address */ \
- struct AcpiGenericAddress xpm1a_event_block; \
- /* 64-bit Extended Power Mgt 1b Event Reg Blk address */ \
- struct AcpiGenericAddress xpm1b_event_block; \
- /* 64-bit Extended Power Mgt 1a Control Reg Blk address */ \
- struct AcpiGenericAddress xpm1a_control_block; \
- /* 64-bit Extended Power Mgt 1b Control Reg Blk address */ \
- struct AcpiGenericAddress xpm1b_control_block; \
- /* 64-bit Extended Power Mgt 2 Control Reg Blk address */ \
- struct AcpiGenericAddress xpm2_control_block; \
- /* 64-bit Extended Power Mgt Timer Ctrl Reg Blk address */ \
- struct AcpiGenericAddress xpm_timer_block; \
- /* 64-bit Extended General Purpose Event 0 Reg Blk address */ \
- struct AcpiGenericAddress xgpe0_block; \
- /* 64-bit Extended General Purpose Event 1 Reg Blk address */ \
- struct AcpiGenericAddress xgpe1_block; \
-
struct AcpiGenericAddress {
uint8_t space_id; /* Address space where struct or register exists */
uint8_t bit_width; /* Size in bits of given register */
@@ -160,11 +84,6 @@ struct AcpiGenericAddress {
uint64_t address; /* 64-bit address of struct or register */
} QEMU_PACKED;
-struct AcpiFadtDescriptorRev3 {
- ACPI_FADT_COMMON_DEF
-} QEMU_PACKED;
-typedef struct AcpiFadtDescriptorRev3 AcpiFadtDescriptorRev3;
-
typedef struct AcpiFadtData {
struct AcpiGenericAddress pm1_cnt; /* PM1a_CNT_BLK */
struct AcpiGenericAddress pm1_evt; /* PM1a_EVT_BLK */
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 65b271a..cd753ff 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -29,7 +29,8 @@ typedef struct {
uint32_t rsdp_addr;
AcpiRsdpDescriptor rsdp_table;
AcpiRsdtDescriptorRev1 rsdt_table;
- AcpiFadtDescriptorRev3 fadt_table;
+ uint32_t dsdt_addr;
+ uint32_t facs_addr;
AcpiFacsDescriptorRev1 facs_table;
uint32_t *rsdt_tables_addr;
int rsdt_tables_nr;
@@ -127,71 +128,18 @@ static void test_acpi_rsdt_table(test_data *data)
data->rsdt_tables_nr = tables_nr;
}
-static void test_acpi_fadt_table(test_data *data)
+static void fadt_fetch_facs_and_dsdt_ptrs(test_data *data)
{
- AcpiFadtDescriptorRev3 *fadt_table = &data->fadt_table;
uint32_t addr;
+ AcpiTableHeader hdr;
/* FADT table comes first */
addr = le32_to_cpu(data->rsdt_tables_addr[0]);
- ACPI_READ_TABLE_HEADER(fadt_table, addr);
-
- ACPI_READ_FIELD(fadt_table->firmware_ctrl, addr);
- ACPI_READ_FIELD(fadt_table->dsdt, addr);
- ACPI_READ_FIELD(fadt_table->model, addr);
- ACPI_READ_FIELD(fadt_table->reserved1, addr);
- ACPI_READ_FIELD(fadt_table->sci_int, addr);
- ACPI_READ_FIELD(fadt_table->smi_cmd, addr);
- ACPI_READ_FIELD(fadt_table->acpi_enable, addr);
- ACPI_READ_FIELD(fadt_table->acpi_disable, addr);
- ACPI_READ_FIELD(fadt_table->S4bios_req, addr);
- ACPI_READ_FIELD(fadt_table->reserved2, addr);
- ACPI_READ_FIELD(fadt_table->pm1a_evt_blk, addr);
- ACPI_READ_FIELD(fadt_table->pm1b_evt_blk, addr);
- ACPI_READ_FIELD(fadt_table->pm1a_cnt_blk, addr);
- ACPI_READ_FIELD(fadt_table->pm1b_cnt_blk, addr);
- ACPI_READ_FIELD(fadt_table->pm2_cnt_blk, addr);
- ACPI_READ_FIELD(fadt_table->pm_tmr_blk, addr);
- ACPI_READ_FIELD(fadt_table->gpe0_blk, addr);
- ACPI_READ_FIELD(fadt_table->gpe1_blk, addr);
- ACPI_READ_FIELD(fadt_table->pm1_evt_len, addr);
- ACPI_READ_FIELD(fadt_table->pm1_cnt_len, addr);
- ACPI_READ_FIELD(fadt_table->pm2_cnt_len, addr);
- ACPI_READ_FIELD(fadt_table->pm_tmr_len, addr);
- ACPI_READ_FIELD(fadt_table->gpe0_blk_len, addr);
- ACPI_READ_FIELD(fadt_table->gpe1_blk_len, addr);
- ACPI_READ_FIELD(fadt_table->gpe1_base, addr);
- ACPI_READ_FIELD(fadt_table->reserved3, addr);
- ACPI_READ_FIELD(fadt_table->plvl2_lat, addr);
- ACPI_READ_FIELD(fadt_table->plvl3_lat, addr);
- ACPI_READ_FIELD(fadt_table->flush_size, addr);
- ACPI_READ_FIELD(fadt_table->flush_stride, addr);
- ACPI_READ_FIELD(fadt_table->duty_offset, addr);
- ACPI_READ_FIELD(fadt_table->duty_width, addr);
- ACPI_READ_FIELD(fadt_table->day_alrm, addr);
- ACPI_READ_FIELD(fadt_table->mon_alrm, addr);
- ACPI_READ_FIELD(fadt_table->century, addr);
- ACPI_READ_FIELD(fadt_table->boot_flags, addr);
- ACPI_READ_FIELD(fadt_table->reserved, addr);
- ACPI_READ_FIELD(fadt_table->flags, addr);
- ACPI_READ_GENERIC_ADDRESS(fadt_table->reset_register, addr);
- ACPI_READ_FIELD(fadt_table->reset_value, addr);
- ACPI_READ_FIELD(fadt_table->arm_boot_flags, addr);
- ACPI_READ_FIELD(fadt_table->minor_revision, addr);
- ACPI_READ_FIELD(fadt_table->x_facs, addr);
- ACPI_READ_FIELD(fadt_table->x_dsdt, addr);
- ACPI_READ_GENERIC_ADDRESS(fadt_table->xpm1a_event_block, addr);
- ACPI_READ_GENERIC_ADDRESS(fadt_table->xpm1b_event_block, addr);
- ACPI_READ_GENERIC_ADDRESS(fadt_table->xpm1a_control_block, addr);
- ACPI_READ_GENERIC_ADDRESS(fadt_table->xpm1b_control_block, addr);
- ACPI_READ_GENERIC_ADDRESS(fadt_table->xpm2_control_block, addr);
- ACPI_READ_GENERIC_ADDRESS(fadt_table->xpm_timer_block, addr);
- ACPI_READ_GENERIC_ADDRESS(fadt_table->xgpe0_block, addr);
- ACPI_READ_GENERIC_ADDRESS(fadt_table->xgpe1_block, addr);
-
- ACPI_ASSERT_CMP(fadt_table->signature, "FACP");
- g_assert(!acpi_calc_checksum((uint8_t *)fadt_table,
- le32_to_cpu(fadt_table->length)));
+ ACPI_READ_TABLE_HEADER(&hdr, addr);
+ ACPI_ASSERT_CMP(hdr.signature, "FACP");
+
+ ACPI_READ_FIELD(data->facs_addr, addr);
+ ACPI_READ_FIELD(data->dsdt_addr, addr);
}
static void sanitize_fadt_ptrs(test_data *data)
@@ -206,6 +154,12 @@ static void sanitize_fadt_ptrs(test_data *data)
continue;
}
+ /* check original FADT checksum before sanitizing table */
+ g_assert(!(uint8_t)(
+ acpi_calc_checksum((uint8_t *)sdt, sizeof(AcpiTableHeader)) +
+ acpi_calc_checksum((uint8_t *)sdt->aml, sdt->aml_len)
+ ));
+
/* sdt->aml field offset := spec offset - header size */
memset(sdt->aml + 0, 0, 4); /* sanitize FIRMWARE_CTRL(36) ptr */
memset(sdt->aml + 4, 0, 4); /* sanitize DSDT(40) ptr */
@@ -226,7 +180,7 @@ static void sanitize_fadt_ptrs(test_data *data)
static void test_acpi_facs_table(test_data *data)
{
AcpiFacsDescriptorRev1 *facs_table = &data->facs_table;
- uint32_t addr = le32_to_cpu(data->fadt_table.firmware_ctrl);
+ uint32_t addr = le32_to_cpu(data->facs_addr);
ACPI_READ_FIELD(facs_table->signature, addr);
ACPI_READ_FIELD(facs_table->length, addr);
@@ -265,7 +219,7 @@ static void fetch_table(AcpiSdtTable *sdt_table, uint32_t addr)
static void test_acpi_dsdt_table(test_data *data)
{
AcpiSdtTable dsdt_table;
- uint32_t addr = le32_to_cpu(data->fadt_table.dsdt);
+ uint32_t addr = le32_to_cpu(data->dsdt_addr);
fetch_table(&dsdt_table, addr);
ACPI_ASSERT_CMP(dsdt_table.header.signature, "DSDT");
@@ -674,7 +628,7 @@ static void test_acpi_one(const char *params, test_data *data)
test_acpi_rsdp_address(data);
test_acpi_rsdp_table(data);
test_acpi_rsdt_table(data);
- test_acpi_fadt_table(data);
+ fadt_fetch_facs_and_dsdt_ptrs(data);
test_acpi_facs_table(data);
test_acpi_dsdt_table(data);
fetch_rsdt_referenced_tables(data);
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH 1/9] acpi: remove unused acpi-dsdt.aml
2018-02-22 12:42 ` [Qemu-devel] [PATCH 1/9] acpi: remove unused acpi-dsdt.aml Igor Mammedov
@ 2018-02-22 14:25 ` Gerd Hoffmann
2018-02-27 12:42 ` Auger Eric
1 sibling, 0 replies; 24+ messages in thread
From: Gerd Hoffmann @ 2018-02-22 14:25 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, Michael S. Tsirkin, Shannon Zhao, Peter Maydell,
Andrew Jones, qemu-arm
On Thu, Feb 22, 2018 at 01:42:48PM +0100, Igor Mammedov wrote:
> SeaBIOS blob which is currently shipped with QEMU
> doesn't need acpi-dsdt.aml nor is able to use it
> and code that loaded it QEMU was removed by
> (commit 9fb7aaaf4c "pc: drop external DSDT loading")
> in 2013.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> CC: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH 3/9] acpi: reuse AcpiGenericAddress instead of Acpi20GenericAddress
2018-02-22 12:42 ` [Qemu-devel] [PATCH 3/9] acpi: reuse AcpiGenericAddress instead of Acpi20GenericAddress Igor Mammedov
@ 2018-02-27 12:42 ` Auger Eric
0 siblings, 0 replies; 24+ messages in thread
From: Auger Eric @ 2018-02-27 12:42 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel
Cc: Peter Maydell, Andrew Jones, Shannon Zhao, qemu-arm, Michael S. Tsirkin
Hi Igor,
On 22/02/18 13:42, Igor Mammedov wrote:
> Drop duplicate in form of Acpi20GenericAddress and reuse
> AcpiGenericAddress.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Eric
> ---
> include/hw/acpi/acpi-defs.h | 17 +++--------------
> 1 file changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index 80c8099..9942bc5 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -40,18 +40,6 @@ enum {
> ACPI_FADT_F_LOW_POWER_S0_IDLE_CAPABLE,
> };
>
> -/*
> - * ACPI 2.0 Generic Address Space definition.
> - */
> -struct Acpi20GenericAddress {
> - uint8_t address_space_id;
> - uint8_t register_bit_width;
> - uint8_t register_bit_offset;
> - uint8_t reserved;
> - uint64_t address;
> -} QEMU_PACKED;
> -typedef struct Acpi20GenericAddress Acpi20GenericAddress;
> -
> struct AcpiRsdpDescriptor { /* Root System Descriptor Pointer */
> uint64_t signature; /* ACPI signature, contains "RSD PTR " */
> uint8_t checksum; /* To make sum of struct == 0 */
> @@ -167,7 +155,8 @@ struct AcpiGenericAddress {
> uint8_t space_id; /* Address space where struct or register exists */
> uint8_t bit_width; /* Size in bits of given register */
> uint8_t bit_offset; /* Bit offset within the register */
> - uint8_t access_width; /* Minimum Access size (ACPI 3.0) */
> + uint8_t access_width; /* ACPI 3.0: Minimum Access size (ACPI 3.0),
> + ACPI 2.0: Reserved, Table 5-1 */
> uint64_t address; /* 64-bit address of struct or register */
> } QEMU_PACKED;
>
> @@ -456,7 +445,7 @@ typedef struct AcpiGenericTimerTable AcpiGenericTimerTable;
> struct Acpi20Hpet {
> ACPI_TABLE_HEADER_DEF /* ACPI common table header */
> uint32_t timer_block_id;
> - Acpi20GenericAddress addr;
> + struct AcpiGenericAddress addr;
> uint8_t hpet_number;
> uint16_t min_tick;
> uint8_t page_protect;
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH 2/9] pc: replace pm object initialization with one-liner in acpi_get_pm_info()
2018-02-22 12:42 ` [Qemu-devel] [PATCH 2/9] pc: replace pm object initialization with one-liner in acpi_get_pm_info() Igor Mammedov
@ 2018-02-27 12:42 ` Auger Eric
2018-02-27 23:41 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
1 sibling, 0 replies; 24+ messages in thread
From: Auger Eric @ 2018-02-27 12:42 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel
Cc: Peter Maydell, Andrew Jones, Shannon Zhao, qemu-arm, Michael S. Tsirkin
Hi,
On 22/02/18 13:42, Igor Mammedov wrote:
> next patch will need it before it gets to piix4/lpc branches
> that initializes 'obj' now.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Eric
> ---
> hw/i386/acpi-build.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index deb440f..b85fefe 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -128,7 +128,7 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
> {
> Object *piix = piix4_pm_find();
> Object *lpc = ich9_lpc_find();
> - Object *obj = NULL;
> + Object *obj = piix ? piix : lpc;
> QObject *o;
>
> pm->force_rev1_fadt = false;
> @@ -138,7 +138,6 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
> if (piix) {
> /* w2k requires FADT(rev1) or it won't boot, keep PC compatible */
> pm->force_rev1_fadt = true;
> - obj = piix;
> pm->cpu_hp_io_base = PIIX4_CPU_HOTPLUG_IO_BASE;
> pm->pcihp_io_base =
> object_property_get_uint(obj, ACPI_PCIHP_IO_BASE_PROP, NULL);
> @@ -146,7 +145,6 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
> object_property_get_uint(obj, ACPI_PCIHP_IO_LEN_PROP, NULL);
> }
> if (lpc) {
> - obj = lpc;
> pm->cpu_hp_io_base = ICH9_CPU_HOTPLUG_IO_BASE;
> }
> assert(obj);
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH 1/9] acpi: remove unused acpi-dsdt.aml
2018-02-22 12:42 ` [Qemu-devel] [PATCH 1/9] acpi: remove unused acpi-dsdt.aml Igor Mammedov
2018-02-22 14:25 ` Gerd Hoffmann
@ 2018-02-27 12:42 ` Auger Eric
2018-02-27 15:09 ` Igor Mammedov
1 sibling, 1 reply; 24+ messages in thread
From: Auger Eric @ 2018-02-27 12:42 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel
Cc: Peter Maydell, Andrew Jones, Michael S. Tsirkin, qemu-arm,
Gerd Hoffmann, Shannon Zhao
Hi,
On 22/02/18 13:42, Igor Mammedov wrote:
> SeaBIOS blob which is currently shipped with QEMU
> doesn't need acpi-dsdt.aml nor is able to use it
> and code that loaded it QEMU was removed by
as code that loaded it in QEMU was removed by?
> (commit 9fb7aaaf4c "pc: drop external DSDT loading")
> in 2013.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> CC: Gerd Hoffmann <kraxel@redhat.com>
Otherwise
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
> ---
> Makefile | 1 -
> pc-bios/acpi-dsdt.aml | Bin 4405 -> 0 bytes
> 2 files changed, 1 deletion(-)
> delete mode 100644 pc-bios/acpi-dsdt.aml
>
> diff --git a/Makefile b/Makefile
> index 90e05ac..f3a4cf9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -648,7 +648,6 @@ bepo cz
> ifdef INSTALL_BLOBS
> BLOBS=bios.bin bios-256k.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \
> vgabios-stdvga.bin vgabios-vmware.bin vgabios-qxl.bin vgabios-virtio.bin \
> -acpi-dsdt.aml \
> ppc_rom.bin openbios-sparc32 openbios-sparc64 openbios-ppc QEMU,tcx.bin QEMU,cgthree.bin \
> pxe-e1000.rom pxe-eepro100.rom pxe-ne2k_pci.rom \
> pxe-pcnet.rom pxe-rtl8139.rom pxe-virtio.rom \
> diff --git a/pc-bios/acpi-dsdt.aml b/pc-bios/acpi-dsdt.aml
> deleted file mode 100644
> index 558c10f51ccbbf9ec2f47a4a998a7055059a8963..0000000000000000000000000000000000000000
> GIT binary patch
> literal 0
> HcmV?d00001
>
> literal 4405
> zcmb7{O>f)C8OI;KNTx=#Ov$uk$9XY?b_=xIjb0q@_EJP5WkrsxFrpHqP*76NE}&CE
> zqzSOzpn&5RO*WTe*G<u*ve0?^5!z#q`3UV-*rJF}QJ-hVlxK_$u;qa>|Kam8znLL9
> z<A?s>dJ#bTx_LkF0GjuGY(WhGo!+3koGWcQ9rFPU5B+94((<~g4WH$C9dAv`{m^gT
> zZEJrW$A5|A$IoMJl)(Ns&a3@V^7|L@K9JFq{e&^9IOQm8M#H0x!0S}3=w`>a8*i9l
> zMGe0XR&=-HYff+FBQoL^UcVI<n+xnWFBU<!u}c6mx@m3g#6Gb#3)?l@pr*I@_{5&;
> z#Tgm?=lKNy@f?7`Y?dceyma7Ch?1^<&1QdpC#wIrasZas-`*<LS>@%=&fLZ0Lo8-=
> z2|2%0`vJHO7J2;;UQ)-|gCMNeL^TdtX>}ZQ>$N1Pgb_W)N-Ls=$)m@-itRY~WHYgk
> zgX+BqrWEW?)FoC3!tE_lT@6}k^@E_hy_E!2ipVPzkypAAJ^BL$Apdw8JA0Oxf|hkN
> zXbsXi&~OfL^l_GN27^7ov3&C`59aWhLwfmMtLJY9eLvcCx1(^-fP`A&gqlWQ#LS5&
> z_E*O-9LM?DYzmXYSH~mx^T>vO|2H#*DO<8=Sc*kf_+yTS?9DqcX}p{p+4*D-kG#yi
> zb|d180Xv{$XK)dCIxy@<o~j1#hl>PNAErQ+8n3KJVco~H$7En{Z-6#s#%p5=&X1+|
> z>%skMU4%Dqj4^z*PT^<HPDV28n4R#f8{BTI;^{1=ethujk0=Ux0^Ga?3*DgA)8G>@
> zyarVauZe}V<Kx}wZd^0cwM;RGM?dcmJR}qgKaWeEhGmVdw6z2haP%@Q?MLtk^y~o)
> zk3PQD^ylV=;pX_@&&QKH#t?&sUZ29JSeA7h*5T1l_HN&uJ1#AsceGfh3=SFYnmfKX
> ze-#(NT@&+50P!S?b2^3B<~-vDTWf3I8Q&RTwzap$TO7yo4fv_alm4<B4CYDAc_<p8
> z?+N9w#kTgj@ws7H<wNe@GQHb-)pT?+n)o23J)-e_Uzii)!~m=8@Gv_Rrgkn2)8}z;
> zg5DcPKhZIcg>jsYb+#sOA%+7j58pBiUkMT(uE)EZc=I=hhhb|Mzl_$me4&!?nw8e>
> zrn?k)tz9iS(8fRw?snkyc5t5KZ$5k#v#U?yN%1MgInZJNd^U)+Nr_tgvleDJyAxVO
> zPWv%F!Kn)RgVL?v9+vVZzHHF#-SR=yHLN$FWK%oSQ8ZIwpzxryXxg(Ge)CX;b46Zg
> zSP;*+ADX6;JTX4^)VU|xo+|Q8P4P9NjA+U|QIaS2hTGy7TG*Z{@=Q$);fbc)6D4`3
> zS@1I<Y`Lcir;Oax6rO44QOcYd?wR%=!#z{ejPOi5k5cB$Dx6vFnVM!*PLwj|g2K7L
> zJyXsFl@q1RX(^nR!fC0TC}mFkpyCLnoH>Ovr*fi{Ihn%A6i%jcqLexDH;OrNO!%zi
> z70$fMiBjgo54$v<w!&$voG4|^MTHap`xqyk&qb9JrOa7SI137ALFGg#b1o^IOA6<b
> z%863ubQDfU;dE3^lrkr7#-#ZyDx5`?6Q#^qQaDQrXG!HmDRV9>oXZO5vdW24=5!TK
> zSK)M3PLwj|io&^~aIUDFC}qx7g>zNmTva(y%AB|xl-BJ9h4X^SiBjfVQ#jWY&NY=2
> zrOdgmaIPzy>nbNone(E;c~RlKsB)r|IX4te+zyF%j(;^bR8EvK=Ou;nlEQgO<wPlS
> zURF3SE1Z{APLwj|6@~MP!g)pIL@9G#RXDFIoL5y&l!9~k>_^uO`jgU*EWn+e7WD5_
> zEWB0eR-;?pa+f=I@RvWyJ!OYu+`;CiEbnf2?s)wi8uTm00?U7yg&aRY9KcIzV;Q`6
> zCiz!mc9@K*KBea2QFnpf=yXS7<8GMt+Vm$6i>qw;%L3#K{9WM*1%OT{c#v2UJ3Z<I
> zb<ZtEekX+AQJo#~mL-1Dm{OOxz7U1|P<uHRy}+$`zeDY(*_-FG<L2rIXRk`xt2}!Z
> z`$y-TG<((k{_NG^(H^mT=dv^X|43hx(${$U+PU<#_oT0#ruWaM$J5Rarmsus>pXq^
> zT>AQZ($|Maw@suE&!;y<`g94=kqD<e-Q4HhET3#QFUFX<icK`TPP;%`LHD{B>@_qz
> zV0*#s-WcMfm}eH?9)hk>GJY{)I`G1PBt~VzbmU(20$kE(UXx6W8+$q?xy%b%yZW%q
> z{)wleKHuy9jcpE}*<9c)y5YFHS*&<~ODl{%!&5$OWOp*J;^)+h)37m&ChTd<7T}A0
> zZU426&7a``5jTMQ$<uue9!|<%ACDd;4|&&Pn6TrAT5quPt5|z&@sb%&VyBmj+Chtt
> z+hW5DI+aRg8*mW1l?u2kQL9pg2WMw1+E%*`w$|W**tBCmxpiGQZHeN#C{81NEYv5W
> U_=PAMqG*c36NN8|mMC`Mf01wzJpcdz
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH 4/9] acpi: add build_append_gas() helper for Generic Address Structure
2018-02-22 12:42 ` [Qemu-devel] [PATCH 4/9] acpi: add build_append_gas() helper for Generic Address Structure Igor Mammedov
@ 2018-02-27 12:48 ` Auger Eric
0 siblings, 0 replies; 24+ messages in thread
From: Auger Eric @ 2018-02-27 12:48 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel
Cc: Peter Maydell, Andrew Jones, Shannon Zhao, qemu-arm, Michael S. Tsirkin
Hi,
On 22/02/18 13:42, Igor Mammedov wrote:
> it will help to add Generic Address Structure to ACPI tables
> without using packed C structures and avoid endianness
> issues as API doesn't need an explicit conversion.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
> ---
> include/hw/acpi/aml-build.h | 20 ++++++++++++++++++++
> hw/acpi/aml-build.c | 16 ++++++++++++++++
> 2 files changed, 36 insertions(+)
>
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 88d0738..8692ccc 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -78,6 +78,15 @@ typedef enum {
> } AmlUpdateRule;
>
> typedef enum {
> + AML_AS_SYSTEM_MEMORY = 0X00,
> + AML_AS_SYSTEM_IO = 0X01,
> + AML_AS_PCI_CONFIG = 0X02,
> + AML_AS_EMBEDDED_CTRL = 0X03,
> + AML_AS_SMBUS = 0X04,
> + AML_AS_FFH = 0X7F,
> +} AmlAddressSpace;
> +
> +typedef enum {
> AML_SYSTEM_MEMORY = 0X00,
> AML_SYSTEM_IO = 0X01,
> AML_PCI_CONFIG = 0X02,
> @@ -389,6 +398,17 @@ int
> build_append_named_dword(GArray *array, const char *name_format, ...)
> GCC_FMT_ATTR(2, 3);
>
> +void build_append_gas(GArray *table, AmlAddressSpace as,
> + uint8_t bit_width, uint8_t bit_offset,
> + uint8_t access_width, uint64_t address);
> +
> +static inline void
> +build_append_gas_from_struct(GArray *table, const struct AcpiGenericAddress *s)
> +{
> + build_append_gas(table, s->space_id, s->bit_width, s->bit_offset,
> + s->access_width, s->address);
> +}
> +
> void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
> uint64_t len, int node, MemoryAffinityFlags flags);
>
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 36a6cc4..3fef5f6 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -258,6 +258,22 @@ static void build_append_int(GArray *table, uint64_t value)
> }
> }
>
> +/* Generic Address Structure (GAS)
> + * ACPI 2.0/3.0: 5.2.3.1 Generic Address Structure
> + * 2.0 compat note:
> + * @access_width must be 0, see ACPI 2.0:Table 5-1
> + */
> +void build_append_gas(GArray *table, AmlAddressSpace as,
> + uint8_t bit_width, uint8_t bit_offset,
> + uint8_t access_width, uint64_t address)
> +{
> + build_append_int_noprefix(table, as, 1);
> + build_append_int_noprefix(table, bit_width, 1);
> + build_append_int_noprefix(table, bit_offset, 1);
> + build_append_int_noprefix(table, access_width, 1);
> + build_append_int_noprefix(table, address, 8);
> +}
> +
> /*
> * Build NAME(XXXX, 0x00000000) where 0x00000000 is encoded as a dword,
> * and return the offset to 0x00000000 for runtime patching.
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH 5/9] pc: acpi: isolate FADT specific data into AcpiFadtData structure
2018-02-22 12:42 ` [Qemu-devel] [PATCH 5/9] pc: acpi: isolate FADT specific data into AcpiFadtData structure Igor Mammedov
@ 2018-02-27 13:47 ` Auger Eric
2018-02-27 15:44 ` Igor Mammedov
0 siblings, 1 reply; 24+ messages in thread
From: Auger Eric @ 2018-02-27 13:47 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel
Cc: Peter Maydell, Andrew Jones, Shannon Zhao, qemu-arm, Michael S. Tsirkin
Hi Igor,
On 22/02/18 13:42, Igor Mammedov wrote:
> move FADT data initialization out of fadt_setup() into dedicated
> init_fadt_data() that will set common for pc/q35 values in
> AcpiFadtData structure and acpi_get_pm_info() will complement
> it with pc/q35 specific values initialization.
acpi_get_pm_info only adds reset stuff + version info on top of
previously initialized common data, right?
>
> That will allow to get rid of fadt_setup() and generalize
> build_fadt() so it could be easily extended for rev5 and
> reused by ARM target.
>
> While at it also move facs/dsdt/xdsdt offsets from build_fadt()
> arg list into AcpiFadtData, as they belong to the same dataset.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> include/hw/acpi/acpi-defs.h | 28 ++++++
> hw/i386/acpi-build.c | 204 ++++++++++++++++++++++++--------------------
> 2 files changed, 138 insertions(+), 94 deletions(-)
>
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index 9942bc5..e0accc4 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -175,6 +175,34 @@ struct AcpiFadtDescriptorRev5_1 {
>
> typedef struct AcpiFadtDescriptorRev5_1 AcpiFadtDescriptorRev5_1;
>
> +typedef struct AcpiFadtData {
> + struct AcpiGenericAddress pm1_cnt; /* PM1a_CNT_BLK */
Any reason why we don't use the same field names as in
AcpiFadtDescriptorRev3?
for instance pm1a_cnt?
> + struct AcpiGenericAddress pm1_evt; /* PM1a_EVT_BLK */
pm1a_evt?
> + struct AcpiGenericAddress pm_tmr; /* PM_TMR_BLK */
> + struct AcpiGenericAddress gpe0_blk; /* GPE0_BLK */
> + struct AcpiGenericAddress reset_reg; /* RESET_REG */
> + uint8_t reset_val; /* RESET_VALUE */
> + uint8_t rev; /* Revision */
> + uint32_t flags; /* Flags */
> + uint32_t smi_cmd; /* SMI_CMD */
> + uint16_t sci_int; /* SCI_INT */
> + uint8_t int_model; /* INT_MODEL */
> + uint8_t acpi_enable_cmd; /* ACPI_ENABLE */
> + uint8_t acpi_disable_cmd; /* ACPI_DISABLE */
> + uint8_t rtc_century; /* CENTURY */
> + uint16_t c2_latency; /* P_LVL2_LAT */
same?
> + uint16_t c3_latency; /* P_LVL3_LAT */
> +
> + /*
> + * respective tables offsets within ACPI_BUILD_TABLE_FILE,
> + * NULL if table doesn't exist (in that case field's value
> + * won't be patched by linker and will be kept set to 0)
> + */
> + unsigned *facs_tbl_offset; /* FACS offset in */
> + unsigned *dsdt_tbl_offset;
> + unsigned *xdsdt_tbl_offset;
> +} AcpiFadtData;
> +
> #define ACPI_FADT_ARM_PSCI_COMPLIANT (1 << 0)
> #define ACPI_FADT_ARM_PSCI_USE_HVC (1 << 1)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index b85fefe..706ba35 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -91,17 +91,11 @@ typedef struct AcpiMcfgInfo {
> } AcpiMcfgInfo;
>
> typedef struct AcpiPmInfo {
> - bool force_rev1_fadt;
> bool s3_disabled;
> bool s4_disabled;
> bool pcihp_bridge_en;
> uint8_t s4_val;
> - uint16_t sci_int;
> - uint8_t acpi_enable_cmd;
> - uint8_t acpi_disable_cmd;
> - uint32_t gpe0_blk;
> - uint32_t gpe0_blk_len;
> - uint32_t io_base;
> + AcpiFadtData fadt;
> uint16_t cpu_hp_io_base;
> uint16_t pcihp_io_base;
> uint16_t pcihp_io_len;
> @@ -124,20 +118,60 @@ typedef struct AcpiBuildPciBusHotplugState {
> bool pcihp_bridge_en;
> } AcpiBuildPciBusHotplugState;
>
> +#define ACPI_PORT_SMI_CMD 0x00b2 /* TODO: this is APM_CNT_IOPORT */
Can't we take benefit of this series to move that define in hw/isa/apm.h?
> +
> +static void init_fadt_data(Object *o, AcpiFadtData *data)
> +{
> + uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL);
> + AmlAddressSpace as = AML_AS_SYSTEM_IO;
> + AcpiFadtData fadt = {
> + .rev = 3,
> + .flags =
> + (1 << ACPI_FADT_F_WBINVD) |
> + (1 << ACPI_FADT_F_PROC_C1) |
> + (1 << ACPI_FADT_F_SLP_BUTTON) |
> + (1 << ACPI_FADT_F_RTC_S4) |
> + (1 << ACPI_FADT_F_USE_PLATFORM_CLOCK) |
> + /* APIC destination mode ("Flat Logical") has an upper limit of 8
> + * CPUs for more than 8 CPUs, "Clustered Logical" mode has to be
> + * used
> + */
> + ((max_cpus > 8) ? (1 << ACPI_FADT_F_FORCE_APIC_CLUSTER_MODEL) : 0),
> + .int_model = 1 /* Multiple APIC */,
> + .rtc_century = RTC_CENTURY,
> + .c2_latency = 0xfff /* C2 state not supported */,
> + .c3_latency = 0xfff /* C3 state not supported */,
> + .smi_cmd = ACPI_PORT_SMI_CMD,
> + .sci_int = object_property_get_uint(o, ACPI_PM_PROP_SCI_INT, NULL),
> + .acpi_enable_cmd =
> + object_property_get_uint(o, ACPI_PM_PROP_ACPI_ENABLE_CMD, NULL),
> + .acpi_disable_cmd =
> + object_property_get_uint(o, ACPI_PM_PROP_ACPI_DISABLE_CMD, NULL),
> + .pm1_evt = { .space_id = as, .bit_width = 4 * 8, .address = io },
> + .pm1_cnt = { .space_id = as, .bit_width = 2 * 8, .address = io + 0x04 },
> + .pm_tmr = { .space_id = as, .bit_width = 4 * 8, .address = io + 0x08 },
> + .gpe0_blk = { .space_id = as, .bit_width =
> + object_property_get_uint(o, ACPI_PM_PROP_GPE0_BLK_LEN, NULL) * 8,
> + .address = object_property_get_uint(o, ACPI_PM_PROP_GPE0_BLK, NULL)
> + },
> + };
> + *data = fadt;
> +}
> +
> static void acpi_get_pm_info(AcpiPmInfo *pm)
> {
> Object *piix = piix4_pm_find();
> Object *lpc = ich9_lpc_find();
> Object *obj = piix ? piix : lpc;
> QObject *o;
> -
> - pm->force_rev1_fadt = false;
> pm->cpu_hp_io_base = 0;
> pm->pcihp_io_base = 0;
> pm->pcihp_io_len = 0;
> +
> + init_fadt_data(obj, &pm->fadt);
> if (piix) {
> /* w2k requires FADT(rev1) or it won't boot, keep PC compatible */
> - pm->force_rev1_fadt = true;
> + pm->fadt.rev = 1;
> pm->cpu_hp_io_base = PIIX4_CPU_HOTPLUG_IO_BASE;
> pm->pcihp_io_base =
> object_property_get_uint(obj, ACPI_PCIHP_IO_BASE_PROP, NULL);
> @@ -145,10 +179,19 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
> object_property_get_uint(obj, ACPI_PCIHP_IO_LEN_PROP, NULL);
> }
> if (lpc) {
> + struct AcpiGenericAddress r = { .space_id = AML_AS_SYSTEM_IO,
> + .bit_width = 8, .address = ICH9_RST_CNT_IOPORT };
> + pm->fadt.reset_reg = r;
> + pm->fadt.reset_val = 0xf;
> + pm->fadt.flags |= 1 << ACPI_FADT_F_RESET_REG_SUP;
> pm->cpu_hp_io_base = ICH9_CPU_HOTPLUG_IO_BASE;
Wouldn't it be possible to pass an other parameter to init_fadt_data()
to tell whether we are in pc or q35 or the target rev and do those
initializations there?
> }
> assert(obj);
>
> + /* The above need not be conditional on machine type because the reset port
> + * happens to be the same on PIIX (pc) and ICH9 (q35). */
> + QEMU_BUILD_BUG_ON(ICH9_RST_CNT_IOPORT != RCR_IOPORT);
> +
> /* Fill in optional s3/s4 related properties */
> o = object_property_get_qobject(obj, ACPI_PM_PROP_S3_DISABLED, NULL);
> if (o) {
> @@ -172,22 +215,6 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
> }
> qobject_decref(o);
>
> - /* Fill in mandatory properties */
> - pm->sci_int = object_property_get_uint(obj, ACPI_PM_PROP_SCI_INT, NULL);
> -
> - pm->acpi_enable_cmd = object_property_get_uint(obj,
> - ACPI_PM_PROP_ACPI_ENABLE_CMD,
> - NULL);
> - pm->acpi_disable_cmd =
> - object_property_get_uint(obj,
> - ACPI_PM_PROP_ACPI_DISABLE_CMD,
> - NULL);
> - pm->io_base = object_property_get_uint(obj, ACPI_PM_PROP_PM_IO_BASE,
> - NULL);
> - pm->gpe0_blk = object_property_get_uint(obj, ACPI_PM_PROP_GPE0_BLK,
> - NULL);
> - pm->gpe0_blk_len = object_property_get_uint(obj, ACPI_PM_PROP_GPE0_BLK_LEN,
> - NULL);
> pm->pcihp_bridge_en =
> object_property_get_bool(obj, "acpi-pci-hotplug-with-bridge-support",
> NULL);
> @@ -255,8 +282,6 @@ static void acpi_get_pci_holes(Range *hole, Range *hole64)
> NULL));
> }
>
> -#define ACPI_PORT_SMI_CMD 0x00b2 /* TODO: this is APM_CNT_IOPORT */
> -
> static void acpi_align_size(GArray *blob, unsigned align)
> {
> /* Align size to multiple of given size. This reduces the chance
> @@ -275,73 +300,53 @@ build_facs(GArray *table_data, BIOSLinker *linker)
> }
>
> /* Load chipset information in FADT */
> -static void fadt_setup(AcpiFadtDescriptorRev3 *fadt, AcpiPmInfo *pm)
> +static void fadt_setup(AcpiFadtDescriptorRev3 *fadt, AcpiFadtData f)
> {
> - fadt->model = 1;
> + fadt->model = f.int_model;
> fadt->reserved1 = 0;
> - fadt->sci_int = cpu_to_le16(pm->sci_int);
> - fadt->smi_cmd = cpu_to_le32(ACPI_PORT_SMI_CMD);
> - fadt->acpi_enable = pm->acpi_enable_cmd;
> - fadt->acpi_disable = pm->acpi_disable_cmd;
> + fadt->sci_int = cpu_to_le16(f.sci_int);
> + fadt->smi_cmd = cpu_to_le32(f.smi_cmd);
> + fadt->acpi_enable = f.acpi_enable_cmd;
> + fadt->acpi_disable = f.acpi_disable_cmd;
> /* EVT, CNT, TMR offset matches hw/acpi/core.c */
> - fadt->pm1a_evt_blk = cpu_to_le32(pm->io_base);
> - fadt->pm1a_cnt_blk = cpu_to_le32(pm->io_base + 0x04);
> - fadt->pm_tmr_blk = cpu_to_le32(pm->io_base + 0x08);
> - fadt->gpe0_blk = cpu_to_le32(pm->gpe0_blk);
> + fadt->pm1a_evt_blk = cpu_to_le32(f.pm1_evt.address);
> + fadt->pm1a_cnt_blk = cpu_to_le32(f.pm1_cnt.address);
> + fadt->pm_tmr_blk = cpu_to_le32(f.pm_tmr.address);
> + fadt->gpe0_blk = cpu_to_le32(f.gpe0_blk.address);
> /* EVT, CNT, TMR length matches hw/acpi/core.c */
> - fadt->pm1_evt_len = 4;
> - fadt->pm1_cnt_len = 2;
> - fadt->pm_tmr_len = 4;
> - fadt->gpe0_blk_len = pm->gpe0_blk_len;
> - fadt->plvl2_lat = cpu_to_le16(0xfff); /* C2 state not supported */
> - fadt->plvl3_lat = cpu_to_le16(0xfff); /* C3 state not supported */
> - fadt->flags = cpu_to_le32((1 << ACPI_FADT_F_WBINVD) |
> - (1 << ACPI_FADT_F_PROC_C1) |
> - (1 << ACPI_FADT_F_SLP_BUTTON) |
> - (1 << ACPI_FADT_F_RTC_S4));
> - fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_USE_PLATFORM_CLOCK);
> - /* APIC destination mode ("Flat Logical") has an upper limit of 8 CPUs
> - * For more than 8 CPUs, "Clustered Logical" mode has to be used
> - */
> - if (max_cpus > 8) {
> - fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_FORCE_APIC_CLUSTER_MODEL);
> - }
> - fadt->century = RTC_CENTURY;
> - if (pm->force_rev1_fadt) {
> + fadt->pm1_evt_len = f.pm1_evt.bit_width / 8;
> + fadt->pm1_cnt_len = f.pm1_cnt.bit_width / 8;
> + fadt->pm_tmr_len = f.pm_tmr.bit_width / 8;
> + fadt->gpe0_blk_len = f.gpe0_blk.bit_width / 8;
> + fadt->plvl2_lat = cpu_to_le16(f.c2_latency);
> + fadt->plvl3_lat = cpu_to_le16(f.c3_latency);
> + fadt->flags = cpu_to_le32(f.flags);
> + fadt->century = f.rtc_century;
> + if (f.rev == 1) {
> return;
> }
>
> - fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_RESET_REG_SUP);
> - fadt->reset_value = 0xf;
> - fadt->reset_register.space_id = AML_SYSTEM_IO;
> - fadt->reset_register.bit_width = 8;
> - fadt->reset_register.address = cpu_to_le64(ICH9_RST_CNT_IOPORT);
> - /* The above need not be conditional on machine type because the reset port
> - * happens to be the same on PIIX (pc) and ICH9 (q35). */
> - QEMU_BUILD_BUG_ON(ICH9_RST_CNT_IOPORT != RCR_IOPORT);
> + fadt->reset_value = f.reset_val;
> + fadt->reset_register = f.reset_reg;
> + fadt->reset_register.address = cpu_to_le64(f.reset_reg.address);
>
> - fadt->xpm1a_event_block.space_id = AML_SYSTEM_IO;
> - fadt->xpm1a_event_block.bit_width = fadt->pm1_evt_len * 8;
> - fadt->xpm1a_event_block.address = cpu_to_le64(pm->io_base);
> + fadt->xpm1a_event_block = f.pm1_evt;
> + fadt->xpm1a_event_block.address = cpu_to_le64(f.pm1_evt.address);
>
> - fadt->xpm1a_control_block.space_id = AML_SYSTEM_IO;
> - fadt->xpm1a_control_block.bit_width = fadt->pm1_cnt_len * 8;
> - fadt->xpm1a_control_block.address = cpu_to_le64(pm->io_base + 0x4);
> + fadt->xpm1a_control_block = f.pm1_cnt;
> + fadt->xpm1a_control_block.address = cpu_to_le64(f.pm1_cnt.address);
>
> - fadt->xpm_timer_block.space_id = AML_SYSTEM_IO;
> - fadt->xpm_timer_block.bit_width = fadt->pm_tmr_len * 8;
> - fadt->xpm_timer_block.address = cpu_to_le64(pm->io_base + 0x8);
> + fadt->xpm_timer_block = f.pm_tmr;
> + fadt->xpm_timer_block.address = cpu_to_le64(f.pm_tmr.address);
>
> - fadt->xgpe0_block.space_id = AML_SYSTEM_IO;
> - fadt->xgpe0_block.bit_width = pm->gpe0_blk_len * 8;
> - fadt->xgpe0_block.address = cpu_to_le64(pm->gpe0_blk);
> + fadt->xgpe0_block = f.gpe0_blk;
> + fadt->xgpe0_block.address = cpu_to_le64(f.gpe0_blk.address);
> }
>
>
> /* FADT */
> static void
> -build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,
> - unsigned facs_tbl_offset, unsigned dsdt_tbl_offset,
> +build_fadt(GArray *table_data, BIOSLinker *linker, AcpiFadtData *f,
> const char *oem_id, const char *oem_table_id)
> {
> AcpiFadtDescriptorRev3 *fadt = acpi_data_push(table_data, sizeof(*fadt));
> @@ -349,29 +354,32 @@ build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,
> unsigned dsdt_entry_offset = (char *)&fadt->dsdt - table_data->data;
> unsigned xdsdt_entry_offset = (char *)&fadt->x_dsdt - table_data->data;
> int fadt_size = sizeof(*fadt);
> - int rev = 3;
>
> /* FACS address to be filled by Guest linker */
> - bios_linker_loader_add_pointer(linker,
> - ACPI_BUILD_TABLE_FILE, fw_ctrl_offset, sizeof(fadt->firmware_ctrl),
> - ACPI_BUILD_TABLE_FILE, facs_tbl_offset);
> + if (f->facs_tbl_offset) {
this test was not there originally. How does it relate to above changes?
> + bios_linker_loader_add_pointer(linker,
> + ACPI_BUILD_TABLE_FILE, fw_ctrl_offset, sizeof(fadt->firmware_ctrl),
> + ACPI_BUILD_TABLE_FILE, *f->facs_tbl_offset);
> + }
>
> /* DSDT address to be filled by Guest linker */
> - fadt_setup(fadt, pm);
> - bios_linker_loader_add_pointer(linker,
> - ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->dsdt),
> - ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
> - if (pm->force_rev1_fadt) {
> - rev = 1;
> + fadt_setup(fadt, *f);
> + if (f->dsdt_tbl_offset) {
same
> + bios_linker_loader_add_pointer(linker,
> + ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->dsdt),
> + ACPI_BUILD_TABLE_FILE, *f->dsdt_tbl_offset);
> + }
> + if (f->rev == 1) {
> fadt_size = offsetof(typeof(*fadt), reset_register);
> - } else {
> + } else if (f->xdsdt_tbl_offset) {
> bios_linker_loader_add_pointer(linker,
> ACPI_BUILD_TABLE_FILE, xdsdt_entry_offset, sizeof(fadt->x_dsdt),
> - ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
> + ACPI_BUILD_TABLE_FILE, *f->xdsdt_tbl_offset);
> }
>
> build_header(linker, table_data,
> - (void *)fadt, "FACP", fadt_size, rev, oem_id, oem_table_id);
> + (void *)fadt, "FACP", fadt_size, f->rev,
> + oem_id, oem_table_id);
> }
>
> void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
> @@ -2051,7 +2059,12 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
> crs = aml_resource_template();
> aml_append(crs,
> - aml_io(AML_DECODE16, pm->gpe0_blk, pm->gpe0_blk, 1, pm->gpe0_blk_len)
> + aml_io(
> + AML_DECODE16,
> + pm->fadt.gpe0_blk.address,
> + pm->fadt.gpe0_blk.address,
> + 1,
> + pm->fadt.gpe0_blk.bit_width / 8)
> );
> aml_append(dev, aml_name_decl("_CRS", crs));
> aml_append(scope, dev);
> @@ -2698,7 +2711,10 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> /* ACPI tables pointed to by RSDT */
> fadt = tables_blob->len;
> acpi_add_table(table_offsets, tables_blob);
> - build_fadt(tables_blob, tables->linker, &pm, facs, dsdt,
> + pm.fadt.facs_tbl_offset = &facs;
> + pm.fadt.dsdt_tbl_offset = &dsdt;
> + pm.fadt.xdsdt_tbl_offset = &dsdt;
> + build_fadt(tables_blob, tables->linker, &pm.fadt,
> slic_oem.id, slic_oem.table_id);
> aml_len += tables_blob->len - fadt;
>
>
Otherwise looks good to me.
Thanks
Eric
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH 6/9] pc: acpi: use build_append_foo() API to construct FADT
2018-02-22 12:42 ` [Qemu-devel] [PATCH 6/9] pc: acpi: use build_append_foo() API to construct FADT Igor Mammedov
@ 2018-02-27 14:10 ` Auger Eric
0 siblings, 0 replies; 24+ messages in thread
From: Auger Eric @ 2018-02-27 14:10 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel
Cc: Peter Maydell, Andrew Jones, Shannon Zhao, qemu-arm, Michael S. Tsirkin
Hi Igor,
On 22/02/18 13:42, Igor Mammedov wrote:
> build_append_foo() API doesn't need explicit endianness
> conversions which eliminates a source of errors and
> it makes build_fadt() look like declarative definition of
> FADT table in ACPI spec, which makes it easy to review.
> Also it allows easily extending FADT to support other
> revisions which will be used by follow up patches
> where build_fadt() will be reused for ARM target.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
> ---
> hw/i386/acpi-build.c | 147 +++++++++++++++++++++++++++++----------------------
> 1 file changed, 85 insertions(+), 62 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 706ba35..544a4bc 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -299,87 +299,110 @@ build_facs(GArray *table_data, BIOSLinker *linker)
> facs->length = cpu_to_le32(sizeof(*facs));
> }
>
> -/* Load chipset information in FADT */
> -static void fadt_setup(AcpiFadtDescriptorRev3 *fadt, AcpiFadtData f)
> -{
> - fadt->model = f.int_model;
> - fadt->reserved1 = 0;
> - fadt->sci_int = cpu_to_le16(f.sci_int);
> - fadt->smi_cmd = cpu_to_le32(f.smi_cmd);
> - fadt->acpi_enable = f.acpi_enable_cmd;
> - fadt->acpi_disable = f.acpi_disable_cmd;
> - /* EVT, CNT, TMR offset matches hw/acpi/core.c */
> - fadt->pm1a_evt_blk = cpu_to_le32(f.pm1_evt.address);
> - fadt->pm1a_cnt_blk = cpu_to_le32(f.pm1_cnt.address);
> - fadt->pm_tmr_blk = cpu_to_le32(f.pm_tmr.address);
> - fadt->gpe0_blk = cpu_to_le32(f.gpe0_blk.address);
> - /* EVT, CNT, TMR length matches hw/acpi/core.c */
> - fadt->pm1_evt_len = f.pm1_evt.bit_width / 8;
> - fadt->pm1_cnt_len = f.pm1_cnt.bit_width / 8;
> - fadt->pm_tmr_len = f.pm_tmr.bit_width / 8;
> - fadt->gpe0_blk_len = f.gpe0_blk.bit_width / 8;
> - fadt->plvl2_lat = cpu_to_le16(f.c2_latency);
> - fadt->plvl3_lat = cpu_to_le16(f.c3_latency);
> - fadt->flags = cpu_to_le32(f.flags);
> - fadt->century = f.rtc_century;
> - if (f.rev == 1) {
> - return;
> - }
> -
> - fadt->reset_value = f.reset_val;
> - fadt->reset_register = f.reset_reg;
> - fadt->reset_register.address = cpu_to_le64(f.reset_reg.address);
> -
> - fadt->xpm1a_event_block = f.pm1_evt;
> - fadt->xpm1a_event_block.address = cpu_to_le64(f.pm1_evt.address);
> -
> - fadt->xpm1a_control_block = f.pm1_cnt;
> - fadt->xpm1a_control_block.address = cpu_to_le64(f.pm1_cnt.address);
> -
> - fadt->xpm_timer_block = f.pm_tmr;
> - fadt->xpm_timer_block.address = cpu_to_le64(f.pm_tmr.address);
> -
> - fadt->xgpe0_block = f.gpe0_blk;
> - fadt->xgpe0_block.address = cpu_to_le64(f.gpe0_blk.address);
> -}
> -
> -
> /* FADT */
> static void
> -build_fadt(GArray *table_data, BIOSLinker *linker, AcpiFadtData *f,
> +build_fadt(GArray *tbl, BIOSLinker *linker, AcpiFadtData *f,
> const char *oem_id, const char *oem_table_id)
> {
> - AcpiFadtDescriptorRev3 *fadt = acpi_data_push(table_data, sizeof(*fadt));
> - unsigned fw_ctrl_offset = (char *)&fadt->firmware_ctrl - table_data->data;
> - unsigned dsdt_entry_offset = (char *)&fadt->dsdt - table_data->data;
> - unsigned xdsdt_entry_offset = (char *)&fadt->x_dsdt - table_data->data;
> - int fadt_size = sizeof(*fadt);
> + int off;
> + int fadt_start = tbl->len;
> +
> + acpi_data_push(tbl, sizeof(AcpiTableHeader));
>
> - /* FACS address to be filled by Guest linker */
> + /* FACS address to be filled by Guest linker at runtime */
> + off = tbl->len;
> + build_append_int_noprefix(tbl, 0, 4); /* FIRMWARE_CTRL */
> if (f->facs_tbl_offset) {
> bios_linker_loader_add_pointer(linker,
> - ACPI_BUILD_TABLE_FILE, fw_ctrl_offset, sizeof(fadt->firmware_ctrl),
> + ACPI_BUILD_TABLE_FILE, off, 4,
> ACPI_BUILD_TABLE_FILE, *f->facs_tbl_offset);
> }
>
> - /* DSDT address to be filled by Guest linker */
> - fadt_setup(fadt, *f);
> + /* DSDT address to be filled by Guest linker at runtime */
> + off = tbl->len;
> + build_append_int_noprefix(tbl, 0, 4); /* DSDT */
> if (f->dsdt_tbl_offset) {
> bios_linker_loader_add_pointer(linker,
> - ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->dsdt),
> + ACPI_BUILD_TABLE_FILE, off, 4,
> ACPI_BUILD_TABLE_FILE, *f->dsdt_tbl_offset);
> }
> +
> + /* ACPI1.0: INT_MODEL, ACPI2.0+: Reserved */
> + build_append_int_noprefix(tbl, f->int_model /* Multiple APIC */, 1);
> + /* Preferred_PM_Profile */
> + build_append_int_noprefix(tbl, 0 /* Unspecified */, 1);
> + build_append_int_noprefix(tbl, f->sci_int, 2); /* SCI_INT */
> + build_append_int_noprefix(tbl, f->smi_cmd, 4); /* SMI_CMD */
> + build_append_int_noprefix(tbl, f->acpi_enable_cmd, 1); /* ACPI_ENABLE */
> + build_append_int_noprefix(tbl, f->acpi_disable_cmd, 1); /* ACPI_DISABLE */
> + build_append_int_noprefix(tbl, 0 /* not supported */, 1); /* S4BIOS_REQ */
> + /* ACPI1.0: Reserved, ACPI2.0+: PSTATE_CNT */
> + build_append_int_noprefix(tbl, 0, 1);
> + build_append_int_noprefix(tbl, f->pm1_evt.address, 4); /* PM1a_EVT_BLK */
> + build_append_int_noprefix(tbl, 0, 4); /* PM1b_EVT_BLK */
> + build_append_int_noprefix(tbl, f->pm1_cnt.address, 4); /* PM1a_CNT_BLK */
> + build_append_int_noprefix(tbl, 0, 4); /* PM1b_CNT_BLK */
> + build_append_int_noprefix(tbl, 0, 4); /* PM2_CNT_BLK */
> + build_append_int_noprefix(tbl, f->pm_tmr.address, 4); /* PM_TMR_BLK */
> + build_append_int_noprefix(tbl, f->gpe0_blk.address, 4); /* GPE0_BLK */
> + build_append_int_noprefix(tbl, 0, 4); /* GPE1_BLK */
> + /* PM1_EVT_LEN */
> + build_append_int_noprefix(tbl, f->pm1_evt.bit_width / 8, 1);
> + /* PM1_CNT_LEN */
> + build_append_int_noprefix(tbl, f->pm1_cnt.bit_width / 8, 1);
> + build_append_int_noprefix(tbl, 0, 1); /* PM2_CNT_LEN */
> + build_append_int_noprefix(tbl, f->pm_tmr.bit_width / 8, 1); /* PM_TMR_LEN */
> + /* GPE0_BLK_LEN */
> + build_append_int_noprefix(tbl, f->gpe0_blk.bit_width / 8, 1);
> + build_append_int_noprefix(tbl, 0, 1); /* GPE1_BLK_LEN */
> + build_append_int_noprefix(tbl, 0, 1); /* GPE1_BASE */
> + build_append_int_noprefix(tbl, 0, 1); /* CST_CNT */
> + build_append_int_noprefix(tbl, f->c2_latency, 2); /* P_LVL2_LAT */
> + build_append_int_noprefix(tbl, f->c3_latency, 2); /* P_LVL3_LAT */
> + build_append_int_noprefix(tbl, 0, 2); /* FLUSH_SIZE */
> + build_append_int_noprefix(tbl, 0, 2); /* FLUSH_STRIDE */
> + build_append_int_noprefix(tbl, 0, 1); /* DUTY_OFFSET */
> + build_append_int_noprefix(tbl, 0, 1); /* DUTY_WIDTH */
> + build_append_int_noprefix(tbl, 0, 1); /* DAY_ALRM */
> + build_append_int_noprefix(tbl, 0, 1); /* MON_ALRM */
> + build_append_int_noprefix(tbl, f->rtc_century, 1); /* CENTURY */
> + build_append_int_noprefix(tbl, 0, 2); /* IAPC_BOOT_ARCH */
> + build_append_int_noprefix(tbl, 0, 1); /* Reserved */
> + build_append_int_noprefix(tbl, f->flags, 4); /* Flags */
> +
> if (f->rev == 1) {
> - fadt_size = offsetof(typeof(*fadt), reset_register);
> - } else if (f->xdsdt_tbl_offset) {
> + goto build_hdr;
> + }
> +
> + build_append_gas_from_struct(tbl, &f->reset_reg); /* RESET_REG */
> + build_append_int_noprefix(tbl, f->reset_val, 1); /* RESET_VALUE */
> + build_append_int_noprefix(tbl, 0, 3); /* Reserved, ACPI 3.0 */
> + build_append_int_noprefix(tbl, 0, 8); /* X_FIRMWARE_CTRL */
> +
> + /* XDSDT address to be filled by Guest linker at runtime */
> + off = tbl->len;
> + build_append_int_noprefix(tbl, 0, 8); /* X_DSDT */
> + if (f->xdsdt_tbl_offset) {
> bios_linker_loader_add_pointer(linker,
> - ACPI_BUILD_TABLE_FILE, xdsdt_entry_offset, sizeof(fadt->x_dsdt),
> + ACPI_BUILD_TABLE_FILE, off, 8,
> ACPI_BUILD_TABLE_FILE, *f->xdsdt_tbl_offset);
> }
>
> - build_header(linker, table_data,
> - (void *)fadt, "FACP", fadt_size, f->rev,
> - oem_id, oem_table_id);
> + build_append_gas_from_struct(tbl, &f->pm1_evt); /* X_PM1a_EVT_BLK */
> + /* X_PM1b_EVT_BLK */
> + build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
> + build_append_gas_from_struct(tbl, &f->pm1_cnt); /* X_PM1a_CNT_BLK */
> + /* X_PM1b_CNT_BLK */
> + build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
> + /* X_PM2_CNT_BLK */
> + build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
> + build_append_gas_from_struct(tbl, &f->pm_tmr); /* X_PM_TMR_BLK */
> + build_append_gas_from_struct(tbl, &f->gpe0_blk); /* X_GPE0_BLK */
> + build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0); /* X_GPE1_BLK */
> +
> +build_hdr:
> + build_header(linker, tbl, (void *)(tbl->data + fadt_start),
> + "FACP", tbl->len - fadt_start, f->rev, oem_id, oem_table_id);
> }
>
> void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH 7/9] acpi: move build_fadt() from i386 specific to generic ACPI source
2018-02-22 12:42 ` [Qemu-devel] [PATCH 7/9] acpi: move build_fadt() from i386 specific to generic ACPI source Igor Mammedov
@ 2018-02-27 14:15 ` Auger Eric
0 siblings, 0 replies; 24+ messages in thread
From: Auger Eric @ 2018-02-27 14:15 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel
Cc: Peter Maydell, Andrew Jones, Shannon Zhao, qemu-arm, Michael S. Tsirkin
On 22/02/18 13:42, Igor Mammedov wrote:
> it will be extended and reused in follow up patch by ARM target
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Eric
> ---
> include/hw/acpi/aml-build.h | 3 ++
> hw/acpi/aml-build.c | 105 +++++++++++++++++++++++++++++++++++++++++++
> hw/arm/virt-acpi-build.c | 6 +--
> hw/i386/acpi-build.c | 106 --------------------------------------------
> 4 files changed, 111 insertions(+), 109 deletions(-)
>
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 8692ccc..6c36903 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -413,4 +413,7 @@ void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
> uint64_t len, int node, MemoryAffinityFlags flags);
>
> void build_slit(GArray *table_data, BIOSLinker *linker);
> +
> +void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
> + const char *oem_id, const char *oem_table_id);
> #endif
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 3fef5f6..17fb71b 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1678,3 +1678,108 @@ void build_slit(GArray *table_data, BIOSLinker *linker)
> "SLIT",
> table_data->len - slit_start, 1, NULL, NULL);
> }
> +
> +/* build rev1/rev3 FADT */
> +void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
> + const char *oem_id, const char *oem_table_id)
> +{
> + int off;
> + int fadt_start = tbl->len;
> +
> + acpi_data_push(tbl, sizeof(AcpiTableHeader));
> +
> + /* FACS address to be filled by Guest linker at runtime */
> + off = tbl->len;
> + build_append_int_noprefix(tbl, 0, 4); /* FIRMWARE_CTRL */
> + if (f->facs_tbl_offset) {
> + bios_linker_loader_add_pointer(linker,
> + ACPI_BUILD_TABLE_FILE, off, 4,
> + ACPI_BUILD_TABLE_FILE, *f->facs_tbl_offset);
> + }
> +
> + /* DSDT address to be filled by Guest linker at runtime */
> + off = tbl->len;
> + build_append_int_noprefix(tbl, 0, 4); /* DSDT */
> + if (f->dsdt_tbl_offset) {
> + bios_linker_loader_add_pointer(linker,
> + ACPI_BUILD_TABLE_FILE, off, 4,
> + ACPI_BUILD_TABLE_FILE, *f->dsdt_tbl_offset);
> + }
> +
> + /* ACPI1.0: INT_MODEL, ACPI2.0+: Reserved */
> + build_append_int_noprefix(tbl, f->int_model /* Multiple APIC */, 1);
> + /* Preferred_PM_Profile */
> + build_append_int_noprefix(tbl, 0 /* Unspecified */, 1);
> + build_append_int_noprefix(tbl, f->sci_int, 2); /* SCI_INT */
> + build_append_int_noprefix(tbl, f->smi_cmd, 4); /* SMI_CMD */
> + build_append_int_noprefix(tbl, f->acpi_enable_cmd, 1); /* ACPI_ENABLE */
> + build_append_int_noprefix(tbl, f->acpi_disable_cmd, 1); /* ACPI_DISABLE */
> + build_append_int_noprefix(tbl, 0 /* not supported */, 1); /* S4BIOS_REQ */
> + /* ACPI1.0: Reserved, ACPI2.0+: PSTATE_CNT */
> + build_append_int_noprefix(tbl, 0, 1);
> + build_append_int_noprefix(tbl, f->pm1_evt.address, 4); /* PM1a_EVT_BLK */
> + build_append_int_noprefix(tbl, 0, 4); /* PM1b_EVT_BLK */
> + build_append_int_noprefix(tbl, f->pm1_cnt.address, 4); /* PM1a_CNT_BLK */
> + build_append_int_noprefix(tbl, 0, 4); /* PM1b_CNT_BLK */
> + build_append_int_noprefix(tbl, 0, 4); /* PM2_CNT_BLK */
> + build_append_int_noprefix(tbl, f->pm_tmr.address, 4); /* PM_TMR_BLK */
> + build_append_int_noprefix(tbl, f->gpe0_blk.address, 4); /* GPE0_BLK */
> + build_append_int_noprefix(tbl, 0, 4); /* GPE1_BLK */
> + /* PM1_EVT_LEN */
> + build_append_int_noprefix(tbl, f->pm1_evt.bit_width / 8, 1);
> + /* PM1_CNT_LEN */
> + build_append_int_noprefix(tbl, f->pm1_cnt.bit_width / 8, 1);
> + build_append_int_noprefix(tbl, 0, 1); /* PM2_CNT_LEN */
> + build_append_int_noprefix(tbl, f->pm_tmr.bit_width / 8, 1); /* PM_TMR_LEN */
> + /* GPE0_BLK_LEN */
> + build_append_int_noprefix(tbl, f->gpe0_blk.bit_width / 8, 1);
> + build_append_int_noprefix(tbl, 0, 1); /* GPE1_BLK_LEN */
> + build_append_int_noprefix(tbl, 0, 1); /* GPE1_BASE */
> + build_append_int_noprefix(tbl, 0, 1); /* CST_CNT */
> + build_append_int_noprefix(tbl, f->c2_latency, 2); /* P_LVL2_LAT */
> + build_append_int_noprefix(tbl, f->c3_latency, 2); /* P_LVL3_LAT */
> + build_append_int_noprefix(tbl, 0, 2); /* FLUSH_SIZE */
> + build_append_int_noprefix(tbl, 0, 2); /* FLUSH_STRIDE */
> + build_append_int_noprefix(tbl, 0, 1); /* DUTY_OFFSET */
> + build_append_int_noprefix(tbl, 0, 1); /* DUTY_WIDTH */
> + build_append_int_noprefix(tbl, 0, 1); /* DAY_ALRM */
> + build_append_int_noprefix(tbl, 0, 1); /* MON_ALRM */
> + build_append_int_noprefix(tbl, f->rtc_century, 1); /* CENTURY */
> + build_append_int_noprefix(tbl, 0, 2); /* IAPC_BOOT_ARCH */
> + build_append_int_noprefix(tbl, 0, 1); /* Reserved */
> + build_append_int_noprefix(tbl, f->flags, 4); /* Flags */
> +
> + if (f->rev == 1) {
> + goto build_hdr;
> + }
> +
> + build_append_gas_from_struct(tbl, &f->reset_reg); /* RESET_REG */
> + build_append_int_noprefix(tbl, f->reset_val, 1); /* RESET_VALUE */
> + build_append_int_noprefix(tbl, 0, 3); /* Reserved, ACPI 3.0 */
> + build_append_int_noprefix(tbl, 0, 8); /* X_FIRMWARE_CTRL */
> +
> + /* XDSDT address to be filled by Guest linker at runtime */
> + off = tbl->len;
> + build_append_int_noprefix(tbl, 0, 8); /* X_DSDT */
> + if (f->xdsdt_tbl_offset) {
> + bios_linker_loader_add_pointer(linker,
> + ACPI_BUILD_TABLE_FILE, off, 8,
> + ACPI_BUILD_TABLE_FILE, *f->xdsdt_tbl_offset);
> + }
> +
> + build_append_gas_from_struct(tbl, &f->pm1_evt); /* X_PM1a_EVT_BLK */
> + /* X_PM1b_EVT_BLK */
> + build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
> + build_append_gas_from_struct(tbl, &f->pm1_cnt); /* X_PM1a_CNT_BLK */
> + /* X_PM1b_CNT_BLK */
> + build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
> + /* X_PM2_CNT_BLK */
> + build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
> + build_append_gas_from_struct(tbl, &f->pm_tmr); /* X_PM_TMR_BLK */
> + build_append_gas_from_struct(tbl, &f->gpe0_blk); /* X_GPE0_BLK */
> + build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0); /* X_GPE1_BLK */
> +
> +build_hdr:
> + build_header(linker, tbl, (void *)(tbl->data + fadt_start),
> + "FACP", tbl->len - fadt_start, f->rev, oem_id, oem_table_id);
> +}
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index f7fa795..b644da9 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -651,8 +651,8 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> }
>
> /* FADT */
> -static void build_fadt(GArray *table_data, BIOSLinker *linker,
> - VirtMachineState *vms, unsigned dsdt_tbl_offset)
> +static void build_fadt_rev5(GArray *table_data, BIOSLinker *linker,
> + VirtMachineState *vms, unsigned dsdt_tbl_offset)
> {
> int fadt_start = table_data->len;
> AcpiFadtDescriptorRev5_1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
> @@ -761,7 +761,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
>
> /* FADT MADT GTDT MCFG SPCR pointed to by RSDT */
> acpi_add_table(table_offsets, tables_blob);
> - build_fadt(tables_blob, tables->linker, vms, dsdt);
> + build_fadt_rev5(tables_blob, tables->linker, vms, dsdt);
>
> acpi_add_table(table_offsets, tables_blob);
> build_madt(tables_blob, tables->linker, vms);
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 544a4bc..898a7e8 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -299,112 +299,6 @@ build_facs(GArray *table_data, BIOSLinker *linker)
> facs->length = cpu_to_le32(sizeof(*facs));
> }
>
> -/* FADT */
> -static void
> -build_fadt(GArray *tbl, BIOSLinker *linker, AcpiFadtData *f,
> - const char *oem_id, const char *oem_table_id)
> -{
> - int off;
> - int fadt_start = tbl->len;
> -
> - acpi_data_push(tbl, sizeof(AcpiTableHeader));
> -
> - /* FACS address to be filled by Guest linker at runtime */
> - off = tbl->len;
> - build_append_int_noprefix(tbl, 0, 4); /* FIRMWARE_CTRL */
> - if (f->facs_tbl_offset) {
> - bios_linker_loader_add_pointer(linker,
> - ACPI_BUILD_TABLE_FILE, off, 4,
> - ACPI_BUILD_TABLE_FILE, *f->facs_tbl_offset);
> - }
> -
> - /* DSDT address to be filled by Guest linker at runtime */
> - off = tbl->len;
> - build_append_int_noprefix(tbl, 0, 4); /* DSDT */
> - if (f->dsdt_tbl_offset) {
> - bios_linker_loader_add_pointer(linker,
> - ACPI_BUILD_TABLE_FILE, off, 4,
> - ACPI_BUILD_TABLE_FILE, *f->dsdt_tbl_offset);
> - }
> -
> - /* ACPI1.0: INT_MODEL, ACPI2.0+: Reserved */
> - build_append_int_noprefix(tbl, f->int_model /* Multiple APIC */, 1);
> - /* Preferred_PM_Profile */
> - build_append_int_noprefix(tbl, 0 /* Unspecified */, 1);
> - build_append_int_noprefix(tbl, f->sci_int, 2); /* SCI_INT */
> - build_append_int_noprefix(tbl, f->smi_cmd, 4); /* SMI_CMD */
> - build_append_int_noprefix(tbl, f->acpi_enable_cmd, 1); /* ACPI_ENABLE */
> - build_append_int_noprefix(tbl, f->acpi_disable_cmd, 1); /* ACPI_DISABLE */
> - build_append_int_noprefix(tbl, 0 /* not supported */, 1); /* S4BIOS_REQ */
> - /* ACPI1.0: Reserved, ACPI2.0+: PSTATE_CNT */
> - build_append_int_noprefix(tbl, 0, 1);
> - build_append_int_noprefix(tbl, f->pm1_evt.address, 4); /* PM1a_EVT_BLK */
> - build_append_int_noprefix(tbl, 0, 4); /* PM1b_EVT_BLK */
> - build_append_int_noprefix(tbl, f->pm1_cnt.address, 4); /* PM1a_CNT_BLK */
> - build_append_int_noprefix(tbl, 0, 4); /* PM1b_CNT_BLK */
> - build_append_int_noprefix(tbl, 0, 4); /* PM2_CNT_BLK */
> - build_append_int_noprefix(tbl, f->pm_tmr.address, 4); /* PM_TMR_BLK */
> - build_append_int_noprefix(tbl, f->gpe0_blk.address, 4); /* GPE0_BLK */
> - build_append_int_noprefix(tbl, 0, 4); /* GPE1_BLK */
> - /* PM1_EVT_LEN */
> - build_append_int_noprefix(tbl, f->pm1_evt.bit_width / 8, 1);
> - /* PM1_CNT_LEN */
> - build_append_int_noprefix(tbl, f->pm1_cnt.bit_width / 8, 1);
> - build_append_int_noprefix(tbl, 0, 1); /* PM2_CNT_LEN */
> - build_append_int_noprefix(tbl, f->pm_tmr.bit_width / 8, 1); /* PM_TMR_LEN */
> - /* GPE0_BLK_LEN */
> - build_append_int_noprefix(tbl, f->gpe0_blk.bit_width / 8, 1);
> - build_append_int_noprefix(tbl, 0, 1); /* GPE1_BLK_LEN */
> - build_append_int_noprefix(tbl, 0, 1); /* GPE1_BASE */
> - build_append_int_noprefix(tbl, 0, 1); /* CST_CNT */
> - build_append_int_noprefix(tbl, f->c2_latency, 2); /* P_LVL2_LAT */
> - build_append_int_noprefix(tbl, f->c3_latency, 2); /* P_LVL3_LAT */
> - build_append_int_noprefix(tbl, 0, 2); /* FLUSH_SIZE */
> - build_append_int_noprefix(tbl, 0, 2); /* FLUSH_STRIDE */
> - build_append_int_noprefix(tbl, 0, 1); /* DUTY_OFFSET */
> - build_append_int_noprefix(tbl, 0, 1); /* DUTY_WIDTH */
> - build_append_int_noprefix(tbl, 0, 1); /* DAY_ALRM */
> - build_append_int_noprefix(tbl, 0, 1); /* MON_ALRM */
> - build_append_int_noprefix(tbl, f->rtc_century, 1); /* CENTURY */
> - build_append_int_noprefix(tbl, 0, 2); /* IAPC_BOOT_ARCH */
> - build_append_int_noprefix(tbl, 0, 1); /* Reserved */
> - build_append_int_noprefix(tbl, f->flags, 4); /* Flags */
> -
> - if (f->rev == 1) {
> - goto build_hdr;
> - }
> -
> - build_append_gas_from_struct(tbl, &f->reset_reg); /* RESET_REG */
> - build_append_int_noprefix(tbl, f->reset_val, 1); /* RESET_VALUE */
> - build_append_int_noprefix(tbl, 0, 3); /* Reserved, ACPI 3.0 */
> - build_append_int_noprefix(tbl, 0, 8); /* X_FIRMWARE_CTRL */
> -
> - /* XDSDT address to be filled by Guest linker at runtime */
> - off = tbl->len;
> - build_append_int_noprefix(tbl, 0, 8); /* X_DSDT */
> - if (f->xdsdt_tbl_offset) {
> - bios_linker_loader_add_pointer(linker,
> - ACPI_BUILD_TABLE_FILE, off, 8,
> - ACPI_BUILD_TABLE_FILE, *f->xdsdt_tbl_offset);
> - }
> -
> - build_append_gas_from_struct(tbl, &f->pm1_evt); /* X_PM1a_EVT_BLK */
> - /* X_PM1b_EVT_BLK */
> - build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
> - build_append_gas_from_struct(tbl, &f->pm1_cnt); /* X_PM1a_CNT_BLK */
> - /* X_PM1b_CNT_BLK */
> - build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
> - /* X_PM2_CNT_BLK */
> - build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
> - build_append_gas_from_struct(tbl, &f->pm_tmr); /* X_PM_TMR_BLK */
> - build_append_gas_from_struct(tbl, &f->gpe0_blk); /* X_GPE0_BLK */
> - build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0); /* X_GPE1_BLK */
> -
> -build_hdr:
> - build_header(linker, tbl, (void *)(tbl->data + fadt_start),
> - "FACP", tbl->len - fadt_start, f->rev, oem_id, oem_table_id);
> -}
> -
> void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
> const CPUArchIdList *apic_ids, GArray *entry)
> {
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH 8/9] virt_arm: acpi: reuse common build_fadt()
2018-02-22 12:42 ` [Qemu-devel] [PATCH 8/9] virt_arm: acpi: reuse common build_fadt() Igor Mammedov
@ 2018-02-27 14:42 ` Auger Eric
2018-02-27 15:07 ` Auger Eric
1 sibling, 0 replies; 24+ messages in thread
From: Auger Eric @ 2018-02-27 14:42 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel
Cc: Peter Maydell, Andrew Jones, Shannon Zhao, qemu-arm, Michael S. Tsirkin
On 22/02/18 13:42, Igor Mammedov wrote:
> Extend generic build_fadt() to support rev5.1 FADT
> and reuse it for 'virt' board, it would allow to
> phase out usage of AcpiFadtDescriptorRev5_1 and
> later ACPI_FADT_COMMON_DEF.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Eric
> ---
> include/hw/acpi/acpi-defs.h | 12 ++----------
> hw/acpi/aml-build.c | 21 ++++++++++++++++++++-
> hw/arm/virt-acpi-build.c | 33 ++++++++++++---------------------
> 3 files changed, 34 insertions(+), 32 deletions(-)
>
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index e0accc4..34e49dc 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -165,16 +165,6 @@ struct AcpiFadtDescriptorRev3 {
> } QEMU_PACKED;
> typedef struct AcpiFadtDescriptorRev3 AcpiFadtDescriptorRev3;
>
> -struct AcpiFadtDescriptorRev5_1 {
> - ACPI_FADT_COMMON_DEF
> - /* 64-bit Sleep Control register (ACPI 5.0) */
> - struct AcpiGenericAddress sleep_control;
> - /* 64-bit Sleep Status register (ACPI 5.0) */
> - struct AcpiGenericAddress sleep_status;
> -} QEMU_PACKED;
> -
> -typedef struct AcpiFadtDescriptorRev5_1 AcpiFadtDescriptorRev5_1;
> -
> typedef struct AcpiFadtData {
> struct AcpiGenericAddress pm1_cnt; /* PM1a_CNT_BLK */
> struct AcpiGenericAddress pm1_evt; /* PM1a_EVT_BLK */
> @@ -192,6 +182,8 @@ typedef struct AcpiFadtData {
> uint8_t rtc_century; /* CENTURY */
> uint16_t c2_latency; /* P_LVL2_LAT */
> uint16_t c3_latency; /* P_LVL3_LAT */
> + uint16_t arm_boot_arch; /* ARM_BOOT_ARCH */
> + uint8_t minor_ver; /* FADT Minor Version */
>
> /*
> * respective tables offsets within ACPI_BUILD_TABLE_FILE,
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 17fb71b..5a33cd0 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1755,7 +1755,14 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
>
> build_append_gas_from_struct(tbl, &f->reset_reg); /* RESET_REG */
> build_append_int_noprefix(tbl, f->reset_val, 1); /* RESET_VALUE */
> - build_append_int_noprefix(tbl, 0, 3); /* Reserved, ACPI 3.0 */
> + /* Since ACPI 5.1 */
> + if ((f->rev >= 6) || ((f->rev == 5) && f->minor_ver > 0)) {
> + build_append_int_noprefix(tbl, f->arm_boot_arch, 2); /* ARM_BOOT_ARCH */
> + /* FADT Minor Version */
> + build_append_int_noprefix(tbl, f->minor_ver, 1);
> + } else {
> + build_append_int_noprefix(tbl, 0, 3); /* Reserved upto ACPI 5.0 */
> + }
> build_append_int_noprefix(tbl, 0, 8); /* X_FIRMWARE_CTRL */
>
> /* XDSDT address to be filled by Guest linker at runtime */
> @@ -1779,6 +1786,18 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
> build_append_gas_from_struct(tbl, &f->gpe0_blk); /* X_GPE0_BLK */
> build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0); /* X_GPE1_BLK */
>
> + if (f->rev <= 4) {
> + goto build_hdr;
> + }
> +
> + /* SLEEP_CONTROL_REG */
> + build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
> + /* SLEEP_STATUS_REG */
> + build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
> +
> + /* TODO: extra fields need to be added to support revisions above rev5 */
> + assert(f->rev == 5);
> +
> build_hdr:
> build_header(linker, tbl, (void *)(tbl->data + fadt_start),
> "FACP", tbl->len - fadt_start, f->rev, oem_id, oem_table_id);
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index b644da9..c7c6a57 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -654,39 +654,30 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> static void build_fadt_rev5(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms, unsigned dsdt_tbl_offset)
> {
> - int fadt_start = table_data->len;
> - AcpiFadtDescriptorRev5_1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
> - unsigned xdsdt_entry_offset = (char *)&fadt->x_dsdt - table_data->data;
> - uint16_t bootflags;
> + /* ACPI v5.1 */
> + AcpiFadtData fadt = {
> + .rev = 5,
> + .minor_ver = 1,
> + .flags = 1 << ACPI_FADT_F_HW_REDUCED_ACPI,
> + .xdsdt_tbl_offset = &dsdt_tbl_offset,
> + };
>
> switch (vms->psci_conduit) {
> case QEMU_PSCI_CONDUIT_DISABLED:
> - bootflags = 0;
> + fadt.arm_boot_arch = 0;
> break;
> case QEMU_PSCI_CONDUIT_HVC:
> - bootflags = ACPI_FADT_ARM_PSCI_COMPLIANT | ACPI_FADT_ARM_PSCI_USE_HVC;
> + fadt.arm_boot_arch = ACPI_FADT_ARM_PSCI_COMPLIANT |
> + ACPI_FADT_ARM_PSCI_USE_HVC;
> break;
> case QEMU_PSCI_CONDUIT_SMC:
> - bootflags = ACPI_FADT_ARM_PSCI_COMPLIANT;
> + fadt.arm_boot_arch = ACPI_FADT_ARM_PSCI_COMPLIANT;
> break;
> default:
> g_assert_not_reached();
> }
>
> - /* Hardware Reduced = 1 and use PSCI 0.2+ */
> - fadt->flags = cpu_to_le32(1 << ACPI_FADT_F_HW_REDUCED_ACPI);
> - fadt->arm_boot_flags = cpu_to_le16(bootflags);
> -
> - /* ACPI v5.1 (fadt->revision.fadt->minor_revision) */
> - fadt->minor_revision = 0x1;
> -
> - /* DSDT address to be filled by Guest linker */
> - bios_linker_loader_add_pointer(linker,
> - ACPI_BUILD_TABLE_FILE, xdsdt_entry_offset, sizeof(fadt->x_dsdt),
> - ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
> -
> - build_header(linker, table_data, (void *)(table_data->data + fadt_start),
> - "FACP", table_data->len - fadt_start, 5, NULL, NULL);
> + build_fadt(table_data, linker, &fadt, NULL, NULL);
> }
>
> /* DSDT */
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH 8/9] virt_arm: acpi: reuse common build_fadt()
2018-02-22 12:42 ` [Qemu-devel] [PATCH 8/9] virt_arm: acpi: reuse common build_fadt() Igor Mammedov
2018-02-27 14:42 ` Auger Eric
@ 2018-02-27 15:07 ` Auger Eric
2018-02-27 15:49 ` Igor Mammedov
1 sibling, 1 reply; 24+ messages in thread
From: Auger Eric @ 2018-02-27 15:07 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel
Cc: Peter Maydell, Andrew Jones, Shannon Zhao, qemu-arm, Michael S. Tsirkin
Hi Igor,
On 22/02/18 13:42, Igor Mammedov wrote:
> Extend generic build_fadt() to support rev5.1 FADT
> and reuse it for 'virt' board, it would allow to
> phase out usage of AcpiFadtDescriptorRev5_1 and
> later ACPI_FADT_COMMON_DEF.
Oups sorry sent the R-b to fast. Some small comments below.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> include/hw/acpi/acpi-defs.h | 12 ++----------
> hw/acpi/aml-build.c | 21 ++++++++++++++++++++-
> hw/arm/virt-acpi-build.c | 33 ++++++++++++---------------------
> 3 files changed, 34 insertions(+), 32 deletions(-)
>
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index e0accc4..34e49dc 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -165,16 +165,6 @@ struct AcpiFadtDescriptorRev3 {
> } QEMU_PACKED;
> typedef struct AcpiFadtDescriptorRev3 AcpiFadtDescriptorRev3;
>
> -struct AcpiFadtDescriptorRev5_1 {
> - ACPI_FADT_COMMON_DEF
> - /* 64-bit Sleep Control register (ACPI 5.0) */
> - struct AcpiGenericAddress sleep_control;
> - /* 64-bit Sleep Status register (ACPI 5.0) */
> - struct AcpiGenericAddress sleep_status;
> -} QEMU_PACKED;
> -
> -typedef struct AcpiFadtDescriptorRev5_1 AcpiFadtDescriptorRev5_1;
> -
> typedef struct AcpiFadtData {
> struct AcpiGenericAddress pm1_cnt; /* PM1a_CNT_BLK */
> struct AcpiGenericAddress pm1_evt; /* PM1a_EVT_BLK */
> @@ -192,6 +182,8 @@ typedef struct AcpiFadtData {
> uint8_t rtc_century; /* CENTURY */
> uint16_t c2_latency; /* P_LVL2_LAT */
> uint16_t c3_latency; /* P_LVL3_LAT */
> + uint16_t arm_boot_arch; /* ARM_BOOT_ARCH */
> + uint8_t minor_ver; /* FADT Minor Version */
>
> /*
> * respective tables offsets within ACPI_BUILD_TABLE_FILE,
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 17fb71b..5a33cd0 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
nit: comment above needs to be extended to rev5.1 support
> @@ -1755,7 +1755,14 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
>
> build_append_gas_from_struct(tbl, &f->reset_reg); /* RESET_REG */
> build_append_int_noprefix(tbl, f->reset_val, 1); /* RESET_VALUE */
> - build_append_int_noprefix(tbl, 0, 3); /* Reserved, ACPI 3.0 */
> + /* Since ACPI 5.1 */
> + if ((f->rev >= 6) || ((f->rev == 5) && f->minor_ver > 0)) {
> + build_append_int_noprefix(tbl, f->arm_boot_arch, 2); /* ARM_BOOT_ARCH */
> + /* FADT Minor Version */
> + build_append_int_noprefix(tbl, f->minor_ver, 1);
> + } else {
> + build_append_int_noprefix(tbl, 0, 3); /* Reserved upto ACPI 5.0 */
> + }
> build_append_int_noprefix(tbl, 0, 8); /* X_FIRMWARE_CTRL */
>
> /* XDSDT address to be filled by Guest linker at runtime */
> @@ -1779,6 +1786,18 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
> build_append_gas_from_struct(tbl, &f->gpe0_blk); /* X_GPE0_BLK */
> build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0); /* X_GPE1_BLK */
>
> + if (f->rev <= 4) {
> + goto build_hdr;
> + }
> +
> + /* SLEEP_CONTROL_REG */
> + build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
> + /* SLEEP_STATUS_REG */
> + build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
what about Hypervisor Vendor Identity field (8 byte) in version 6.0?
> +
> + /* TODO: extra fields need to be added to support revisions above rev5 */
> + assert(f->rev == 5);
OK so can't we assert before the (f->rev >= 6) check above?
Thanks
Eric
> +
> build_hdr:
> build_header(linker, tbl, (void *)(tbl->data + fadt_start),
> "FACP", tbl->len - fadt_start, f->rev, oem_id, oem_table_id);
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index b644da9..c7c6a57 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -654,39 +654,30 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> static void build_fadt_rev5(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms, unsigned dsdt_tbl_offset)
> {
> - int fadt_start = table_data->len;
> - AcpiFadtDescriptorRev5_1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
> - unsigned xdsdt_entry_offset = (char *)&fadt->x_dsdt - table_data->data;
> - uint16_t bootflags;
> + /* ACPI v5.1 */
> + AcpiFadtData fadt = {
> + .rev = 5,
> + .minor_ver = 1,
> + .flags = 1 << ACPI_FADT_F_HW_REDUCED_ACPI,
> + .xdsdt_tbl_offset = &dsdt_tbl_offset,
> + };
>
> switch (vms->psci_conduit) {
> case QEMU_PSCI_CONDUIT_DISABLED:
> - bootflags = 0;
> + fadt.arm_boot_arch = 0;
> break;
> case QEMU_PSCI_CONDUIT_HVC:
> - bootflags = ACPI_FADT_ARM_PSCI_COMPLIANT | ACPI_FADT_ARM_PSCI_USE_HVC;
> + fadt.arm_boot_arch = ACPI_FADT_ARM_PSCI_COMPLIANT |
> + ACPI_FADT_ARM_PSCI_USE_HVC;
> break;
> case QEMU_PSCI_CONDUIT_SMC:
> - bootflags = ACPI_FADT_ARM_PSCI_COMPLIANT;
> + fadt.arm_boot_arch = ACPI_FADT_ARM_PSCI_COMPLIANT;
> break;
> default:
> g_assert_not_reached();
> }
>
> - /* Hardware Reduced = 1 and use PSCI 0.2+ */
> - fadt->flags = cpu_to_le32(1 << ACPI_FADT_F_HW_REDUCED_ACPI);
> - fadt->arm_boot_flags = cpu_to_le16(bootflags);
> -
> - /* ACPI v5.1 (fadt->revision.fadt->minor_revision) */
> - fadt->minor_revision = 0x1;
> -
> - /* DSDT address to be filled by Guest linker */
> - bios_linker_loader_add_pointer(linker,
> - ACPI_BUILD_TABLE_FILE, xdsdt_entry_offset, sizeof(fadt->x_dsdt),
> - ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
> -
> - build_header(linker, table_data, (void *)(table_data->data + fadt_start),
> - "FACP", table_data->len - fadt_start, 5, NULL, NULL);
> + build_fadt(table_data, linker, &fadt, NULL, NULL);
> }
>
> /* DSDT */
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH 1/9] acpi: remove unused acpi-dsdt.aml
2018-02-27 12:42 ` Auger Eric
@ 2018-02-27 15:09 ` Igor Mammedov
0 siblings, 0 replies; 24+ messages in thread
From: Igor Mammedov @ 2018-02-27 15:09 UTC (permalink / raw)
To: Auger Eric
Cc: qemu-devel, Peter Maydell, Andrew Jones, Michael S. Tsirkin,
qemu-arm, Gerd Hoffmann, Shannon Zhao
On Tue, 27 Feb 2018 13:42:38 +0100
Auger Eric <eric.auger@redhat.com> wrote:
> Hi,
>
> On 22/02/18 13:42, Igor Mammedov wrote:
> > SeaBIOS blob which is currently shipped with QEMU
> > doesn't need acpi-dsdt.aml nor is able to use it
> > and code that loaded it QEMU was removed by
> as code that loaded it in QEMU was removed by?
Thanks, I'll fix it on respin.
> > (commit 9fb7aaaf4c "pc: drop external DSDT loading")
> > in 2013.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > CC: Gerd Hoffmann <kraxel@redhat.com>
> Otherwise
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
>
> Thanks
>
> Eric
> > ---
> > Makefile | 1 -
> > pc-bios/acpi-dsdt.aml | Bin 4405 -> 0 bytes
> > 2 files changed, 1 deletion(-)
> > delete mode 100644 pc-bios/acpi-dsdt.aml
> >
> > diff --git a/Makefile b/Makefile
> > index 90e05ac..f3a4cf9 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -648,7 +648,6 @@ bepo cz
> > ifdef INSTALL_BLOBS
> > BLOBS=bios.bin bios-256k.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \
> > vgabios-stdvga.bin vgabios-vmware.bin vgabios-qxl.bin vgabios-virtio.bin \
> > -acpi-dsdt.aml \
> > ppc_rom.bin openbios-sparc32 openbios-sparc64 openbios-ppc QEMU,tcx.bin QEMU,cgthree.bin \
> > pxe-e1000.rom pxe-eepro100.rom pxe-ne2k_pci.rom \
> > pxe-pcnet.rom pxe-rtl8139.rom pxe-virtio.rom \
> > diff --git a/pc-bios/acpi-dsdt.aml b/pc-bios/acpi-dsdt.aml
> > deleted file mode 100644
> > index 558c10f51ccbbf9ec2f47a4a998a7055059a8963..0000000000000000000000000000000000000000
> > GIT binary patch
> > literal 0
> > HcmV?d00001
> >
> > literal 4405
> > zcmb7{O>f)C8OI;KNTx=#Ov$uk$9XY?b_=xIjb0q@_EJP5WkrsxFrpHqP*76NE}&CE
> > zqzSOzpn&5RO*WTe*G<u*ve0?^5!z#q`3UV-*rJF}QJ-hVlxK_$u;qa>|Kam8znLL9
> > z<A?s>dJ#bTx_LkF0GjuGY(WhGo!+3koGWcQ9rFPU5B+94((<~g4WH$C9dAv`{m^gT
> > zZEJrW$A5|A$IoMJl)(Ns&a3@V^7|L@K9JFq{e&^9IOQm8M#H0x!0S}3=w`>a8*i9l
> > zMGe0XR&=-HYff+FBQoL^UcVI<n+xnWFBU<!u}c6mx@m3g#6Gb#3)?l@pr*I@_{5&;
> > z#Tgm?=lKNy@f?7`Y?dceyma7Ch?1^<&1QdpC#wIrasZas-`*<LS>@%=&fLZ0Lo8-=
> > z2|2%0`vJHO7J2;;UQ)-|gCMNeL^TdtX>}ZQ>$N1Pgb_W)N-Ls=$)m@-itRY~WHYgk
> > zgX+BqrWEW?)FoC3!tE_lT@6}k^@E_hy_E!2ipVPzkypAAJ^BL$Apdw8JA0Oxf|hkN
> > zXbsXi&~OfL^l_GN27^7ov3&C`59aWhLwfmMtLJY9eLvcCx1(^-fP`A&gqlWQ#LS5&
> > z_E*O-9LM?DYzmXYSH~mx^T>vO|2H#*DO<8=Sc*kf_+yTS?9DqcX}p{p+4*D-kG#yi
> > zb|d180Xv{$XK)dCIxy@<o~j1#hl>PNAErQ+8n3KJVco~H$7En{Z-6#s#%p5=&X1+|
> > z>%skMU4%Dqj4^z*PT^<HPDV28n4R#f8{BTI;^{1=ethujk0=Ux0^Ga?3*DgA)8G>@
> > zyarVauZe}V<Kx}wZd^0cwM;RGM?dcmJR}qgKaWeEhGmVdw6z2haP%@Q?MLtk^y~o)
> > zk3PQD^ylV=;pX_@&&QKH#t?&sUZ29JSeA7h*5T1l_HN&uJ1#AsceGfh3=SFYnmfKX
> > ze-#(NT@&+50P!S?b2^3B<~-vDTWf3I8Q&RTwzap$TO7yo4fv_alm4<B4CYDAc_<p8
> > z?+N9w#kTgj@ws7H<wNe@GQHb-)pT?+n)o23J)-e_Uzii)!~m=8@Gv_Rrgkn2)8}z;
> > zg5DcPKhZIcg>jsYb+#sOA%+7j58pBiUkMT(uE)EZc=I=hhhb|Mzl_$me4&!?nw8e>
> > zrn?k)tz9iS(8fRw?snkyc5t5KZ$5k#v#U?yN%1MgInZJNd^U)+Nr_tgvleDJyAxVO
> > zPWv%F!Kn)RgVL?v9+vVZzHHF#-SR=yHLN$FWK%oSQ8ZIwpzxryXxg(Ge)CX;b46Zg
> > zSP;*+ADX6;JTX4^)VU|xo+|Q8P4P9NjA+U|QIaS2hTGy7TG*Z{@=Q$);fbc)6D4`3
> > zS@1I<Y`Lcir;Oax6rO44QOcYd?wR%=!#z{ejPOi5k5cB$Dx6vFnVM!*PLwj|g2K7L
> > zJyXsFl@q1RX(^nR!fC0TC}mFkpyCLnoH>Ovr*fi{Ihn%A6i%jcqLexDH;OrNO!%zi
> > z70$fMiBjgo54$v<w!&$voG4|^MTHap`xqyk&qb9JrOa7SI137ALFGg#b1o^IOA6<b
> > z%863ubQDfU;dE3^lrkr7#-#ZyDx5`?6Q#^qQaDQrXG!HmDRV9>oXZO5vdW24=5!TK
> > zSK)M3PLwj|io&^~aIUDFC}qx7g>zNmTva(y%AB|xl-BJ9h4X^SiBjfVQ#jWY&NY=2
> > zrOdgmaIPzy>nbNone(E;c~RlKsB)r|IX4te+zyF%j(;^bR8EvK=Ou;nlEQgO<wPlS
> > zURF3SE1Z{APLwj|6@~MP!g)pIL@9G#RXDFIoL5y&l!9~k>_^uO`jgU*EWn+e7WD5_
> > zEWB0eR-;?pa+f=I@RvWyJ!OYu+`;CiEbnf2?s)wi8uTm00?U7yg&aRY9KcIzV;Q`6
> > zCiz!mc9@K*KBea2QFnpf=yXS7<8GMt+Vm$6i>qw;%L3#K{9WM*1%OT{c#v2UJ3Z<I
> > zb<ZtEekX+AQJo#~mL-1Dm{OOxz7U1|P<uHRy}+$`zeDY(*_-FG<L2rIXRk`xt2}!Z
> > z`$y-TG<((k{_NG^(H^mT=dv^X|43hx(${$U+PU<#_oT0#ruWaM$J5Rarmsus>pXq^
> > zT>AQZ($|Maw@suE&!;y<`g94=kqD<e-Q4HhET3#QFUFX<icK`TPP;%`LHD{B>@_qz
> > zV0*#s-WcMfm}eH?9)hk>GJY{)I`G1PBt~VzbmU(20$kE(UXx6W8+$q?xy%b%yZW%q
> > z{)wleKHuy9jcpE}*<9c)y5YFHS*&<~ODl{%!&5$OWOp*J;^)+h)37m&ChTd<7T}A0
> > zZU426&7a``5jTMQ$<uue9!|<%ACDd;4|&&Pn6TrAT5quPt5|z&@sb%&VyBmj+Chtt
> > z+hW5DI+aRg8*mW1l?u2kQL9pg2WMw1+E%*`w$|W**tBCmxpiGQZHeN#C{81NEYv5W
> > U_=PAMqG*c36NN8|mMC`Mf01wzJpcdz
> >
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH 5/9] pc: acpi: isolate FADT specific data into AcpiFadtData structure
2018-02-27 13:47 ` Auger Eric
@ 2018-02-27 15:44 ` Igor Mammedov
0 siblings, 0 replies; 24+ messages in thread
From: Igor Mammedov @ 2018-02-27 15:44 UTC (permalink / raw)
To: Auger Eric
Cc: qemu-devel, Peter Maydell, Andrew Jones, Shannon Zhao, qemu-arm,
Michael S. Tsirkin
On Tue, 27 Feb 2018 14:47:16 +0100
Auger Eric <eric.auger@redhat.com> wrote:
> Hi Igor,
>
> On 22/02/18 13:42, Igor Mammedov wrote:
> > move FADT data initialization out of fadt_setup() into dedicated
> > init_fadt_data() that will set common for pc/q35 values in
> > AcpiFadtData structure and acpi_get_pm_info() will complement
> > it with pc/q35 specific values initialization.
> acpi_get_pm_info only adds reset stuff + version info on top of
> previously initialized common data, right?
yep
> > That will allow to get rid of fadt_setup() and generalize
> > build_fadt() so it could be easily extended for rev5 and
> > reused by ARM target.
> >
> > While at it also move facs/dsdt/xdsdt offsets from build_fadt()
> > arg list into AcpiFadtData, as they belong to the same dataset.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > include/hw/acpi/acpi-defs.h | 28 ++++++
> > hw/i386/acpi-build.c | 204 ++++++++++++++++++++++++--------------------
> > 2 files changed, 138 insertions(+), 94 deletions(-)
> >
> > diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> > index 9942bc5..e0accc4 100644
> > --- a/include/hw/acpi/acpi-defs.h
> > +++ b/include/hw/acpi/acpi-defs.h
> > @@ -175,6 +175,34 @@ struct AcpiFadtDescriptorRev5_1 {
> >
> > typedef struct AcpiFadtDescriptorRev5_1 AcpiFadtDescriptorRev5_1;
> >
> > +typedef struct AcpiFadtData {
> > + struct AcpiGenericAddress pm1_cnt; /* PM1a_CNT_BLK */
> Any reason why we don't use the same field names as in
> AcpiFadtDescriptorRev3?
I'll amend it on respin as you suggest.
>
> for instance pm1a_cnt?
> > + struct AcpiGenericAddress pm1_evt; /* PM1a_EVT_BLK */
> pm1a_evt?
> > + struct AcpiGenericAddress pm_tmr; /* PM_TMR_BLK */
> > + struct AcpiGenericAddress gpe0_blk; /* GPE0_BLK */
> > + struct AcpiGenericAddress reset_reg; /* RESET_REG */
> > + uint8_t reset_val; /* RESET_VALUE */
> > + uint8_t rev; /* Revision */
> > + uint32_t flags; /* Flags */
> > + uint32_t smi_cmd; /* SMI_CMD */
> > + uint16_t sci_int; /* SCI_INT */
> > + uint8_t int_model; /* INT_MODEL */
> > + uint8_t acpi_enable_cmd; /* ACPI_ENABLE */
> > + uint8_t acpi_disable_cmd; /* ACPI_DISABLE */
> > + uint8_t rtc_century; /* CENTURY */
> > + uint16_t c2_latency; /* P_LVL2_LAT */
> same?
> > + uint16_t c3_latency; /* P_LVL3_LAT */
> > +
> > + /*
> > + * respective tables offsets within ACPI_BUILD_TABLE_FILE,
> > + * NULL if table doesn't exist (in that case field's value
> > + * won't be patched by linker and will be kept set to 0)
> > + */
> > + unsigned *facs_tbl_offset; /* FACS offset in */
> > + unsigned *dsdt_tbl_offset;
> > + unsigned *xdsdt_tbl_offset;
> > +} AcpiFadtData;
> > +
> > #define ACPI_FADT_ARM_PSCI_COMPLIANT (1 << 0)
> > #define ACPI_FADT_ARM_PSCI_USE_HVC (1 << 1)
> >
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index b85fefe..706ba35 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -91,17 +91,11 @@ typedef struct AcpiMcfgInfo {
> > } AcpiMcfgInfo;
> >
> > typedef struct AcpiPmInfo {
> > - bool force_rev1_fadt;
> > bool s3_disabled;
> > bool s4_disabled;
> > bool pcihp_bridge_en;
> > uint8_t s4_val;
> > - uint16_t sci_int;
> > - uint8_t acpi_enable_cmd;
> > - uint8_t acpi_disable_cmd;
> > - uint32_t gpe0_blk;
> > - uint32_t gpe0_blk_len;
> > - uint32_t io_base;
> > + AcpiFadtData fadt;
> > uint16_t cpu_hp_io_base;
> > uint16_t pcihp_io_base;
> > uint16_t pcihp_io_len;
> > @@ -124,20 +118,60 @@ typedef struct AcpiBuildPciBusHotplugState {
> > bool pcihp_bridge_en;
> > } AcpiBuildPciBusHotplugState;
> >
> > +#define ACPI_PORT_SMI_CMD 0x00b2 /* TODO: this is APM_CNT_IOPORT */
> Can't we take benefit of this series to move that define in hw/isa/apm.h?
> > +
> > +static void init_fadt_data(Object *o, AcpiFadtData *data)
> > +{
> > + uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL);
> > + AmlAddressSpace as = AML_AS_SYSTEM_IO;
> > + AcpiFadtData fadt = {
> > + .rev = 3,
> > + .flags =
> > + (1 << ACPI_FADT_F_WBINVD) |
> > + (1 << ACPI_FADT_F_PROC_C1) |
> > + (1 << ACPI_FADT_F_SLP_BUTTON) |
> > + (1 << ACPI_FADT_F_RTC_S4) |
> > + (1 << ACPI_FADT_F_USE_PLATFORM_CLOCK) |
> > + /* APIC destination mode ("Flat Logical") has an upper limit of 8
> > + * CPUs for more than 8 CPUs, "Clustered Logical" mode has to be
> > + * used
> > + */
> > + ((max_cpus > 8) ? (1 << ACPI_FADT_F_FORCE_APIC_CLUSTER_MODEL) : 0),
> > + .int_model = 1 /* Multiple APIC */,
> > + .rtc_century = RTC_CENTURY,
> > + .c2_latency = 0xfff /* C2 state not supported */,
> > + .c3_latency = 0xfff /* C3 state not supported */,
> > + .smi_cmd = ACPI_PORT_SMI_CMD,
> > + .sci_int = object_property_get_uint(o, ACPI_PM_PROP_SCI_INT, NULL),
> > + .acpi_enable_cmd =
> > + object_property_get_uint(o, ACPI_PM_PROP_ACPI_ENABLE_CMD, NULL),
> > + .acpi_disable_cmd =
> > + object_property_get_uint(o, ACPI_PM_PROP_ACPI_DISABLE_CMD, NULL),
> > + .pm1_evt = { .space_id = as, .bit_width = 4 * 8, .address = io },
> > + .pm1_cnt = { .space_id = as, .bit_width = 2 * 8, .address = io + 0x04 },
> > + .pm_tmr = { .space_id = as, .bit_width = 4 * 8, .address = io + 0x08 },
> > + .gpe0_blk = { .space_id = as, .bit_width =
> > + object_property_get_uint(o, ACPI_PM_PROP_GPE0_BLK_LEN, NULL) * 8,
> > + .address = object_property_get_uint(o, ACPI_PM_PROP_GPE0_BLK, NULL)
> > + },
> > + };
> > + *data = fadt;
> > +}
> > +
> > static void acpi_get_pm_info(AcpiPmInfo *pm)
> > {
> > Object *piix = piix4_pm_find();
> > Object *lpc = ich9_lpc_find();
> > Object *obj = piix ? piix : lpc;
> > QObject *o;
> > -
> > - pm->force_rev1_fadt = false;
> > pm->cpu_hp_io_base = 0;
> > pm->pcihp_io_base = 0;
> > pm->pcihp_io_len = 0;
> > +
> > + init_fadt_data(obj, &pm->fadt);
> > if (piix) {
> > /* w2k requires FADT(rev1) or it won't boot, keep PC compatible */
> > - pm->force_rev1_fadt = true;
> > + pm->fadt.rev = 1;
> > pm->cpu_hp_io_base = PIIX4_CPU_HOTPLUG_IO_BASE;
> > pm->pcihp_io_base =
> > object_property_get_uint(obj, ACPI_PCIHP_IO_BASE_PROP, NULL);
> > @@ -145,10 +179,19 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
> > object_property_get_uint(obj, ACPI_PCIHP_IO_LEN_PROP, NULL);
> > }
> > if (lpc) {
> > + struct AcpiGenericAddress r = { .space_id = AML_AS_SYSTEM_IO,
> > + .bit_width = 8, .address = ICH9_RST_CNT_IOPORT };
> > + pm->fadt.reset_reg = r;
> > + pm->fadt.reset_val = 0xf;
> > + pm->fadt.flags |= 1 << ACPI_FADT_F_RESET_REG_SUP;
> > pm->cpu_hp_io_base = ICH9_CPU_HOTPLUG_IO_BASE;
> Wouldn't it be possible to pass an other parameter to init_fadt_data()
> to tell whether we are in pc or q35 or the target rev and do those
> initializations there?
It's possible but I wanted to keep pc/q35 specific pieces explicitly
split from common data initialization as personally for me this way
it's easier to see difference (i.e. there is no need to jump into init_fadt_data()).
Perhaps I should rename init_fadt_data() to init_common_fadt_data().
> > }
> > assert(obj);
> >
> > + /* The above need not be conditional on machine type because the reset port
> > + * happens to be the same on PIIX (pc) and ICH9 (q35). */
> > + QEMU_BUILD_BUG_ON(ICH9_RST_CNT_IOPORT != RCR_IOPORT);
> > +
> > /* Fill in optional s3/s4 related properties */
> > o = object_property_get_qobject(obj, ACPI_PM_PROP_S3_DISABLED, NULL);
> > if (o) {
> > @@ -172,22 +215,6 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
> > }
> > qobject_decref(o);
> >
> > - /* Fill in mandatory properties */
> > - pm->sci_int = object_property_get_uint(obj, ACPI_PM_PROP_SCI_INT, NULL);
> > -
> > - pm->acpi_enable_cmd = object_property_get_uint(obj,
> > - ACPI_PM_PROP_ACPI_ENABLE_CMD,
> > - NULL);
> > - pm->acpi_disable_cmd =
> > - object_property_get_uint(obj,
> > - ACPI_PM_PROP_ACPI_DISABLE_CMD,
> > - NULL);
> > - pm->io_base = object_property_get_uint(obj, ACPI_PM_PROP_PM_IO_BASE,
> > - NULL);
> > - pm->gpe0_blk = object_property_get_uint(obj, ACPI_PM_PROP_GPE0_BLK,
> > - NULL);
> > - pm->gpe0_blk_len = object_property_get_uint(obj, ACPI_PM_PROP_GPE0_BLK_LEN,
> > - NULL);
> > pm->pcihp_bridge_en =
> > object_property_get_bool(obj, "acpi-pci-hotplug-with-bridge-support",
> > NULL);
> > @@ -255,8 +282,6 @@ static void acpi_get_pci_holes(Range *hole, Range *hole64)
> > NULL));
> > }
> >
> > -#define ACPI_PORT_SMI_CMD 0x00b2 /* TODO: this is APM_CNT_IOPORT */
> > -
> > static void acpi_align_size(GArray *blob, unsigned align)
> > {
> > /* Align size to multiple of given size. This reduces the chance
> > @@ -275,73 +300,53 @@ build_facs(GArray *table_data, BIOSLinker *linker)
> > }
> >
> > /* Load chipset information in FADT */
> > -static void fadt_setup(AcpiFadtDescriptorRev3 *fadt, AcpiPmInfo *pm)
> > +static void fadt_setup(AcpiFadtDescriptorRev3 *fadt, AcpiFadtData f)
> > {
> > - fadt->model = 1;
> > + fadt->model = f.int_model;
> > fadt->reserved1 = 0;
> > - fadt->sci_int = cpu_to_le16(pm->sci_int);
> > - fadt->smi_cmd = cpu_to_le32(ACPI_PORT_SMI_CMD);
> > - fadt->acpi_enable = pm->acpi_enable_cmd;
> > - fadt->acpi_disable = pm->acpi_disable_cmd;
> > + fadt->sci_int = cpu_to_le16(f.sci_int);
> > + fadt->smi_cmd = cpu_to_le32(f.smi_cmd);
> > + fadt->acpi_enable = f.acpi_enable_cmd;
> > + fadt->acpi_disable = f.acpi_disable_cmd;
> > /* EVT, CNT, TMR offset matches hw/acpi/core.c */
> > - fadt->pm1a_evt_blk = cpu_to_le32(pm->io_base);
> > - fadt->pm1a_cnt_blk = cpu_to_le32(pm->io_base + 0x04);
> > - fadt->pm_tmr_blk = cpu_to_le32(pm->io_base + 0x08);
> > - fadt->gpe0_blk = cpu_to_le32(pm->gpe0_blk);
> > + fadt->pm1a_evt_blk = cpu_to_le32(f.pm1_evt.address);
> > + fadt->pm1a_cnt_blk = cpu_to_le32(f.pm1_cnt.address);
> > + fadt->pm_tmr_blk = cpu_to_le32(f.pm_tmr.address);
> > + fadt->gpe0_blk = cpu_to_le32(f.gpe0_blk.address);
> > /* EVT, CNT, TMR length matches hw/acpi/core.c */
> > - fadt->pm1_evt_len = 4;
> > - fadt->pm1_cnt_len = 2;
> > - fadt->pm_tmr_len = 4;
> > - fadt->gpe0_blk_len = pm->gpe0_blk_len;
> > - fadt->plvl2_lat = cpu_to_le16(0xfff); /* C2 state not supported */
> > - fadt->plvl3_lat = cpu_to_le16(0xfff); /* C3 state not supported */
> > - fadt->flags = cpu_to_le32((1 << ACPI_FADT_F_WBINVD) |
> > - (1 << ACPI_FADT_F_PROC_C1) |
> > - (1 << ACPI_FADT_F_SLP_BUTTON) |
> > - (1 << ACPI_FADT_F_RTC_S4));
> > - fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_USE_PLATFORM_CLOCK);
> > - /* APIC destination mode ("Flat Logical") has an upper limit of 8 CPUs
> > - * For more than 8 CPUs, "Clustered Logical" mode has to be used
> > - */
> > - if (max_cpus > 8) {
> > - fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_FORCE_APIC_CLUSTER_MODEL);
> > - }
> > - fadt->century = RTC_CENTURY;
> > - if (pm->force_rev1_fadt) {
> > + fadt->pm1_evt_len = f.pm1_evt.bit_width / 8;
> > + fadt->pm1_cnt_len = f.pm1_cnt.bit_width / 8;
> > + fadt->pm_tmr_len = f.pm_tmr.bit_width / 8;
> > + fadt->gpe0_blk_len = f.gpe0_blk.bit_width / 8;
> > + fadt->plvl2_lat = cpu_to_le16(f.c2_latency);
> > + fadt->plvl3_lat = cpu_to_le16(f.c3_latency);
> > + fadt->flags = cpu_to_le32(f.flags);
> > + fadt->century = f.rtc_century;
> > + if (f.rev == 1) {
> > return;
> > }
> >
> > - fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_RESET_REG_SUP);
> > - fadt->reset_value = 0xf;
> > - fadt->reset_register.space_id = AML_SYSTEM_IO;
> > - fadt->reset_register.bit_width = 8;
> > - fadt->reset_register.address = cpu_to_le64(ICH9_RST_CNT_IOPORT);
> > - /* The above need not be conditional on machine type because the reset port
> > - * happens to be the same on PIIX (pc) and ICH9 (q35). */
> > - QEMU_BUILD_BUG_ON(ICH9_RST_CNT_IOPORT != RCR_IOPORT);
> > + fadt->reset_value = f.reset_val;
> > + fadt->reset_register = f.reset_reg;
> > + fadt->reset_register.address = cpu_to_le64(f.reset_reg.address);
> >
> > - fadt->xpm1a_event_block.space_id = AML_SYSTEM_IO;
> > - fadt->xpm1a_event_block.bit_width = fadt->pm1_evt_len * 8;
> > - fadt->xpm1a_event_block.address = cpu_to_le64(pm->io_base);
> > + fadt->xpm1a_event_block = f.pm1_evt;
> > + fadt->xpm1a_event_block.address = cpu_to_le64(f.pm1_evt.address);
> >
> > - fadt->xpm1a_control_block.space_id = AML_SYSTEM_IO;
> > - fadt->xpm1a_control_block.bit_width = fadt->pm1_cnt_len * 8;
> > - fadt->xpm1a_control_block.address = cpu_to_le64(pm->io_base + 0x4);
> > + fadt->xpm1a_control_block = f.pm1_cnt;
> > + fadt->xpm1a_control_block.address = cpu_to_le64(f.pm1_cnt.address);
> >
> > - fadt->xpm_timer_block.space_id = AML_SYSTEM_IO;
> > - fadt->xpm_timer_block.bit_width = fadt->pm_tmr_len * 8;
> > - fadt->xpm_timer_block.address = cpu_to_le64(pm->io_base + 0x8);
> > + fadt->xpm_timer_block = f.pm_tmr;
> > + fadt->xpm_timer_block.address = cpu_to_le64(f.pm_tmr.address);
> >
> > - fadt->xgpe0_block.space_id = AML_SYSTEM_IO;
> > - fadt->xgpe0_block.bit_width = pm->gpe0_blk_len * 8;
> > - fadt->xgpe0_block.address = cpu_to_le64(pm->gpe0_blk);
> > + fadt->xgpe0_block = f.gpe0_blk;
> > + fadt->xgpe0_block.address = cpu_to_le64(f.gpe0_blk.address);
> > }
> >
> >
> > /* FADT */
> > static void
> > -build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,
> > - unsigned facs_tbl_offset, unsigned dsdt_tbl_offset,
> > +build_fadt(GArray *table_data, BIOSLinker *linker, AcpiFadtData *f,
> > const char *oem_id, const char *oem_table_id)
> > {
> > AcpiFadtDescriptorRev3 *fadt = acpi_data_push(table_data, sizeof(*fadt));
> > @@ -349,29 +354,32 @@ build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,
> > unsigned dsdt_entry_offset = (char *)&fadt->dsdt - table_data->data;
> > unsigned xdsdt_entry_offset = (char *)&fadt->x_dsdt - table_data->data;
> > int fadt_size = sizeof(*fadt);
> > - int rev = 3;
> >
> > /* FACS address to be filled by Guest linker */
> > - bios_linker_loader_add_pointer(linker,
> > - ACPI_BUILD_TABLE_FILE, fw_ctrl_offset, sizeof(fadt->firmware_ctrl),
> > - ACPI_BUILD_TABLE_FILE, facs_tbl_offset);
> > + if (f->facs_tbl_offset) {
> this test was not there originally. How does it relate to above changes?
[...]
> > + if (f->dsdt_tbl_offset) {
> same
I guess it is sort of slipped in from (7/9) patch,
as this fields can be 0 and don't need dynamic patching.
Theoretically facs/dsdt could be 0 for x86 too if we drop support
for 32bit guests (which isn't going to happen).
I can move it to 7/9 which moves build_fadt into generic code from x86 specific,
which is more appropriate place for it.
> > + bios_linker_loader_add_pointer(linker,
> > + ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->dsdt),
> > + ACPI_BUILD_TABLE_FILE, *f->dsdt_tbl_offset);
> > + }
> > + if (f->rev == 1) {
> > fadt_size = offsetof(typeof(*fadt), reset_register);
> > - } else {
> > + } else if (f->xdsdt_tbl_offset) {
> > bios_linker_loader_add_pointer(linker,
> > ACPI_BUILD_TABLE_FILE, xdsdt_entry_offset, sizeof(fadt->x_dsdt),
> > - ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
> > + ACPI_BUILD_TABLE_FILE, *f->xdsdt_tbl_offset);
> > }
> >
> > build_header(linker, table_data,
> > - (void *)fadt, "FACP", fadt_size, rev, oem_id, oem_table_id);
> > + (void *)fadt, "FACP", fadt_size, f->rev,
> > + oem_id, oem_table_id);
> > }
> >
> > void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
> > @@ -2051,7 +2059,12 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
> > crs = aml_resource_template();
> > aml_append(crs,
> > - aml_io(AML_DECODE16, pm->gpe0_blk, pm->gpe0_blk, 1, pm->gpe0_blk_len)
> > + aml_io(
> > + AML_DECODE16,
> > + pm->fadt.gpe0_blk.address,
> > + pm->fadt.gpe0_blk.address,
> > + 1,
> > + pm->fadt.gpe0_blk.bit_width / 8)
> > );
> > aml_append(dev, aml_name_decl("_CRS", crs));
> > aml_append(scope, dev);
> > @@ -2698,7 +2711,10 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> > /* ACPI tables pointed to by RSDT */
> > fadt = tables_blob->len;
> > acpi_add_table(table_offsets, tables_blob);
> > - build_fadt(tables_blob, tables->linker, &pm, facs, dsdt,
> > + pm.fadt.facs_tbl_offset = &facs;
> > + pm.fadt.dsdt_tbl_offset = &dsdt;
> > + pm.fadt.xdsdt_tbl_offset = &dsdt;
> > + build_fadt(tables_blob, tables->linker, &pm.fadt,
> > slic_oem.id, slic_oem.table_id);
> > aml_len += tables_blob->len - fadt;
> >
> >
> Otherwise looks good to me.
>
> Thanks
>
> Eric
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH 8/9] virt_arm: acpi: reuse common build_fadt()
2018-02-27 15:07 ` Auger Eric
@ 2018-02-27 15:49 ` Igor Mammedov
0 siblings, 0 replies; 24+ messages in thread
From: Igor Mammedov @ 2018-02-27 15:49 UTC (permalink / raw)
To: Auger Eric
Cc: qemu-devel, Peter Maydell, Andrew Jones, Shannon Zhao, qemu-arm,
Michael S. Tsirkin
On Tue, 27 Feb 2018 16:07:49 +0100
Auger Eric <eric.auger@redhat.com> wrote:
> Hi Igor,
> On 22/02/18 13:42, Igor Mammedov wrote:
> > Extend generic build_fadt() to support rev5.1 FADT
> > and reuse it for 'virt' board, it would allow to
> > phase out usage of AcpiFadtDescriptorRev5_1 and
> > later ACPI_FADT_COMMON_DEF.
> Oups sorry sent the R-b to fast. Some small comments below.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > include/hw/acpi/acpi-defs.h | 12 ++----------
> > hw/acpi/aml-build.c | 21 ++++++++++++++++++++-
> > hw/arm/virt-acpi-build.c | 33 ++++++++++++---------------------
> > 3 files changed, 34 insertions(+), 32 deletions(-)
> >
> > diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> > index e0accc4..34e49dc 100644
> > --- a/include/hw/acpi/acpi-defs.h
> > +++ b/include/hw/acpi/acpi-defs.h
> > @@ -165,16 +165,6 @@ struct AcpiFadtDescriptorRev3 {
> > } QEMU_PACKED;
> > typedef struct AcpiFadtDescriptorRev3 AcpiFadtDescriptorRev3;
> >
> > -struct AcpiFadtDescriptorRev5_1 {
> > - ACPI_FADT_COMMON_DEF
> > - /* 64-bit Sleep Control register (ACPI 5.0) */
> > - struct AcpiGenericAddress sleep_control;
> > - /* 64-bit Sleep Status register (ACPI 5.0) */
> > - struct AcpiGenericAddress sleep_status;
> > -} QEMU_PACKED;
> > -
> > -typedef struct AcpiFadtDescriptorRev5_1 AcpiFadtDescriptorRev5_1;
> > -
> > typedef struct AcpiFadtData {
> > struct AcpiGenericAddress pm1_cnt; /* PM1a_CNT_BLK */
> > struct AcpiGenericAddress pm1_evt; /* PM1a_EVT_BLK */
> > @@ -192,6 +182,8 @@ typedef struct AcpiFadtData {
> > uint8_t rtc_century; /* CENTURY */
> > uint16_t c2_latency; /* P_LVL2_LAT */
> > uint16_t c3_latency; /* P_LVL3_LAT */
> > + uint16_t arm_boot_arch; /* ARM_BOOT_ARCH */
> > + uint8_t minor_ver; /* FADT Minor Version */
> >
> > /*
> > * respective tables offsets within ACPI_BUILD_TABLE_FILE,
> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > index 17fb71b..5a33cd0 100644
> > --- a/hw/acpi/aml-build.c
> > +++ b/hw/acpi/aml-build.c
>
> nit: comment above needs to be extended to rev5.1 support
Thanks, I'll fix it up on respin.
> > @@ -1755,7 +1755,14 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
> >
> > build_append_gas_from_struct(tbl, &f->reset_reg); /* RESET_REG */
> > build_append_int_noprefix(tbl, f->reset_val, 1); /* RESET_VALUE */
> > - build_append_int_noprefix(tbl, 0, 3); /* Reserved, ACPI 3.0 */
> > + /* Since ACPI 5.1 */
> > + if ((f->rev >= 6) || ((f->rev == 5) && f->minor_ver > 0)) {
> > + build_append_int_noprefix(tbl, f->arm_boot_arch, 2); /* ARM_BOOT_ARCH */
> > + /* FADT Minor Version */
> > + build_append_int_noprefix(tbl, f->minor_ver, 1);
> > + } else {
> > + build_append_int_noprefix(tbl, 0, 3); /* Reserved upto ACPI 5.0 */
> > + }
> > build_append_int_noprefix(tbl, 0, 8); /* X_FIRMWARE_CTRL */
> >
> > /* XDSDT address to be filled by Guest linker at runtime */
> > @@ -1779,6 +1786,18 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
> > build_append_gas_from_struct(tbl, &f->gpe0_blk); /* X_GPE0_BLK */
> > build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0); /* X_GPE1_BLK */
> >
> > + if (f->rev <= 4) {
> > + goto build_hdr;
> > + }
> > +
> > + /* SLEEP_CONTROL_REG */
> > + build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
> > + /* SLEEP_STATUS_REG */
> > + build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
>
> what about Hypervisor Vendor Identity field (8 byte) in version 6.0?
not supported yet, hence assert below.
When it's added we should update assert to the next supported revision.
> > +
> > + /* TODO: extra fields need to be added to support revisions above rev5 */
> > + assert(f->rev == 5);
> OK so can't we assert before the (f->rev >= 6) check above?
>
> Thanks
>
> Eric
> > +
> > build_hdr:
> > build_header(linker, tbl, (void *)(tbl->data + fadt_start),
> > "FACP", tbl->len - fadt_start, f->rev, oem_id, oem_table_id);
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index b644da9..c7c6a57 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -654,39 +654,30 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> > static void build_fadt_rev5(GArray *table_data, BIOSLinker *linker,
> > VirtMachineState *vms, unsigned dsdt_tbl_offset)
> > {
> > - int fadt_start = table_data->len;
> > - AcpiFadtDescriptorRev5_1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
> > - unsigned xdsdt_entry_offset = (char *)&fadt->x_dsdt - table_data->data;
> > - uint16_t bootflags;
> > + /* ACPI v5.1 */
> > + AcpiFadtData fadt = {
> > + .rev = 5,
> > + .minor_ver = 1,
> > + .flags = 1 << ACPI_FADT_F_HW_REDUCED_ACPI,
> > + .xdsdt_tbl_offset = &dsdt_tbl_offset,
> > + };
> >
> > switch (vms->psci_conduit) {
> > case QEMU_PSCI_CONDUIT_DISABLED:
> > - bootflags = 0;
> > + fadt.arm_boot_arch = 0;
> > break;
> > case QEMU_PSCI_CONDUIT_HVC:
> > - bootflags = ACPI_FADT_ARM_PSCI_COMPLIANT | ACPI_FADT_ARM_PSCI_USE_HVC;
> > + fadt.arm_boot_arch = ACPI_FADT_ARM_PSCI_COMPLIANT |
> > + ACPI_FADT_ARM_PSCI_USE_HVC;
> > break;
> > case QEMU_PSCI_CONDUIT_SMC:
> > - bootflags = ACPI_FADT_ARM_PSCI_COMPLIANT;
> > + fadt.arm_boot_arch = ACPI_FADT_ARM_PSCI_COMPLIANT;
> > break;
> > default:
> > g_assert_not_reached();
> > }
> >
> > - /* Hardware Reduced = 1 and use PSCI 0.2+ */
> > - fadt->flags = cpu_to_le32(1 << ACPI_FADT_F_HW_REDUCED_ACPI);
> > - fadt->arm_boot_flags = cpu_to_le16(bootflags);
> > -
> > - /* ACPI v5.1 (fadt->revision.fadt->minor_revision) */
> > - fadt->minor_revision = 0x1;
> > -
> > - /* DSDT address to be filled by Guest linker */
> > - bios_linker_loader_add_pointer(linker,
> > - ACPI_BUILD_TABLE_FILE, xdsdt_entry_offset, sizeof(fadt->x_dsdt),
> > - ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
> > -
> > - build_header(linker, table_data, (void *)(table_data->data + fadt_start),
> > - "FACP", table_data->len - fadt_start, 5, NULL, NULL);
> > + build_fadt(table_data, linker, &fadt, NULL, NULL);
> > }
> >
> > /* DSDT */
> >
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [Qemu-arm] [PATCH 2/9] pc: replace pm object initialization with one-liner in acpi_get_pm_info()
2018-02-22 12:42 ` [Qemu-devel] [PATCH 2/9] pc: replace pm object initialization with one-liner in acpi_get_pm_info() Igor Mammedov
2018-02-27 12:42 ` Auger Eric
@ 2018-02-27 23:41 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 24+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-02-27 23:41 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel
Cc: Peter Maydell, Andrew Jones, Shannon Zhao, qemu-arm, Michael S. Tsirkin
On 02/22/2018 09:42 AM, Igor Mammedov wrote:
> next patch will need it before it gets to piix4/lpc branches
> that initializes 'obj' now.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/i386/acpi-build.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index deb440f..b85fefe 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -128,7 +128,7 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
> {
> Object *piix = piix4_pm_find();
> Object *lpc = ich9_lpc_find();
> - Object *obj = NULL;
> + Object *obj = piix ? piix : lpc;
> QObject *o;
>
> pm->force_rev1_fadt = false;
> @@ -138,7 +138,6 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
> if (piix) {
> /* w2k requires FADT(rev1) or it won't boot, keep PC compatible */
> pm->force_rev1_fadt = true;
> - obj = piix;
> pm->cpu_hp_io_base = PIIX4_CPU_HOTPLUG_IO_BASE;
> pm->pcihp_io_base =
> object_property_get_uint(obj, ACPI_PCIHP_IO_BASE_PROP, NULL);
> @@ -146,7 +145,6 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
> object_property_get_uint(obj, ACPI_PCIHP_IO_LEN_PROP, NULL);
> }
> if (lpc) {
> - obj = lpc;
> pm->cpu_hp_io_base = ICH9_CPU_HOTPLUG_IO_BASE;
> }
> assert(obj);
>
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2018-02-27 23:41 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-22 12:42 [Qemu-devel] [PATCH 0/9] generalize build_fadt() Igor Mammedov
2018-02-22 12:42 ` [Qemu-devel] [PATCH 1/9] acpi: remove unused acpi-dsdt.aml Igor Mammedov
2018-02-22 14:25 ` Gerd Hoffmann
2018-02-27 12:42 ` Auger Eric
2018-02-27 15:09 ` Igor Mammedov
2018-02-22 12:42 ` [Qemu-devel] [PATCH 2/9] pc: replace pm object initialization with one-liner in acpi_get_pm_info() Igor Mammedov
2018-02-27 12:42 ` Auger Eric
2018-02-27 23:41 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2018-02-22 12:42 ` [Qemu-devel] [PATCH 3/9] acpi: reuse AcpiGenericAddress instead of Acpi20GenericAddress Igor Mammedov
2018-02-27 12:42 ` Auger Eric
2018-02-22 12:42 ` [Qemu-devel] [PATCH 4/9] acpi: add build_append_gas() helper for Generic Address Structure Igor Mammedov
2018-02-27 12:48 ` Auger Eric
2018-02-22 12:42 ` [Qemu-devel] [PATCH 5/9] pc: acpi: isolate FADT specific data into AcpiFadtData structure Igor Mammedov
2018-02-27 13:47 ` Auger Eric
2018-02-27 15:44 ` Igor Mammedov
2018-02-22 12:42 ` [Qemu-devel] [PATCH 6/9] pc: acpi: use build_append_foo() API to construct FADT Igor Mammedov
2018-02-27 14:10 ` Auger Eric
2018-02-22 12:42 ` [Qemu-devel] [PATCH 7/9] acpi: move build_fadt() from i386 specific to generic ACPI source Igor Mammedov
2018-02-27 14:15 ` Auger Eric
2018-02-22 12:42 ` [Qemu-devel] [PATCH 8/9] virt_arm: acpi: reuse common build_fadt() Igor Mammedov
2018-02-27 14:42 ` Auger Eric
2018-02-27 15:07 ` Auger Eric
2018-02-27 15:49 ` Igor Mammedov
2018-02-22 12:42 ` [Qemu-devel] [PATCH 9/9] tests: acpi: don't read all fields in test_acpi_fadt_table() Igor Mammedov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.