* [Qemu-devel] [PATCH v2 01/11] tests: acpi: test multiple SSDT tables
2016-01-12 18:49 [Qemu-devel] [PATCH v2 00/11] NVDIMM ACPI: introduce the framework of QEMU emulated Xiao Guangrong
@ 2016-01-12 18:50 ` Xiao Guangrong
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 02/11] tests: acpi: test NVDIMM tables Xiao Guangrong
` (11 subsequent siblings)
12 siblings, 0 replies; 51+ messages in thread
From: Xiao Guangrong @ 2016-01-12 18:50 UTC (permalink / raw)
To: pbonzini, imammedo
Cc: Xiao Guangrong, ehabkost, kvm, mst, gleb, mtosatti, qemu-devel,
stefanha, dan.j.williams, rth
Currently, only one SSDT with default oem-table-id can be tested,
this patch extents it to test all SSDT tables
Other tables except the default one will be named as SSDT-$oem-table-id
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
tests/bios-tables-test.c | 43 ++++++++++++++++++++++++++++++++-----------
1 file changed, 32 insertions(+), 11 deletions(-)
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 75ec330..fc01cce 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -18,6 +18,7 @@
#include "libqtest.h"
#include "qemu/compiler.h"
#include "hw/acpi/acpi-defs.h"
+#include "hw/acpi/aml-build.h"
#include "hw/smbios/smbios.h"
#include "qemu/bitmap.h"
@@ -379,6 +380,33 @@ static void test_acpi_tables(test_data *data)
}
}
+static gchar *get_sdt_aml_file_name(test_data *data, AcpiSdtTable *sdt,
+ bool skip_variant)
+{
+ gchar *aml_file, *def_oem_table_id, *oem_table_id, *sig_oem;
+ uint32_t signature = cpu_to_le32(sdt->header.signature);
+ const char *ext = data->variant && !skip_variant ? data->variant : "";
+
+ def_oem_table_id = g_strdup_printf("%s%s", ACPI_BUILD_APPNAME4,
+ (gchar *)&signature);
+ oem_table_id = g_strndup((char *)sdt->header.oem_table_id,
+ sizeof(sdt->header.oem_table_id));
+
+ if (strcmp(oem_table_id, def_oem_table_id)) {
+ sig_oem = g_strdup_printf("%.4s-%s", (gchar *)&signature,
+ oem_table_id);
+ } else {
+ sig_oem = g_strdup_printf("%.4s", (gchar *)&signature);
+ }
+
+ aml_file = g_strdup_printf("%s/%s/%s%s", data_dir, data->machine,
+ sig_oem, ext);
+ g_free(sig_oem);
+ g_free(oem_table_id);
+ g_free(def_oem_table_id);
+ return aml_file;
+}
+
static void dump_aml_files(test_data *data, bool rebuild)
{
AcpiSdtTable *sdt;
@@ -389,14 +417,11 @@ static void dump_aml_files(test_data *data, bool rebuild)
int i;
for (i = 0; i < data->tables->len; ++i) {
- const char *ext = data->variant ? data->variant : "";
sdt = &g_array_index(data->tables, AcpiSdtTable, i);
g_assert(sdt->aml);
if (rebuild) {
- uint32_t signature = cpu_to_le32(sdt->header.signature);
- aml_file = g_strdup_printf("%s/%s/%.4s%s", data_dir, data->machine,
- (gchar *)&signature, ext);
+ aml_file = get_sdt_aml_file_name(data, sdt, false);
fd = g_open(aml_file, O_WRONLY|O_TRUNC|O_CREAT,
S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH);
} else {
@@ -508,22 +533,18 @@ static GArray *load_expected_aml(test_data *data)
GArray *exp_tables = g_array_new(false, true, sizeof(AcpiSdtTable));
for (i = 0; i < data->tables->len; ++i) {
AcpiSdtTable exp_sdt;
- uint32_t signature;
- const char *ext = data->variant ? data->variant : "";
+ bool skip_variant = false;
sdt = &g_array_index(data->tables, AcpiSdtTable, i);
memset(&exp_sdt, 0, sizeof(exp_sdt));
exp_sdt.header.signature = sdt->header.signature;
- signature = cpu_to_le32(sdt->header.signature);
-
try_again:
- aml_file = g_strdup_printf("%s/%s/%.4s%s", data_dir, data->machine,
- (gchar *)&signature, ext);
+ aml_file = get_sdt_aml_file_name(data, sdt, skip_variant);
if (data->variant && !g_file_test(aml_file, G_FILE_TEST_EXISTS)) {
g_free(aml_file);
- ext = "";
+ skip_variant = true;
goto try_again;
}
exp_sdt.aml_file = aml_file;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* [Qemu-devel] [PATCH v2 02/11] tests: acpi: test NVDIMM tables
2016-01-12 18:49 [Qemu-devel] [PATCH v2 00/11] NVDIMM ACPI: introduce the framework of QEMU emulated Xiao Guangrong
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 01/11] tests: acpi: test multiple SSDT tables Xiao Guangrong
@ 2016-01-12 18:50 ` Xiao Guangrong
2016-02-04 16:20 ` Michael S. Tsirkin
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 03/11] acpi: add aml_create_field() Xiao Guangrong
` (10 subsequent siblings)
12 siblings, 1 reply; 51+ messages in thread
From: Xiao Guangrong @ 2016-01-12 18:50 UTC (permalink / raw)
To: pbonzini, imammedo
Cc: Xiao Guangrong, ehabkost, kvm, mst, gleb, mtosatti, qemu-devel,
stefanha, dan.j.williams, rth
Add nvdimm test, two tables are created which are NFIT and
SSDT
Max memory size and multiple slots are needed to enable NVDIMM which
cause the primal SSDT table is also updated
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
tests/acpi-test-data/pc/NFIT | Bin 0 -> 224 bytes
tests/acpi-test-data/pc/NFIT.bridge | Bin 0 -> 224 bytes
tests/acpi-test-data/pc/SSDT | Bin 2486 -> 2885 bytes
tests/acpi-test-data/pc/SSDT-NVDIMM | Bin 0 -> 134 bytes
tests/acpi-test-data/pc/SSDT-NVDIMM.bridge | Bin 0 -> 134 bytes
tests/acpi-test-data/pc/SSDT.bridge | Bin 4345 -> 4745 bytes
tests/acpi-test-data/q35/NFIT | Bin 0 -> 224 bytes
tests/acpi-test-data/q35/NFIT.bridge | Bin 0 -> 224 bytes
tests/acpi-test-data/q35/SSDT | Bin 691 -> 1090 bytes
tests/acpi-test-data/q35/SSDT-NVDIMM | Bin 0 -> 134 bytes
tests/acpi-test-data/q35/SSDT-NVDIMM.bridge | Bin 0 -> 134 bytes
tests/acpi-test-data/q35/SSDT.bridge | Bin 708 -> 1107 bytes
tests/bios-tables-test.c | 15 ++++++++++-----
13 files changed, 10 insertions(+), 5 deletions(-)
create mode 100644 tests/acpi-test-data/pc/NFIT
create mode 100644 tests/acpi-test-data/pc/NFIT.bridge
create mode 100644 tests/acpi-test-data/pc/SSDT-NVDIMM
create mode 100644 tests/acpi-test-data/pc/SSDT-NVDIMM.bridge
create mode 100644 tests/acpi-test-data/q35/NFIT
create mode 100644 tests/acpi-test-data/q35/NFIT.bridge
create mode 100644 tests/acpi-test-data/q35/SSDT-NVDIMM
create mode 100644 tests/acpi-test-data/q35/SSDT-NVDIMM.bridge
diff --git a/tests/acpi-test-data/pc/NFIT b/tests/acpi-test-data/pc/NFIT
new file mode 100644
index 0000000000000000000000000000000000000000..2d2a190bd0a603dac228177428f4884e23393a7c
GIT binary patch
literal 224
zcmeZs^9*^wz`($m?Bwt45v<@85#a0x6k`O6f!H7#0xTGq7?@!khRVwy(mrn~aaiNb
zYb>$7=Qc<Js+I%9=4b$sATa}&I7~lS9wG}NA^IRB3qt_VmbL~)xGspAFcTpLMkZuk
G#IXUIOA`SA
literal 0
HcmV?d00001
diff --git a/tests/acpi-test-data/pc/NFIT.bridge b/tests/acpi-test-data/pc/NFIT.bridge
new file mode 100644
index 0000000000000000000000000000000000000000..2d2a190bd0a603dac228177428f4884e23393a7c
GIT binary patch
literal 224
zcmeZs^9*^wz`($m?Bwt45v<@85#a0x6k`O6f!H7#0xTGq7?@!khRVwy(mrn~aaiNb
zYb>$7=Qc<Js+I%9=4b$sATa}&I7~lS9wG}NA^IRB3qt_VmbL~)xGspAFcTpLMkZuk
G#IXUIOA`SA
literal 0
HcmV?d00001
diff --git a/tests/acpi-test-data/pc/SSDT b/tests/acpi-test-data/pc/SSDT
index 210d6a71e58aa34ce8e94121d25bcf58c3bd503c..83c5fcc88786f3f8c2ccb3e1fd935ab6b940cbf8 100644
GIT binary patch
delta 469
zcmdlcd{m4pIM^k`m79TqQFbF&G$WJ4yvgZ|dnNohV)U8ggPr07oIMSEJpx=fd|mv4
zxR@qeGnLgxH+gaU1{fG{#D{vi@ETMY7%*_edw9C=I9}js5Rr>_4hm*i5~I(8tlt+X
z2vQD|4i0g|lnx3Gfl3EN_+m-}1;Nsa@&3W}A<P)2`$M#6WM+xT$GdtNFk+|x3W7|?
zfI8j~?s!8|9bb>>JQ7?_k>f>Vd_&wMBAFI&P0nCw1I0lmh{*_I8fI?Z!Ss&}0M+w@
Ae*gdg
delta 67
zcmX>qwoRBTIM^j*8z%z;<MoYP(Tq%vt&`Ik_lo;+#OO1}2Rp?FIC~oSdIY#|_`3K7
XF-*2$Dq|O6^9^x}WZLY`@{bJwhvpK3
diff --git a/tests/acpi-test-data/pc/SSDT-NVDIMM b/tests/acpi-test-data/pc/SSDT-NVDIMM
new file mode 100644
index 0000000000000000000000000000000000000000..22265278c3a4df9e9fee0ea9e04b37f2c80baa29
GIT binary patch
literal 134
zcmWFzb_r=?U|?Xp<K*w`5v<@B=Hlt=3*-aEoFW38L9~D)TTFbgQ+#xj2P;rMh$G&^
z(}ma3Il$Avz|e?6MAXmO(T8OL=j1GgC4$V1V1iMEKi(zSmt_e^E+aE5JGx2QFU-Jz
LBi_*^2tyJ8_y8aQ
literal 0
HcmV?d00001
diff --git a/tests/acpi-test-data/pc/SSDT-NVDIMM.bridge b/tests/acpi-test-data/pc/SSDT-NVDIMM.bridge
new file mode 100644
index 0000000000000000000000000000000000000000..22265278c3a4df9e9fee0ea9e04b37f2c80baa29
GIT binary patch
literal 134
zcmWFzb_r=?U|?Xp<K*w`5v<@B=Hlt=3*-aEoFW38L9~D)TTFbgQ+#xj2P;rMh$G&^
z(}ma3Il$Avz|e?6MAXmO(T8OL=j1GgC4$V1V1iMEKi(zSmt_e^E+aE5JGx2QFU-Jz
LBi_*^2tyJ8_y8aQ
literal 0
HcmV?d00001
diff --git a/tests/acpi-test-data/pc/SSDT.bridge b/tests/acpi-test-data/pc/SSDT.bridge
index 6e6660b1fbe0bdb7fe0b257cb53c31fa9bb21a14..051c2e34aebbe6214a8c1e37b39f744bb4943051 100644
GIT binary patch
delta 470
zcmeyV*s01D9PAR(Da63QsJxLYnvuChh;ec{<6a3rju?IB_+Y2_0B27FUylG64qq3)
zATFlK)=Xvf(M?|5z5xaX9Py!^F1!X61_lfq@gANoJdPK58bsvcor8iImc;0@AnW%9
z3WAgarGrBpF{OioL!i<D5x$txKtZsyV!VHFeF!s#>HZMy8JSrk^6{=-28<XgfPx?s
zGN6t(ggf4lRL9q2I*$a`Q{;FN8Q%~$iAbgeT$3{x+CXuT31TvWn1-2~cQ85f0RS^@
Bg82Xd
delta 67
zcmeBF{i(<m9PASEQ-FbiQDGxjG$WJC&&e5#d&PY@V)U8ggPr07oIMSEJpx=fd|mv4
X7$(~?m9dMk`G&YfGHv!?apVI4kDw9$
diff --git a/tests/acpi-test-data/q35/NFIT b/tests/acpi-test-data/q35/NFIT
new file mode 100644
index 0000000000000000000000000000000000000000..2d2a190bd0a603dac228177428f4884e23393a7c
GIT binary patch
literal 224
zcmeZs^9*^wz`($m?Bwt45v<@85#a0x6k`O6f!H7#0xTGq7?@!khRVwy(mrn~aaiNb
zYb>$7=Qc<Js+I%9=4b$sATa}&I7~lS9wG}NA^IRB3qt_VmbL~)xGspAFcTpLMkZuk
G#IXUIOA`SA
literal 0
HcmV?d00001
diff --git a/tests/acpi-test-data/q35/NFIT.bridge b/tests/acpi-test-data/q35/NFIT.bridge
new file mode 100644
index 0000000000000000000000000000000000000000..2d2a190bd0a603dac228177428f4884e23393a7c
GIT binary patch
literal 224
zcmeZs^9*^wz`($m?Bwt45v<@85#a0x6k`O6f!H7#0xTGq7?@!khRVwy(mrn~aaiNb
zYb>$7=Qc<Js+I%9=4b$sATa}&I7~lS9wG}NA^IRB3qt_VmbL~)xGspAFcTpLMkZuk
G#IXUIOA`SA
literal 0
HcmV?d00001
diff --git a/tests/acpi-test-data/q35/SSDT b/tests/acpi-test-data/q35/SSDT
index 0970c67ddb1456707c0111f7e5e659ef385af408..f75d0745ce6d34ae7416409d4f8f42402e6c20b6 100644
GIT binary patch
delta 468
zcmdnYdWeH7IM^k`iG_iI@$g12enuuwqsh{YdnNohV)U8ggPr07oIMSEJpx=fd|mv4
zxR@rtXDq9aZt~*x4KOg^h!6F2;Wel*Fks+__waP#alF9OAR-s<92Cs3Bu1YFS-&q(
z5TqO^9US6_DIF9X0+kMk@Wqq{3WB8-<Nbr{Lzpp4_lIcD$jlOvk9YMlV8l=X6a<-&
z0d>3~-0_B_I=&v$c_g@=BFBr!_=dPiL^3Vlnw-JV28x4B5R(zaG|Zfw!{iPClKFy&
delta 66
zcmX@av6+=CIM^j*GZO;?W9>#RenuuQ$;r};d&PY@V)U8ggPr07oIMSEJpx=fd|mv4
W7$$#UEMpg8^9^x}WSab&*&P5k77>a7
diff --git a/tests/acpi-test-data/q35/SSDT-NVDIMM b/tests/acpi-test-data/q35/SSDT-NVDIMM
new file mode 100644
index 0000000000000000000000000000000000000000..22265278c3a4df9e9fee0ea9e04b37f2c80baa29
GIT binary patch
literal 134
zcmWFzb_r=?U|?Xp<K*w`5v<@B=Hlt=3*-aEoFW38L9~D)TTFbgQ+#xj2P;rMh$G&^
z(}ma3Il$Avz|e?6MAXmO(T8OL=j1GgC4$V1V1iMEKi(zSmt_e^E+aE5JGx2QFU-Jz
LBi_*^2tyJ8_y8aQ
literal 0
HcmV?d00001
diff --git a/tests/acpi-test-data/q35/SSDT-NVDIMM.bridge b/tests/acpi-test-data/q35/SSDT-NVDIMM.bridge
new file mode 100644
index 0000000000000000000000000000000000000000..22265278c3a4df9e9fee0ea9e04b37f2c80baa29
GIT binary patch
literal 134
zcmWFzb_r=?U|?Xp<K*w`5v<@B=Hlt=3*-aEoFW38L9~D)TTFbgQ+#xj2P;rMh$G&^
z(}ma3Il$Avz|e?6MAXmO(T8OL=j1GgC4$V1V1iMEKi(zSmt_e^E+aE5JGx2QFU-Jz
LBi_*^2tyJ8_y8aQ
literal 0
HcmV?d00001
diff --git a/tests/acpi-test-data/q35/SSDT.bridge b/tests/acpi-test-data/q35/SSDT.bridge
index a77868861754ce0b3333b2b7bc8091c03a53754d..87586ac9455a4933fc2a069e98a19e07cbf04670 100644
GIT binary patch
delta 468
zcmX@YdYOYOIM^j5n1z9XF?k~wKO>Wu@nmVny%K&LG5XB$!A|i3&YlLo9sw>KzAk=2
zTuhVSGnUmyH+gaU1{fG{#D{vi@ETMY7%*_edw9C=I9}js5Rr>_4hm*i5~I(8tlt+X
z2vQD|4i0g|lnx3Gfl3EN_+m-}1;Nsa@&3W}A<P)2`$M#6WM+xT$GdtNFk+|x3W7|?
zfI8j~?s!8|9bb>>JQ7?_k>f>Vd_&wMBAFI&P0nCw1I0lmh{*_I8fH$;VTu3%NXmk4
delta 66
zcmcc2afFpCIM^lR2onPXqwGd5enuv5smaoed&PY@V)U8ggPr07oIMSEJpx=fd|mv4
W7$$#UEMpg8^9^x}WSab&IRXGMZ4qz)
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index fc01cce..87cf2e5 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -157,6 +157,10 @@ static const char *iasl = stringify(CONFIG_IASL);
static const char *iasl;
#endif
+#define NVDIMM_PARAMS " -m 128M,maxmem=5G,slots=2 " \
+ "-object memory-backend-ram,id=mem1,size=128M " \
+ "-device nvdimm,memdev=mem1"
+
static void free_test_data(test_data *data)
{
AcpiSdtTable *temp;
@@ -823,7 +827,7 @@ static void test_acpi_piix4_tcg(void)
*/
memset(&data, 0, sizeof(data));
data.machine = MACHINE_PC;
- test_acpi_one("-machine accel=tcg", &data);
+ test_acpi_one("-machine accel=tcg,nvdimm" NVDIMM_PARAMS, &data);
free_test_data(&data);
}
@@ -834,7 +838,8 @@ static void test_acpi_piix4_tcg_bridge(void)
memset(&data, 0, sizeof(data));
data.machine = MACHINE_PC;
data.variant = ".bridge";
- test_acpi_one("-machine accel=tcg -device pci-bridge,chassis_nr=1", &data);
+ test_acpi_one("-machine accel=tcg,nvdimm -device pci-bridge,chassis_nr=1"
+ NVDIMM_PARAMS, &data);
free_test_data(&data);
}
@@ -844,7 +849,7 @@ static void test_acpi_q35_tcg(void)
memset(&data, 0, sizeof(data));
data.machine = MACHINE_Q35;
- test_acpi_one("-machine q35,accel=tcg", &data);
+ test_acpi_one("-machine q35,accel=tcg,nvdimm" NVDIMM_PARAMS, &data);
free_test_data(&data);
}
@@ -855,8 +860,8 @@ static void test_acpi_q35_tcg_bridge(void)
memset(&data, 0, sizeof(data));
data.machine = MACHINE_Q35;
data.variant = ".bridge";
- test_acpi_one("-machine q35,accel=tcg -device pci-bridge,chassis_nr=1",
- &data);
+ test_acpi_one("-machine q35,accel=tcg,nvdimm "
+ "-device pci-bridge,chassis_nr=1" NVDIMM_PARAMS, &data);
free_test_data(&data);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH v2 02/11] tests: acpi: test NVDIMM tables
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 02/11] tests: acpi: test NVDIMM tables Xiao Guangrong
@ 2016-02-04 16:20 ` Michael S. Tsirkin
2016-02-14 5:36 ` Xiao Guangrong
0 siblings, 1 reply; 51+ messages in thread
From: Michael S. Tsirkin @ 2016-02-04 16:20 UTC (permalink / raw)
To: Xiao Guangrong
Cc: ehabkost, kvm, gleb, mtosatti, qemu-devel, stefanha, imammedo,
pbonzini, dan.j.williams, rth
On Wed, Jan 13, 2016 at 02:50:01AM +0800, Xiao Guangrong wrote:
> Add nvdimm test, two tables are created which are NFIT and
> SSDT
>
> Max memory size and multiple slots are needed to enable NVDIMM which
> cause the primal SSDT table is also updated
>
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
First don't include binary in patches directly please.
> ---
> tests/acpi-test-data/pc/NFIT | Bin 0 -> 224 bytes
> tests/acpi-test-data/pc/NFIT.bridge | Bin 0 -> 224 bytes
> tests/acpi-test-data/pc/SSDT | Bin 2486 -> 2885 bytes
> tests/acpi-test-data/pc/SSDT-NVDIMM | Bin 0 -> 134 bytes
> tests/acpi-test-data/pc/SSDT-NVDIMM.bridge | Bin 0 -> 134 bytes
> tests/acpi-test-data/pc/SSDT.bridge | Bin 4345 -> 4745 bytes
> tests/acpi-test-data/q35/NFIT | Bin 0 -> 224 bytes
> tests/acpi-test-data/q35/NFIT.bridge | Bin 0 -> 224 bytes
> tests/acpi-test-data/q35/SSDT | Bin 691 -> 1090 bytes
> tests/acpi-test-data/q35/SSDT-NVDIMM | Bin 0 -> 134 bytes
> tests/acpi-test-data/q35/SSDT-NVDIMM.bridge | Bin 0 -> 134 bytes
> tests/acpi-test-data/q35/SSDT.bridge | Bin 708 -> 1107 bytes
> tests/bios-tables-test.c | 15 ++++++++++-----
So let's add an new test, and there only test the NVDIMM tables and only
with one configuration.
Otherwise number of tables to maintain explodes.
You will have to teach test to ignore everything
that does not have NVDIMM in name for this test.
> 13 files changed, 10 insertions(+), 5 deletions(-)
> create mode 100644 tests/acpi-test-data/pc/NFIT
> create mode 100644 tests/acpi-test-data/pc/NFIT.bridge
> create mode 100644 tests/acpi-test-data/pc/SSDT-NVDIMM
> create mode 100644 tests/acpi-test-data/pc/SSDT-NVDIMM.bridge
> create mode 100644 tests/acpi-test-data/q35/NFIT
> create mode 100644 tests/acpi-test-data/q35/NFIT.bridge
> create mode 100644 tests/acpi-test-data/q35/SSDT-NVDIMM
> create mode 100644 tests/acpi-test-data/q35/SSDT-NVDIMM.bridge
>
> diff --git a/tests/acpi-test-data/pc/NFIT b/tests/acpi-test-data/pc/NFIT
> new file mode 100644
> index 0000000000000000000000000000000000000000..2d2a190bd0a603dac228177428f4884e23393a7c
> GIT binary patch
> literal 224
> zcmeZs^9*^wz`($m?Bwt45v<@85#a0x6k`O6f!H7#0xTGq7?@!khRVwy(mrn~aaiNb
> zYb>$7=Qc<Js+I%9=4b$sATa}&I7~lS9wG}NA^IRB3qt_VmbL~)xGspAFcTpLMkZuk
> G#IXUIOA`SA
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/acpi-test-data/pc/NFIT.bridge b/tests/acpi-test-data/pc/NFIT.bridge
> new file mode 100644
> index 0000000000000000000000000000000000000000..2d2a190bd0a603dac228177428f4884e23393a7c
> GIT binary patch
> literal 224
> zcmeZs^9*^wz`($m?Bwt45v<@85#a0x6k`O6f!H7#0xTGq7?@!khRVwy(mrn~aaiNb
> zYb>$7=Qc<Js+I%9=4b$sATa}&I7~lS9wG}NA^IRB3qt_VmbL~)xGspAFcTpLMkZuk
> G#IXUIOA`SA
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/acpi-test-data/pc/SSDT b/tests/acpi-test-data/pc/SSDT
> index 210d6a71e58aa34ce8e94121d25bcf58c3bd503c..83c5fcc88786f3f8c2ccb3e1fd935ab6b940cbf8 100644
> GIT binary patch
> delta 469
> zcmdlcd{m4pIM^k`m79TqQFbF&G$WJ4yvgZ|dnNohV)U8ggPr07oIMSEJpx=fd|mv4
> zxR@qeGnLgxH+gaU1{fG{#D{vi@ETMY7%*_edw9C=I9}js5Rr>_4hm*i5~I(8tlt+X
> z2vQD|4i0g|lnx3Gfl3EN_+m-}1;Nsa@&3W}A<P)2`$M#6WM+xT$GdtNFk+|x3W7|?
> zfI8j~?s!8|9bb>>JQ7?_k>f>Vd_&wMBAFI&P0nCw1I0lmh{*_I8fI?Z!Ss&}0M+w@
> Ae*gdg
>
> delta 67
> zcmX>qwoRBTIM^j*8z%z;<MoYP(Tq%vt&`Ik_lo;+#OO1}2Rp?FIC~oSdIY#|_`3K7
> XF-*2$Dq|O6^9^x}WZLY`@{bJwhvpK3
>
> diff --git a/tests/acpi-test-data/pc/SSDT-NVDIMM b/tests/acpi-test-data/pc/SSDT-NVDIMM
> new file mode 100644
> index 0000000000000000000000000000000000000000..22265278c3a4df9e9fee0ea9e04b37f2c80baa29
> GIT binary patch
> literal 134
> zcmWFzb_r=?U|?Xp<K*w`5v<@B=Hlt=3*-aEoFW38L9~D)TTFbgQ+#xj2P;rMh$G&^
> z(}ma3Il$Avz|e?6MAXmO(T8OL=j1GgC4$V1V1iMEKi(zSmt_e^E+aE5JGx2QFU-Jz
> LBi_*^2tyJ8_y8aQ
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/acpi-test-data/pc/SSDT-NVDIMM.bridge b/tests/acpi-test-data/pc/SSDT-NVDIMM.bridge
> new file mode 100644
> index 0000000000000000000000000000000000000000..22265278c3a4df9e9fee0ea9e04b37f2c80baa29
> GIT binary patch
> literal 134
> zcmWFzb_r=?U|?Xp<K*w`5v<@B=Hlt=3*-aEoFW38L9~D)TTFbgQ+#xj2P;rMh$G&^
> z(}ma3Il$Avz|e?6MAXmO(T8OL=j1GgC4$V1V1iMEKi(zSmt_e^E+aE5JGx2QFU-Jz
> LBi_*^2tyJ8_y8aQ
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/acpi-test-data/pc/SSDT.bridge b/tests/acpi-test-data/pc/SSDT.bridge
> index 6e6660b1fbe0bdb7fe0b257cb53c31fa9bb21a14..051c2e34aebbe6214a8c1e37b39f744bb4943051 100644
> GIT binary patch
> delta 470
> zcmeyV*s01D9PAR(Da63QsJxLYnvuChh;ec{<6a3rju?IB_+Y2_0B27FUylG64qq3)
> zATFlK)=Xvf(M?|5z5xaX9Py!^F1!X61_lfq@gANoJdPK58bsvcor8iImc;0@AnW%9
> z3WAgarGrBpF{OioL!i<D5x$txKtZsyV!VHFeF!s#>HZMy8JSrk^6{=-28<XgfPx?s
> zGN6t(ggf4lRL9q2I*$a`Q{;FN8Q%~$iAbgeT$3{x+CXuT31TvWn1-2~cQ85f0RS^@
> Bg82Xd
>
> delta 67
> zcmeBF{i(<m9PASEQ-FbiQDGxjG$WJC&&e5#d&PY@V)U8ggPr07oIMSEJpx=fd|mv4
> X7$(~?m9dMk`G&YfGHv!?apVI4kDw9$
>
> diff --git a/tests/acpi-test-data/q35/NFIT b/tests/acpi-test-data/q35/NFIT
> new file mode 100644
> index 0000000000000000000000000000000000000000..2d2a190bd0a603dac228177428f4884e23393a7c
> GIT binary patch
> literal 224
> zcmeZs^9*^wz`($m?Bwt45v<@85#a0x6k`O6f!H7#0xTGq7?@!khRVwy(mrn~aaiNb
> zYb>$7=Qc<Js+I%9=4b$sATa}&I7~lS9wG}NA^IRB3qt_VmbL~)xGspAFcTpLMkZuk
> G#IXUIOA`SA
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/acpi-test-data/q35/NFIT.bridge b/tests/acpi-test-data/q35/NFIT.bridge
> new file mode 100644
> index 0000000000000000000000000000000000000000..2d2a190bd0a603dac228177428f4884e23393a7c
> GIT binary patch
> literal 224
> zcmeZs^9*^wz`($m?Bwt45v<@85#a0x6k`O6f!H7#0xTGq7?@!khRVwy(mrn~aaiNb
> zYb>$7=Qc<Js+I%9=4b$sATa}&I7~lS9wG}NA^IRB3qt_VmbL~)xGspAFcTpLMkZuk
> G#IXUIOA`SA
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/acpi-test-data/q35/SSDT b/tests/acpi-test-data/q35/SSDT
> index 0970c67ddb1456707c0111f7e5e659ef385af408..f75d0745ce6d34ae7416409d4f8f42402e6c20b6 100644
> GIT binary patch
> delta 468
> zcmdnYdWeH7IM^k`iG_iI@$g12enuuwqsh{YdnNohV)U8ggPr07oIMSEJpx=fd|mv4
> zxR@rtXDq9aZt~*x4KOg^h!6F2;Wel*Fks+__waP#alF9OAR-s<92Cs3Bu1YFS-&q(
> z5TqO^9US6_DIF9X0+kMk@Wqq{3WB8-<Nbr{Lzpp4_lIcD$jlOvk9YMlV8l=X6a<-&
> z0d>3~-0_B_I=&v$c_g@=BFBr!_=dPiL^3Vlnw-JV28x4B5R(zaG|Zfw!{iPClKFy&
>
> delta 66
> zcmX@av6+=CIM^j*GZO;?W9>#RenuuQ$;r};d&PY@V)U8ggPr07oIMSEJpx=fd|mv4
> W7$$#UEMpg8^9^x}WSab&*&P5k77>a7
>
> diff --git a/tests/acpi-test-data/q35/SSDT-NVDIMM b/tests/acpi-test-data/q35/SSDT-NVDIMM
> new file mode 100644
> index 0000000000000000000000000000000000000000..22265278c3a4df9e9fee0ea9e04b37f2c80baa29
> GIT binary patch
> literal 134
> zcmWFzb_r=?U|?Xp<K*w`5v<@B=Hlt=3*-aEoFW38L9~D)TTFbgQ+#xj2P;rMh$G&^
> z(}ma3Il$Avz|e?6MAXmO(T8OL=j1GgC4$V1V1iMEKi(zSmt_e^E+aE5JGx2QFU-Jz
> LBi_*^2tyJ8_y8aQ
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/acpi-test-data/q35/SSDT-NVDIMM.bridge b/tests/acpi-test-data/q35/SSDT-NVDIMM.bridge
> new file mode 100644
> index 0000000000000000000000000000000000000000..22265278c3a4df9e9fee0ea9e04b37f2c80baa29
> GIT binary patch
> literal 134
> zcmWFzb_r=?U|?Xp<K*w`5v<@B=Hlt=3*-aEoFW38L9~D)TTFbgQ+#xj2P;rMh$G&^
> z(}ma3Il$Avz|e?6MAXmO(T8OL=j1GgC4$V1V1iMEKi(zSmt_e^E+aE5JGx2QFU-Jz
> LBi_*^2tyJ8_y8aQ
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/acpi-test-data/q35/SSDT.bridge b/tests/acpi-test-data/q35/SSDT.bridge
> index a77868861754ce0b3333b2b7bc8091c03a53754d..87586ac9455a4933fc2a069e98a19e07cbf04670 100644
> GIT binary patch
> delta 468
> zcmX@YdYOYOIM^j5n1z9XF?k~wKO>Wu@nmVny%K&LG5XB$!A|i3&YlLo9sw>KzAk=2
> zTuhVSGnUmyH+gaU1{fG{#D{vi@ETMY7%*_edw9C=I9}js5Rr>_4hm*i5~I(8tlt+X
> z2vQD|4i0g|lnx3Gfl3EN_+m-}1;Nsa@&3W}A<P)2`$M#6WM+xT$GdtNFk+|x3W7|?
> zfI8j~?s!8|9bb>>JQ7?_k>f>Vd_&wMBAFI&P0nCw1I0lmh{*_I8fH$;VTu3%NXmk4
>
> delta 66
> zcmcc2afFpCIM^lR2onPXqwGd5enuv5smaoed&PY@V)U8ggPr07oIMSEJpx=fd|mv4
> W7$$#UEMpg8^9^x}WSab&IRXGMZ4qz)
>
> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> index fc01cce..87cf2e5 100644
> --- a/tests/bios-tables-test.c
> +++ b/tests/bios-tables-test.c
> @@ -157,6 +157,10 @@ static const char *iasl = stringify(CONFIG_IASL);
> static const char *iasl;
> #endif
>
> +#define NVDIMM_PARAMS " -m 128M,maxmem=5G,slots=2 " \
> + "-object memory-backend-ram,id=mem1,size=128M " \
> + "-device nvdimm,memdev=mem1"
> +
> static void free_test_data(test_data *data)
> {
> AcpiSdtTable *temp;
> @@ -823,7 +827,7 @@ static void test_acpi_piix4_tcg(void)
> */
> memset(&data, 0, sizeof(data));
> data.machine = MACHINE_PC;
> - test_acpi_one("-machine accel=tcg", &data);
> + test_acpi_one("-machine accel=tcg,nvdimm" NVDIMM_PARAMS, &data);
> free_test_data(&data);
> }
>
> @@ -834,7 +838,8 @@ static void test_acpi_piix4_tcg_bridge(void)
> memset(&data, 0, sizeof(data));
> data.machine = MACHINE_PC;
> data.variant = ".bridge";
> - test_acpi_one("-machine accel=tcg -device pci-bridge,chassis_nr=1", &data);
> + test_acpi_one("-machine accel=tcg,nvdimm -device pci-bridge,chassis_nr=1"
> + NVDIMM_PARAMS, &data);
> free_test_data(&data);
> }
>
> @@ -844,7 +849,7 @@ static void test_acpi_q35_tcg(void)
>
> memset(&data, 0, sizeof(data));
> data.machine = MACHINE_Q35;
> - test_acpi_one("-machine q35,accel=tcg", &data);
> + test_acpi_one("-machine q35,accel=tcg,nvdimm" NVDIMM_PARAMS, &data);
> free_test_data(&data);
> }
>
> @@ -855,8 +860,8 @@ static void test_acpi_q35_tcg_bridge(void)
> memset(&data, 0, sizeof(data));
> data.machine = MACHINE_Q35;
> data.variant = ".bridge";
> - test_acpi_one("-machine q35,accel=tcg -device pci-bridge,chassis_nr=1",
> - &data);
> + test_acpi_one("-machine q35,accel=tcg,nvdimm "
> + "-device pci-bridge,chassis_nr=1" NVDIMM_PARAMS, &data);
> free_test_data(&data);
> }
>
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH v2 02/11] tests: acpi: test NVDIMM tables
2016-02-04 16:20 ` Michael S. Tsirkin
@ 2016-02-14 5:36 ` Xiao Guangrong
0 siblings, 0 replies; 51+ messages in thread
From: Xiao Guangrong @ 2016-02-14 5:36 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: ehabkost, kvm, gleb, mtosatti, qemu-devel, stefanha, imammedo,
pbonzini, dan.j.williams, rth
On 02/05/2016 12:20 AM, Michael S. Tsirkin wrote:
> On Wed, Jan 13, 2016 at 02:50:01AM +0800, Xiao Guangrong wrote:
>> Add nvdimm test, two tables are created which are NFIT and
>> SSDT
>>
>> Max memory size and multiple slots are needed to enable NVDIMM which
>> cause the primal SSDT table is also updated
>>
>> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>
> First don't include binary in patches directly please.
Okay.
>
>> ---
>> tests/acpi-test-data/pc/NFIT | Bin 0 -> 224 bytes
>> tests/acpi-test-data/pc/NFIT.bridge | Bin 0 -> 224 bytes
>> tests/acpi-test-data/pc/SSDT | Bin 2486 -> 2885 bytes
>> tests/acpi-test-data/pc/SSDT-NVDIMM | Bin 0 -> 134 bytes
>> tests/acpi-test-data/pc/SSDT-NVDIMM.bridge | Bin 0 -> 134 bytes
>> tests/acpi-test-data/pc/SSDT.bridge | Bin 4345 -> 4745 bytes
>> tests/acpi-test-data/q35/NFIT | Bin 0 -> 224 bytes
>> tests/acpi-test-data/q35/NFIT.bridge | Bin 0 -> 224 bytes
>> tests/acpi-test-data/q35/SSDT | Bin 691 -> 1090 bytes
>> tests/acpi-test-data/q35/SSDT-NVDIMM | Bin 0 -> 134 bytes
>> tests/acpi-test-data/q35/SSDT-NVDIMM.bridge | Bin 0 -> 134 bytes
>> tests/acpi-test-data/q35/SSDT.bridge | Bin 708 -> 1107 bytes
>> tests/bios-tables-test.c | 15 ++++++++++-----
>
> So let's add an new test, and there only test the NVDIMM tables and only
> with one configuration.
>
> Otherwise number of tables to maintain explodes.
>
> You will have to teach test to ignore everything
> that does not have NVDIMM in name for this test.
>
Okay, i will it as the single test and make it as a separate patchset.
^ permalink raw reply [flat|nested] 51+ messages in thread
* [Qemu-devel] [PATCH v2 03/11] acpi: add aml_create_field()
2016-01-12 18:49 [Qemu-devel] [PATCH v2 00/11] NVDIMM ACPI: introduce the framework of QEMU emulated Xiao Guangrong
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 01/11] tests: acpi: test multiple SSDT tables Xiao Guangrong
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 02/11] tests: acpi: test NVDIMM tables Xiao Guangrong
@ 2016-01-12 18:50 ` Xiao Guangrong
2016-02-08 10:47 ` Igor Mammedov
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 04/11] acpi: add aml_concatenate() Xiao Guangrong
` (9 subsequent siblings)
12 siblings, 1 reply; 51+ messages in thread
From: Xiao Guangrong @ 2016-01-12 18:50 UTC (permalink / raw)
To: pbonzini, imammedo
Cc: Xiao Guangrong, ehabkost, kvm, mst, gleb, mtosatti, qemu-devel,
stefanha, dan.j.williams, rth
It will be used by nvdimm acpi
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
hw/acpi/aml-build.c | 13 +++++++++++++
include/hw/acpi/aml-build.h | 1 +
2 files changed, 14 insertions(+)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 78e1290..97c9efb 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1001,6 +1001,19 @@ Aml *create_field_common(int opcode, Aml *srcbuf, Aml *index, const char *name)
return var;
}
+/* ACPI 1.0b: 16.2.5.2 Named Objects Encoding: DefCreateField */
+Aml *aml_create_field(Aml *srcbuf, Aml *index, Aml *len, const char *name)
+{
+ Aml *var = aml_alloc();
+ build_append_byte(var->buf, 0x5B); /* ExtOpPrefix */
+ build_append_byte(var->buf, 0x13); /* CreateFieldOp */
+ aml_append(var, srcbuf);
+ aml_append(var, index);
+ aml_append(var, len);
+ build_append_namestring(var->buf, "%s", name);
+ return var;
+}
+
/* ACPI 1.0b: 16.2.5.2 Named Objects Encoding: DefCreateDWordField */
Aml *aml_create_dword_field(Aml *srcbuf, Aml *index, const char *name)
{
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 6d6f705..e1ba534 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -346,6 +346,7 @@ Aml *aml_mutex(const char *name, uint8_t sync_level);
Aml *aml_acquire(Aml *mutex, uint16_t timeout);
Aml *aml_release(Aml *mutex);
Aml *aml_alias(const char *source_object, const char *alias_object);
+Aml *aml_create_field(Aml *srcbuf, Aml *index, Aml *len, const char *name);
Aml *aml_create_dword_field(Aml *srcbuf, Aml *index, const char *name);
Aml *aml_create_qword_field(Aml *srcbuf, Aml *index, const char *name);
Aml *aml_varpackage(uint32_t num_elements);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH v2 03/11] acpi: add aml_create_field()
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 03/11] acpi: add aml_create_field() Xiao Guangrong
@ 2016-02-08 10:47 ` Igor Mammedov
2016-02-14 5:41 ` Xiao Guangrong
0 siblings, 1 reply; 51+ messages in thread
From: Igor Mammedov @ 2016-02-08 10:47 UTC (permalink / raw)
To: Xiao Guangrong
Cc: ehabkost, kvm, mst, gleb, mtosatti, qemu-devel, stefanha,
pbonzini, dan.j.williams, rth
On Wed, 13 Jan 2016 02:50:02 +0800
Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> It will be used by nvdimm acpi
>
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
> hw/acpi/aml-build.c | 13 +++++++++++++
> include/hw/acpi/aml-build.h | 1 +
> 2 files changed, 14 insertions(+)
>
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 78e1290..97c9efb 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1001,6 +1001,19 @@ Aml *create_field_common(int opcode, Aml *srcbuf, Aml *index, const char *name)
> return var;
> }
>
> +/* ACPI 1.0b: 16.2.5.2 Named Objects Encoding: DefCreateField */
> +Aml *aml_create_field(Aml *srcbuf, Aml *index, Aml *len, const char *name)
to match spec more closely pls do following:
s/index/bit_index/
s/len/num_bits/
> +{
> + Aml *var = aml_alloc();
> + build_append_byte(var->buf, 0x5B); /* ExtOpPrefix */
> + build_append_byte(var->buf, 0x13); /* CreateFieldOp */
> + aml_append(var, srcbuf);
> + aml_append(var, index);
> + aml_append(var, len);
> + build_append_namestring(var->buf, "%s", name);
> + return var;
> +}
> +
> /* ACPI 1.0b: 16.2.5.2 Named Objects Encoding: DefCreateDWordField */
> Aml *aml_create_dword_field(Aml *srcbuf, Aml *index, const char *name)
> {
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 6d6f705..e1ba534 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -346,6 +346,7 @@ Aml *aml_mutex(const char *name, uint8_t sync_level);
> Aml *aml_acquire(Aml *mutex, uint16_t timeout);
> Aml *aml_release(Aml *mutex);
> Aml *aml_alias(const char *source_object, const char *alias_object);
> +Aml *aml_create_field(Aml *srcbuf, Aml *index, Aml *len, const char *name);
> Aml *aml_create_dword_field(Aml *srcbuf, Aml *index, const char *name);
> Aml *aml_create_qword_field(Aml *srcbuf, Aml *index, const char *name);
> Aml *aml_varpackage(uint32_t num_elements);
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH v2 03/11] acpi: add aml_create_field()
2016-02-08 10:47 ` Igor Mammedov
@ 2016-02-14 5:41 ` Xiao Guangrong
0 siblings, 0 replies; 51+ messages in thread
From: Xiao Guangrong @ 2016-02-14 5:41 UTC (permalink / raw)
To: Igor Mammedov
Cc: ehabkost, kvm, mst, gleb, mtosatti, qemu-devel, stefanha,
pbonzini, dan.j.williams, rth
On 02/08/2016 06:47 PM, Igor Mammedov wrote:
> On Wed, 13 Jan 2016 02:50:02 +0800
> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>
>> It will be used by nvdimm acpi
>>
>> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>> ---
>> hw/acpi/aml-build.c | 13 +++++++++++++
>> include/hw/acpi/aml-build.h | 1 +
>> 2 files changed, 14 insertions(+)
>>
>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> index 78e1290..97c9efb 100644
>> --- a/hw/acpi/aml-build.c
>> +++ b/hw/acpi/aml-build.c
>> @@ -1001,6 +1001,19 @@ Aml *create_field_common(int opcode, Aml *srcbuf, Aml *index, const char *name)
>> return var;
>> }
>>
>> +/* ACPI 1.0b: 16.2.5.2 Named Objects Encoding: DefCreateField */
>> +Aml *aml_create_field(Aml *srcbuf, Aml *index, Aml *len, const char *name)
> to match spec more closely pls do following:
> s/index/bit_index/
> s/len/num_bits/
>
Good to me, will do it in the next version.
^ permalink raw reply [flat|nested] 51+ messages in thread
* [Qemu-devel] [PATCH v2 04/11] acpi: add aml_concatenate()
2016-01-12 18:49 [Qemu-devel] [PATCH v2 00/11] NVDIMM ACPI: introduce the framework of QEMU emulated Xiao Guangrong
` (2 preceding siblings ...)
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 03/11] acpi: add aml_create_field() Xiao Guangrong
@ 2016-01-12 18:50 ` Xiao Guangrong
2016-02-08 10:51 ` Igor Mammedov
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 05/11] acpi: allow using object as offset for OperationRegion Xiao Guangrong
` (8 subsequent siblings)
12 siblings, 1 reply; 51+ messages in thread
From: Xiao Guangrong @ 2016-01-12 18:50 UTC (permalink / raw)
To: pbonzini, imammedo
Cc: Xiao Guangrong, ehabkost, kvm, mst, gleb, mtosatti, qemu-devel,
stefanha, dan.j.williams, rth
It will be used by nvdimm acpi
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
hw/acpi/aml-build.c | 14 ++++++++++++++
include/hw/acpi/aml-build.h | 1 +
2 files changed, 15 insertions(+)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 97c9efb..421dd84 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1440,6 +1440,20 @@ Aml *aml_alias(const char *source_object, const char *alias_object)
return var;
}
+/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefConcat */
+Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target)
+{
+ Aml *var = aml_opcode(0x73 /* ConcatOp */);
+ aml_append(var, source1);
+ aml_append(var, source2);
+
+ if (target) {
+ aml_append(var, target);
+ }
+
+ return var;
+}
+
void
build_header(GArray *linker, GArray *table_data,
AcpiTableHeader *h, const char *sig, int len, uint8_t rev,
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index e1ba534..4a5168a 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -354,6 +354,7 @@ Aml *aml_touuid(const char *uuid);
Aml *aml_unicode(const char *str);
Aml *aml_derefof(Aml *arg);
Aml *aml_sizeof(Aml *arg);
+Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target);
void
build_header(GArray *linker, GArray *table_data,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH v2 04/11] acpi: add aml_concatenate()
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 04/11] acpi: add aml_concatenate() Xiao Guangrong
@ 2016-02-08 10:51 ` Igor Mammedov
2016-02-14 5:52 ` Xiao Guangrong
0 siblings, 1 reply; 51+ messages in thread
From: Igor Mammedov @ 2016-02-08 10:51 UTC (permalink / raw)
To: Xiao Guangrong
Cc: ehabkost, kvm, mst, gleb, mtosatti, qemu-devel, stefanha,
pbonzini, dan.j.williams, rth
On Wed, 13 Jan 2016 02:50:03 +0800
Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> It will be used by nvdimm acpi
>
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
> hw/acpi/aml-build.c | 14 ++++++++++++++
> include/hw/acpi/aml-build.h | 1 +
> 2 files changed, 15 insertions(+)
>
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 97c9efb..421dd84 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1440,6 +1440,20 @@ Aml *aml_alias(const char *source_object, const char *alias_object)
> return var;
> }
>
> +/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefConcat */
> +Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target)
> +{
> + Aml *var = aml_opcode(0x73 /* ConcatOp */);
> + aml_append(var, source1);
> + aml_append(var, source2);
> +
> + if (target) {
> + aml_append(var, target);
> + }
target is not an optional, pls looks at aml_add and use
helper to make patch correct and smaller
> +
> + return var;
> +}
> +
> void
> build_header(GArray *linker, GArray *table_data,
> AcpiTableHeader *h, const char *sig, int len, uint8_t rev,
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index e1ba534..4a5168a 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -354,6 +354,7 @@ Aml *aml_touuid(const char *uuid);
> Aml *aml_unicode(const char *str);
> Aml *aml_derefof(Aml *arg);
> Aml *aml_sizeof(Aml *arg);
> +Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target);
>
> void
> build_header(GArray *linker, GArray *table_data,
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH v2 04/11] acpi: add aml_concatenate()
2016-02-08 10:51 ` Igor Mammedov
@ 2016-02-14 5:52 ` Xiao Guangrong
2016-02-14 5:55 ` Xiao Guangrong
0 siblings, 1 reply; 51+ messages in thread
From: Xiao Guangrong @ 2016-02-14 5:52 UTC (permalink / raw)
To: Igor Mammedov
Cc: ehabkost, kvm, mst, gleb, mtosatti, qemu-devel, stefanha,
pbonzini, dan.j.williams, rth
On 02/08/2016 06:51 PM, Igor Mammedov wrote:
> On Wed, 13 Jan 2016 02:50:03 +0800
> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>
>> It will be used by nvdimm acpi
>>
>> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>> ---
>> hw/acpi/aml-build.c | 14 ++++++++++++++
>> include/hw/acpi/aml-build.h | 1 +
>> 2 files changed, 15 insertions(+)
>>
>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> index 97c9efb..421dd84 100644
>> --- a/hw/acpi/aml-build.c
>> +++ b/hw/acpi/aml-build.c
>> @@ -1440,6 +1440,20 @@ Aml *aml_alias(const char *source_object, const char *alias_object)
>> return var;
>> }
>>
>> +/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefConcat */
>> +Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target)
>> +{
>> + Aml *var = aml_opcode(0x73 /* ConcatOp */);
>> + aml_append(var, source1);
>> + aml_append(var, source2);
>> +
>> + if (target) {
>> + aml_append(var, target);
>> + }
> target is not an optional, pls looks at aml_add and use
> helper to make patch correct and smaller
>
Indeed, i will change it to:
/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefConcat */
Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target)
{
return build_opcode_2arg_dst(0x73 /* ConcatOp */, arg1, arg2, dst);
}
Thank you, Igor!
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH v2 04/11] acpi: add aml_concatenate()
2016-02-14 5:52 ` Xiao Guangrong
@ 2016-02-14 5:55 ` Xiao Guangrong
2016-02-15 9:02 ` Igor Mammedov
0 siblings, 1 reply; 51+ messages in thread
From: Xiao Guangrong @ 2016-02-14 5:55 UTC (permalink / raw)
To: Igor Mammedov
Cc: ehabkost, kvm, mst, gleb, mtosatti, qemu-devel, stefanha,
pbonzini, dan.j.williams, rth
On 02/14/2016 01:52 PM, Xiao Guangrong wrote:
>
>
> On 02/08/2016 06:51 PM, Igor Mammedov wrote:
>> On Wed, 13 Jan 2016 02:50:03 +0800
>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>
>>> It will be used by nvdimm acpi
>>>
>>> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>>> ---
>>> hw/acpi/aml-build.c | 14 ++++++++++++++
>>> include/hw/acpi/aml-build.h | 1 +
>>> 2 files changed, 15 insertions(+)
>>>
>>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>>> index 97c9efb..421dd84 100644
>>> --- a/hw/acpi/aml-build.c
>>> +++ b/hw/acpi/aml-build.c
>>> @@ -1440,6 +1440,20 @@ Aml *aml_alias(const char *source_object, const char *alias_object)
>>> return var;
>>> }
>>>
>>> +/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefConcat */
>>> +Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target)
>>> +{
>>> + Aml *var = aml_opcode(0x73 /* ConcatOp */);
>>> + aml_append(var, source1);
>>> + aml_append(var, source2);
>>> +
>>> + if (target) {
>>> + aml_append(var, target);
>>> + }
>> target is not an optional, pls looks at aml_add and use
>> helper to make patch correct and smaller
>>
>
> Indeed, i will change it to:
>
> /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefConcat */
> Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target)
> {
> return build_opcode_2arg_dst(0x73 /* ConcatOp */, arg1, arg2, dst);
> }
>
And make a assert for @target to ensure it can not be NULL.
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH v2 04/11] acpi: add aml_concatenate()
2016-02-14 5:55 ` Xiao Guangrong
@ 2016-02-15 9:02 ` Igor Mammedov
2016-02-15 10:32 ` Xiao Guangrong
0 siblings, 1 reply; 51+ messages in thread
From: Igor Mammedov @ 2016-02-15 9:02 UTC (permalink / raw)
To: Xiao Guangrong
Cc: ehabkost, kvm, mst, gleb, mtosatti, qemu-devel, stefanha,
pbonzini, dan.j.williams, rth
On Sun, 14 Feb 2016 13:55:24 +0800
Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> On 02/14/2016 01:52 PM, Xiao Guangrong wrote:
> >
> >
> > On 02/08/2016 06:51 PM, Igor Mammedov wrote:
> >> On Wed, 13 Jan 2016 02:50:03 +0800
> >> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> >>
> >>> It will be used by nvdimm acpi
> >>>
> >>> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> >>> ---
> >>> hw/acpi/aml-build.c | 14 ++++++++++++++
> >>> include/hw/acpi/aml-build.h | 1 +
> >>> 2 files changed, 15 insertions(+)
> >>>
> >>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> >>> index 97c9efb..421dd84 100644
> >>> --- a/hw/acpi/aml-build.c
> >>> +++ b/hw/acpi/aml-build.c
> >>> @@ -1440,6 +1440,20 @@ Aml *aml_alias(const char *source_object, const char *alias_object)
> >>> return var;
> >>> }
> >>>
> >>> +/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefConcat */
> >>> +Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target)
> >>> +{
> >>> + Aml *var = aml_opcode(0x73 /* ConcatOp */);
> >>> + aml_append(var, source1);
> >>> + aml_append(var, source2);
> >>> +
> >>> + if (target) {
> >>> + aml_append(var, target);
> >>> + }
> >> target is not an optional, pls looks at aml_add and use
> >> helper to make patch correct and smaller
> >>
> >
> > Indeed, i will change it to:
> >
> > /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefConcat */
> > Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target)
> > {
> > return build_opcode_2arg_dst(0x73 /* ConcatOp */, arg1, arg2, dst);
> > }
> >
>
> And make a assert for @target to ensure it can not be NULL.
Don't do that, see build_opcode_2arg_dst(), target can be NULL
that allows to express implicit target like:
aml_add(a, aml_add(b, c, NULL), NULL)
where one doesn't need an intermediate variable to store result.
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH v2 04/11] acpi: add aml_concatenate()
2016-02-15 9:02 ` Igor Mammedov
@ 2016-02-15 10:32 ` Xiao Guangrong
0 siblings, 0 replies; 51+ messages in thread
From: Xiao Guangrong @ 2016-02-15 10:32 UTC (permalink / raw)
To: Igor Mammedov
Cc: ehabkost, kvm, mst, gleb, mtosatti, qemu-devel, stefanha,
pbonzini, dan.j.williams, rth
On 02/15/2016 05:02 PM, Igor Mammedov wrote:
> On Sun, 14 Feb 2016 13:55:24 +0800
> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>
>> On 02/14/2016 01:52 PM, Xiao Guangrong wrote:
>>>
>>>
>>> On 02/08/2016 06:51 PM, Igor Mammedov wrote:
>>>> On Wed, 13 Jan 2016 02:50:03 +0800
>>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>>>
>>>>> It will be used by nvdimm acpi
>>>>>
>>>>> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>>>>> ---
>>>>> hw/acpi/aml-build.c | 14 ++++++++++++++
>>>>> include/hw/acpi/aml-build.h | 1 +
>>>>> 2 files changed, 15 insertions(+)
>>>>>
>>>>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>>>>> index 97c9efb..421dd84 100644
>>>>> --- a/hw/acpi/aml-build.c
>>>>> +++ b/hw/acpi/aml-build.c
>>>>> @@ -1440,6 +1440,20 @@ Aml *aml_alias(const char *source_object, const char *alias_object)
>>>>> return var;
>>>>> }
>>>>>
>>>>> +/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefConcat */
>>>>> +Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target)
>>>>> +{
>>>>> + Aml *var = aml_opcode(0x73 /* ConcatOp */);
>>>>> + aml_append(var, source1);
>>>>> + aml_append(var, source2);
>>>>> +
>>>>> + if (target) {
>>>>> + aml_append(var, target);
>>>>> + }
>>>> target is not an optional, pls looks at aml_add and use
>>>> helper to make patch correct and smaller
>>>>
>>>
>>> Indeed, i will change it to:
>>>
>>> /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefConcat */
>>> Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target)
>>> {
>>> return build_opcode_2arg_dst(0x73 /* ConcatOp */, arg1, arg2, dst);
>>> }
>>>
>>
>> And make a assert for @target to ensure it can not be NULL.
> Don't do that, see build_opcode_2arg_dst(), target can be NULL
> that allows to express implicit target like:
>
> aml_add(a, aml_add(b, c, NULL), NULL)
>
> where one doesn't need an intermediate variable to store result.
>
Yes, you are right, i re-checked the spec and it said that "Source2 is
concatenated to Source1 and the result data is optionally stored into Result" so that
@target can be eliminated.
^ permalink raw reply [flat|nested] 51+ messages in thread
* [Qemu-devel] [PATCH v2 05/11] acpi: allow using object as offset for OperationRegion
2016-01-12 18:49 [Qemu-devel] [PATCH v2 00/11] NVDIMM ACPI: introduce the framework of QEMU emulated Xiao Guangrong
` (3 preceding siblings ...)
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 04/11] acpi: add aml_concatenate() Xiao Guangrong
@ 2016-01-12 18:50 ` Xiao Guangrong
2016-02-08 10:57 ` Igor Mammedov
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI Xiao Guangrong
` (7 subsequent siblings)
12 siblings, 1 reply; 51+ messages in thread
From: Xiao Guangrong @ 2016-01-12 18:50 UTC (permalink / raw)
To: pbonzini, imammedo
Cc: Xiao Guangrong, ehabkost, kvm, mst, gleb, mtosatti, qemu-devel,
stefanha, dan.j.williams, rth
Extend aml_operation_region() to use object as offset
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
hw/acpi/aml-build.c | 4 ++--
hw/i386/acpi-build.c | 31 ++++++++++++++++---------------
include/hw/acpi/aml-build.h | 2 +-
3 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 421dd84..208f22e 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -946,14 +946,14 @@ Aml *aml_package(uint8_t num_elements)
/* ACPI 1.0b: 16.2.5.2 Named Objects Encoding: DefOpRegion */
Aml *aml_operation_region(const char *name, AmlRegionSpace rs,
- uint32_t offset, uint32_t len)
+ Aml *offset, uint32_t len)
{
Aml *var = aml_alloc();
build_append_byte(var->buf, 0x5B); /* ExtOpPrefix */
build_append_byte(var->buf, 0x80); /* OpRegionOp */
build_append_namestring(var->buf, "%s", name);
build_append_byte(var->buf, rs);
- build_append_int(var->buf, offset);
+ aml_append(var, offset);
build_append_int(var->buf, len);
return var;
}
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 78758e2..1ca044f 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -993,7 +993,7 @@ static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus,
aml_append(sb_scope, dev);
/* declare CPU hotplug MMIO region and PRS field to access it */
aml_append(sb_scope, aml_operation_region(
- "PRST", AML_SYSTEM_IO, pm->cpu_hp_io_base, pm->cpu_hp_io_len));
+ "PRST", AML_SYSTEM_IO, aml_int(pm->cpu_hp_io_base), pm->cpu_hp_io_len));
field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
aml_append(field, aml_named_field("PRS", 256));
aml_append(sb_scope, field);
@@ -1078,7 +1078,7 @@ static void build_memory_devices(Aml *sb_scope, int nr_mem,
aml_append(scope, aml_operation_region(
MEMORY_HOTPLUG_IO_REGION, AML_SYSTEM_IO,
- io_base, io_len)
+ aml_int(io_base), io_len)
);
field = aml_field(MEMORY_HOTPLUG_IO_REGION, AML_DWORD_ACC,
@@ -1192,7 +1192,8 @@ static void build_hpet_aml(Aml *table)
aml_append(dev, aml_name_decl("_UID", zero));
aml_append(dev,
- aml_operation_region("HPTM", AML_SYSTEM_MEMORY, HPET_BASE, HPET_LEN));
+ aml_operation_region("HPTM", AML_SYSTEM_MEMORY, aml_int(HPET_BASE),
+ HPET_LEN));
field = aml_field("HPTM", AML_DWORD_ACC, AML_LOCK, AML_PRESERVE);
aml_append(field, aml_named_field("VEND", 32));
aml_append(field, aml_named_field("PRD", 32));
@@ -1430,7 +1431,7 @@ static void build_dbg_aml(Aml *table)
Aml *idx = aml_local(2);
aml_append(scope,
- aml_operation_region("DBG", AML_SYSTEM_IO, 0x0402, 0x01));
+ aml_operation_region("DBG", AML_SYSTEM_IO, aml_int(0x0402), 0x01));
field = aml_field("DBG", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
aml_append(field, aml_named_field("DBGB", 8));
aml_append(scope, field);
@@ -1770,10 +1771,10 @@ static void build_q35_isa_bridge(Aml *table)
/* ICH9 PCI to ISA irq remapping */
aml_append(dev, aml_operation_region("PIRQ", AML_PCI_CONFIG,
- 0x60, 0x0C));
+ aml_int(0x60), 0x0C));
aml_append(dev, aml_operation_region("LPCD", AML_PCI_CONFIG,
- 0x80, 0x02));
+ aml_int(0x80), 0x02));
field = aml_field("LPCD", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
aml_append(field, aml_named_field("COMA", 3));
aml_append(field, aml_reserved_field(1));
@@ -1785,7 +1786,7 @@ static void build_q35_isa_bridge(Aml *table)
aml_append(dev, field);
aml_append(dev, aml_operation_region("LPCE", AML_PCI_CONFIG,
- 0x82, 0x02));
+ aml_int(0x82), 0x02));
/* enable bits */
field = aml_field("LPCE", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
aml_append(field, aml_named_field("CAEN", 1));
@@ -1808,7 +1809,7 @@ static void build_piix4_pm(Aml *table)
aml_append(dev, aml_name_decl("_ADR", aml_int(0x00010003)));
aml_append(dev, aml_operation_region("P13C", AML_PCI_CONFIG,
- 0x00, 0xff));
+ aml_int(0x00), 0xff));
aml_append(scope, dev);
aml_append(table, scope);
}
@@ -1825,7 +1826,7 @@ static void build_piix4_isa_bridge(Aml *table)
/* PIIX PCI to ISA irq remapping */
aml_append(dev, aml_operation_region("P40C", AML_PCI_CONFIG,
- 0x60, 0x04));
+ aml_int(0x60), 0x04));
/* enable bits */
field = aml_field("^PX13.P13C", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
/* Offset(0x5f),, 7, */
@@ -1854,20 +1855,20 @@ static void build_piix4_pci_hotplug(Aml *table)
scope = aml_scope("_SB.PCI0");
aml_append(scope,
- aml_operation_region("PCST", AML_SYSTEM_IO, 0xae00, 0x08));
+ aml_operation_region("PCST", AML_SYSTEM_IO, aml_int(0xae00), 0x08));
field = aml_field("PCST", AML_DWORD_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS);
aml_append(field, aml_named_field("PCIU", 32));
aml_append(field, aml_named_field("PCID", 32));
aml_append(scope, field);
aml_append(scope,
- aml_operation_region("SEJ", AML_SYSTEM_IO, 0xae08, 0x04));
+ aml_operation_region("SEJ", AML_SYSTEM_IO, aml_int(0xae08), 0x04));
field = aml_field("SEJ", AML_DWORD_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS);
aml_append(field, aml_named_field("B0EJ", 32));
aml_append(scope, field);
aml_append(scope,
- aml_operation_region("BNMR", AML_SYSTEM_IO, 0xae10, 0x04));
+ aml_operation_region("BNMR", AML_SYSTEM_IO, aml_int(0xae10), 0x04));
field = aml_field("BNMR", AML_DWORD_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS);
aml_append(field, aml_named_field("BNUM", 32));
aml_append(scope, field);
@@ -2133,7 +2134,7 @@ build_ssdt(GArray *table_data, GArray *linker,
aml_append(dev, aml_name_decl("_CRS", crs));
aml_append(dev, aml_operation_region("PEOR", AML_SYSTEM_IO,
- misc->pvpanic_port, 1));
+ aml_int(misc->pvpanic_port), 1));
field = aml_field("PEOR", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
aml_append(field, aml_named_field("PEPT", 8));
aml_append(dev, field);
@@ -2455,9 +2456,9 @@ build_dsdt(GArray *table_data, GArray *linker,
} else {
sb_scope = aml_scope("_SB");
aml_append(sb_scope,
- aml_operation_region("PCST", AML_SYSTEM_IO, 0xae00, 0x0c));
+ aml_operation_region("PCST", AML_SYSTEM_IO, aml_int(0xae00), 0x0c));
aml_append(sb_scope,
- aml_operation_region("PCSB", AML_SYSTEM_IO, 0xae0c, 0x01));
+ aml_operation_region("PCSB", AML_SYSTEM_IO, aml_int(0xae0c), 0x01));
field = aml_field("PCSB", AML_ANY_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS);
aml_append(field, aml_named_field("PCIB", 8));
aml_append(sb_scope, field);
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 4a5168a..7c8db8f 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -287,7 +287,7 @@ Aml *aml_interrupt(AmlConsumerAndProducer con_and_pro,
Aml *aml_io(AmlIODecode dec, uint16_t min_base, uint16_t max_base,
uint8_t aln, uint8_t len);
Aml *aml_operation_region(const char *name, AmlRegionSpace rs,
- uint32_t offset, uint32_t len);
+ Aml *offset, uint32_t len);
Aml *aml_irq_no_flags(uint8_t irq);
Aml *aml_named_field(const char *name, unsigned length);
Aml *aml_reserved_field(unsigned length);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH v2 05/11] acpi: allow using object as offset for OperationRegion
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 05/11] acpi: allow using object as offset for OperationRegion Xiao Guangrong
@ 2016-02-08 10:57 ` Igor Mammedov
0 siblings, 0 replies; 51+ messages in thread
From: Igor Mammedov @ 2016-02-08 10:57 UTC (permalink / raw)
To: Xiao Guangrong
Cc: ehabkost, kvm, mst, gleb, mtosatti, qemu-devel, stefanha,
pbonzini, dan.j.williams, rth
On Wed, 13 Jan 2016 02:50:04 +0800
Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> Extend aml_operation_region() to use object as offset
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
> hw/acpi/aml-build.c | 4 ++--
> hw/i386/acpi-build.c | 31 ++++++++++++++++---------------
> include/hw/acpi/aml-build.h | 2 +-
> 3 files changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 421dd84..208f22e 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -946,14 +946,14 @@ Aml *aml_package(uint8_t num_elements)
>
> /* ACPI 1.0b: 16.2.5.2 Named Objects Encoding: DefOpRegion */
> Aml *aml_operation_region(const char *name, AmlRegionSpace rs,
> - uint32_t offset, uint32_t len)
> + Aml *offset, uint32_t len)
> {
> Aml *var = aml_alloc();
> build_append_byte(var->buf, 0x5B); /* ExtOpPrefix */
> build_append_byte(var->buf, 0x80); /* OpRegionOp */
> build_append_namestring(var->buf, "%s", name);
> build_append_byte(var->buf, rs);
> - build_append_int(var->buf, offset);
> + aml_append(var, offset);
> build_append_int(var->buf, len);
> return var;
> }
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 78758e2..1ca044f 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -993,7 +993,7 @@ static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus,
> aml_append(sb_scope, dev);
> /* declare CPU hotplug MMIO region and PRS field to access it */
> aml_append(sb_scope, aml_operation_region(
> - "PRST", AML_SYSTEM_IO, pm->cpu_hp_io_base, pm->cpu_hp_io_len));
> + "PRST", AML_SYSTEM_IO, aml_int(pm->cpu_hp_io_base), pm->cpu_hp_io_len));
> field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
> aml_append(field, aml_named_field("PRS", 256));
> aml_append(sb_scope, field);
> @@ -1078,7 +1078,7 @@ static void build_memory_devices(Aml *sb_scope, int nr_mem,
>
> aml_append(scope, aml_operation_region(
> MEMORY_HOTPLUG_IO_REGION, AML_SYSTEM_IO,
> - io_base, io_len)
> + aml_int(io_base), io_len)
> );
>
> field = aml_field(MEMORY_HOTPLUG_IO_REGION, AML_DWORD_ACC,
> @@ -1192,7 +1192,8 @@ static void build_hpet_aml(Aml *table)
> aml_append(dev, aml_name_decl("_UID", zero));
>
> aml_append(dev,
> - aml_operation_region("HPTM", AML_SYSTEM_MEMORY, HPET_BASE, HPET_LEN));
> + aml_operation_region("HPTM", AML_SYSTEM_MEMORY, aml_int(HPET_BASE),
> + HPET_LEN));
> field = aml_field("HPTM", AML_DWORD_ACC, AML_LOCK, AML_PRESERVE);
> aml_append(field, aml_named_field("VEND", 32));
> aml_append(field, aml_named_field("PRD", 32));
> @@ -1430,7 +1431,7 @@ static void build_dbg_aml(Aml *table)
> Aml *idx = aml_local(2);
>
> aml_append(scope,
> - aml_operation_region("DBG", AML_SYSTEM_IO, 0x0402, 0x01));
> + aml_operation_region("DBG", AML_SYSTEM_IO, aml_int(0x0402), 0x01));
> field = aml_field("DBG", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
> aml_append(field, aml_named_field("DBGB", 8));
> aml_append(scope, field);
> @@ -1770,10 +1771,10 @@ static void build_q35_isa_bridge(Aml *table)
>
> /* ICH9 PCI to ISA irq remapping */
> aml_append(dev, aml_operation_region("PIRQ", AML_PCI_CONFIG,
> - 0x60, 0x0C));
> + aml_int(0x60), 0x0C));
>
> aml_append(dev, aml_operation_region("LPCD", AML_PCI_CONFIG,
> - 0x80, 0x02));
> + aml_int(0x80), 0x02));
> field = aml_field("LPCD", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
> aml_append(field, aml_named_field("COMA", 3));
> aml_append(field, aml_reserved_field(1));
> @@ -1785,7 +1786,7 @@ static void build_q35_isa_bridge(Aml *table)
> aml_append(dev, field);
>
> aml_append(dev, aml_operation_region("LPCE", AML_PCI_CONFIG,
> - 0x82, 0x02));
> + aml_int(0x82), 0x02));
> /* enable bits */
> field = aml_field("LPCE", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
> aml_append(field, aml_named_field("CAEN", 1));
> @@ -1808,7 +1809,7 @@ static void build_piix4_pm(Aml *table)
> aml_append(dev, aml_name_decl("_ADR", aml_int(0x00010003)));
>
> aml_append(dev, aml_operation_region("P13C", AML_PCI_CONFIG,
> - 0x00, 0xff));
> + aml_int(0x00), 0xff));
> aml_append(scope, dev);
> aml_append(table, scope);
> }
> @@ -1825,7 +1826,7 @@ static void build_piix4_isa_bridge(Aml *table)
>
> /* PIIX PCI to ISA irq remapping */
> aml_append(dev, aml_operation_region("P40C", AML_PCI_CONFIG,
> - 0x60, 0x04));
> + aml_int(0x60), 0x04));
> /* enable bits */
> field = aml_field("^PX13.P13C", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
> /* Offset(0x5f),, 7, */
> @@ -1854,20 +1855,20 @@ static void build_piix4_pci_hotplug(Aml *table)
> scope = aml_scope("_SB.PCI0");
>
> aml_append(scope,
> - aml_operation_region("PCST", AML_SYSTEM_IO, 0xae00, 0x08));
> + aml_operation_region("PCST", AML_SYSTEM_IO, aml_int(0xae00), 0x08));
> field = aml_field("PCST", AML_DWORD_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS);
> aml_append(field, aml_named_field("PCIU", 32));
> aml_append(field, aml_named_field("PCID", 32));
> aml_append(scope, field);
>
> aml_append(scope,
> - aml_operation_region("SEJ", AML_SYSTEM_IO, 0xae08, 0x04));
> + aml_operation_region("SEJ", AML_SYSTEM_IO, aml_int(0xae08), 0x04));
> field = aml_field("SEJ", AML_DWORD_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS);
> aml_append(field, aml_named_field("B0EJ", 32));
> aml_append(scope, field);
>
> aml_append(scope,
> - aml_operation_region("BNMR", AML_SYSTEM_IO, 0xae10, 0x04));
> + aml_operation_region("BNMR", AML_SYSTEM_IO, aml_int(0xae10), 0x04));
> field = aml_field("BNMR", AML_DWORD_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS);
> aml_append(field, aml_named_field("BNUM", 32));
> aml_append(scope, field);
> @@ -2133,7 +2134,7 @@ build_ssdt(GArray *table_data, GArray *linker,
> aml_append(dev, aml_name_decl("_CRS", crs));
>
> aml_append(dev, aml_operation_region("PEOR", AML_SYSTEM_IO,
> - misc->pvpanic_port, 1));
> + aml_int(misc->pvpanic_port), 1));
> field = aml_field("PEOR", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
> aml_append(field, aml_named_field("PEPT", 8));
> aml_append(dev, field);
> @@ -2455,9 +2456,9 @@ build_dsdt(GArray *table_data, GArray *linker,
> } else {
> sb_scope = aml_scope("_SB");
> aml_append(sb_scope,
> - aml_operation_region("PCST", AML_SYSTEM_IO, 0xae00, 0x0c));
> + aml_operation_region("PCST", AML_SYSTEM_IO, aml_int(0xae00), 0x0c));
> aml_append(sb_scope,
> - aml_operation_region("PCSB", AML_SYSTEM_IO, 0xae0c, 0x01));
> + aml_operation_region("PCSB", AML_SYSTEM_IO, aml_int(0xae0c), 0x01));
> field = aml_field("PCSB", AML_ANY_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS);
> aml_append(field, aml_named_field("PCIB", 8));
> aml_append(sb_scope, field);
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 4a5168a..7c8db8f 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -287,7 +287,7 @@ Aml *aml_interrupt(AmlConsumerAndProducer con_and_pro,
> Aml *aml_io(AmlIODecode dec, uint16_t min_base, uint16_t max_base,
> uint8_t aln, uint8_t len);
> Aml *aml_operation_region(const char *name, AmlRegionSpace rs,
> - uint32_t offset, uint32_t len);
> + Aml *offset, uint32_t len);
> Aml *aml_irq_no_flags(uint8_t irq);
> Aml *aml_named_field(const char *name, unsigned length);
> Aml *aml_reserved_field(unsigned length);
^ permalink raw reply [flat|nested] 51+ messages in thread
* [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-01-12 18:49 [Qemu-devel] [PATCH v2 00/11] NVDIMM ACPI: introduce the framework of QEMU emulated Xiao Guangrong
` (4 preceding siblings ...)
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 05/11] acpi: allow using object as offset for OperationRegion Xiao Guangrong
@ 2016-01-12 18:50 ` Xiao Guangrong
2016-02-04 16:22 ` Michael S. Tsirkin
2016-02-08 11:03 ` Igor Mammedov
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 07/11] nvdimm acpi: introduce patched dsm memory Xiao Guangrong
` (6 subsequent siblings)
12 siblings, 2 replies; 51+ messages in thread
From: Xiao Guangrong @ 2016-01-12 18:50 UTC (permalink / raw)
To: pbonzini, imammedo
Cc: Xiao Guangrong, ehabkost, kvm, mst, gleb, mtosatti, qemu-devel,
stefanha, dan.j.williams, rth
32 bits IO port starting from 0x0a18 in guest is reserved for NVDIMM
ACPI emulation. The table, NVDIMM_DSM_MEM_FILE, will be patched into
NVDIMM ACPI binary code
OSPM uses this port to tell QEMU the final address of the DSM memory
and notify QEMU to emulate the DSM method
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
hw/acpi/Makefile.objs | 2 +-
hw/acpi/nvdimm.c | 35 +++++++++++++++++++++++++++++++++++
hw/i386/acpi-build.c | 10 +---------
hw/i386/pc.c | 8 +++++---
hw/i386/pc_piix.c | 5 +++++
hw/i386/pc_q35.c | 8 +++++++-
include/hw/i386/pc.h | 5 ++++-
include/hw/mem/nvdimm.h | 28 +++++++++++++++++++++++++++-
8 files changed, 85 insertions(+), 16 deletions(-)
diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
index f3ade9a..faee86c 100644
--- a/hw/acpi/Makefile.objs
+++ b/hw/acpi/Makefile.objs
@@ -2,7 +2,7 @@ common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o
common-obj-$(CONFIG_ACPI_X86_ICH) += ich9.o tco.o
common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o cpu_hotplug_acpi_table.o
common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o memory_hotplug_acpi_table.o
-common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
+obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
common-obj-$(CONFIG_ACPI) += acpi_interface.o
common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
common-obj-$(CONFIG_ACPI) += aml-build.o
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index df1b176..256cedd 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -28,6 +28,7 @@
#include "hw/acpi/acpi.h"
#include "hw/acpi/aml-build.h"
+#include "hw/nvram/fw_cfg.h"
#include "hw/mem/nvdimm.h"
static int nvdimm_plugged_device_list(Object *obj, void *opaque)
@@ -369,6 +370,40 @@ static void nvdimm_build_nfit(GSList *device_list, GArray *table_offsets,
g_array_free(structures, true);
}
+static uint64_t
+nvdimm_dsm_read(void *opaque, hwaddr addr, unsigned size)
+{
+ return 0;
+}
+
+static void
+nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
+{
+}
+
+static const MemoryRegionOps nvdimm_dsm_ops = {
+ .read = nvdimm_dsm_read,
+ .write = nvdimm_dsm_write,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+ .valid = {
+ .min_access_size = 4,
+ .max_access_size = 4,
+ },
+};
+
+void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
+ FWCfgState *fw_cfg, Object *owner)
+{
+ memory_region_init_io(&state->io_mr, owner, &nvdimm_dsm_ops, state,
+ "nvdimm-acpi-io", NVDIMM_ACPI_IO_LEN);
+ memory_region_add_subregion(io, NVDIMM_ACPI_IO_BASE, &state->io_mr);
+
+ state->dsm_mem = g_array_new(false, true /* clear */, 1);
+ acpi_data_push(state->dsm_mem, TARGET_PAGE_SIZE);
+ fw_cfg_add_file(fw_cfg, NVDIMM_DSM_MEM_FILE, state->dsm_mem->data,
+ state->dsm_mem->len);
+}
+
#define NVDIMM_COMMON_DSM "NCAL"
static void nvdimm_build_common_dsm(Aml *dev)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 1ca044f..c68cfb8 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -39,7 +39,6 @@
#include "hw/loader.h"
#include "hw/isa/isa.h"
#include "hw/acpi/memory_hotplug.h"
-#include "hw/mem/nvdimm.h"
#include "sysemu/tpm.h"
#include "hw/acpi/tpm.h"
#include "sysemu/tpm_backend.h"
@@ -2602,13 +2601,6 @@ static bool acpi_has_iommu(void)
return intel_iommu && !ambiguous;
}
-static bool acpi_has_nvdimm(void)
-{
- PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
-
- return pcms->nvdimm;
-}
-
static
void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
{
@@ -2692,7 +2684,7 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
build_dmar_q35(tables_blob, tables->linker);
}
- if (acpi_has_nvdimm()) {
+ if (guest_info->has_nvdimm) {
nvdimm_build_acpi(table_offsets, tables_blob, tables->linker);
}
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c36b8cf..397de28 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1228,6 +1228,8 @@ PcGuestInfo *pc_guest_info_init(PCMachineState *pcms)
}
}
+ guest_info->has_nvdimm = pcms->acpi_nvdimm_state.is_enabled;
+
guest_info_state->machine_done.notify = pc_guest_info_machine_done;
qemu_add_machine_init_done_notifier(&guest_info_state->machine_done);
return guest_info;
@@ -1877,14 +1879,14 @@ static bool pc_machine_get_nvdimm(Object *obj, Error **errp)
{
PCMachineState *pcms = PC_MACHINE(obj);
- return pcms->nvdimm;
+ return pcms->acpi_nvdimm_state.is_enabled;
}
static void pc_machine_set_nvdimm(Object *obj, bool value, Error **errp)
{
PCMachineState *pcms = PC_MACHINE(obj);
- pcms->nvdimm = value;
+ pcms->acpi_nvdimm_state.is_enabled = value;
}
static void pc_machine_initfn(Object *obj)
@@ -1923,7 +1925,7 @@ static void pc_machine_initfn(Object *obj)
&error_abort);
/* nvdimm is disabled on default. */
- pcms->nvdimm = false;
+ pcms->acpi_nvdimm_state.is_enabled = false;
object_property_add_bool(obj, PC_MACHINE_NVDIMM, pc_machine_get_nvdimm,
pc_machine_set_nvdimm, &error_abort);
}
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 438cdae..2fee478 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -281,6 +281,11 @@ static void pc_init1(MachineState *machine,
if (pcmc->pci_enabled) {
pc_pci_device_init(pci_bus);
}
+
+ if (guest_info->has_nvdimm) {
+ nvdimm_init_acpi_state(&pcms->acpi_nvdimm_state, system_io,
+ guest_info->fw_cfg, OBJECT(pcms));
+ }
}
/* Looking for a pc_compat_2_4() function? It doesn't exist.
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 412b3cd..c9334d5 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -60,6 +60,7 @@ static void pc_q35_init(MachineState *machine)
PCIDevice *lpc;
BusState *idebus[MAX_SATA_PORTS];
ISADevice *rtc_state;
+ MemoryRegion *system_io = get_system_io();
MemoryRegion *pci_memory;
MemoryRegion *rom_memory;
MemoryRegion *ram_memory;
@@ -176,7 +177,7 @@ static void pc_q35_init(MachineState *machine)
q35_host->mch.ram_memory = ram_memory;
q35_host->mch.pci_address_space = pci_memory;
q35_host->mch.system_memory = get_system_memory();
- q35_host->mch.address_space_io = get_system_io();
+ q35_host->mch.address_space_io = system_io;
q35_host->mch.below_4g_mem_size = pcms->below_4g_mem_size;
q35_host->mch.above_4g_mem_size = pcms->above_4g_mem_size;
/* pci */
@@ -267,6 +268,11 @@ static void pc_q35_init(MachineState *machine)
if (pcmc->pci_enabled) {
pc_pci_device_init(host_bus);
}
+
+ if (guest_info->has_nvdimm) {
+ nvdimm_init_acpi_state(&pcms->acpi_nvdimm_state, system_io,
+ guest_info->fw_cfg, OBJECT(pcms));
+ }
}
/* Looking for a pc_compat_2_4() function? It doesn't exist.
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 8122229..362ddc4 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -17,6 +17,7 @@
#include "hw/boards.h"
#include "hw/compat.h"
#include "hw/mem/pc-dimm.h"
+#include "hw/mem/nvdimm.h"
#define HPET_INTCAP "hpet-intcap"
@@ -55,7 +56,8 @@ struct PCMachineState {
uint64_t max_ram_below_4g;
OnOffAuto vmport;
OnOffAuto smm;
- bool nvdimm;
+
+ AcpiNVDIMMState acpi_nvdimm_state;
/* RAM information (sizes, addresses, configuration): */
ram_addr_t below_4g_mem_size, above_4g_mem_size;
@@ -161,6 +163,7 @@ struct PcGuestInfo {
bool has_acpi_build;
bool has_reserved_memory;
bool rsdp_in_ram;
+ bool has_nvdimm;
};
/* parallel.c */
diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
index 49183c1..634c60b 100644
--- a/include/hw/mem/nvdimm.h
+++ b/include/hw/mem/nvdimm.h
@@ -25,8 +25,34 @@
#include "hw/mem/pc-dimm.h"
-#define TYPE_NVDIMM "nvdimm"
+#define TYPE_NVDIMM "nvdimm"
+#define NVDIMM_DSM_MEM_FILE "etc/acpi/nvdimm-mem"
+
+/*
+ * 32 bits IO port starting from 0x0a18 in guest is reserved for
+ * NVDIMM ACPI emulation.
+ */
+#define NVDIMM_ACPI_IO_BASE 0x0a18
+#define NVDIMM_ACPI_IO_LEN 4
+
+/*
+ * AcpiNVDIMMState:
+ * @is_enabled: detect if NVDIMM support is enabled.
+ *
+ * @dsm_mem: the data of the fw_cfg file NVDIMM_DSM_MEM_FILE.
+ * @io_mr: the IO region used by OSPM to transfer control to QEMU.
+ */
+struct AcpiNVDIMMState {
+ bool is_enabled;
+
+ GArray *dsm_mem;
+ MemoryRegion io_mr;
+};
+typedef struct AcpiNVDIMMState AcpiNVDIMMState;
+
+void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
+ FWCfgState *fw_cfg, Object *owner);
void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
GArray *linker);
#endif
--
1.8.3.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI Xiao Guangrong
@ 2016-02-04 16:22 ` Michael S. Tsirkin
2016-02-08 11:03 ` Igor Mammedov
1 sibling, 0 replies; 51+ messages in thread
From: Michael S. Tsirkin @ 2016-02-04 16:22 UTC (permalink / raw)
To: Xiao Guangrong
Cc: ehabkost, kvm, gleb, mtosatti, qemu-devel, stefanha, imammedo,
pbonzini, dan.j.williams, rth
On Wed, Jan 13, 2016 at 02:50:05AM +0800, Xiao Guangrong wrote:
> 32 bits IO port starting from 0x0a18 in guest is reserved for NVDIMM
> ACPI emulation. The table, NVDIMM_DSM_MEM_FILE, will be patched into
> NVDIMM ACPI binary code
>
> OSPM uses this port to tell QEMU the final address of the DSM memory
> and notify QEMU to emulate the DSM method
>
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
This will have to be rebased now that guest info is gone.
> ---
> hw/acpi/Makefile.objs | 2 +-
> hw/acpi/nvdimm.c | 35 +++++++++++++++++++++++++++++++++++
> hw/i386/acpi-build.c | 10 +---------
> hw/i386/pc.c | 8 +++++---
> hw/i386/pc_piix.c | 5 +++++
> hw/i386/pc_q35.c | 8 +++++++-
> include/hw/i386/pc.h | 5 ++++-
> include/hw/mem/nvdimm.h | 28 +++++++++++++++++++++++++++-
> 8 files changed, 85 insertions(+), 16 deletions(-)
>
> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> index f3ade9a..faee86c 100644
> --- a/hw/acpi/Makefile.objs
> +++ b/hw/acpi/Makefile.objs
> @@ -2,7 +2,7 @@ common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o
> common-obj-$(CONFIG_ACPI_X86_ICH) += ich9.o tco.o
> common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o cpu_hotplug_acpi_table.o
> common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o memory_hotplug_acpi_table.o
> -common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
> +obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
> common-obj-$(CONFIG_ACPI) += acpi_interface.o
> common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
> common-obj-$(CONFIG_ACPI) += aml-build.o
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index df1b176..256cedd 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -28,6 +28,7 @@
>
> #include "hw/acpi/acpi.h"
> #include "hw/acpi/aml-build.h"
> +#include "hw/nvram/fw_cfg.h"
> #include "hw/mem/nvdimm.h"
>
> static int nvdimm_plugged_device_list(Object *obj, void *opaque)
> @@ -369,6 +370,40 @@ static void nvdimm_build_nfit(GSList *device_list, GArray *table_offsets,
> g_array_free(structures, true);
> }
>
> +static uint64_t
> +nvdimm_dsm_read(void *opaque, hwaddr addr, unsigned size)
> +{
> + return 0;
> +}
> +
> +static void
> +nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
> +{
> +}
> +
> +static const MemoryRegionOps nvdimm_dsm_ops = {
> + .read = nvdimm_dsm_read,
> + .write = nvdimm_dsm_write,
> + .endianness = DEVICE_LITTLE_ENDIAN,
> + .valid = {
> + .min_access_size = 4,
> + .max_access_size = 4,
> + },
> +};
> +
> +void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
> + FWCfgState *fw_cfg, Object *owner)
> +{
> + memory_region_init_io(&state->io_mr, owner, &nvdimm_dsm_ops, state,
> + "nvdimm-acpi-io", NVDIMM_ACPI_IO_LEN);
> + memory_region_add_subregion(io, NVDIMM_ACPI_IO_BASE, &state->io_mr);
> +
> + state->dsm_mem = g_array_new(false, true /* clear */, 1);
> + acpi_data_push(state->dsm_mem, TARGET_PAGE_SIZE);
> + fw_cfg_add_file(fw_cfg, NVDIMM_DSM_MEM_FILE, state->dsm_mem->data,
> + state->dsm_mem->len);
> +}
> +
> #define NVDIMM_COMMON_DSM "NCAL"
>
> static void nvdimm_build_common_dsm(Aml *dev)
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 1ca044f..c68cfb8 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -39,7 +39,6 @@
> #include "hw/loader.h"
> #include "hw/isa/isa.h"
> #include "hw/acpi/memory_hotplug.h"
> -#include "hw/mem/nvdimm.h"
> #include "sysemu/tpm.h"
> #include "hw/acpi/tpm.h"
> #include "sysemu/tpm_backend.h"
> @@ -2602,13 +2601,6 @@ static bool acpi_has_iommu(void)
> return intel_iommu && !ambiguous;
> }
>
> -static bool acpi_has_nvdimm(void)
> -{
> - PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
> -
> - return pcms->nvdimm;
> -}
> -
> static
> void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
> {
> @@ -2692,7 +2684,7 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
> build_dmar_q35(tables_blob, tables->linker);
> }
>
> - if (acpi_has_nvdimm()) {
> + if (guest_info->has_nvdimm) {
> nvdimm_build_acpi(table_offsets, tables_blob, tables->linker);
> }
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index c36b8cf..397de28 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1228,6 +1228,8 @@ PcGuestInfo *pc_guest_info_init(PCMachineState *pcms)
> }
> }
>
> + guest_info->has_nvdimm = pcms->acpi_nvdimm_state.is_enabled;
> +
> guest_info_state->machine_done.notify = pc_guest_info_machine_done;
> qemu_add_machine_init_done_notifier(&guest_info_state->machine_done);
> return guest_info;
> @@ -1877,14 +1879,14 @@ static bool pc_machine_get_nvdimm(Object *obj, Error **errp)
> {
> PCMachineState *pcms = PC_MACHINE(obj);
>
> - return pcms->nvdimm;
> + return pcms->acpi_nvdimm_state.is_enabled;
> }
>
> static void pc_machine_set_nvdimm(Object *obj, bool value, Error **errp)
> {
> PCMachineState *pcms = PC_MACHINE(obj);
>
> - pcms->nvdimm = value;
> + pcms->acpi_nvdimm_state.is_enabled = value;
> }
>
> static void pc_machine_initfn(Object *obj)
> @@ -1923,7 +1925,7 @@ static void pc_machine_initfn(Object *obj)
> &error_abort);
>
> /* nvdimm is disabled on default. */
> - pcms->nvdimm = false;
> + pcms->acpi_nvdimm_state.is_enabled = false;
> object_property_add_bool(obj, PC_MACHINE_NVDIMM, pc_machine_get_nvdimm,
> pc_machine_set_nvdimm, &error_abort);
> }
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 438cdae..2fee478 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -281,6 +281,11 @@ static void pc_init1(MachineState *machine,
> if (pcmc->pci_enabled) {
> pc_pci_device_init(pci_bus);
> }
> +
> + if (guest_info->has_nvdimm) {
> + nvdimm_init_acpi_state(&pcms->acpi_nvdimm_state, system_io,
> + guest_info->fw_cfg, OBJECT(pcms));
> + }
> }
>
> /* Looking for a pc_compat_2_4() function? It doesn't exist.
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 412b3cd..c9334d5 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -60,6 +60,7 @@ static void pc_q35_init(MachineState *machine)
> PCIDevice *lpc;
> BusState *idebus[MAX_SATA_PORTS];
> ISADevice *rtc_state;
> + MemoryRegion *system_io = get_system_io();
> MemoryRegion *pci_memory;
> MemoryRegion *rom_memory;
> MemoryRegion *ram_memory;
> @@ -176,7 +177,7 @@ static void pc_q35_init(MachineState *machine)
> q35_host->mch.ram_memory = ram_memory;
> q35_host->mch.pci_address_space = pci_memory;
> q35_host->mch.system_memory = get_system_memory();
> - q35_host->mch.address_space_io = get_system_io();
> + q35_host->mch.address_space_io = system_io;
> q35_host->mch.below_4g_mem_size = pcms->below_4g_mem_size;
> q35_host->mch.above_4g_mem_size = pcms->above_4g_mem_size;
> /* pci */
> @@ -267,6 +268,11 @@ static void pc_q35_init(MachineState *machine)
> if (pcmc->pci_enabled) {
> pc_pci_device_init(host_bus);
> }
> +
> + if (guest_info->has_nvdimm) {
> + nvdimm_init_acpi_state(&pcms->acpi_nvdimm_state, system_io,
> + guest_info->fw_cfg, OBJECT(pcms));
> + }
> }
>
> /* Looking for a pc_compat_2_4() function? It doesn't exist.
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 8122229..362ddc4 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -17,6 +17,7 @@
> #include "hw/boards.h"
> #include "hw/compat.h"
> #include "hw/mem/pc-dimm.h"
> +#include "hw/mem/nvdimm.h"
>
> #define HPET_INTCAP "hpet-intcap"
>
> @@ -55,7 +56,8 @@ struct PCMachineState {
> uint64_t max_ram_below_4g;
> OnOffAuto vmport;
> OnOffAuto smm;
> - bool nvdimm;
> +
> + AcpiNVDIMMState acpi_nvdimm_state;
>
> /* RAM information (sizes, addresses, configuration): */
> ram_addr_t below_4g_mem_size, above_4g_mem_size;
> @@ -161,6 +163,7 @@ struct PcGuestInfo {
> bool has_acpi_build;
> bool has_reserved_memory;
> bool rsdp_in_ram;
> + bool has_nvdimm;
> };
>
> /* parallel.c */
> diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
> index 49183c1..634c60b 100644
> --- a/include/hw/mem/nvdimm.h
> +++ b/include/hw/mem/nvdimm.h
> @@ -25,8 +25,34 @@
>
> #include "hw/mem/pc-dimm.h"
>
> -#define TYPE_NVDIMM "nvdimm"
> +#define TYPE_NVDIMM "nvdimm"
>
> +#define NVDIMM_DSM_MEM_FILE "etc/acpi/nvdimm-mem"
> +
> +/*
> + * 32 bits IO port starting from 0x0a18 in guest is reserved for
> + * NVDIMM ACPI emulation.
> + */
> +#define NVDIMM_ACPI_IO_BASE 0x0a18
> +#define NVDIMM_ACPI_IO_LEN 4
> +
> +/*
> + * AcpiNVDIMMState:
> + * @is_enabled: detect if NVDIMM support is enabled.
> + *
> + * @dsm_mem: the data of the fw_cfg file NVDIMM_DSM_MEM_FILE.
> + * @io_mr: the IO region used by OSPM to transfer control to QEMU.
> + */
> +struct AcpiNVDIMMState {
> + bool is_enabled;
> +
> + GArray *dsm_mem;
> + MemoryRegion io_mr;
> +};
> +typedef struct AcpiNVDIMMState AcpiNVDIMMState;
> +
> +void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
> + FWCfgState *fw_cfg, Object *owner);
> void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
> GArray *linker);
> #endif
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI Xiao Guangrong
2016-02-04 16:22 ` Michael S. Tsirkin
@ 2016-02-08 11:03 ` Igor Mammedov
2016-02-14 5:57 ` Xiao Guangrong
1 sibling, 1 reply; 51+ messages in thread
From: Igor Mammedov @ 2016-02-08 11:03 UTC (permalink / raw)
To: Xiao Guangrong
Cc: ehabkost, kvm, mst, gleb, mtosatti, qemu-devel, stefanha,
pbonzini, dan.j.williams, rth
On Wed, 13 Jan 2016 02:50:05 +0800
Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> 32 bits IO port starting from 0x0a18 in guest is reserved for NVDIMM
> ACPI emulation. The table, NVDIMM_DSM_MEM_FILE, will be patched into
> NVDIMM ACPI binary code
>
> OSPM uses this port to tell QEMU the final address of the DSM memory
> and notify QEMU to emulate the DSM method
Would you need to pass control to QEMU if each NVDIMM had its whole
label area MemoryRegion mapped right after its storage MemoryRegion?
>
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
> hw/acpi/Makefile.objs | 2 +-
> hw/acpi/nvdimm.c | 35 +++++++++++++++++++++++++++++++++++
> hw/i386/acpi-build.c | 10 +---------
> hw/i386/pc.c | 8 +++++---
> hw/i386/pc_piix.c | 5 +++++
> hw/i386/pc_q35.c | 8 +++++++-
> include/hw/i386/pc.h | 5 ++++-
> include/hw/mem/nvdimm.h | 28 +++++++++++++++++++++++++++-
> 8 files changed, 85 insertions(+), 16 deletions(-)
>
> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> index f3ade9a..faee86c 100644
> --- a/hw/acpi/Makefile.objs
> +++ b/hw/acpi/Makefile.objs
> @@ -2,7 +2,7 @@ common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o
> common-obj-$(CONFIG_ACPI_X86_ICH) += ich9.o tco.o
> common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o cpu_hotplug_acpi_table.o
> common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o memory_hotplug_acpi_table.o
> -common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
> +obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
> common-obj-$(CONFIG_ACPI) += acpi_interface.o
> common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
> common-obj-$(CONFIG_ACPI) += aml-build.o
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index df1b176..256cedd 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -28,6 +28,7 @@
>
> #include "hw/acpi/acpi.h"
> #include "hw/acpi/aml-build.h"
> +#include "hw/nvram/fw_cfg.h"
> #include "hw/mem/nvdimm.h"
>
> static int nvdimm_plugged_device_list(Object *obj, void *opaque)
> @@ -369,6 +370,40 @@ static void nvdimm_build_nfit(GSList *device_list, GArray *table_offsets,
> g_array_free(structures, true);
> }
>
> +static uint64_t
> +nvdimm_dsm_read(void *opaque, hwaddr addr, unsigned size)
> +{
> + return 0;
> +}
> +
> +static void
> +nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
> +{
> +}
> +
> +static const MemoryRegionOps nvdimm_dsm_ops = {
> + .read = nvdimm_dsm_read,
> + .write = nvdimm_dsm_write,
> + .endianness = DEVICE_LITTLE_ENDIAN,
> + .valid = {
> + .min_access_size = 4,
> + .max_access_size = 4,
> + },
> +};
> +
> +void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
> + FWCfgState *fw_cfg, Object *owner)
> +{
> + memory_region_init_io(&state->io_mr, owner, &nvdimm_dsm_ops, state,
> + "nvdimm-acpi-io", NVDIMM_ACPI_IO_LEN);
> + memory_region_add_subregion(io, NVDIMM_ACPI_IO_BASE, &state->io_mr);
> +
> + state->dsm_mem = g_array_new(false, true /* clear */, 1);
> + acpi_data_push(state->dsm_mem, TARGET_PAGE_SIZE);
> + fw_cfg_add_file(fw_cfg, NVDIMM_DSM_MEM_FILE, state->dsm_mem->data,
> + state->dsm_mem->len);
> +}
> +
> #define NVDIMM_COMMON_DSM "NCAL"
>
> static void nvdimm_build_common_dsm(Aml *dev)
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 1ca044f..c68cfb8 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -39,7 +39,6 @@
> #include "hw/loader.h"
> #include "hw/isa/isa.h"
> #include "hw/acpi/memory_hotplug.h"
> -#include "hw/mem/nvdimm.h"
> #include "sysemu/tpm.h"
> #include "hw/acpi/tpm.h"
> #include "sysemu/tpm_backend.h"
> @@ -2602,13 +2601,6 @@ static bool acpi_has_iommu(void)
> return intel_iommu && !ambiguous;
> }
>
> -static bool acpi_has_nvdimm(void)
> -{
> - PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
> -
> - return pcms->nvdimm;
> -}
> -
> static
> void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
> {
> @@ -2692,7 +2684,7 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
> build_dmar_q35(tables_blob, tables->linker);
> }
>
> - if (acpi_has_nvdimm()) {
> + if (guest_info->has_nvdimm) {
> nvdimm_build_acpi(table_offsets, tables_blob, tables->linker);
> }
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index c36b8cf..397de28 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1228,6 +1228,8 @@ PcGuestInfo *pc_guest_info_init(PCMachineState *pcms)
> }
> }
>
> + guest_info->has_nvdimm = pcms->acpi_nvdimm_state.is_enabled;
> +
> guest_info_state->machine_done.notify = pc_guest_info_machine_done;
> qemu_add_machine_init_done_notifier(&guest_info_state->machine_done);
> return guest_info;
> @@ -1877,14 +1879,14 @@ static bool pc_machine_get_nvdimm(Object *obj, Error **errp)
> {
> PCMachineState *pcms = PC_MACHINE(obj);
>
> - return pcms->nvdimm;
> + return pcms->acpi_nvdimm_state.is_enabled;
> }
>
> static void pc_machine_set_nvdimm(Object *obj, bool value, Error **errp)
> {
> PCMachineState *pcms = PC_MACHINE(obj);
>
> - pcms->nvdimm = value;
> + pcms->acpi_nvdimm_state.is_enabled = value;
> }
>
> static void pc_machine_initfn(Object *obj)
> @@ -1923,7 +1925,7 @@ static void pc_machine_initfn(Object *obj)
> &error_abort);
>
> /* nvdimm is disabled on default. */
> - pcms->nvdimm = false;
> + pcms->acpi_nvdimm_state.is_enabled = false;
> object_property_add_bool(obj, PC_MACHINE_NVDIMM, pc_machine_get_nvdimm,
> pc_machine_set_nvdimm, &error_abort);
> }
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 438cdae..2fee478 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -281,6 +281,11 @@ static void pc_init1(MachineState *machine,
> if (pcmc->pci_enabled) {
> pc_pci_device_init(pci_bus);
> }
> +
> + if (guest_info->has_nvdimm) {
> + nvdimm_init_acpi_state(&pcms->acpi_nvdimm_state, system_io,
> + guest_info->fw_cfg, OBJECT(pcms));
> + }
> }
>
> /* Looking for a pc_compat_2_4() function? It doesn't exist.
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 412b3cd..c9334d5 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -60,6 +60,7 @@ static void pc_q35_init(MachineState *machine)
> PCIDevice *lpc;
> BusState *idebus[MAX_SATA_PORTS];
> ISADevice *rtc_state;
> + MemoryRegion *system_io = get_system_io();
> MemoryRegion *pci_memory;
> MemoryRegion *rom_memory;
> MemoryRegion *ram_memory;
> @@ -176,7 +177,7 @@ static void pc_q35_init(MachineState *machine)
> q35_host->mch.ram_memory = ram_memory;
> q35_host->mch.pci_address_space = pci_memory;
> q35_host->mch.system_memory = get_system_memory();
> - q35_host->mch.address_space_io = get_system_io();
> + q35_host->mch.address_space_io = system_io;
> q35_host->mch.below_4g_mem_size = pcms->below_4g_mem_size;
> q35_host->mch.above_4g_mem_size = pcms->above_4g_mem_size;
> /* pci */
> @@ -267,6 +268,11 @@ static void pc_q35_init(MachineState *machine)
> if (pcmc->pci_enabled) {
> pc_pci_device_init(host_bus);
> }
> +
> + if (guest_info->has_nvdimm) {
> + nvdimm_init_acpi_state(&pcms->acpi_nvdimm_state, system_io,
> + guest_info->fw_cfg, OBJECT(pcms));
> + }
> }
>
> /* Looking for a pc_compat_2_4() function? It doesn't exist.
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 8122229..362ddc4 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -17,6 +17,7 @@
> #include "hw/boards.h"
> #include "hw/compat.h"
> #include "hw/mem/pc-dimm.h"
> +#include "hw/mem/nvdimm.h"
>
> #define HPET_INTCAP "hpet-intcap"
>
> @@ -55,7 +56,8 @@ struct PCMachineState {
> uint64_t max_ram_below_4g;
> OnOffAuto vmport;
> OnOffAuto smm;
> - bool nvdimm;
> +
> + AcpiNVDIMMState acpi_nvdimm_state;
>
> /* RAM information (sizes, addresses, configuration): */
> ram_addr_t below_4g_mem_size, above_4g_mem_size;
> @@ -161,6 +163,7 @@ struct PcGuestInfo {
> bool has_acpi_build;
> bool has_reserved_memory;
> bool rsdp_in_ram;
> + bool has_nvdimm;
> };
>
> /* parallel.c */
> diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
> index 49183c1..634c60b 100644
> --- a/include/hw/mem/nvdimm.h
> +++ b/include/hw/mem/nvdimm.h
> @@ -25,8 +25,34 @@
>
> #include "hw/mem/pc-dimm.h"
>
> -#define TYPE_NVDIMM "nvdimm"
> +#define TYPE_NVDIMM "nvdimm"
>
> +#define NVDIMM_DSM_MEM_FILE "etc/acpi/nvdimm-mem"
> +
> +/*
> + * 32 bits IO port starting from 0x0a18 in guest is reserved for
> + * NVDIMM ACPI emulation.
> + */
> +#define NVDIMM_ACPI_IO_BASE 0x0a18
> +#define NVDIMM_ACPI_IO_LEN 4
> +
> +/*
> + * AcpiNVDIMMState:
> + * @is_enabled: detect if NVDIMM support is enabled.
> + *
> + * @dsm_mem: the data of the fw_cfg file NVDIMM_DSM_MEM_FILE.
> + * @io_mr: the IO region used by OSPM to transfer control to QEMU.
> + */
> +struct AcpiNVDIMMState {
> + bool is_enabled;
> +
> + GArray *dsm_mem;
> + MemoryRegion io_mr;
> +};
> +typedef struct AcpiNVDIMMState AcpiNVDIMMState;
> +
> +void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
> + FWCfgState *fw_cfg, Object *owner);
> void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
> GArray *linker);
> #endif
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-08 11:03 ` Igor Mammedov
@ 2016-02-14 5:57 ` Xiao Guangrong
2016-02-15 9:11 ` Igor Mammedov
0 siblings, 1 reply; 51+ messages in thread
From: Xiao Guangrong @ 2016-02-14 5:57 UTC (permalink / raw)
To: Igor Mammedov
Cc: ehabkost, kvm, mst, gleb, mtosatti, qemu-devel, stefanha,
pbonzini, dan.j.williams, rth
On 02/08/2016 07:03 PM, Igor Mammedov wrote:
> On Wed, 13 Jan 2016 02:50:05 +0800
> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>
>> 32 bits IO port starting from 0x0a18 in guest is reserved for NVDIMM
>> ACPI emulation. The table, NVDIMM_DSM_MEM_FILE, will be patched into
>> NVDIMM ACPI binary code
>>
>> OSPM uses this port to tell QEMU the final address of the DSM memory
>> and notify QEMU to emulate the DSM method
> Would you need to pass control to QEMU if each NVDIMM had its whole
> label area MemoryRegion mapped right after its storage MemoryRegion?
>
No, label data is not mapped into guest's address space and it only
can be accessed by DSM method indirectly.
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-14 5:57 ` Xiao Guangrong
@ 2016-02-15 9:11 ` Igor Mammedov
2016-02-15 9:18 ` Michael S. Tsirkin
0 siblings, 1 reply; 51+ messages in thread
From: Igor Mammedov @ 2016-02-15 9:11 UTC (permalink / raw)
To: Xiao Guangrong
Cc: ehabkost, kvm, mst, gleb, mtosatti, qemu-devel, stefanha,
pbonzini, dan.j.williams, rth
On Sun, 14 Feb 2016 13:57:27 +0800
Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> On 02/08/2016 07:03 PM, Igor Mammedov wrote:
> > On Wed, 13 Jan 2016 02:50:05 +0800
> > Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> >
> >> 32 bits IO port starting from 0x0a18 in guest is reserved for NVDIMM
> >> ACPI emulation. The table, NVDIMM_DSM_MEM_FILE, will be patched into
> >> NVDIMM ACPI binary code
> >>
> >> OSPM uses this port to tell QEMU the final address of the DSM memory
> >> and notify QEMU to emulate the DSM method
> > Would you need to pass control to QEMU if each NVDIMM had its whole
> > label area MemoryRegion mapped right after its storage MemoryRegion?
> >
>
> No, label data is not mapped into guest's address space and it only
> can be accessed by DSM method indirectly.
Yep, per spec label data should be accessed via _DSM but question
wasn't about it,
Why would one map only 4Kb window and serialize label data
via it if it could be mapped as whole, that way _DMS method will be
much less complicated and there won't be need to add/support a protocol
for its serialization.
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-15 9:11 ` Igor Mammedov
@ 2016-02-15 9:18 ` Michael S. Tsirkin
2016-02-15 10:13 ` Xiao Guangrong
0 siblings, 1 reply; 51+ messages in thread
From: Michael S. Tsirkin @ 2016-02-15 9:18 UTC (permalink / raw)
To: Igor Mammedov
Cc: Xiao Guangrong, ehabkost, kvm, gleb, mtosatti, qemu-devel,
stefanha, pbonzini, dan.j.williams, rth
On Mon, Feb 15, 2016 at 10:11:05AM +0100, Igor Mammedov wrote:
> On Sun, 14 Feb 2016 13:57:27 +0800
> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>
> > On 02/08/2016 07:03 PM, Igor Mammedov wrote:
> > > On Wed, 13 Jan 2016 02:50:05 +0800
> > > Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> > >
> > >> 32 bits IO port starting from 0x0a18 in guest is reserved for NVDIMM
> > >> ACPI emulation. The table, NVDIMM_DSM_MEM_FILE, will be patched into
> > >> NVDIMM ACPI binary code
> > >>
> > >> OSPM uses this port to tell QEMU the final address of the DSM memory
> > >> and notify QEMU to emulate the DSM method
> > > Would you need to pass control to QEMU if each NVDIMM had its whole
> > > label area MemoryRegion mapped right after its storage MemoryRegion?
> > >
> >
> > No, label data is not mapped into guest's address space and it only
> > can be accessed by DSM method indirectly.
> Yep, per spec label data should be accessed via _DSM but question
> wasn't about it,
> Why would one map only 4Kb window and serialize label data
> via it if it could be mapped as whole, that way _DMS method will be
> much less complicated and there won't be need to add/support a protocol
> for its serialization.
>
Is it ever accessed on data path? If not I prefer the current approach:
limit the window used, the serialization protocol seems rather simple.
--
MST
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-15 9:18 ` Michael S. Tsirkin
@ 2016-02-15 10:13 ` Xiao Guangrong
2016-02-15 10:30 ` Michael S. Tsirkin
` (2 more replies)
0 siblings, 3 replies; 51+ messages in thread
From: Xiao Guangrong @ 2016-02-15 10:13 UTC (permalink / raw)
To: Michael S. Tsirkin, Igor Mammedov
Cc: ehabkost, kvm, gleb, mtosatti, qemu-devel, stefanha, pbonzini,
dan.j.williams, rth
On 02/15/2016 05:18 PM, Michael S. Tsirkin wrote:
> On Mon, Feb 15, 2016 at 10:11:05AM +0100, Igor Mammedov wrote:
>> On Sun, 14 Feb 2016 13:57:27 +0800
>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>
>>> On 02/08/2016 07:03 PM, Igor Mammedov wrote:
>>>> On Wed, 13 Jan 2016 02:50:05 +0800
>>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>>>
>>>>> 32 bits IO port starting from 0x0a18 in guest is reserved for NVDIMM
>>>>> ACPI emulation. The table, NVDIMM_DSM_MEM_FILE, will be patched into
>>>>> NVDIMM ACPI binary code
>>>>>
>>>>> OSPM uses this port to tell QEMU the final address of the DSM memory
>>>>> and notify QEMU to emulate the DSM method
>>>> Would you need to pass control to QEMU if each NVDIMM had its whole
>>>> label area MemoryRegion mapped right after its storage MemoryRegion?
>>>>
>>>
>>> No, label data is not mapped into guest's address space and it only
>>> can be accessed by DSM method indirectly.
>> Yep, per spec label data should be accessed via _DSM but question
>> wasn't about it,
Ah, sorry, i missed your question.
>> Why would one map only 4Kb window and serialize label data
>> via it if it could be mapped as whole, that way _DMS method will be
>> much less complicated and there won't be need to add/support a protocol
>> for its serialization.
>>
>
> Is it ever accessed on data path? If not I prefer the current approach:
The label data is only accessed via two DSM commands - Get Namespace Label
Data and Set Namespace Label Data, no other place need to be emulated.
> limit the window used, the serialization protocol seems rather simple.
>
Yes.
Label data is at least 128k which is big enough for BIOS as it allocates
memory at 0 ~ 4G which is tight region. It also needs guest OS to support
lager max-xfer (the max size that can be transferred one time), the size
in current Linux NVDIMM driver is 4k.
However, using lager DSM buffer can help us to simplify NVDIMM hotplug for
the case that too many nvdimm devices present in the system and their FIT
info can not be filled into one page. Each PMEM-only device needs 0xb8 bytes
and we can append 256 memory devices at most, so 12 pages are needed to
contain this info. The prototype we implemented is using ourself-defined
protocol to read piece of _FIT and concatenate them before return to Guest,
please refer to:
https://github.com/xiaogr/qemu/commit/c46ce01c8433ac0870670304360b3c4aa414143a
As 12 pages are not small region for BIOS and the _FIT size may be extended in the
future development (eg, if PBLK is introduced) i am not sure if we need this. Of
course, another approach to simplify it is that we limit the number of NVDIMM
device to make sure their _FIT < 4k.
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-15 10:13 ` Xiao Guangrong
@ 2016-02-15 10:30 ` Michael S. Tsirkin
2016-02-15 10:47 ` Igor Mammedov
2016-02-15 11:36 ` Michael S. Tsirkin
2 siblings, 0 replies; 51+ messages in thread
From: Michael S. Tsirkin @ 2016-02-15 10:30 UTC (permalink / raw)
To: Xiao Guangrong
Cc: ehabkost, kvm, gleb, mtosatti, qemu-devel, stefanha, pbonzini,
Igor Mammedov, dan.j.williams, rth
On Mon, Feb 15, 2016 at 06:13:38PM +0800, Xiao Guangrong wrote:
>
>
> On 02/15/2016 05:18 PM, Michael S. Tsirkin wrote:
> >On Mon, Feb 15, 2016 at 10:11:05AM +0100, Igor Mammedov wrote:
> >>On Sun, 14 Feb 2016 13:57:27 +0800
> >>Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> >>
> >>>On 02/08/2016 07:03 PM, Igor Mammedov wrote:
> >>>>On Wed, 13 Jan 2016 02:50:05 +0800
> >>>>Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> >>>>
> >>>>>32 bits IO port starting from 0x0a18 in guest is reserved for NVDIMM
> >>>>>ACPI emulation. The table, NVDIMM_DSM_MEM_FILE, will be patched into
> >>>>>NVDIMM ACPI binary code
> >>>>>
> >>>>>OSPM uses this port to tell QEMU the final address of the DSM memory
> >>>>>and notify QEMU to emulate the DSM method
> >>>>Would you need to pass control to QEMU if each NVDIMM had its whole
> >>>>label area MemoryRegion mapped right after its storage MemoryRegion?
> >>>>
> >>>
> >>>No, label data is not mapped into guest's address space and it only
> >>>can be accessed by DSM method indirectly.
> >>Yep, per spec label data should be accessed via _DSM but question
> >>wasn't about it,
>
> Ah, sorry, i missed your question.
>
> >>Why would one map only 4Kb window and serialize label data
> >>via it if it could be mapped as whole, that way _DMS method will be
> >>much less complicated and there won't be need to add/support a protocol
> >>for its serialization.
> >>
> >
> >Is it ever accessed on data path? If not I prefer the current approach:
>
> The label data is only accessed via two DSM commands - Get Namespace Label
> Data and Set Namespace Label Data, no other place need to be emulated.
>
> >limit the window used, the serialization protocol seems rather simple.
> >
>
> Yes.
>
> Label data is at least 128k which is big enough for BIOS as it allocates
> memory at 0 ~ 4G which is tight region. It also needs guest OS to support
> lager max-xfer (the max size that can be transferred one time), the size
> in current Linux NVDIMM driver is 4k.
>
> However, using lager DSM buffer can help us to simplify NVDIMM hotplug for
> the case that too many nvdimm devices present in the system and their FIT
> info can not be filled into one page. Each PMEM-only device needs 0xb8 bytes
> and we can append 256 memory devices at most, so 12 pages are needed to
> contain this info. The prototype we implemented is using ourself-defined
> protocol to read piece of _FIT and concatenate them before return to Guest,
> please refer to:
> https://github.com/xiaogr/qemu/commit/c46ce01c8433ac0870670304360b3c4aa414143a
>
> As 12 pages are not small region for BIOS and the _FIT size may be extended in the
> future development (eg, if PBLK is introduced) i am not sure if we need this. Of
> course, another approach to simplify it is that we limit the number of NVDIMM
> device to make sure their _FIT < 4k.
>
>
>
Bigger buffer would only be reasonable in 64 bit window (>4G),
<4G area is too busy for that.
Would that be a problem?
--
MST
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-15 10:13 ` Xiao Guangrong
2016-02-15 10:30 ` Michael S. Tsirkin
@ 2016-02-15 10:47 ` Igor Mammedov
2016-02-15 11:22 ` Xiao Guangrong
2016-02-15 11:45 ` Michael S. Tsirkin
2016-02-15 11:36 ` Michael S. Tsirkin
2 siblings, 2 replies; 51+ messages in thread
From: Igor Mammedov @ 2016-02-15 10:47 UTC (permalink / raw)
To: Xiao Guangrong
Cc: ehabkost, kvm, Michael S. Tsirkin, gleb, mtosatti, qemu-devel,
stefanha, pbonzini, dan.j.williams, rth
On Mon, 15 Feb 2016 18:13:38 +0800
Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> On 02/15/2016 05:18 PM, Michael S. Tsirkin wrote:
> > On Mon, Feb 15, 2016 at 10:11:05AM +0100, Igor Mammedov wrote:
> >> On Sun, 14 Feb 2016 13:57:27 +0800
> >> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> >>
> >>> On 02/08/2016 07:03 PM, Igor Mammedov wrote:
> >>>> On Wed, 13 Jan 2016 02:50:05 +0800
> >>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> >>>>
> >>>>> 32 bits IO port starting from 0x0a18 in guest is reserved for NVDIMM
> >>>>> ACPI emulation. The table, NVDIMM_DSM_MEM_FILE, will be patched into
> >>>>> NVDIMM ACPI binary code
> >>>>>
> >>>>> OSPM uses this port to tell QEMU the final address of the DSM memory
> >>>>> and notify QEMU to emulate the DSM method
> >>>> Would you need to pass control to QEMU if each NVDIMM had its whole
> >>>> label area MemoryRegion mapped right after its storage MemoryRegion?
> >>>>
> >>>
> >>> No, label data is not mapped into guest's address space and it only
> >>> can be accessed by DSM method indirectly.
> >> Yep, per spec label data should be accessed via _DSM but question
> >> wasn't about it,
>
> Ah, sorry, i missed your question.
>
> >> Why would one map only 4Kb window and serialize label data
> >> via it if it could be mapped as whole, that way _DMS method will be
> >> much less complicated and there won't be need to add/support a protocol
> >> for its serialization.
> >>
> >
> > Is it ever accessed on data path? If not I prefer the current approach:
>
> The label data is only accessed via two DSM commands - Get Namespace Label
> Data and Set Namespace Label Data, no other place need to be emulated.
>
> > limit the window used, the serialization protocol seems rather simple.
> >
>
> Yes.
>
> Label data is at least 128k which is big enough for BIOS as it allocates
> memory at 0 ~ 4G which is tight region. It also needs guest OS to support
> lager max-xfer (the max size that can be transferred one time), the size
> in current Linux NVDIMM driver is 4k.
>
> However, using lager DSM buffer can help us to simplify NVDIMM hotplug for
> the case that too many nvdimm devices present in the system and their FIT
> info can not be filled into one page. Each PMEM-only device needs 0xb8 bytes
> and we can append 256 memory devices at most, so 12 pages are needed to
> contain this info. The prototype we implemented is using ourself-defined
> protocol to read piece of _FIT and concatenate them before return to Guest,
> please refer to:
> https://github.com/xiaogr/qemu/commit/c46ce01c8433ac0870670304360b3c4aa414143a
>
> As 12 pages are not small region for BIOS and the _FIT size may be extended in the
> future development (eg, if PBLK is introduced) i am not sure if we need this. Of
> course, another approach to simplify it is that we limit the number of NVDIMM
> device to make sure their _FIT < 4k.
My suggestion is not to have only one label area for every NVDIMM but
rather to map each label area right after each NVDIMM's data memory.
That way _DMS can be made non-serialized and guest could handle
label data in parallel.
As for a _FIT we can use the same approach as mem hotplug
(IO port window) or Michael's idea to add vendor specific
PCI_config region to a current PM device to avoid using
IO ports.
>
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-15 10:47 ` Igor Mammedov
@ 2016-02-15 11:22 ` Xiao Guangrong
2016-02-15 11:45 ` Michael S. Tsirkin
1 sibling, 0 replies; 51+ messages in thread
From: Xiao Guangrong @ 2016-02-15 11:22 UTC (permalink / raw)
To: Igor Mammedov
Cc: ehabkost, kvm, Michael S. Tsirkin, gleb, mtosatti, qemu-devel,
stefanha, pbonzini, dan.j.williams, rth
On 02/15/2016 06:47 PM, Igor Mammedov wrote:
> On Mon, 15 Feb 2016 18:13:38 +0800
> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>
>> On 02/15/2016 05:18 PM, Michael S. Tsirkin wrote:
>>> On Mon, Feb 15, 2016 at 10:11:05AM +0100, Igor Mammedov wrote:
>>>> On Sun, 14 Feb 2016 13:57:27 +0800
>>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>>>
>>>>> On 02/08/2016 07:03 PM, Igor Mammedov wrote:
>>>>>> On Wed, 13 Jan 2016 02:50:05 +0800
>>>>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>>>>>
>>>>>>> 32 bits IO port starting from 0x0a18 in guest is reserved for NVDIMM
>>>>>>> ACPI emulation. The table, NVDIMM_DSM_MEM_FILE, will be patched into
>>>>>>> NVDIMM ACPI binary code
>>>>>>>
>>>>>>> OSPM uses this port to tell QEMU the final address of the DSM memory
>>>>>>> and notify QEMU to emulate the DSM method
>>>>>> Would you need to pass control to QEMU if each NVDIMM had its whole
>>>>>> label area MemoryRegion mapped right after its storage MemoryRegion?
>>>>>>
>>>>>
>>>>> No, label data is not mapped into guest's address space and it only
>>>>> can be accessed by DSM method indirectly.
>>>> Yep, per spec label data should be accessed via _DSM but question
>>>> wasn't about it,
>>
>> Ah, sorry, i missed your question.
>>
>>>> Why would one map only 4Kb window and serialize label data
>>>> via it if it could be mapped as whole, that way _DMS method will be
>>>> much less complicated and there won't be need to add/support a protocol
>>>> for its serialization.
>>>>
>>>
>>> Is it ever accessed on data path? If not I prefer the current approach:
>>
>> The label data is only accessed via two DSM commands - Get Namespace Label
>> Data and Set Namespace Label Data, no other place need to be emulated.
>>
>>> limit the window used, the serialization protocol seems rather simple.
>>>
>>
>> Yes.
>>
>> Label data is at least 128k which is big enough for BIOS as it allocates
>> memory at 0 ~ 4G which is tight region. It also needs guest OS to support
>> lager max-xfer (the max size that can be transferred one time), the size
>> in current Linux NVDIMM driver is 4k.
>>
>> However, using lager DSM buffer can help us to simplify NVDIMM hotplug for
>> the case that too many nvdimm devices present in the system and their FIT
>> info can not be filled into one page. Each PMEM-only device needs 0xb8 bytes
>> and we can append 256 memory devices at most, so 12 pages are needed to
>> contain this info. The prototype we implemented is using ourself-defined
>> protocol to read piece of _FIT and concatenate them before return to Guest,
>> please refer to:
>> https://github.com/xiaogr/qemu/commit/c46ce01c8433ac0870670304360b3c4aa414143a
>>
>> As 12 pages are not small region for BIOS and the _FIT size may be extended in the
>> future development (eg, if PBLK is introduced) i am not sure if we need this. Of
>> course, another approach to simplify it is that we limit the number of NVDIMM
>> device to make sure their _FIT < 4k.
> My suggestion is not to have only one label area for every NVDIMM but
> rather to map each label area right after each NVDIMM's data memory.
> That way _DMS can be made non-serialized and guest could handle
> label data in parallel.
>
Sounds great to me. I like this idea. :D
> As for a _FIT we can use the same approach as mem hotplug
> (IO port window) or Michael's idea to add vendor specific
> PCI_config region to a current PM device to avoid using
> IO ports.
Thanks for your reminder, i will learn it.
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-15 10:47 ` Igor Mammedov
2016-02-15 11:22 ` Xiao Guangrong
@ 2016-02-15 11:45 ` Michael S. Tsirkin
2016-02-15 13:32 ` Igor Mammedov
1 sibling, 1 reply; 51+ messages in thread
From: Michael S. Tsirkin @ 2016-02-15 11:45 UTC (permalink / raw)
To: Igor Mammedov
Cc: Xiao Guangrong, ehabkost, kvm, gleb, mtosatti, qemu-devel,
stefanha, pbonzini, dan.j.williams, rth
On Mon, Feb 15, 2016 at 11:47:42AM +0100, Igor Mammedov wrote:
> On Mon, 15 Feb 2016 18:13:38 +0800
> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>
> > On 02/15/2016 05:18 PM, Michael S. Tsirkin wrote:
> > > On Mon, Feb 15, 2016 at 10:11:05AM +0100, Igor Mammedov wrote:
> > >> On Sun, 14 Feb 2016 13:57:27 +0800
> > >> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> > >>
> > >>> On 02/08/2016 07:03 PM, Igor Mammedov wrote:
> > >>>> On Wed, 13 Jan 2016 02:50:05 +0800
> > >>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> > >>>>
> > >>>>> 32 bits IO port starting from 0x0a18 in guest is reserved for NVDIMM
> > >>>>> ACPI emulation. The table, NVDIMM_DSM_MEM_FILE, will be patched into
> > >>>>> NVDIMM ACPI binary code
> > >>>>>
> > >>>>> OSPM uses this port to tell QEMU the final address of the DSM memory
> > >>>>> and notify QEMU to emulate the DSM method
> > >>>> Would you need to pass control to QEMU if each NVDIMM had its whole
> > >>>> label area MemoryRegion mapped right after its storage MemoryRegion?
> > >>>>
> > >>>
> > >>> No, label data is not mapped into guest's address space and it only
> > >>> can be accessed by DSM method indirectly.
> > >> Yep, per spec label data should be accessed via _DSM but question
> > >> wasn't about it,
> >
> > Ah, sorry, i missed your question.
> >
> > >> Why would one map only 4Kb window and serialize label data
> > >> via it if it could be mapped as whole, that way _DMS method will be
> > >> much less complicated and there won't be need to add/support a protocol
> > >> for its serialization.
> > >>
> > >
> > > Is it ever accessed on data path? If not I prefer the current approach:
> >
> > The label data is only accessed via two DSM commands - Get Namespace Label
> > Data and Set Namespace Label Data, no other place need to be emulated.
> >
> > > limit the window used, the serialization protocol seems rather simple.
> > >
> >
> > Yes.
> >
> > Label data is at least 128k which is big enough for BIOS as it allocates
> > memory at 0 ~ 4G which is tight region. It also needs guest OS to support
> > lager max-xfer (the max size that can be transferred one time), the size
> > in current Linux NVDIMM driver is 4k.
> >
> > However, using lager DSM buffer can help us to simplify NVDIMM hotplug for
> > the case that too many nvdimm devices present in the system and their FIT
> > info can not be filled into one page. Each PMEM-only device needs 0xb8 bytes
> > and we can append 256 memory devices at most, so 12 pages are needed to
> > contain this info. The prototype we implemented is using ourself-defined
> > protocol to read piece of _FIT and concatenate them before return to Guest,
> > please refer to:
> > https://github.com/xiaogr/qemu/commit/c46ce01c8433ac0870670304360b3c4aa414143a
> >
> > As 12 pages are not small region for BIOS and the _FIT size may be extended in the
> > future development (eg, if PBLK is introduced) i am not sure if we need this. Of
> > course, another approach to simplify it is that we limit the number of NVDIMM
> > device to make sure their _FIT < 4k.
> My suggestion is not to have only one label area for every NVDIMM but
> rather to map each label area right after each NVDIMM's data memory.
> That way _DMS can be made non-serialized and guest could handle
> label data in parallel.
I think that alignment considerations would mean we are burning up
1G of phys address space for this. For PAE we only have 64G
of this address space, so this would be a problem.
> As for a _FIT we can use the same approach as mem hotplug
> (IO port window) or Michael's idea to add vendor specific
> PCI_config region to a current PM device to avoid using
> IO ports.
>
>
> >
> >
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-15 11:45 ` Michael S. Tsirkin
@ 2016-02-15 13:32 ` Igor Mammedov
2016-02-15 15:53 ` Xiao Guangrong
0 siblings, 1 reply; 51+ messages in thread
From: Igor Mammedov @ 2016-02-15 13:32 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Xiao Guangrong, ehabkost, kvm, gleb, mtosatti, qemu-devel,
stefanha, pbonzini, dan.j.williams, rth
On Mon, 15 Feb 2016 13:45:59 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Mon, Feb 15, 2016 at 11:47:42AM +0100, Igor Mammedov wrote:
> > On Mon, 15 Feb 2016 18:13:38 +0800
> > Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> >
> > > On 02/15/2016 05:18 PM, Michael S. Tsirkin wrote:
> > > > On Mon, Feb 15, 2016 at 10:11:05AM +0100, Igor Mammedov wrote:
> > > >> On Sun, 14 Feb 2016 13:57:27 +0800
> > > >> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> > > >>
> > > >>> On 02/08/2016 07:03 PM, Igor Mammedov wrote:
> > > >>>> On Wed, 13 Jan 2016 02:50:05 +0800
> > > >>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> > > >>>>
> > > >>>>> 32 bits IO port starting from 0x0a18 in guest is reserved for NVDIMM
> > > >>>>> ACPI emulation. The table, NVDIMM_DSM_MEM_FILE, will be patched into
> > > >>>>> NVDIMM ACPI binary code
> > > >>>>>
> > > >>>>> OSPM uses this port to tell QEMU the final address of the DSM memory
> > > >>>>> and notify QEMU to emulate the DSM method
> > > >>>> Would you need to pass control to QEMU if each NVDIMM had its whole
> > > >>>> label area MemoryRegion mapped right after its storage MemoryRegion?
> > > >>>>
> > > >>>
> > > >>> No, label data is not mapped into guest's address space and it only
> > > >>> can be accessed by DSM method indirectly.
> > > >> Yep, per spec label data should be accessed via _DSM but question
> > > >> wasn't about it,
> > >
> > > Ah, sorry, i missed your question.
> > >
> > > >> Why would one map only 4Kb window and serialize label data
> > > >> via it if it could be mapped as whole, that way _DMS method will be
> > > >> much less complicated and there won't be need to add/support a protocol
> > > >> for its serialization.
> > > >>
> > > >
> > > > Is it ever accessed on data path? If not I prefer the current approach:
> > >
> > > The label data is only accessed via two DSM commands - Get Namespace Label
> > > Data and Set Namespace Label Data, no other place need to be emulated.
> > >
> > > > limit the window used, the serialization protocol seems rather simple.
> > > >
> > >
> > > Yes.
> > >
> > > Label data is at least 128k which is big enough for BIOS as it allocates
> > > memory at 0 ~ 4G which is tight region. It also needs guest OS to support
> > > lager max-xfer (the max size that can be transferred one time), the size
> > > in current Linux NVDIMM driver is 4k.
> > >
> > > However, using lager DSM buffer can help us to simplify NVDIMM hotplug for
> > > the case that too many nvdimm devices present in the system and their FIT
> > > info can not be filled into one page. Each PMEM-only device needs 0xb8 bytes
> > > and we can append 256 memory devices at most, so 12 pages are needed to
> > > contain this info. The prototype we implemented is using ourself-defined
> > > protocol to read piece of _FIT and concatenate them before return to Guest,
> > > please refer to:
> > > https://github.com/xiaogr/qemu/commit/c46ce01c8433ac0870670304360b3c4aa414143a
> > >
> > > As 12 pages are not small region for BIOS and the _FIT size may be extended in the
> > > future development (eg, if PBLK is introduced) i am not sure if we need this. Of
> > > course, another approach to simplify it is that we limit the number of NVDIMM
> > > device to make sure their _FIT < 4k.
> > My suggestion is not to have only one label area for every NVDIMM but
> > rather to map each label area right after each NVDIMM's data memory.
> > That way _DMS can be made non-serialized and guest could handle
> > label data in parallel.
>
> I think that alignment considerations would mean we are burning up
> 1G of phys address space for this. For PAE we only have 64G
> of this address space, so this would be a problem.
That's true that it will burning away address space, however that
just means that PAE guests would not be able to handle as many
NVDIMMs as 64bit guests. The same applies to DIMMs as well, with
alignment enforced. If one needs more DIMMs he/she can switch
to 64bit guest to use them.
It's trade of inefficient GPA consumption vs efficient NVDIMMs access.
Also with fully mapped label area for each NVDIMM we don't have to
introduce and maintain any guest visible serialization protocol
(protocol for serializing _DSM via 4K window) which becomes ABI.
If I were to choose I'd pick more efficient NVDIMM access vs
a lot of densely packed inefficient NVDIMMs and use 64bit
kernel to utilize it vs legacy PAE one.
> > As for a _FIT we can use the same approach as mem hotplug
> > (IO port window) or Michael's idea to add vendor specific
> > PCI_config region to a current PM device to avoid using
> > IO ports.
> >
> >
> > >
> > >
> > >
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-15 13:32 ` Igor Mammedov
@ 2016-02-15 15:53 ` Xiao Guangrong
2016-02-15 17:24 ` Igor Mammedov
0 siblings, 1 reply; 51+ messages in thread
From: Xiao Guangrong @ 2016-02-15 15:53 UTC (permalink / raw)
To: Igor Mammedov, Michael S. Tsirkin
Cc: ehabkost, kvm, gleb, mtosatti, qemu-devel, stefanha, pbonzini,
dan.j.williams, rth
On 02/15/2016 09:32 PM, Igor Mammedov wrote:
> On Mon, 15 Feb 2016 13:45:59 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
>> On Mon, Feb 15, 2016 at 11:47:42AM +0100, Igor Mammedov wrote:
>>> On Mon, 15 Feb 2016 18:13:38 +0800
>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>>
>>>> On 02/15/2016 05:18 PM, Michael S. Tsirkin wrote:
>>>>> On Mon, Feb 15, 2016 at 10:11:05AM +0100, Igor Mammedov wrote:
>>>>>> On Sun, 14 Feb 2016 13:57:27 +0800
>>>>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>>>>>
>>>>>>> On 02/08/2016 07:03 PM, Igor Mammedov wrote:
>>>>>>>> On Wed, 13 Jan 2016 02:50:05 +0800
>>>>>>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>>>>>>>
>>>>>>>>> 32 bits IO port starting from 0x0a18 in guest is reserved for NVDIMM
>>>>>>>>> ACPI emulation. The table, NVDIMM_DSM_MEM_FILE, will be patched into
>>>>>>>>> NVDIMM ACPI binary code
>>>>>>>>>
>>>>>>>>> OSPM uses this port to tell QEMU the final address of the DSM memory
>>>>>>>>> and notify QEMU to emulate the DSM method
>>>>>>>> Would you need to pass control to QEMU if each NVDIMM had its whole
>>>>>>>> label area MemoryRegion mapped right after its storage MemoryRegion?
>>>>>>>>
>>>>>>>
>>>>>>> No, label data is not mapped into guest's address space and it only
>>>>>>> can be accessed by DSM method indirectly.
>>>>>> Yep, per spec label data should be accessed via _DSM but question
>>>>>> wasn't about it,
>>>>
>>>> Ah, sorry, i missed your question.
>>>>
>>>>>> Why would one map only 4Kb window and serialize label data
>>>>>> via it if it could be mapped as whole, that way _DMS method will be
>>>>>> much less complicated and there won't be need to add/support a protocol
>>>>>> for its serialization.
>>>>>>
>>>>>
>>>>> Is it ever accessed on data path? If not I prefer the current approach:
>>>>
>>>> The label data is only accessed via two DSM commands - Get Namespace Label
>>>> Data and Set Namespace Label Data, no other place need to be emulated.
>>>>
>>>>> limit the window used, the serialization protocol seems rather simple.
>>>>>
>>>>
>>>> Yes.
>>>>
>>>> Label data is at least 128k which is big enough for BIOS as it allocates
>>>> memory at 0 ~ 4G which is tight region. It also needs guest OS to support
>>>> lager max-xfer (the max size that can be transferred one time), the size
>>>> in current Linux NVDIMM driver is 4k.
>>>>
>>>> However, using lager DSM buffer can help us to simplify NVDIMM hotplug for
>>>> the case that too many nvdimm devices present in the system and their FIT
>>>> info can not be filled into one page. Each PMEM-only device needs 0xb8 bytes
>>>> and we can append 256 memory devices at most, so 12 pages are needed to
>>>> contain this info. The prototype we implemented is using ourself-defined
>>>> protocol to read piece of _FIT and concatenate them before return to Guest,
>>>> please refer to:
>>>> https://github.com/xiaogr/qemu/commit/c46ce01c8433ac0870670304360b3c4aa414143a
>>>>
>>>> As 12 pages are not small region for BIOS and the _FIT size may be extended in the
>>>> future development (eg, if PBLK is introduced) i am not sure if we need this. Of
>>>> course, another approach to simplify it is that we limit the number of NVDIMM
>>>> device to make sure their _FIT < 4k.
>>> My suggestion is not to have only one label area for every NVDIMM but
>>> rather to map each label area right after each NVDIMM's data memory.
>>> That way _DMS can be made non-serialized and guest could handle
>>> label data in parallel.
>>
>> I think that alignment considerations would mean we are burning up
>> 1G of phys address space for this. For PAE we only have 64G
>> of this address space, so this would be a problem.
> That's true that it will burning away address space, however that
> just means that PAE guests would not be able to handle as many
> NVDIMMs as 64bit guests. The same applies to DIMMs as well, with
> alignment enforced. If one needs more DIMMs he/she can switch
> to 64bit guest to use them.
>
> It's trade of inefficient GPA consumption vs efficient NVDIMMs access.
> Also with fully mapped label area for each NVDIMM we don't have to
> introduce and maintain any guest visible serialization protocol
> (protocol for serializing _DSM via 4K window) which becomes ABI.
It's true for label access but it is not for the long term as we will
need to support other _DSM commands such as vendor specific command,
PBLK dsm command, also NVDIMM MCE related commands will be introduced
in the future, so we will come back here at that time. :(
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-15 15:53 ` Xiao Guangrong
@ 2016-02-15 17:24 ` Igor Mammedov
2016-02-15 18:35 ` Xiao Guangrong
0 siblings, 1 reply; 51+ messages in thread
From: Igor Mammedov @ 2016-02-15 17:24 UTC (permalink / raw)
To: Xiao Guangrong
Cc: ehabkost, kvm, Michael S. Tsirkin, gleb, mtosatti, qemu-devel,
stefanha, pbonzini, dan.j.williams, rth
On Mon, 15 Feb 2016 23:53:13 +0800
Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> On 02/15/2016 09:32 PM, Igor Mammedov wrote:
> > On Mon, 15 Feb 2016 13:45:59 +0200
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >
> >> On Mon, Feb 15, 2016 at 11:47:42AM +0100, Igor Mammedov wrote:
> >>> On Mon, 15 Feb 2016 18:13:38 +0800
> >>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> >>>
> >>>> On 02/15/2016 05:18 PM, Michael S. Tsirkin wrote:
> >>>>> On Mon, Feb 15, 2016 at 10:11:05AM +0100, Igor Mammedov wrote:
> >>>>>> On Sun, 14 Feb 2016 13:57:27 +0800
> >>>>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> >>>>>>
> >>>>>>> On 02/08/2016 07:03 PM, Igor Mammedov wrote:
> >>>>>>>> On Wed, 13 Jan 2016 02:50:05 +0800
> >>>>>>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> >>>>>>>>
> >>>>>>>>> 32 bits IO port starting from 0x0a18 in guest is reserved for NVDIMM
> >>>>>>>>> ACPI emulation. The table, NVDIMM_DSM_MEM_FILE, will be patched into
> >>>>>>>>> NVDIMM ACPI binary code
> >>>>>>>>>
> >>>>>>>>> OSPM uses this port to tell QEMU the final address of the DSM memory
> >>>>>>>>> and notify QEMU to emulate the DSM method
> >>>>>>>> Would you need to pass control to QEMU if each NVDIMM had its whole
> >>>>>>>> label area MemoryRegion mapped right after its storage MemoryRegion?
> >>>>>>>>
> >>>>>>>
> >>>>>>> No, label data is not mapped into guest's address space and it only
> >>>>>>> can be accessed by DSM method indirectly.
> >>>>>> Yep, per spec label data should be accessed via _DSM but question
> >>>>>> wasn't about it,
> >>>>
> >>>> Ah, sorry, i missed your question.
> >>>>
> >>>>>> Why would one map only 4Kb window and serialize label data
> >>>>>> via it if it could be mapped as whole, that way _DMS method will be
> >>>>>> much less complicated and there won't be need to add/support a protocol
> >>>>>> for its serialization.
> >>>>>>
> >>>>>
> >>>>> Is it ever accessed on data path? If not I prefer the current approach:
> >>>>
> >>>> The label data is only accessed via two DSM commands - Get Namespace Label
> >>>> Data and Set Namespace Label Data, no other place need to be emulated.
> >>>>
> >>>>> limit the window used, the serialization protocol seems rather simple.
> >>>>>
> >>>>
> >>>> Yes.
> >>>>
> >>>> Label data is at least 128k which is big enough for BIOS as it allocates
> >>>> memory at 0 ~ 4G which is tight region. It also needs guest OS to support
> >>>> lager max-xfer (the max size that can be transferred one time), the size
> >>>> in current Linux NVDIMM driver is 4k.
> >>>>
> >>>> However, using lager DSM buffer can help us to simplify NVDIMM hotplug for
> >>>> the case that too many nvdimm devices present in the system and their FIT
> >>>> info can not be filled into one page. Each PMEM-only device needs 0xb8 bytes
> >>>> and we can append 256 memory devices at most, so 12 pages are needed to
> >>>> contain this info. The prototype we implemented is using ourself-defined
> >>>> protocol to read piece of _FIT and concatenate them before return to Guest,
> >>>> please refer to:
> >>>> https://github.com/xiaogr/qemu/commit/c46ce01c8433ac0870670304360b3c4aa414143a
> >>>>
> >>>> As 12 pages are not small region for BIOS and the _FIT size may be extended in the
> >>>> future development (eg, if PBLK is introduced) i am not sure if we need this. Of
> >>>> course, another approach to simplify it is that we limit the number of NVDIMM
> >>>> device to make sure their _FIT < 4k.
> >>> My suggestion is not to have only one label area for every NVDIMM but
> >>> rather to map each label area right after each NVDIMM's data memory.
> >>> That way _DMS can be made non-serialized and guest could handle
> >>> label data in parallel.
> >>
> >> I think that alignment considerations would mean we are burning up
> >> 1G of phys address space for this. For PAE we only have 64G
> >> of this address space, so this would be a problem.
> > That's true that it will burning away address space, however that
> > just means that PAE guests would not be able to handle as many
> > NVDIMMs as 64bit guests. The same applies to DIMMs as well, with
> > alignment enforced. If one needs more DIMMs he/she can switch
> > to 64bit guest to use them.
> >
> > It's trade of inefficient GPA consumption vs efficient NVDIMMs access.
> > Also with fully mapped label area for each NVDIMM we don't have to
> > introduce and maintain any guest visible serialization protocol
> > (protocol for serializing _DSM via 4K window) which becomes ABI.
>
> It's true for label access but it is not for the long term as we will
> need to support other _DSM commands such as vendor specific command,
> PBLK dsm command, also NVDIMM MCE related commands will be introduced
> in the future, so we will come back here at that time. :(
I believe for block mode NVDIMM would also need per NVDIMM mapping
for performance reasons (parallel access).
As for the rest could that commands go via MMIO that we usually
use for control path?
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-15 17:24 ` Igor Mammedov
@ 2016-02-15 18:35 ` Xiao Guangrong
2016-02-16 11:00 ` Igor Mammedov
0 siblings, 1 reply; 51+ messages in thread
From: Xiao Guangrong @ 2016-02-15 18:35 UTC (permalink / raw)
To: Igor Mammedov
Cc: ehabkost, kvm, Michael S. Tsirkin, gleb, mtosatti, qemu-devel,
stefanha, pbonzini, dan.j.williams, rth
On 02/16/2016 01:24 AM, Igor Mammedov wrote:
> On Mon, 15 Feb 2016 23:53:13 +0800
> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>
>> On 02/15/2016 09:32 PM, Igor Mammedov wrote:
>>> On Mon, 15 Feb 2016 13:45:59 +0200
>>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>>
>>>> On Mon, Feb 15, 2016 at 11:47:42AM +0100, Igor Mammedov wrote:
>>>>> On Mon, 15 Feb 2016 18:13:38 +0800
>>>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>>>>
>>>>>> On 02/15/2016 05:18 PM, Michael S. Tsirkin wrote:
>>>>>>> On Mon, Feb 15, 2016 at 10:11:05AM +0100, Igor Mammedov wrote:
>>>>>>>> On Sun, 14 Feb 2016 13:57:27 +0800
>>>>>>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>>>>>>>
>>>>>>>>> On 02/08/2016 07:03 PM, Igor Mammedov wrote:
>>>>>>>>>> On Wed, 13 Jan 2016 02:50:05 +0800
>>>>>>>>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>>>>>>>>>
>>>>>>>>>>> 32 bits IO port starting from 0x0a18 in guest is reserved for NVDIMM
>>>>>>>>>>> ACPI emulation. The table, NVDIMM_DSM_MEM_FILE, will be patched into
>>>>>>>>>>> NVDIMM ACPI binary code
>>>>>>>>>>>
>>>>>>>>>>> OSPM uses this port to tell QEMU the final address of the DSM memory
>>>>>>>>>>> and notify QEMU to emulate the DSM method
>>>>>>>>>> Would you need to pass control to QEMU if each NVDIMM had its whole
>>>>>>>>>> label area MemoryRegion mapped right after its storage MemoryRegion?
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> No, label data is not mapped into guest's address space and it only
>>>>>>>>> can be accessed by DSM method indirectly.
>>>>>>>> Yep, per spec label data should be accessed via _DSM but question
>>>>>>>> wasn't about it,
>>>>>>
>>>>>> Ah, sorry, i missed your question.
>>>>>>
>>>>>>>> Why would one map only 4Kb window and serialize label data
>>>>>>>> via it if it could be mapped as whole, that way _DMS method will be
>>>>>>>> much less complicated and there won't be need to add/support a protocol
>>>>>>>> for its serialization.
>>>>>>>>
>>>>>>>
>>>>>>> Is it ever accessed on data path? If not I prefer the current approach:
>>>>>>
>>>>>> The label data is only accessed via two DSM commands - Get Namespace Label
>>>>>> Data and Set Namespace Label Data, no other place need to be emulated.
>>>>>>
>>>>>>> limit the window used, the serialization protocol seems rather simple.
>>>>>>>
>>>>>>
>>>>>> Yes.
>>>>>>
>>>>>> Label data is at least 128k which is big enough for BIOS as it allocates
>>>>>> memory at 0 ~ 4G which is tight region. It also needs guest OS to support
>>>>>> lager max-xfer (the max size that can be transferred one time), the size
>>>>>> in current Linux NVDIMM driver is 4k.
>>>>>>
>>>>>> However, using lager DSM buffer can help us to simplify NVDIMM hotplug for
>>>>>> the case that too many nvdimm devices present in the system and their FIT
>>>>>> info can not be filled into one page. Each PMEM-only device needs 0xb8 bytes
>>>>>> and we can append 256 memory devices at most, so 12 pages are needed to
>>>>>> contain this info. The prototype we implemented is using ourself-defined
>>>>>> protocol to read piece of _FIT and concatenate them before return to Guest,
>>>>>> please refer to:
>>>>>> https://github.com/xiaogr/qemu/commit/c46ce01c8433ac0870670304360b3c4aa414143a
>>>>>>
>>>>>> As 12 pages are not small region for BIOS and the _FIT size may be extended in the
>>>>>> future development (eg, if PBLK is introduced) i am not sure if we need this. Of
>>>>>> course, another approach to simplify it is that we limit the number of NVDIMM
>>>>>> device to make sure their _FIT < 4k.
>>>>> My suggestion is not to have only one label area for every NVDIMM but
>>>>> rather to map each label area right after each NVDIMM's data memory.
>>>>> That way _DMS can be made non-serialized and guest could handle
>>>>> label data in parallel.
>>>>
>>>> I think that alignment considerations would mean we are burning up
>>>> 1G of phys address space for this. For PAE we only have 64G
>>>> of this address space, so this would be a problem.
>>> That's true that it will burning away address space, however that
>>> just means that PAE guests would not be able to handle as many
>>> NVDIMMs as 64bit guests. The same applies to DIMMs as well, with
>>> alignment enforced. If one needs more DIMMs he/she can switch
>>> to 64bit guest to use them.
>>>
>>> It's trade of inefficient GPA consumption vs efficient NVDIMMs access.
>>> Also with fully mapped label area for each NVDIMM we don't have to
>>> introduce and maintain any guest visible serialization protocol
>>> (protocol for serializing _DSM via 4K window) which becomes ABI.
>>
>> It's true for label access but it is not for the long term as we will
>> need to support other _DSM commands such as vendor specific command,
>> PBLK dsm command, also NVDIMM MCE related commands will be introduced
>> in the future, so we will come back here at that time. :(
> I believe for block mode NVDIMM would also need per NVDIMM mapping
> for performance reasons (parallel access).
> As for the rest could that commands go via MMIO that we usually
> use for control path?
So both input data and output data go through single MMIO, we need to
introduce a protocol to pass these data, that is complex?
And is any MMIO we can reuse (more complexer？) or we should allocate this
MMIO page （the old question - where to allocated?）?
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-15 18:35 ` Xiao Guangrong
@ 2016-02-16 11:00 ` Igor Mammedov
2016-02-17 2:04 ` Xiao Guangrong
0 siblings, 1 reply; 51+ messages in thread
From: Igor Mammedov @ 2016-02-16 11:00 UTC (permalink / raw)
To: Xiao Guangrong
Cc: ehabkost, kvm, Michael S. Tsirkin, gleb, mtosatti, qemu-devel,
stefanha, pbonzini, dan.j.williams, rth
On Tue, 16 Feb 2016 02:35:41 +0800
Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> On 02/16/2016 01:24 AM, Igor Mammedov wrote:
> > On Mon, 15 Feb 2016 23:53:13 +0800
> > Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> >
> >> On 02/15/2016 09:32 PM, Igor Mammedov wrote:
> >>> On Mon, 15 Feb 2016 13:45:59 +0200
> >>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >>>
> >>>> On Mon, Feb 15, 2016 at 11:47:42AM +0100, Igor Mammedov wrote:
> >>>>> On Mon, 15 Feb 2016 18:13:38 +0800
> >>>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> >>>>>
> >>>>>> On 02/15/2016 05:18 PM, Michael S. Tsirkin wrote:
> >>>>>>> On Mon, Feb 15, 2016 at 10:11:05AM +0100, Igor Mammedov wrote:
> >>>>>>>> On Sun, 14 Feb 2016 13:57:27 +0800
> >>>>>>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> >>>>>>>>
> >>>>>>>>> On 02/08/2016 07:03 PM, Igor Mammedov wrote:
> >>>>>>>>>> On Wed, 13 Jan 2016 02:50:05 +0800
> >>>>>>>>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> >>>>>>>>>>
> >>>>>>>>>>> 32 bits IO port starting from 0x0a18 in guest is reserved for NVDIMM
> >>>>>>>>>>> ACPI emulation. The table, NVDIMM_DSM_MEM_FILE, will be patched into
> >>>>>>>>>>> NVDIMM ACPI binary code
> >>>>>>>>>>>
> >>>>>>>>>>> OSPM uses this port to tell QEMU the final address of the DSM memory
> >>>>>>>>>>> and notify QEMU to emulate the DSM method
> >>>>>>>>>> Would you need to pass control to QEMU if each NVDIMM had its whole
> >>>>>>>>>> label area MemoryRegion mapped right after its storage MemoryRegion?
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> No, label data is not mapped into guest's address space and it only
> >>>>>>>>> can be accessed by DSM method indirectly.
> >>>>>>>> Yep, per spec label data should be accessed via _DSM but question
> >>>>>>>> wasn't about it,
> >>>>>>
> >>>>>> Ah, sorry, i missed your question.
> >>>>>>
> >>>>>>>> Why would one map only 4Kb window and serialize label data
> >>>>>>>> via it if it could be mapped as whole, that way _DMS method will be
> >>>>>>>> much less complicated and there won't be need to add/support a protocol
> >>>>>>>> for its serialization.
> >>>>>>>>
> >>>>>>>
> >>>>>>> Is it ever accessed on data path? If not I prefer the current approach:
> >>>>>>
> >>>>>> The label data is only accessed via two DSM commands - Get Namespace Label
> >>>>>> Data and Set Namespace Label Data, no other place need to be emulated.
> >>>>>>
> >>>>>>> limit the window used, the serialization protocol seems rather simple.
> >>>>>>>
> >>>>>>
> >>>>>> Yes.
> >>>>>>
> >>>>>> Label data is at least 128k which is big enough for BIOS as it allocates
> >>>>>> memory at 0 ~ 4G which is tight region. It also needs guest OS to support
> >>>>>> lager max-xfer (the max size that can be transferred one time), the size
> >>>>>> in current Linux NVDIMM driver is 4k.
> >>>>>>
> >>>>>> However, using lager DSM buffer can help us to simplify NVDIMM hotplug for
> >>>>>> the case that too many nvdimm devices present in the system and their FIT
> >>>>>> info can not be filled into one page. Each PMEM-only device needs 0xb8 bytes
> >>>>>> and we can append 256 memory devices at most, so 12 pages are needed to
> >>>>>> contain this info. The prototype we implemented is using ourself-defined
> >>>>>> protocol to read piece of _FIT and concatenate them before return to Guest,
> >>>>>> please refer to:
> >>>>>> https://github.com/xiaogr/qemu/commit/c46ce01c8433ac0870670304360b3c4aa414143a
> >>>>>>
> >>>>>> As 12 pages are not small region for BIOS and the _FIT size may be extended in the
> >>>>>> future development (eg, if PBLK is introduced) i am not sure if we need this. Of
> >>>>>> course, another approach to simplify it is that we limit the number of NVDIMM
> >>>>>> device to make sure their _FIT < 4k.
> >>>>> My suggestion is not to have only one label area for every NVDIMM but
> >>>>> rather to map each label area right after each NVDIMM's data memory.
> >>>>> That way _DMS can be made non-serialized and guest could handle
> >>>>> label data in parallel.
> >>>>
> >>>> I think that alignment considerations would mean we are burning up
> >>>> 1G of phys address space for this. For PAE we only have 64G
> >>>> of this address space, so this would be a problem.
> >>> That's true that it will burning away address space, however that
> >>> just means that PAE guests would not be able to handle as many
> >>> NVDIMMs as 64bit guests. The same applies to DIMMs as well, with
> >>> alignment enforced. If one needs more DIMMs he/she can switch
> >>> to 64bit guest to use them.
> >>>
> >>> It's trade of inefficient GPA consumption vs efficient NVDIMMs access.
> >>> Also with fully mapped label area for each NVDIMM we don't have to
> >>> introduce and maintain any guest visible serialization protocol
> >>> (protocol for serializing _DSM via 4K window) which becomes ABI.
> >>
> >> It's true for label access but it is not for the long term as we will
> >> need to support other _DSM commands such as vendor specific command,
> >> PBLK dsm command, also NVDIMM MCE related commands will be introduced
> >> in the future, so we will come back here at that time. :(
> > I believe for block mode NVDIMM would also need per NVDIMM mapping
> > for performance reasons (parallel access).
> > As for the rest could that commands go via MMIO that we usually
> > use for control path?
>
> So both input data and output data go through single MMIO, we need to
> introduce a protocol to pass these data, that is complex?
>
> And is any MMIO we can reuse (more complexer？) or we should allocate this
> MMIO page （the old question - where to allocated?）?
Maybe you could reuse/extend memhotplug IO interface,
or alternatively as Michael suggested add a vendor specific PCI_Config,
I'd suggest PM device for that (hw/acpi/[piix4.c|ihc9.c])
which I like even better since you won't need to care about which ports
to allocate at all.
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-16 11:00 ` Igor Mammedov
@ 2016-02-17 2:04 ` Xiao Guangrong
2016-02-17 17:26 ` Michael S. Tsirkin
0 siblings, 1 reply; 51+ messages in thread
From: Xiao Guangrong @ 2016-02-17 2:04 UTC (permalink / raw)
To: Igor Mammedov
Cc: ehabkost, kvm, Michael S. Tsirkin, gleb, mtosatti, qemu-devel,
stefanha, pbonzini, dan.j.williams, rth
On 02/16/2016 07:00 PM, Igor Mammedov wrote:
> On Tue, 16 Feb 2016 02:35:41 +0800
> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>
>> On 02/16/2016 01:24 AM, Igor Mammedov wrote:
>>> On Mon, 15 Feb 2016 23:53:13 +0800
>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>>
>>>> On 02/15/2016 09:32 PM, Igor Mammedov wrote:
>>>>> On Mon, 15 Feb 2016 13:45:59 +0200
>>>>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>>>>
>>>>>> On Mon, Feb 15, 2016 at 11:47:42AM +0100, Igor Mammedov wrote:
>>>>>>> On Mon, 15 Feb 2016 18:13:38 +0800
>>>>>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>>>>>>
>>>>>>>> On 02/15/2016 05:18 PM, Michael S. Tsirkin wrote:
>>>>>>>>> On Mon, Feb 15, 2016 at 10:11:05AM +0100, Igor Mammedov wrote:
>>>>>>>>>> On Sun, 14 Feb 2016 13:57:27 +0800
>>>>>>>>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>>>>>>>>>
>>>>>>>>>>> On 02/08/2016 07:03 PM, Igor Mammedov wrote:
>>>>>>>>>>>> On Wed, 13 Jan 2016 02:50:05 +0800
>>>>>>>>>>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> 32 bits IO port starting from 0x0a18 in guest is reserved for NVDIMM
>>>>>>>>>>>>> ACPI emulation. The table, NVDIMM_DSM_MEM_FILE, will be patched into
>>>>>>>>>>>>> NVDIMM ACPI binary code
>>>>>>>>>>>>>
>>>>>>>>>>>>> OSPM uses this port to tell QEMU the final address of the DSM memory
>>>>>>>>>>>>> and notify QEMU to emulate the DSM method
>>>>>>>>>>>> Would you need to pass control to QEMU if each NVDIMM had its whole
>>>>>>>>>>>> label area MemoryRegion mapped right after its storage MemoryRegion?
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> No, label data is not mapped into guest's address space and it only
>>>>>>>>>>> can be accessed by DSM method indirectly.
>>>>>>>>>> Yep, per spec label data should be accessed via _DSM but question
>>>>>>>>>> wasn't about it,
>>>>>>>>
>>>>>>>> Ah, sorry, i missed your question.
>>>>>>>>
>>>>>>>>>> Why would one map only 4Kb window and serialize label data
>>>>>>>>>> via it if it could be mapped as whole, that way _DMS method will be
>>>>>>>>>> much less complicated and there won't be need to add/support a protocol
>>>>>>>>>> for its serialization.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Is it ever accessed on data path? If not I prefer the current approach:
>>>>>>>>
>>>>>>>> The label data is only accessed via two DSM commands - Get Namespace Label
>>>>>>>> Data and Set Namespace Label Data, no other place need to be emulated.
>>>>>>>>
>>>>>>>>> limit the window used, the serialization protocol seems rather simple.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Yes.
>>>>>>>>
>>>>>>>> Label data is at least 128k which is big enough for BIOS as it allocates
>>>>>>>> memory at 0 ~ 4G which is tight region. It also needs guest OS to support
>>>>>>>> lager max-xfer (the max size that can be transferred one time), the size
>>>>>>>> in current Linux NVDIMM driver is 4k.
>>>>>>>>
>>>>>>>> However, using lager DSM buffer can help us to simplify NVDIMM hotplug for
>>>>>>>> the case that too many nvdimm devices present in the system and their FIT
>>>>>>>> info can not be filled into one page. Each PMEM-only device needs 0xb8 bytes
>>>>>>>> and we can append 256 memory devices at most, so 12 pages are needed to
>>>>>>>> contain this info. The prototype we implemented is using ourself-defined
>>>>>>>> protocol to read piece of _FIT and concatenate them before return to Guest,
>>>>>>>> please refer to:
>>>>>>>> https://github.com/xiaogr/qemu/commit/c46ce01c8433ac0870670304360b3c4aa414143a
>>>>>>>>
>>>>>>>> As 12 pages are not small region for BIOS and the _FIT size may be extended in the
>>>>>>>> future development (eg, if PBLK is introduced) i am not sure if we need this. Of
>>>>>>>> course, another approach to simplify it is that we limit the number of NVDIMM
>>>>>>>> device to make sure their _FIT < 4k.
>>>>>>> My suggestion is not to have only one label area for every NVDIMM but
>>>>>>> rather to map each label area right after each NVDIMM's data memory.
>>>>>>> That way _DMS can be made non-serialized and guest could handle
>>>>>>> label data in parallel.
>>>>>>
>>>>>> I think that alignment considerations would mean we are burning up
>>>>>> 1G of phys address space for this. For PAE we only have 64G
>>>>>> of this address space, so this would be a problem.
>>>>> That's true that it will burning away address space, however that
>>>>> just means that PAE guests would not be able to handle as many
>>>>> NVDIMMs as 64bit guests. The same applies to DIMMs as well, with
>>>>> alignment enforced. If one needs more DIMMs he/she can switch
>>>>> to 64bit guest to use them.
>>>>>
>>>>> It's trade of inefficient GPA consumption vs efficient NVDIMMs access.
>>>>> Also with fully mapped label area for each NVDIMM we don't have to
>>>>> introduce and maintain any guest visible serialization protocol
>>>>> (protocol for serializing _DSM via 4K window) which becomes ABI.
>>>>
>>>> It's true for label access but it is not for the long term as we will
>>>> need to support other _DSM commands such as vendor specific command,
>>>> PBLK dsm command, also NVDIMM MCE related commands will be introduced
>>>> in the future, so we will come back here at that time. :(
>>> I believe for block mode NVDIMM would also need per NVDIMM mapping
>>> for performance reasons (parallel access).
>>> As for the rest could that commands go via MMIO that we usually
>>> use for control path?
>>
>> So both input data and output data go through single MMIO, we need to
>> introduce a protocol to pass these data, that is complex?
>>
>> And is any MMIO we can reuse (more complexer？) or we should allocate this
>> MMIO page （the old question - where to allocated?）?
> Maybe you could reuse/extend memhotplug IO interface,
> or alternatively as Michael suggested add a vendor specific PCI_Config,
> I'd suggest PM device for that (hw/acpi/[piix4.c|ihc9.c])
> which I like even better since you won't need to care about which ports
> to allocate at all.
Well, if Michael does not object, i will do it in the next version. :)
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-17 2:04 ` Xiao Guangrong
@ 2016-02-17 17:26 ` Michael S. Tsirkin
2016-02-18 4:03 ` Xiao Guangrong
0 siblings, 1 reply; 51+ messages in thread
From: Michael S. Tsirkin @ 2016-02-17 17:26 UTC (permalink / raw)
To: Xiao Guangrong
Cc: ehabkost, kvm, gleb, mtosatti, qemu-devel, stefanha, pbonzini,
Igor Mammedov, dan.j.williams, rth
On Wed, Feb 17, 2016 at 10:04:18AM +0800, Xiao Guangrong wrote:
> >>>As for the rest could that commands go via MMIO that we usually
> >>>use for control path?
> >>
> >>So both input data and output data go through single MMIO, we need to
> >>introduce a protocol to pass these data, that is complex?
> >>
> >>And is any MMIO we can reuse (more complexer？) or we should allocate this
> >>MMIO page （the old question - where to allocated?）?
> >Maybe you could reuse/extend memhotplug IO interface,
> >or alternatively as Michael suggested add a vendor specific PCI_Config,
> >I'd suggest PM device for that (hw/acpi/[piix4.c|ihc9.c])
> >which I like even better since you won't need to care about which ports
> >to allocate at all.
>
> Well, if Michael does not object, i will do it in the next version. :)
Sorry, the thread's so long by now that I'm no longer sure what does "it" refer to.
--
MST
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-17 17:26 ` Michael S. Tsirkin
@ 2016-02-18 4:03 ` Xiao Guangrong
2016-02-18 10:05 ` Igor Mammedov
2016-02-18 10:20 ` Michael S. Tsirkin
0 siblings, 2 replies; 51+ messages in thread
From: Xiao Guangrong @ 2016-02-18 4:03 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: ehabkost, kvm, gleb, mtosatti, qemu-devel, stefanha, pbonzini,
Igor Mammedov, dan.j.williams, rth
On 02/18/2016 01:26 AM, Michael S. Tsirkin wrote:
> On Wed, Feb 17, 2016 at 10:04:18AM +0800, Xiao Guangrong wrote:
>>>>> As for the rest could that commands go via MMIO that we usually
>>>>> use for control path?
>>>>
>>>> So both input data and output data go through single MMIO, we need to
>>>> introduce a protocol to pass these data, that is complex?
>>>>
>>>> And is any MMIO we can reuse (more complexer？) or we should allocate this
>>>> MMIO page （the old question - where to allocated?）?
>>> Maybe you could reuse/extend memhotplug IO interface,
>>> or alternatively as Michael suggested add a vendor specific PCI_Config,
>>> I'd suggest PM device for that (hw/acpi/[piix4.c|ihc9.c])
>>> which I like even better since you won't need to care about which ports
>>> to allocate at all.
>>
>> Well, if Michael does not object, i will do it in the next version. :)
>
> Sorry, the thread's so long by now that I'm no longer sure what does "it" refer to.
Never mind i saw you were busy on other loops.
"It" means the suggestion of Igor that "map each label area right after each
NVDIMM's data memory" so we do not emulate it in QEMU and is good for the performance
of label these are the points i like. However it also brings complexity/limitation for
later DSM commands supports since both dsm input & output need to go through single MMIO.
Your idea?
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-18 4:03 ` Xiao Guangrong
@ 2016-02-18 10:05 ` Igor Mammedov
2016-02-19 8:08 ` Michael S. Tsirkin
2016-02-18 10:20 ` Michael S. Tsirkin
1 sibling, 1 reply; 51+ messages in thread
From: Igor Mammedov @ 2016-02-18 10:05 UTC (permalink / raw)
To: Xiao Guangrong
Cc: ehabkost, kvm, Michael S. Tsirkin, gleb, mtosatti, qemu-devel,
stefanha, pbonzini, dan.j.williams, rth
On Thu, 18 Feb 2016 12:03:36 +0800
Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> On 02/18/2016 01:26 AM, Michael S. Tsirkin wrote:
> > On Wed, Feb 17, 2016 at 10:04:18AM +0800, Xiao Guangrong wrote:
> >>>>> As for the rest could that commands go via MMIO that we usually
> >>>>> use for control path?
> >>>>
> >>>> So both input data and output data go through single MMIO, we need to
> >>>> introduce a protocol to pass these data, that is complex?
> >>>>
> >>>> And is any MMIO we can reuse (more complexer？) or we should allocate this
> >>>> MMIO page （the old question - where to allocated?）?
> >>> Maybe you could reuse/extend memhotplug IO interface,
> >>> or alternatively as Michael suggested add a vendor specific PCI_Config,
> >>> I'd suggest PM device for that (hw/acpi/[piix4.c|ihc9.c])
> >>> which I like even better since you won't need to care about which ports
> >>> to allocate at all.
> >>
> >> Well, if Michael does not object, i will do it in the next version. :)
> >
> > Sorry, the thread's so long by now that I'm no longer sure what does "it" refer to.
>
> Never mind i saw you were busy on other loops.
>
> "It" means the suggestion of Igor that "map each label area right after each
> NVDIMM's data memory"
Michael pointed out that putting label right after each NVDIMM
might burn up to 256GB of address space due to DIMM's alignment for 256 NVDIMMs.
However if address for each label is picked with pc_dimm_get_free_addr()
and label's MemoryRegion alignment is default 2MB then all labels
would be allocated close to each other within a single 1GB range.
That would burn only 1GB for 500 labels which is more than possible 256 NVDIMMs.
Assuming labels are mapped before storage MemoryRegion is mapped, layout with 1Gb hugepage backend CLI
-device nvdimm,size=1G -device nvdimm,size=1G -device nvdimm,size=1G
would look like:
0 2M 4M 1G 2G 3G 4G
L1 | L2 | L3 ... | NV1 | NV2 | NV3 |
> so we do not emulate it in QEMU and is good for the performance
> of label these are the points i like. However it also brings complexity/limitation for
> later DSM commands supports since both dsm input & output need to go through single MMIO.
>
> Your idea?
>
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-18 10:05 ` Igor Mammedov
@ 2016-02-19 8:08 ` Michael S. Tsirkin
2016-02-19 8:43 ` Dan Williams
2016-02-22 10:34 ` Xiao Guangrong
0 siblings, 2 replies; 51+ messages in thread
From: Michael S. Tsirkin @ 2016-02-19 8:08 UTC (permalink / raw)
To: Igor Mammedov
Cc: Xiao Guangrong, ehabkost, kvm, gleb, mtosatti, qemu-devel,
stefanha, pbonzini, dan.j.williams, rth
On Thu, Feb 18, 2016 at 11:05:23AM +0100, Igor Mammedov wrote:
> On Thu, 18 Feb 2016 12:03:36 +0800
> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>
> > On 02/18/2016 01:26 AM, Michael S. Tsirkin wrote:
> > > On Wed, Feb 17, 2016 at 10:04:18AM +0800, Xiao Guangrong wrote:
> > >>>>> As for the rest could that commands go via MMIO that we usually
> > >>>>> use for control path?
> > >>>>
> > >>>> So both input data and output data go through single MMIO, we need to
> > >>>> introduce a protocol to pass these data, that is complex?
> > >>>>
> > >>>> And is any MMIO we can reuse (more complexer？) or we should allocate this
> > >>>> MMIO page （the old question - where to allocated?）?
> > >>> Maybe you could reuse/extend memhotplug IO interface,
> > >>> or alternatively as Michael suggested add a vendor specific PCI_Config,
> > >>> I'd suggest PM device for that (hw/acpi/[piix4.c|ihc9.c])
> > >>> which I like even better since you won't need to care about which ports
> > >>> to allocate at all.
> > >>
> > >> Well, if Michael does not object, i will do it in the next version. :)
> > >
> > > Sorry, the thread's so long by now that I'm no longer sure what does "it" refer to.
> >
> > Never mind i saw you were busy on other loops.
> >
> > "It" means the suggestion of Igor that "map each label area right after each
> > NVDIMM's data memory"
> Michael pointed out that putting label right after each NVDIMM
> might burn up to 256GB of address space due to DIMM's alignment for 256 NVDIMMs.
> However if address for each label is picked with pc_dimm_get_free_addr()
> and label's MemoryRegion alignment is default 2MB then all labels
> would be allocated close to each other within a single 1GB range.
>
> That would burn only 1GB for 500 labels which is more than possible 256 NVDIMMs.
I thought about it, once we support hotplug, this means that one will
have to pre-declare how much is needed so QEMU can mark the correct
memory reserved, that would be nasty. Maybe we always pre-reserve 1Gbyte.
Okay but next time we need something, do we steal another Gigabyte?
It seems too much, I'll think it over on the weekend.
Really, most other devices manage to get by with 4K chunks just fine, I
don't see why do we are so special and need to steal gigabytes of
physically contigious phy ranges.
> Assuming labels are mapped before storage MemoryRegion is mapped, layout with 1Gb hugepage backend CLI
> -device nvdimm,size=1G -device nvdimm,size=1G -device nvdimm,size=1G
> would look like:
>
> 0 2M 4M 1G 2G 3G 4G
> L1 | L2 | L3 ... | NV1 | NV2 | NV3 |
>
> > so we do not emulate it in QEMU and is good for the performance
> > of label these are the points i like. However it also brings complexity/limitation for
> > later DSM commands supports since both dsm input & output need to go through single MMIO.
> >
> > Your idea?
> >
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-19 8:08 ` Michael S. Tsirkin
@ 2016-02-19 8:43 ` Dan Williams
2016-02-22 10:30 ` Xiao Guangrong
2016-02-22 10:34 ` Xiao Guangrong
1 sibling, 1 reply; 51+ messages in thread
From: Dan Williams @ 2016-02-19 8:43 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Xiao Guangrong, ehabkost, KVM list, Gleb Natapov, mtosatti,
qemu-devel, stefanha, Paolo Bonzini, Igor Mammedov, rth
On Fri, Feb 19, 2016 at 12:08 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Thu, Feb 18, 2016 at 11:05:23AM +0100, Igor Mammedov wrote:
>> On Thu, 18 Feb 2016 12:03:36 +0800
>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>
>> > On 02/18/2016 01:26 AM, Michael S. Tsirkin wrote:
>> > > On Wed, Feb 17, 2016 at 10:04:18AM +0800, Xiao Guangrong wrote:
>> > >>>>> As for the rest could that commands go via MMIO that we usually
>> > >>>>> use for control path?
>> > >>>>
>> > >>>> So both input data and output data go through single MMIO, we need to
>> > >>>> introduce a protocol to pass these data, that is complex?
>> > >>>>
>> > >>>> And is any MMIO we can reuse (more complexer？) or we should allocate this
>> > >>>> MMIO page （the old question - where to allocated?）?
>> > >>> Maybe you could reuse/extend memhotplug IO interface,
>> > >>> or alternatively as Michael suggested add a vendor specific PCI_Config,
>> > >>> I'd suggest PM device for that (hw/acpi/[piix4.c|ihc9.c])
>> > >>> which I like even better since you won't need to care about which ports
>> > >>> to allocate at all.
>> > >>
>> > >> Well, if Michael does not object, i will do it in the next version. :)
>> > >
>> > > Sorry, the thread's so long by now that I'm no longer sure what does "it" refer to.
>> >
>> > Never mind i saw you were busy on other loops.
>> >
>> > "It" means the suggestion of Igor that "map each label area right after each
>> > NVDIMM's data memory"
>> Michael pointed out that putting label right after each NVDIMM
>> might burn up to 256GB of address space due to DIMM's alignment for 256 NVDIMMs.
>> However if address for each label is picked with pc_dimm_get_free_addr()
>> and label's MemoryRegion alignment is default 2MB then all labels
>> would be allocated close to each other within a single 1GB range.
>>
>> That would burn only 1GB for 500 labels which is more than possible 256 NVDIMMs.
>
> I thought about it, once we support hotplug, this means that one will
> have to pre-declare how much is needed so QEMU can mark the correct
> memory reserved, that would be nasty. Maybe we always pre-reserve 1Gbyte.
> Okay but next time we need something, do we steal another Gigabyte?
> It seems too much, I'll think it over on the weekend.
>
> Really, most other devices manage to get by with 4K chunks just fine, I
> don't see why do we are so special and need to steal gigabytes of
> physically contigious phy ranges.
What's the driving use case for labels in the guest? For example,
NVDIMM-N devices are supported by the kernel without labels.
I certainly would not want to sacrifice 1GB alignment for a label area.
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-19 8:43 ` Dan Williams
@ 2016-02-22 10:30 ` Xiao Guangrong
0 siblings, 0 replies; 51+ messages in thread
From: Xiao Guangrong @ 2016-02-22 10:30 UTC (permalink / raw)
To: Dan Williams, Michael S. Tsirkin
Cc: ehabkost, KVM list, Gleb Natapov, mtosatti, qemu-devel, stefanha,
Paolo Bonzini, Igor Mammedov, rth
On 02/19/2016 04:43 PM, Dan Williams wrote:
> On Fri, Feb 19, 2016 at 12:08 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> On Thu, Feb 18, 2016 at 11:05:23AM +0100, Igor Mammedov wrote:
>>> On Thu, 18 Feb 2016 12:03:36 +0800
>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>>
>>>> On 02/18/2016 01:26 AM, Michael S. Tsirkin wrote:
>>>>> On Wed, Feb 17, 2016 at 10:04:18AM +0800, Xiao Guangrong wrote:
>>>>>>>>> As for the rest could that commands go via MMIO that we usually
>>>>>>>>> use for control path?
>>>>>>>>
>>>>>>>> So both input data and output data go through single MMIO, we need to
>>>>>>>> introduce a protocol to pass these data, that is complex?
>>>>>>>>
>>>>>>>> And is any MMIO we can reuse (more complexer？) or we should allocate this
>>>>>>>> MMIO page （the old question - where to allocated?）?
>>>>>>> Maybe you could reuse/extend memhotplug IO interface,
>>>>>>> or alternatively as Michael suggested add a vendor specific PCI_Config,
>>>>>>> I'd suggest PM device for that (hw/acpi/[piix4.c|ihc9.c])
>>>>>>> which I like even better since you won't need to care about which ports
>>>>>>> to allocate at all.
>>>>>>
>>>>>> Well, if Michael does not object, i will do it in the next version. :)
>>>>>
>>>>> Sorry, the thread's so long by now that I'm no longer sure what does "it" refer to.
>>>>
>>>> Never mind i saw you were busy on other loops.
>>>>
>>>> "It" means the suggestion of Igor that "map each label area right after each
>>>> NVDIMM's data memory"
>>> Michael pointed out that putting label right after each NVDIMM
>>> might burn up to 256GB of address space due to DIMM's alignment for 256 NVDIMMs.
>>> However if address for each label is picked with pc_dimm_get_free_addr()
>>> and label's MemoryRegion alignment is default 2MB then all labels
>>> would be allocated close to each other within a single 1GB range.
>>>
>>> That would burn only 1GB for 500 labels which is more than possible 256 NVDIMMs.
>>
>> I thought about it, once we support hotplug, this means that one will
>> have to pre-declare how much is needed so QEMU can mark the correct
>> memory reserved, that would be nasty. Maybe we always pre-reserve 1Gbyte.
>> Okay but next time we need something, do we steal another Gigabyte?
>> It seems too much, I'll think it over on the weekend.
>>
>> Really, most other devices manage to get by with 4K chunks just fine, I
>> don't see why do we are so special and need to steal gigabytes of
>> physically contigious phy ranges.
>
> What's the driving use case for labels in the guest? For example,
> NVDIMM-N devices are supported by the kernel without labels.
Yes, I see Linux driver supports label-less vNVDIMM that is exact current QEMU
doing. However, label-less is only Linux specific implementation (as it
completely bypasses namespace), other OS vendors (e.g Microsoft) will use label
storage to address their own requirements，or they do not follow namespace spec
at all. Another reason is that label is essential for PBLK support.
BTW, the label support can be dynamically configured and it will be disabled
on default.
>
> I certainly would not want to sacrifice 1GB alignment for a label area.
>
Yup, me too.
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-19 8:08 ` Michael S. Tsirkin
2016-02-19 8:43 ` Dan Williams
@ 2016-02-22 10:34 ` Xiao Guangrong
1 sibling, 0 replies; 51+ messages in thread
From: Xiao Guangrong @ 2016-02-22 10:34 UTC (permalink / raw)
To: Michael S. Tsirkin, Igor Mammedov
Cc: ehabkost, kvm, gleb, mtosatti, qemu-devel, stefanha, pbonzini,
dan.j.williams, rth
On 02/19/2016 04:08 PM, Michael S. Tsirkin wrote:
> On Thu, Feb 18, 2016 at 11:05:23AM +0100, Igor Mammedov wrote:
>> On Thu, 18 Feb 2016 12:03:36 +0800
>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>
>>> On 02/18/2016 01:26 AM, Michael S. Tsirkin wrote:
>>>> On Wed, Feb 17, 2016 at 10:04:18AM +0800, Xiao Guangrong wrote:
>>>>>>>> As for the rest could that commands go via MMIO that we usually
>>>>>>>> use for control path?
>>>>>>>
>>>>>>> So both input data and output data go through single MMIO, we need to
>>>>>>> introduce a protocol to pass these data, that is complex?
>>>>>>>
>>>>>>> And is any MMIO we can reuse (more complexer？) or we should allocate this
>>>>>>> MMIO page （the old question - where to allocated?）?
>>>>>> Maybe you could reuse/extend memhotplug IO interface,
>>>>>> or alternatively as Michael suggested add a vendor specific PCI_Config,
>>>>>> I'd suggest PM device for that (hw/acpi/[piix4.c|ihc9.c])
>>>>>> which I like even better since you won't need to care about which ports
>>>>>> to allocate at all.
>>>>>
>>>>> Well, if Michael does not object, i will do it in the next version. :)
>>>>
>>>> Sorry, the thread's so long by now that I'm no longer sure what does "it" refer to.
>>>
>>> Never mind i saw you were busy on other loops.
>>>
>>> "It" means the suggestion of Igor that "map each label area right after each
>>> NVDIMM's data memory"
>> Michael pointed out that putting label right after each NVDIMM
>> might burn up to 256GB of address space due to DIMM's alignment for 256 NVDIMMs.
>> However if address for each label is picked with pc_dimm_get_free_addr()
>> and label's MemoryRegion alignment is default 2MB then all labels
>> would be allocated close to each other within a single 1GB range.
>>
>> That would burn only 1GB for 500 labels which is more than possible 256 NVDIMMs.
>
> I thought about it, once we support hotplug, this means that one will
> have to pre-declare how much is needed so QEMU can mark the correct
> memory reserved, that would be nasty. Maybe we always pre-reserve 1Gbyte.
> Okay but next time we need something, do we steal another Gigabyte?
> It seems too much, I'll think it over on the weekend.
>
It sounds like this approach (reserve-and-allocate) is very similar as the
old propose that dynamically allocates memory from the end of hotplug-mem. :)
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-18 4:03 ` Xiao Guangrong
2016-02-18 10:05 ` Igor Mammedov
@ 2016-02-18 10:20 ` Michael S. Tsirkin
1 sibling, 0 replies; 51+ messages in thread
From: Michael S. Tsirkin @ 2016-02-18 10:20 UTC (permalink / raw)
To: Xiao Guangrong
Cc: ehabkost, kvm, gleb, mtosatti, qemu-devel, stefanha, pbonzini,
Igor Mammedov, dan.j.williams, rth
On Thu, Feb 18, 2016 at 12:03:36PM +0800, Xiao Guangrong wrote:
>
>
> On 02/18/2016 01:26 AM, Michael S. Tsirkin wrote:
> >On Wed, Feb 17, 2016 at 10:04:18AM +0800, Xiao Guangrong wrote:
> >>>>>As for the rest could that commands go via MMIO that we usually
> >>>>>use for control path?
> >>>>
> >>>>So both input data and output data go through single MMIO, we need to
> >>>>introduce a protocol to pass these data, that is complex?
> >>>>
> >>>>And is any MMIO we can reuse (more complexer？) or we should allocate this
> >>>>MMIO page （the old question - where to allocated?）?
> >>>Maybe you could reuse/extend memhotplug IO interface,
> >>>or alternatively as Michael suggested add a vendor specific PCI_Config,
> >>>I'd suggest PM device for that (hw/acpi/[piix4.c|ihc9.c])
> >>>which I like even better since you won't need to care about which ports
> >>>to allocate at all.
> >>
> >>Well, if Michael does not object, i will do it in the next version. :)
> >
> >Sorry, the thread's so long by now that I'm no longer sure what does "it" refer to.
>
> Never mind i saw you were busy on other loops.
>
> "It" means the suggestion of Igor that "map each label area right after each
> NVDIMM's data memory" so we do not emulate it in QEMU and is good for the performance
> of label these are the points i like. However it also brings complexity/limitation for
> later DSM commands supports since both dsm input & output need to go through single MMIO.
>
> Your idea?
I think mapping right after data is problematic since it might
use 1G of address space if alignment is used (and alignment is
good for performance, so we typically do want people to use it).
Given you will add more DSM commands anyway,
I don't see a reason not to pass label data this way too.
I don't care much how are commands passed exactly.
Igor, do you have a preference or if it's not MMIO beyong DIMM
then you don't care?
--
MST
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI
2016-02-15 10:13 ` Xiao Guangrong
2016-02-15 10:30 ` Michael S. Tsirkin
2016-02-15 10:47 ` Igor Mammedov
@ 2016-02-15 11:36 ` Michael S. Tsirkin
2 siblings, 0 replies; 51+ messages in thread
From: Michael S. Tsirkin @ 2016-02-15 11:36 UTC (permalink / raw)
To: Xiao Guangrong
Cc: ehabkost, kvm, gleb, mtosatti, qemu-devel, stefanha, pbonzini,
Igor Mammedov, dan.j.williams, rth
On Mon, Feb 15, 2016 at 06:13:38PM +0800, Xiao Guangrong wrote:
>
>
> On 02/15/2016 05:18 PM, Michael S. Tsirkin wrote:
> >On Mon, Feb 15, 2016 at 10:11:05AM +0100, Igor Mammedov wrote:
> >>On Sun, 14 Feb 2016 13:57:27 +0800
> >>Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> >>
> >>>On 02/08/2016 07:03 PM, Igor Mammedov wrote:
> >>>>On Wed, 13 Jan 2016 02:50:05 +0800
> >>>>Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> >>>>
> >>>>>32 bits IO port starting from 0x0a18 in guest is reserved for NVDIMM
> >>>>>ACPI emulation. The table, NVDIMM_DSM_MEM_FILE, will be patched into
> >>>>>NVDIMM ACPI binary code
> >>>>>
> >>>>>OSPM uses this port to tell QEMU the final address of the DSM memory
> >>>>>and notify QEMU to emulate the DSM method
> >>>>Would you need to pass control to QEMU if each NVDIMM had its whole
> >>>>label area MemoryRegion mapped right after its storage MemoryRegion?
> >>>>
> >>>
> >>>No, label data is not mapped into guest's address space and it only
> >>>can be accessed by DSM method indirectly.
> >>Yep, per spec label data should be accessed via _DSM but question
> >>wasn't about it,
>
> Ah, sorry, i missed your question.
>
> >>Why would one map only 4Kb window and serialize label data
> >>via it if it could be mapped as whole, that way _DMS method will be
> >>much less complicated and there won't be need to add/support a protocol
> >>for its serialization.
> >>
> >
> >Is it ever accessed on data path? If not I prefer the current approach:
>
> The label data is only accessed via two DSM commands - Get Namespace Label
> Data and Set Namespace Label Data, no other place need to be emulated.
>
> >limit the window used, the serialization protocol seems rather simple.
> >
>
> Yes.
>
> Label data is at least 128k which is big enough for BIOS as it allocates
> memory at 0 ~ 4G which is tight region. It also needs guest OS to support
> lager max-xfer (the max size that can be transferred one time), the size
> in current Linux NVDIMM driver is 4k.
>
> However, using lager DSM buffer can help us to simplify NVDIMM hotplug for
> the case that too many nvdimm devices present in the system and their FIT
> info can not be filled into one page. Each PMEM-only device needs 0xb8 bytes
> and we can append 256 memory devices at most, so 12 pages are needed to
> contain this info.
BTW 12 pages isn't too scary even for BIOS.
> The prototype we implemented is using ourself-defined
> protocol to read piece of _FIT and concatenate them before return to Guest,
> please refer to:
> https://github.com/xiaogr/qemu/commit/c46ce01c8433ac0870670304360b3c4aa414143a
>
> As 12 pages are not small region for BIOS and the _FIT size may be extended in the
> future development (eg, if PBLK is introduced) i am not sure if we need this. Of
> course, another approach to simplify it is that we limit the number of NVDIMM
> device to make sure their _FIT < 4k.
>
>
>
^ permalink raw reply [flat|nested] 51+ messages in thread
* [Qemu-devel] [PATCH v2 07/11] nvdimm acpi: introduce patched dsm memory
2016-01-12 18:49 [Qemu-devel] [PATCH v2 00/11] NVDIMM ACPI: introduce the framework of QEMU emulated Xiao Guangrong
` (5 preceding siblings ...)
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 06/11] nvdimm acpi: initialize the resource used by NVDIMM ACPI Xiao Guangrong
@ 2016-01-12 18:50 ` Xiao Guangrong
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 08/11] nvdimm acpi: let qemu handle _DSM method Xiao Guangrong
` (5 subsequent siblings)
12 siblings, 0 replies; 51+ messages in thread
From: Xiao Guangrong @ 2016-01-12 18:50 UTC (permalink / raw)
To: pbonzini, imammedo
Cc: Xiao Guangrong, ehabkost, kvm, mst, gleb, mtosatti, qemu-devel,
stefanha, dan.j.williams, rth
The dsm memory is used to save the input parameters and store
the dsm result which is filled by QEMU.
The address of dsm memory is decided by bios and patched into
int32 object named "MEMA"
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
hw/acpi/nvdimm.c | 34 +++++++++++++++++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 256cedd..e7acc98 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -28,6 +28,7 @@
#include "hw/acpi/acpi.h"
#include "hw/acpi/aml-build.h"
+#include "hw/acpi/bios-linker-loader.h"
#include "hw/nvram/fw_cfg.h"
#include "hw/mem/nvdimm.h"
@@ -405,6 +406,7 @@ void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
}
#define NVDIMM_COMMON_DSM "NCAL"
+#define NVDIMM_ACPI_MEM_ADDR "MEMA"
static void nvdimm_build_common_dsm(Aml *dev)
{
@@ -469,7 +471,9 @@ static void nvdimm_build_nvdimm_devices(GSList *device_list, Aml *root_dev)
static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
GArray *table_data, GArray *linker)
{
- Aml *ssdt, *sb_scope, *dev;
+ Aml *ssdt, *sb_scope, *dev, *mem_addr;
+ uint32_t zero_offset = 0;
+ int offset;
acpi_add_table(table_offsets, table_data);
@@ -500,9 +504,37 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
aml_append(sb_scope, dev);
+ /*
+ * leave it at the end of ssdt so that we can conveniently get the
+ * offset of int32 object which will be patched with the real address
+ * of the dsm memory by BIOS.
+ *
+ * 0x32000000 is the magic number to let aml_int() create int32 object.
+ * It will be zeroed later to make bios_linker_loader_add_pointer()
+ * happy.
+ */
+ mem_addr = aml_name_decl(NVDIMM_ACPI_MEM_ADDR, aml_int(0x32000000));
+
+ aml_append(sb_scope, mem_addr);
aml_append(ssdt, sb_scope);
/* copy AML table into ACPI tables blob and patch header there */
g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len);
+
+ offset = table_data->len - 4;
+
+ /*
+ * zero the last 4 bytes, i.e, it is the offset of
+ * NVDIMM_ACPI_MEM_ADDR object.
+ */
+ g_array_remove_range(table_data, offset, 4);
+ g_array_append_vals(table_data, &zero_offset, 4);
+
+ bios_linker_loader_alloc(linker, NVDIMM_DSM_MEM_FILE, TARGET_PAGE_SIZE,
+ false /* high memory */);
+ bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
+ NVDIMM_DSM_MEM_FILE, table_data,
+ table_data->data + offset,
+ sizeof(uint32_t));
build_header(linker, table_data,
(void *)(table_data->data + table_data->len - ssdt->buf->len),
"SSDT", ssdt->buf->len, 1, "NVDIMM");
--
1.8.3.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* [Qemu-devel] [PATCH v2 08/11] nvdimm acpi: let qemu handle _DSM method
2016-01-12 18:49 [Qemu-devel] [PATCH v2 00/11] NVDIMM ACPI: introduce the framework of QEMU emulated Xiao Guangrong
` (6 preceding siblings ...)
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 07/11] nvdimm acpi: introduce patched dsm memory Xiao Guangrong
@ 2016-01-12 18:50 ` Xiao Guangrong
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 09/11] nvdimm acpi: emulate dsm method Xiao Guangrong
` (4 subsequent siblings)
12 siblings, 0 replies; 51+ messages in thread
From: Xiao Guangrong @ 2016-01-12 18:50 UTC (permalink / raw)
To: pbonzini, imammedo
Cc: Xiao Guangrong, ehabkost, kvm, mst, gleb, mtosatti, qemu-devel,
stefanha, dan.j.williams, rth
If dsm memory is successfully patched, we let qemu fully emulate
the dsm method
This patch saves _DSM input parameters into dsm memory, tell dsm
memory address to QEMU, then fetch the result from the dsm memory
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
hw/acpi/nvdimm.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 112 insertions(+), 5 deletions(-)
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index e7acc98..358e340 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -371,6 +371,24 @@ static void nvdimm_build_nfit(GSList *device_list, GArray *table_offsets,
g_array_free(structures, true);
}
+struct NvdimmDsmIn {
+ uint32_t handle;
+ uint32_t revision;
+ uint32_t function;
+ /* the remaining size in the page is used by arg3. */
+ union {
+ uint8_t arg3[0];
+ };
+} QEMU_PACKED;
+typedef struct NvdimmDsmIn NvdimmDsmIn;
+
+struct NvdimmDsmOut {
+ /* the size of buffer filled by QEMU. */
+ uint32_t len;
+ uint8_t data[0];
+} QEMU_PACKED;
+typedef struct NvdimmDsmOut NvdimmDsmOut;
+
static uint64_t
nvdimm_dsm_read(void *opaque, hwaddr addr, unsigned size)
{
@@ -410,11 +428,18 @@ void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
static void nvdimm_build_common_dsm(Aml *dev)
{
- Aml *method, *ifctx, *function;
+ Aml *method, *ifctx, *function, *dsm_mem, *unpatched, *result_size;
uint8_t byte_list[1];
- method = aml_method(NVDIMM_COMMON_DSM, 4, AML_NOTSERIALIZED);
+ method = aml_method(NVDIMM_COMMON_DSM, 4, AML_SERIALIZED);
function = aml_arg(2);
+ dsm_mem = aml_name(NVDIMM_ACPI_MEM_ADDR);
+
+ /*
+ * do not support any method if DSM memory address has not been
+ * patched.
+ */
+ unpatched = aml_if(aml_equal(dsm_mem, aml_int(0x0)));
/*
* function 0 is called to inquire what functions are supported by
@@ -423,12 +448,36 @@ static void nvdimm_build_common_dsm(Aml *dev)
ifctx = aml_if(aml_equal(function, aml_int(0)));
byte_list[0] = 0 /* No function Supported */;
aml_append(ifctx, aml_return(aml_buffer(1, byte_list)));
- aml_append(method, ifctx);
+ aml_append(unpatched, ifctx);
/* No function is supported yet. */
byte_list[0] = 1 /* Not Supported */;
- aml_append(method, aml_return(aml_buffer(1, byte_list)));
+ aml_append(unpatched, aml_return(aml_buffer(1, byte_list)));
+ aml_append(method, unpatched);
+
+ /*
+ * Currently no function is supported for both root device and NVDIMM
+ * devices, let's temporarily set handle to 0x0 at this time.
+ */
+ aml_append(method, aml_store(aml_int(0x0), aml_name("HDLE")));
+ aml_append(method, aml_store(aml_arg(1), aml_name("REVS")));
+ aml_append(method, aml_store(aml_arg(2), aml_name("FUNC")));
+ /*
+ * tell QEMU about the real address of DSM memory, then QEMU begins
+ * to emulate the method and fills the result to DSM memory.
+ */
+ aml_append(method, aml_store(dsm_mem, aml_name("NTFI")));
+
+ result_size = aml_local(1);
+ aml_append(method, aml_store(aml_name("RLEN"), result_size));
+ aml_append(method, aml_store(aml_shiftleft(result_size, aml_int(3)),
+ result_size));
+ aml_append(method, aml_create_field(aml_name("ODAT"), aml_int(0),
+ result_size, "OBUF"));
+ aml_append(method, aml_concatenate(aml_buffer(0, NULL), aml_name("OBUF"),
+ aml_arg(6)));
+ aml_append(method, aml_return(aml_arg(6)));
aml_append(dev, method);
}
@@ -471,7 +520,7 @@ static void nvdimm_build_nvdimm_devices(GSList *device_list, Aml *root_dev)
static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
GArray *table_data, GArray *linker)
{
- Aml *ssdt, *sb_scope, *dev, *mem_addr;
+ Aml *ssdt, *sb_scope, *dev, *field, *mem_addr;
uint32_t zero_offset = 0;
int offset;
@@ -497,6 +546,64 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
*/
aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0012")));
+ /* map DSM memory and IO into ACPI namespace. */
+ aml_append(dev, aml_operation_region("NPIO", AML_SYSTEM_IO,
+ aml_int(NVDIMM_ACPI_IO_BASE), NVDIMM_ACPI_IO_LEN));
+ aml_append(dev, aml_operation_region("NRAM", AML_SYSTEM_MEMORY,
+ aml_name(NVDIMM_ACPI_MEM_ADDR), TARGET_PAGE_SIZE));
+
+ /*
+ * DSM notifier:
+ * NTFI: write the address of DSM memory and notify QEMU to emulate
+ * the access.
+ *
+ * It is the IO port so that accessing them will cause VM-exit, the
+ * control will be transferred to QEMU.
+ */
+ field = aml_field("NPIO", AML_DWORD_ACC, AML_NOLOCK, AML_PRESERVE);
+ aml_append(field, aml_named_field("NTFI",
+ sizeof(uint32_t) * BITS_PER_BYTE));
+ aml_append(dev, field);
+
+ /*
+ * DSM input:
+ * @HDLE: store device's handle, it's zero if the _DSM call happens
+ * on NVDIMM Root Device.
+ * @REVS: store the Arg1 of _DSM call.
+ * @FUNC: store the Arg2 of _DSM call.
+ * @ARG3: store the Arg3 of _DSM call.
+ *
+ * They are RAM mapping on host so that these accesses never cause
+ * VM-EXIT.
+ */
+ field = aml_field("NRAM", AML_DWORD_ACC, AML_NOLOCK, AML_PRESERVE);
+ aml_append(field, aml_named_field("HDLE",
+ sizeof(typeof_field(NvdimmDsmIn, handle)) * BITS_PER_BYTE));
+ aml_append(field, aml_named_field("REVS",
+ sizeof(typeof_field(NvdimmDsmIn, revision)) * BITS_PER_BYTE));
+ aml_append(field, aml_named_field("FUNC",
+ sizeof(typeof_field(NvdimmDsmIn, function)) * BITS_PER_BYTE));
+ aml_append(field, aml_named_field("ARG3",
+ (TARGET_PAGE_SIZE - offsetof(NvdimmDsmIn, arg3)) *
+ BITS_PER_BYTE));
+ aml_append(dev, field);
+
+ /*
+ * DSM output:
+ * @RLEN: the size of the buffer filled by QEMU.
+ * @ODAT: the buffer QEMU uses to store the result.
+ *
+ * Since the page is reused by both input and out, the input data
+ * will be lost after storing new result into @ODAT.
+ */
+ field = aml_field("NRAM", AML_DWORD_ACC, AML_NOLOCK, AML_PRESERVE);
+ aml_append(field, aml_named_field("RLEN",
+ sizeof(typeof_field(NvdimmDsmOut, len)) * BITS_PER_BYTE));
+ aml_append(field, aml_named_field("ODAT",
+ (TARGET_PAGE_SIZE - offsetof(NvdimmDsmOut, data)) *
+ BITS_PER_BYTE));
+ aml_append(dev, field);
+
nvdimm_build_common_dsm(dev);
nvdimm_build_device_dsm(dev);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* [Qemu-devel] [PATCH v2 09/11] nvdimm acpi: emulate dsm method
2016-01-12 18:49 [Qemu-devel] [PATCH v2 00/11] NVDIMM ACPI: introduce the framework of QEMU emulated Xiao Guangrong
` (7 preceding siblings ...)
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 08/11] nvdimm acpi: let qemu handle _DSM method Xiao Guangrong
@ 2016-01-12 18:50 ` Xiao Guangrong
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 10/11] nvdimm acpi: add _CRS Xiao Guangrong
` (3 subsequent siblings)
12 siblings, 0 replies; 51+ messages in thread
From: Xiao Guangrong @ 2016-01-12 18:50 UTC (permalink / raw)
To: pbonzini, imammedo
Cc: Xiao Guangrong, ehabkost, kvm, mst, gleb, mtosatti, qemu-devel,
stefanha, dan.j.williams, rth
Emulate dsm method after IO VM-exit
Currently, we only introduce the framework and no function is actually
supported
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
hw/acpi/aml-build.c | 2 +-
hw/acpi/nvdimm.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
include/hw/acpi/aml-build.h | 1 +
include/hw/mem/nvdimm.h | 8 ++++++++
4 files changed, 54 insertions(+), 1 deletion(-)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 208f22e..e9841be 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -231,7 +231,7 @@ static void build_extop_package(GArray *package, uint8_t op)
build_prepend_byte(package, 0x5B); /* ExtOpPrefix */
}
-static void build_append_int_noprefix(GArray *table, uint64_t value, int size)
+void build_append_int_noprefix(GArray *table, uint64_t value, int size)
{
int i;
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 358e340..d298da6 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -392,12 +392,56 @@ typedef struct NvdimmDsmOut NvdimmDsmOut;
static uint64_t
nvdimm_dsm_read(void *opaque, hwaddr addr, unsigned size)
{
+ fprintf(stderr, "BUG: we never read _DSM IO Port.\n");
return 0;
}
static void
nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
{
+ NvdimmDsmIn *in;
+ GArray *out;
+ uint32_t buf_size;
+ hwaddr dsm_mem_addr = val;
+
+ nvdimm_debug("dsm memory address %#lx.\n", dsm_mem_addr);
+
+ /*
+ * The DSM memory is mapped to guest address space so an evil guest
+ * can change its content while we are doing DSM emulation. Avoid
+ * this by copying DSM memory to QEMU local memory.
+ */
+ in = g_malloc(TARGET_PAGE_SIZE);
+ cpu_physical_memory_read(dsm_mem_addr, in, TARGET_PAGE_SIZE);
+
+ le32_to_cpus(&in->revision);
+ le32_to_cpus(&in->function);
+ le32_to_cpus(&in->handle);
+
+ nvdimm_debug("Revision %#x Handler %#x Function %#x.\n", in->revision,
+ in->handle, in->function);
+
+ out = g_array_new(false, true /* clear */, 1);
+
+ /*
+ * function 0 is called to inquire what functions are supported by
+ * OSPM
+ */
+ if (in->function == 0) {
+ build_append_int_noprefix(out, 0 /* No function Supported */,
+ sizeof(uint8_t));
+ } else {
+ /* No function is supported yet. */
+ build_append_int_noprefix(out, 1 /* Not Supported */,
+ sizeof(uint8_t));
+ }
+
+ buf_size = cpu_to_le32(out->len);
+ cpu_physical_memory_write(dsm_mem_addr, &buf_size, sizeof(buf_size));
+ cpu_physical_memory_write(dsm_mem_addr + sizeof(buf_size), out->data,
+ out->len);
+ g_free(in);
+ g_array_free(out, true);
}
static const MemoryRegionOps nvdimm_dsm_ops = {
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 7c8db8f..c5715c4 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -356,6 +356,7 @@ Aml *aml_derefof(Aml *arg);
Aml *aml_sizeof(Aml *arg);
Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target);
+void build_append_int_noprefix(GArray *table, uint64_t value, int size);
void
build_header(GArray *linker, GArray *table_data,
AcpiTableHeader *h, const char *sig, int len, uint8_t rev,
diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
index 634c60b..aaa2608 100644
--- a/include/hw/mem/nvdimm.h
+++ b/include/hw/mem/nvdimm.h
@@ -25,6 +25,14 @@
#include "hw/mem/pc-dimm.h"
+#define NVDIMM_DEBUG 0
+#define nvdimm_debug(fmt, ...) \
+ do { \
+ if (NVDIMM_DEBUG) { \
+ fprintf(stderr, "nvdimm: " fmt, ## __VA_ARGS__); \
+ } \
+ } while (0)
+
#define TYPE_NVDIMM "nvdimm"
#define NVDIMM_DSM_MEM_FILE "etc/acpi/nvdimm-mem"
--
1.8.3.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* [Qemu-devel] [PATCH v2 10/11] nvdimm acpi: add _CRS
2016-01-12 18:49 [Qemu-devel] [PATCH v2 00/11] NVDIMM ACPI: introduce the framework of QEMU emulated Xiao Guangrong
` (8 preceding siblings ...)
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 09/11] nvdimm acpi: emulate dsm method Xiao Guangrong
@ 2016-01-12 18:50 ` Xiao Guangrong
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 11/11] tests: acpi: update nvdimm ssdt table Xiao Guangrong
` (2 subsequent siblings)
12 siblings, 0 replies; 51+ messages in thread
From: Xiao Guangrong @ 2016-01-12 18:50 UTC (permalink / raw)
To: pbonzini, imammedo
Cc: Xiao Guangrong, ehabkost, kvm, mst, gleb, mtosatti, qemu-devel,
stefanha, dan.j.williams, rth
As Igor suggested that we can report the BIOS patched operation region
so that OSPM could see that particular range is in use and be able to
notice conflicts if it happens some day
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
hw/acpi/nvdimm.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index d298da6..5e36bbd 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -565,6 +565,7 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
GArray *table_data, GArray *linker)
{
Aml *ssdt, *sb_scope, *dev, *field, *mem_addr;
+ Aml *min_addr, *max_addr, *mr32, *method, *crs;
uint32_t zero_offset = 0;
int offset;
@@ -590,6 +591,32 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
*/
aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0012")));
+ /*
+ * report the dsm memory so that OSPM could see that particular range is
+ * in use and be able to notice conflicts if it happens some day.
+ */
+ method = aml_method("_CRS", 0, AML_SERIALIZED);
+ crs = aml_resource_template();
+ aml_append(crs, aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED,
+ AML_MAX_FIXED, AML_CACHEABLE,
+ AML_READ_WRITE,
+ 0, 0x0, 0xFFFFFFFE, 0,
+ TARGET_PAGE_SIZE));
+ aml_append(method, aml_name_decl("MR32", crs));
+ mr32 = aml_name("MR32");
+ aml_append(method, aml_create_dword_field(mr32, aml_int(10), "MIN"));
+ aml_append(method, aml_create_dword_field(mr32, aml_int(14), "MAX"));
+
+ min_addr = aml_name("MIN");
+ max_addr = aml_name("MAX");
+
+ aml_append(method, aml_store(aml_name(NVDIMM_ACPI_MEM_ADDR), min_addr));
+ aml_append(method, aml_add(min_addr, aml_int(TARGET_PAGE_SIZE),
+ max_addr));
+ aml_append(method, aml_decrement(max_addr));
+ aml_append(method, aml_return(mr32));
+ aml_append(dev, method);
+
/* map DSM memory and IO into ACPI namespace. */
aml_append(dev, aml_operation_region("NPIO", AML_SYSTEM_IO,
aml_int(NVDIMM_ACPI_IO_BASE), NVDIMM_ACPI_IO_LEN));
--
1.8.3.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* [Qemu-devel] [PATCH v2 11/11] tests: acpi: update nvdimm ssdt table
2016-01-12 18:49 [Qemu-devel] [PATCH v2 00/11] NVDIMM ACPI: introduce the framework of QEMU emulated Xiao Guangrong
` (9 preceding siblings ...)
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 10/11] nvdimm acpi: add _CRS Xiao Guangrong
@ 2016-01-12 18:50 ` Xiao Guangrong
2016-01-20 2:21 ` [Qemu-devel] [PATCH v2 00/11] NVDIMM ACPI: introduce the framework of QEMU emulated Xiao Guangrong
2016-02-04 16:24 ` Michael S. Tsirkin
12 siblings, 0 replies; 51+ messages in thread
From: Xiao Guangrong @ 2016-01-12 18:50 UTC (permalink / raw)
To: pbonzini, imammedo
Cc: Xiao Guangrong, ehabkost, kvm, mst, gleb, mtosatti, qemu-devel,
stefanha, dan.j.williams, rth
update the nvdimm ssdt table as its acpi code has been updated
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
tests/acpi-test-data/pc/SSDT-NVDIMM | Bin 134 -> 403 bytes
tests/acpi-test-data/pc/SSDT-NVDIMM.bridge | Bin 134 -> 403 bytes
tests/acpi-test-data/q35/SSDT-NVDIMM | Bin 134 -> 403 bytes
tests/acpi-test-data/q35/SSDT-NVDIMM.bridge | Bin 134 -> 403 bytes
4 files changed, 0 insertions(+), 0 deletions(-)
diff --git a/tests/acpi-test-data/pc/SSDT-NVDIMM b/tests/acpi-test-data/pc/SSDT-NVDIMM
index 22265278c3a4df9e9fee0ea9e04b37f2c80baa29..eaf816003b6ba60248aa28d54726fef5d1cc3cb0 100644
GIT binary patch
literal 403
zcmZ8b!A`<J5S?0-+ExWcG?B!9LWM8bc8fH!Te1s;WKA}`2_~e`#Dp7gG4aX|aOekm
z_CW2_n|pZkcHWyeBPAX03;?4SB;i1Md`e@+%0B=Evr&k(!Q0PT23l>Ou1j55dJxky
z5{_cuf9OJsoaj(U%VMI}ZQk3R&l>>OEC;K1D2k#Y4S*GVp~<#c%=I5VXJn=`7BPao
zHv|V7*!)1q(XZBg6eosrYumMw3&H>gN}t*|D4BfRk3F^Npk@x}F!Dq+m0o|sLyw5Z
z-ukDmHkXwuh7tD?O2+F$y~U1sNQUmdy~9A*pY9frH%+4gBLj@e=9rZ&FswQhmt`1}
zW~<aTAyrqH2Wb*a`tQvnz`D$rtqV;htL3=jWxjY-+dDq>eM=K6jQ^sA2e|NE)PDig
CC}#lx
delta 59
zcmbQt+{VZi9PAR(#=yY9cxNJ)G`AyLOnk6Yd~}ls>qG+)QBgl<M<132oRhN{mIyL4
Lf(gdSF^swZxqJ=*
diff --git a/tests/acpi-test-data/pc/SSDT-NVDIMM.bridge b/tests/acpi-test-data/pc/SSDT-NVDIMM.bridge
index 22265278c3a4df9e9fee0ea9e04b37f2c80baa29..eaf816003b6ba60248aa28d54726fef5d1cc3cb0 100644
GIT binary patch
literal 403
zcmZ8b!A`<J5S?0-+ExWcG?B!9LWM8bc8fH!Te1s;WKA}`2_~e`#Dp7gG4aX|aOekm
z_CW2_n|pZkcHWyeBPAX03;?4SB;i1Md`e@+%0B=Evr&k(!Q0PT23l>Ou1j55dJxky
z5{_cuf9OJsoaj(U%VMI}ZQk3R&l>>OEC;K1D2k#Y4S*GVp~<#c%=I5VXJn=`7BPao
zHv|V7*!)1q(XZBg6eosrYumMw3&H>gN}t*|D4BfRk3F^Npk@x}F!Dq+m0o|sLyw5Z
z-ukDmHkXwuh7tD?O2+F$y~U1sNQUmdy~9A*pY9frH%+4gBLj@e=9rZ&FswQhmt`1}
zW~<aTAyrqH2Wb*a`tQvnz`D$rtqV;htL3=jWxjY-+dDq>eM=K6jQ^sA2e|NE)PDig
CC}#lx
delta 59
zcmbQt+{VZi9PAR(#=yY9cxNJ)G`AyLOnk6Yd~}ls>qG+)QBgl<M<132oRhN{mIyL4
Lf(gdSF^swZxqJ=*
diff --git a/tests/acpi-test-data/q35/SSDT-NVDIMM b/tests/acpi-test-data/q35/SSDT-NVDIMM
index 22265278c3a4df9e9fee0ea9e04b37f2c80baa29..eaf816003b6ba60248aa28d54726fef5d1cc3cb0 100644
GIT binary patch
literal 403
zcmZ8b!A`<J5S?0-+ExWcG?B!9LWM8bc8fH!Te1s;WKA}`2_~e`#Dp7gG4aX|aOekm
z_CW2_n|pZkcHWyeBPAX03;?4SB;i1Md`e@+%0B=Evr&k(!Q0PT23l>Ou1j55dJxky
z5{_cuf9OJsoaj(U%VMI}ZQk3R&l>>OEC;K1D2k#Y4S*GVp~<#c%=I5VXJn=`7BPao
zHv|V7*!)1q(XZBg6eosrYumMw3&H>gN}t*|D4BfRk3F^Npk@x}F!Dq+m0o|sLyw5Z
z-ukDmHkXwuh7tD?O2+F$y~U1sNQUmdy~9A*pY9frH%+4gBLj@e=9rZ&FswQhmt`1}
zW~<aTAyrqH2Wb*a`tQvnz`D$rtqV;htL3=jWxjY-+dDq>eM=K6jQ^sA2e|NE)PDig
CC}#lx
delta 59
zcmbQt+{VZi9PAR(#=yY9cxNJ)G`AyLOnk6Yd~}ls>qG+)QBgl<M<132oRhN{mIyL4
Lf(gdSF^swZxqJ=*
diff --git a/tests/acpi-test-data/q35/SSDT-NVDIMM.bridge b/tests/acpi-test-data/q35/SSDT-NVDIMM.bridge
index 22265278c3a4df9e9fee0ea9e04b37f2c80baa29..eaf816003b6ba60248aa28d54726fef5d1cc3cb0 100644
GIT binary patch
literal 403
zcmZ8b!A`<J5S?0-+ExWcG?B!9LWM8bc8fH!Te1s;WKA}`2_~e`#Dp7gG4aX|aOekm
z_CW2_n|pZkcHWyeBPAX03;?4SB;i1Md`e@+%0B=Evr&k(!Q0PT23l>Ou1j55dJxky
z5{_cuf9OJsoaj(U%VMI}ZQk3R&l>>OEC;K1D2k#Y4S*GVp~<#c%=I5VXJn=`7BPao
zHv|V7*!)1q(XZBg6eosrYumMw3&H>gN}t*|D4BfRk3F^Npk@x}F!Dq+m0o|sLyw5Z
z-ukDmHkXwuh7tD?O2+F$y~U1sNQUmdy~9A*pY9frH%+4gBLj@e=9rZ&FswQhmt`1}
zW~<aTAyrqH2Wb*a`tQvnz`D$rtqV;htL3=jWxjY-+dDq>eM=K6jQ^sA2e|NE)PDig
CC}#lx
delta 59
zcmbQt+{VZi9PAR(#=yY9cxNJ)G`AyLOnk6Yd~}ls>qG+)QBgl<M<132oRhN{mIyL4
Lf(gdSF^swZxqJ=*
--
1.8.3.1
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH v2 00/11] NVDIMM ACPI: introduce the framework of QEMU emulated
2016-01-12 18:49 [Qemu-devel] [PATCH v2 00/11] NVDIMM ACPI: introduce the framework of QEMU emulated Xiao Guangrong
` (10 preceding siblings ...)
2016-01-12 18:50 ` [Qemu-devel] [PATCH v2 11/11] tests: acpi: update nvdimm ssdt table Xiao Guangrong
@ 2016-01-20 2:21 ` Xiao Guangrong
2016-01-28 4:42 ` Xiao Guangrong
2016-02-04 16:24 ` Michael S. Tsirkin
12 siblings, 1 reply; 51+ messages in thread
From: Xiao Guangrong @ 2016-01-20 2:21 UTC (permalink / raw)
To: pbonzini, imammedo
Cc: ehabkost, kvm, mst, gleb, mtosatti, qemu-devel, stefanha,
dan.j.williams, rth
Hi Michael, Igor, Paolo,
Any comment?
On 01/13/2016 02:49 AM, Xiao Guangrong wrote:
> This patchset is against commit 8a1be662a69 (virtio: fix error message for
> number of queues) on pci branch of Michael's git tree
> and can be found at:
> https://github.com/xiaogr/qemu.git nvdimm-acpi-v2
>
> Changelog:
> These changes are based on Igor's comments:
> - drop ssdt.rev2 support as the memory address allocated by BIOS/OVMF
> are always 32 bits
> - support to test NVDIMM tables (NFIT and NVDIMM SSDT)
> - add _CRS to report its operation region
> - make AML APIs change be the separated patches
>
> This is the second part of vNVDIMM implementation which implements the
> BIOS patched dsm memory and introduces the framework that allows QEMU
> to emulate DSM method
>
> Thanks to Michael's idea, we do not reserve any memory for NVDIMM ACPI,
> instead we let BIOS allocate the memory and patch the address to the
> offset we want
>
> IO port is still enabled as it plays as the way to notify QEMU and pass
> the patched dsm memory address, so that IO port region, 0x0a18 - 0xa20,
> is reserved and it is divided into two 32 bits ports and used to pass
> the low 32 bits and high 32 bits of dsm memory address to QEMU
>
> Thanks Igor's idea, this patchset also extends DSDT/SSDT to revision 2
> to apply 64 bit operations, in order to keeping compatibility, old
> version (<= 2.5) still uses revision 1. Since 64 bit operations breaks
> old guests (such as windows XP), we should keep the 64 bits stuff in
> the private place where common ACPI operation does not touch it
>
> Xiao Guangrong (11):
> tests: acpi: test multiple SSDT tables
> tests: acpi: test NVDIMM tables
> acpi: add aml_create_field()
> acpi: add aml_concatenate()
> acpi: allow using object as offset for OperationRegion
> nvdimm acpi: initialize the resource used by NVDIMM ACPI
> nvdimm acpi: introduce patched dsm memory
> nvdimm acpi: let qemu handle _DSM method
> nvdimm acpi: emulate dsm method
> nvdimm acpi: add _CRS
> tests: acpi: update nvdimm ssdt table
>
> hw/acpi/Makefile.objs | 2 +-
> hw/acpi/aml-build.c | 33 +++-
> hw/acpi/nvdimm.c | 255 +++++++++++++++++++++++++++-
> hw/i386/acpi-build.c | 41 ++---
> hw/i386/pc.c | 8 +-
> hw/i386/pc_piix.c | 5 +
> hw/i386/pc_q35.c | 8 +-
> include/hw/acpi/aml-build.h | 5 +-
> include/hw/i386/pc.h | 5 +-
> include/hw/mem/nvdimm.h | 36 +++-
> tests/acpi-test-data/pc/NFIT | Bin 0 -> 224 bytes
> tests/acpi-test-data/pc/NFIT.bridge | Bin 0 -> 224 bytes
> tests/acpi-test-data/pc/SSDT | Bin 2486 -> 2885 bytes
> tests/acpi-test-data/pc/SSDT-NVDIMM | Bin 0 -> 403 bytes
> tests/acpi-test-data/pc/SSDT-NVDIMM.bridge | Bin 0 -> 403 bytes
> tests/acpi-test-data/pc/SSDT.bridge | Bin 4345 -> 4745 bytes
> tests/acpi-test-data/q35/NFIT | Bin 0 -> 224 bytes
> tests/acpi-test-data/q35/NFIT.bridge | Bin 0 -> 224 bytes
> tests/acpi-test-data/q35/SSDT | Bin 691 -> 1090 bytes
> tests/acpi-test-data/q35/SSDT-NVDIMM | Bin 0 -> 403 bytes
> tests/acpi-test-data/q35/SSDT-NVDIMM.bridge | Bin 0 -> 403 bytes
> tests/acpi-test-data/q35/SSDT.bridge | Bin 708 -> 1107 bytes
> tests/bios-tables-test.c | 58 +++++--
> 23 files changed, 400 insertions(+), 56 deletions(-)
> create mode 100644 tests/acpi-test-data/pc/NFIT
> create mode 100644 tests/acpi-test-data/pc/NFIT.bridge
> create mode 100644 tests/acpi-test-data/pc/SSDT-NVDIMM
> create mode 100644 tests/acpi-test-data/pc/SSDT-NVDIMM.bridge
> create mode 100644 tests/acpi-test-data/q35/NFIT
> create mode 100644 tests/acpi-test-data/q35/NFIT.bridge
> create mode 100644 tests/acpi-test-data/q35/SSDT-NVDIMM
> create mode 100644 tests/acpi-test-data/q35/SSDT-NVDIMM.bridge
>
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH v2 00/11] NVDIMM ACPI: introduce the framework of QEMU emulated
2016-01-20 2:21 ` [Qemu-devel] [PATCH v2 00/11] NVDIMM ACPI: introduce the framework of QEMU emulated Xiao Guangrong
@ 2016-01-28 4:42 ` Xiao Guangrong
0 siblings, 0 replies; 51+ messages in thread
From: Xiao Guangrong @ 2016-01-28 4:42 UTC (permalink / raw)
To: pbonzini, imammedo
Cc: ehabkost, kvm, mst, gleb, mtosatti, qemu-devel, stefanha,
dan.j.williams, rth
Ping...
On 01/20/2016 10:21 AM, Xiao Guangrong wrote:
>
> Hi Michael, Igor, Paolo,
>
> Any comment?
>
>
>
> On 01/13/2016 02:49 AM, Xiao Guangrong wrote:
>> This patchset is against commit 8a1be662a69 (virtio: fix error message for
>> number of queues) on pci branch of Michael's git tree
>> and can be found at:
>> https://github.com/xiaogr/qemu.git nvdimm-acpi-v2
>>
>> Changelog:
>> These changes are based on Igor's comments:
>> - drop ssdt.rev2 support as the memory address allocated by BIOS/OVMF
>> are always 32 bits
>> - support to test NVDIMM tables (NFIT and NVDIMM SSDT)
>> - add _CRS to report its operation region
>> - make AML APIs change be the separated patches
>>
>> This is the second part of vNVDIMM implementation which implements the
>> BIOS patched dsm memory and introduces the framework that allows QEMU
>> to emulate DSM method
>>
>> Thanks to Michael's idea, we do not reserve any memory for NVDIMM ACPI,
>> instead we let BIOS allocate the memory and patch the address to the
>> offset we want
>>
>> IO port is still enabled as it plays as the way to notify QEMU and pass
>> the patched dsm memory address, so that IO port region, 0x0a18 - 0xa20,
>> is reserved and it is divided into two 32 bits ports and used to pass
>> the low 32 bits and high 32 bits of dsm memory address to QEMU
>>
>> Thanks Igor's idea, this patchset also extends DSDT/SSDT to revision 2
>> to apply 64 bit operations, in order to keeping compatibility, old
>> version (<= 2.5) still uses revision 1. Since 64 bit operations breaks
>> old guests (such as windows XP), we should keep the 64 bits stuff in
>> the private place where common ACPI operation does not touch it
>>
>> Xiao Guangrong (11):
>> tests: acpi: test multiple SSDT tables
>> tests: acpi: test NVDIMM tables
>> acpi: add aml_create_field()
>> acpi: add aml_concatenate()
>> acpi: allow using object as offset for OperationRegion
>> nvdimm acpi: initialize the resource used by NVDIMM ACPI
>> nvdimm acpi: introduce patched dsm memory
>> nvdimm acpi: let qemu handle _DSM method
>> nvdimm acpi: emulate dsm method
>> nvdimm acpi: add _CRS
>> tests: acpi: update nvdimm ssdt table
>>
>> hw/acpi/Makefile.objs | 2 +-
>> hw/acpi/aml-build.c | 33 +++-
>> hw/acpi/nvdimm.c | 255 +++++++++++++++++++++++++++-
>> hw/i386/acpi-build.c | 41 ++---
>> hw/i386/pc.c | 8 +-
>> hw/i386/pc_piix.c | 5 +
>> hw/i386/pc_q35.c | 8 +-
>> include/hw/acpi/aml-build.h | 5 +-
>> include/hw/i386/pc.h | 5 +-
>> include/hw/mem/nvdimm.h | 36 +++-
>> tests/acpi-test-data/pc/NFIT | Bin 0 -> 224 bytes
>> tests/acpi-test-data/pc/NFIT.bridge | Bin 0 -> 224 bytes
>> tests/acpi-test-data/pc/SSDT | Bin 2486 -> 2885 bytes
>> tests/acpi-test-data/pc/SSDT-NVDIMM | Bin 0 -> 403 bytes
>> tests/acpi-test-data/pc/SSDT-NVDIMM.bridge | Bin 0 -> 403 bytes
>> tests/acpi-test-data/pc/SSDT.bridge | Bin 4345 -> 4745 bytes
>> tests/acpi-test-data/q35/NFIT | Bin 0 -> 224 bytes
>> tests/acpi-test-data/q35/NFIT.bridge | Bin 0 -> 224 bytes
>> tests/acpi-test-data/q35/SSDT | Bin 691 -> 1090 bytes
>> tests/acpi-test-data/q35/SSDT-NVDIMM | Bin 0 -> 403 bytes
>> tests/acpi-test-data/q35/SSDT-NVDIMM.bridge | Bin 0 -> 403 bytes
>> tests/acpi-test-data/q35/SSDT.bridge | Bin 708 -> 1107 bytes
>> tests/bios-tables-test.c | 58 +++++--
>> 23 files changed, 400 insertions(+), 56 deletions(-)
>> create mode 100644 tests/acpi-test-data/pc/NFIT
>> create mode 100644 tests/acpi-test-data/pc/NFIT.bridge
>> create mode 100644 tests/acpi-test-data/pc/SSDT-NVDIMM
>> create mode 100644 tests/acpi-test-data/pc/SSDT-NVDIMM.bridge
>> create mode 100644 tests/acpi-test-data/q35/NFIT
>> create mode 100644 tests/acpi-test-data/q35/NFIT.bridge
>> create mode 100644 tests/acpi-test-data/q35/SSDT-NVDIMM
>> create mode 100644 tests/acpi-test-data/q35/SSDT-NVDIMM.bridge
>>
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH v2 00/11] NVDIMM ACPI: introduce the framework of QEMU emulated
2016-01-12 18:49 [Qemu-devel] [PATCH v2 00/11] NVDIMM ACPI: introduce the framework of QEMU emulated Xiao Guangrong
` (11 preceding siblings ...)
2016-01-20 2:21 ` [Qemu-devel] [PATCH v2 00/11] NVDIMM ACPI: introduce the framework of QEMU emulated Xiao Guangrong
@ 2016-02-04 16:24 ` Michael S. Tsirkin
2016-02-14 5:38 ` Xiao Guangrong
12 siblings, 1 reply; 51+ messages in thread
From: Michael S. Tsirkin @ 2016-02-04 16:24 UTC (permalink / raw)
To: Xiao Guangrong
Cc: ehabkost, kvm, gleb, mtosatti, qemu-devel, stefanha, imammedo,
pbonzini, dan.j.williams, rth
On Wed, Jan 13, 2016 at 02:49:59AM +0800, Xiao Guangrong wrote:
> This patchset is against commit 8a1be662a69 (virtio: fix error message for
> number of queues) on pci branch of Michael's git tree
> and can be found at:
> https://github.com/xiaogr/qemu.git nvdimm-acpi-v2
Looks good, thanks!
There's been a bunch of changes in my tree so
this no longer applies.
Pls rebase and repost, and I'll apply.
> Changelog:
> These changes are based on Igor's comments:
> - drop ssdt.rev2 support as the memory address allocated by BIOS/OVMF
> are always 32 bits
> - support to test NVDIMM tables (NFIT and NVDIMM SSDT)
> - add _CRS to report its operation region
> - make AML APIs change be the separated patches
>
> This is the second part of vNVDIMM implementation which implements the
> BIOS patched dsm memory and introduces the framework that allows QEMU
> to emulate DSM method
>
> Thanks to Michael's idea, we do not reserve any memory for NVDIMM ACPI,
> instead we let BIOS allocate the memory and patch the address to the
> offset we want
>
> IO port is still enabled as it plays as the way to notify QEMU and pass
> the patched dsm memory address, so that IO port region, 0x0a18 - 0xa20,
> is reserved and it is divided into two 32 bits ports and used to pass
> the low 32 bits and high 32 bits of dsm memory address to QEMU
>
> Thanks Igor's idea, this patchset also extends DSDT/SSDT to revision 2
> to apply 64 bit operations, in order to keeping compatibility, old
> version (<= 2.5) still uses revision 1. Since 64 bit operations breaks
> old guests (such as windows XP), we should keep the 64 bits stuff in
> the private place where common ACPI operation does not touch it
>
> Xiao Guangrong (11):
> tests: acpi: test multiple SSDT tables
> tests: acpi: test NVDIMM tables
> acpi: add aml_create_field()
> acpi: add aml_concatenate()
> acpi: allow using object as offset for OperationRegion
> nvdimm acpi: initialize the resource used by NVDIMM ACPI
> nvdimm acpi: introduce patched dsm memory
> nvdimm acpi: let qemu handle _DSM method
> nvdimm acpi: emulate dsm method
> nvdimm acpi: add _CRS
> tests: acpi: update nvdimm ssdt table
>
> hw/acpi/Makefile.objs | 2 +-
> hw/acpi/aml-build.c | 33 +++-
> hw/acpi/nvdimm.c | 255 +++++++++++++++++++++++++++-
> hw/i386/acpi-build.c | 41 ++---
> hw/i386/pc.c | 8 +-
> hw/i386/pc_piix.c | 5 +
> hw/i386/pc_q35.c | 8 +-
> include/hw/acpi/aml-build.h | 5 +-
> include/hw/i386/pc.h | 5 +-
> include/hw/mem/nvdimm.h | 36 +++-
> tests/acpi-test-data/pc/NFIT | Bin 0 -> 224 bytes
> tests/acpi-test-data/pc/NFIT.bridge | Bin 0 -> 224 bytes
> tests/acpi-test-data/pc/SSDT | Bin 2486 -> 2885 bytes
> tests/acpi-test-data/pc/SSDT-NVDIMM | Bin 0 -> 403 bytes
> tests/acpi-test-data/pc/SSDT-NVDIMM.bridge | Bin 0 -> 403 bytes
> tests/acpi-test-data/pc/SSDT.bridge | Bin 4345 -> 4745 bytes
> tests/acpi-test-data/q35/NFIT | Bin 0 -> 224 bytes
> tests/acpi-test-data/q35/NFIT.bridge | Bin 0 -> 224 bytes
> tests/acpi-test-data/q35/SSDT | Bin 691 -> 1090 bytes
> tests/acpi-test-data/q35/SSDT-NVDIMM | Bin 0 -> 403 bytes
> tests/acpi-test-data/q35/SSDT-NVDIMM.bridge | Bin 0 -> 403 bytes
> tests/acpi-test-data/q35/SSDT.bridge | Bin 708 -> 1107 bytes
> tests/bios-tables-test.c | 58 +++++--
> 23 files changed, 400 insertions(+), 56 deletions(-)
> create mode 100644 tests/acpi-test-data/pc/NFIT
> create mode 100644 tests/acpi-test-data/pc/NFIT.bridge
> create mode 100644 tests/acpi-test-data/pc/SSDT-NVDIMM
> create mode 100644 tests/acpi-test-data/pc/SSDT-NVDIMM.bridge
> create mode 100644 tests/acpi-test-data/q35/NFIT
> create mode 100644 tests/acpi-test-data/q35/NFIT.bridge
> create mode 100644 tests/acpi-test-data/q35/SSDT-NVDIMM
> create mode 100644 tests/acpi-test-data/q35/SSDT-NVDIMM.bridge
>
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH v2 00/11] NVDIMM ACPI: introduce the framework of QEMU emulated
2016-02-04 16:24 ` Michael S. Tsirkin
@ 2016-02-14 5:38 ` Xiao Guangrong
0 siblings, 0 replies; 51+ messages in thread
From: Xiao Guangrong @ 2016-02-14 5:38 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: ehabkost, kvm, gleb, mtosatti, qemu-devel, stefanha, pbonzini,
imammedo, dan.j.williams, rth
On 02/05/2016 12:24 AM, Michael S. Tsirkin wrote:
> On Wed, Jan 13, 2016 at 02:49:59AM +0800, Xiao Guangrong wrote:
>> This patchset is against commit 8a1be662a69 (virtio: fix error message for
>> number of queues) on pci branch of Michael's git tree
>> and can be found at:
>> https://github.com/xiaogr/qemu.git nvdimm-acpi-v2
>
> Looks good, thanks!
> There's been a bunch of changes in my tree so
> this no longer applies.
> Pls rebase and repost, and I'll apply.
>
Good to hear that. :) Will address some comments from Igor and post it
out asap.
^ permalink raw reply [flat|nested] 51+ messages in thread