* [PATCH v3 0/5] Only generate cluster node in PPTT when specified
@ 2022-10-31 9:05 Yicong Yang via
2022-10-31 9:05 ` [PATCH v3 1/5] hw/acpi/aml-build: " Yicong Yang via
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Yicong Yang via @ 2022-10-31 9:05 UTC (permalink / raw)
To: mst, peter.maydell, imammedo, ani, eduardo, marcel.apfelbaum,
f4bug, wangyanan55, qemu-devel
Cc: jonathan.cameron, linuxarm, yangyicong, prime.zeng,
hesham.almatary, ionela.voinescu, darren
From: Yicong Yang <yangyicong@hisilicon.com>
This series mainly change the policy for building a cluster topology node
in PPTT. Previously we'll always build a cluster node in PPTT without
asking the user, after this set the cluster node will be built only the
the user specify through "-smp clusters=X".
One problem is related to this but not fully caused by this, see the
discussion in [*]. When booting the VM with `-smp 8` and 4 numa nodes,
the linux scheduling domains in the VM misses the NUMA domains. It's
because the MC level span extends to Cluster level (which is generated
by the Qemu by default) that spans all the cpus in the system, then the
scheduling domain building stops at MC level since it already includes all
the cpus.
Considering cluster is an optional level and most platforms don't have it,
they may even don't realize this is built and a always build policy cannot
emulate the topology on these platforms. So in this series improve the
policy to only generate cluster when the user explicitly want.
Update the tests and test tables accordingly.
[*] https://lore.kernel.org/lkml/2c079860-ee82-7719-d3d2-756192f41704@huawei.com/
Change since v2:
- Add tag from Micheal, thanks
- Handle the tests changes with bios-tables-test-allowed-diff.h, Per Micheal
- Address the comments per Yanan
Link: https://lore.kernel.org/qemu-devel/20221027032613.18377-1-yangyicong@huawei.com/
Change since v1:
- Only includes the test tables which is really needed
- Enrich the commit
Link: https://lore.kernel.org/qemu-devel/20220922131143.58003-1-yangyicong@huawei.com/
Yicong Yang (5):
hw/acpi/aml-build: Only generate cluster node in PPTT when specified
tests: virt: update expected ACPI tables for virt test
tests: acpi: add and whitelist *.topology blobs
tests: acpi: aarch64: add topology test for aarch64
tests: acpi: aarch64: add *.topology tables
hw/acpi/aml-build.c | 2 +-
hw/core/machine-smp.c | 3 +++
include/hw/boards.h | 3 +++
qemu-options.hx | 3 +++
tests/data/acpi/virt/APIC.topology | Bin 0 -> 700 bytes
tests/data/acpi/virt/DSDT.topology | Bin 0 -> 5398 bytes
tests/data/acpi/virt/PPTT | Bin 96 -> 76 bytes
tests/data/acpi/virt/PPTT.topology | Bin 0 -> 336 bytes
tests/qtest/bios-tables-test.c | 22 ++++++++++++++++++++++
9 files changed, 32 insertions(+), 1 deletion(-)
create mode 100644 tests/data/acpi/virt/APIC.topology
create mode 100644 tests/data/acpi/virt/DSDT.topology
create mode 100644 tests/data/acpi/virt/PPTT.topology
--
2.24.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 1/5] hw/acpi/aml-build: Only generate cluster node in PPTT when specified
2022-10-31 9:05 [PATCH v3 0/5] Only generate cluster node in PPTT when specified Yicong Yang via
@ 2022-10-31 9:05 ` Yicong Yang via
2022-10-31 11:17 ` wangyanan (Y) via
2022-10-31 9:05 ` [PATCH v3 2/5] tests: virt: update expected ACPI tables for virt test Yicong Yang via
` (3 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Yicong Yang via @ 2022-10-31 9:05 UTC (permalink / raw)
To: mst, peter.maydell, imammedo, ani, eduardo, marcel.apfelbaum,
f4bug, wangyanan55, qemu-devel
Cc: jonathan.cameron, linuxarm, yangyicong, prime.zeng,
hesham.almatary, ionela.voinescu, darren
From: Yicong Yang <yangyicong@hisilicon.com>
Currently we'll always generate a cluster node no matter user has
specified '-smp clusters=X' or not. Cluster is an optional level
and will participant the building of Linux scheduling domains and
only appears on a few platforms. It's unncessary to always build
it which cannot reflect the real topology on platforms have no
cluster and to avoid affecting the linux scheduling domains in the
VM. So only generate it when user specified explicitly.
Tested qemu-system-aarch64 with `-smp 8` and linux 6.1-rc1, without
this patch:
estuary:/sys/devices/system/cpu/cpu0/topology$ cat cluster_*
ff # cluster_cpus
0-7 # cluster_cpus_list
56 # cluster_id
with this patch:
estuary:/sys/devices/system/cpu/cpu0/topology$ cat cluster_*
ff # cluster_cpus
0-7 # cluster_cpus_list
36 # cluster_id, with no cluster node kernel will make it to
physical package id
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
hw/acpi/aml-build.c | 2 +-
hw/core/machine-smp.c | 3 +++
include/hw/boards.h | 3 +++
qemu-options.hx | 3 +++
tests/qtest/bios-tables-test-allowed-diff.h | 1 +
5 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index e6bfac95c7..60c1acf3da 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -2030,7 +2030,7 @@ void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
0, socket_id, NULL, 0);
}
- if (mc->smp_props.clusters_supported) {
+ if (mc->smp_props.clusters_supported && mc->smp_props.has_clusters) {
if (cpus->cpus[n].props.cluster_id != cluster_id) {
assert(cpus->cpus[n].props.cluster_id > cluster_id);
cluster_id = cpus->cpus[n].props.cluster_id;
diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
index b39ed21e65..9c8950b2b0 100644
--- a/hw/core/machine-smp.c
+++ b/hw/core/machine-smp.c
@@ -158,6 +158,9 @@ void machine_parse_smp_config(MachineState *ms,
ms->smp.threads = threads;
ms->smp.max_cpus = maxcpus;
+ if (config->has_clusters)
+ mc->smp_props.has_clusters = true;
+
/* sanity-check of the computed topology */
if (sockets * dies * clusters * cores * threads != maxcpus) {
g_autofree char *topo_msg = cpu_hierarchy_to_string(ms);
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 311ed17e18..06ed66453f 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -130,11 +130,14 @@ typedef struct {
* @prefer_sockets - whether sockets are preferred over cores in smp parsing
* @dies_supported - whether dies are supported by the machine
* @clusters_supported - whether clusters are supported by the machine
+ * @has_clusters - whether clusters are explicitly specified in the user
+ * provided SMP configuration
*/
typedef struct {
bool prefer_sockets;
bool dies_supported;
bool clusters_supported;
+ bool has_clusters;
} SMPCompatProps;
/**
diff --git a/qemu-options.hx b/qemu-options.hx
index eb38e5dc40..bbdbdef0af 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -349,6 +349,9 @@ SRST
::
-smp 2
+
+ Note: The cluster topology will only be generated in ACPI and exposed
+ to guest if it's explicitly specified in -smp.
ERST
DEF("numa", HAS_ARG, QEMU_OPTION_numa,
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..cb143a55a6 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,2 @@
/* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/virt/PPTT",
--
2.24.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 2/5] tests: virt: update expected ACPI tables for virt test
2022-10-31 9:05 [PATCH v3 0/5] Only generate cluster node in PPTT when specified Yicong Yang via
2022-10-31 9:05 ` [PATCH v3 1/5] hw/acpi/aml-build: " Yicong Yang via
@ 2022-10-31 9:05 ` Yicong Yang via
2022-10-31 11:21 ` wangyanan (Y) via
2022-10-31 9:05 ` [PATCH v3 3/5] tests: acpi: add and whitelist *.topology blobs Yicong Yang via
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Yicong Yang via @ 2022-10-31 9:05 UTC (permalink / raw)
To: mst, peter.maydell, imammedo, ani, eduardo, marcel.apfelbaum,
f4bug, wangyanan55, qemu-devel
Cc: jonathan.cameron, linuxarm, yangyicong, prime.zeng,
hesham.almatary, ionela.voinescu, darren
From: Yicong Yang <yangyicong@hisilicon.com>
Update the ACPI tables according to the acpi aml_build change.
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
tests/data/acpi/virt/PPTT | Bin 96 -> 76 bytes
tests/qtest/bios-tables-test-allowed-diff.h | 1 -
2 files changed, 1 deletion(-)
diff --git a/tests/data/acpi/virt/PPTT b/tests/data/acpi/virt/PPTT
index f56ea63b369a604877374ad696c396e796ab1c83..7a1258ecf123555b24462c98ccbb76b4ac1d0c2b 100644
GIT binary patch
delta 32
fcmYfB;R*-{3GrcIU|?D?k;`ae01J-_kOKn%ZFdCM
delta 53
pcmeZC;0g!`2}xjJU|{l?$YrDgWH5jU5Ca567#O&Klm(arApowi1QY-O
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index cb143a55a6..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,2 +1 @@
/* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/virt/PPTT",
--
2.24.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 3/5] tests: acpi: add and whitelist *.topology blobs
2022-10-31 9:05 [PATCH v3 0/5] Only generate cluster node in PPTT when specified Yicong Yang via
2022-10-31 9:05 ` [PATCH v3 1/5] hw/acpi/aml-build: " Yicong Yang via
2022-10-31 9:05 ` [PATCH v3 2/5] tests: virt: update expected ACPI tables for virt test Yicong Yang via
@ 2022-10-31 9:05 ` Yicong Yang via
2022-10-31 9:05 ` [PATCH v3 4/5] tests: acpi: aarch64: add topology test for aarch64 Yicong Yang via
2022-10-31 9:05 ` [PATCH v3 5/5] tests: acpi: aarch64: add *.topology tables Yicong Yang via
4 siblings, 0 replies; 14+ messages in thread
From: Yicong Yang via @ 2022-10-31 9:05 UTC (permalink / raw)
To: mst, peter.maydell, imammedo, ani, eduardo, marcel.apfelbaum,
f4bug, wangyanan55, qemu-devel
Cc: jonathan.cameron, linuxarm, yangyicong, prime.zeng,
hesham.almatary, ionela.voinescu, darren
From: Yicong Yang <yangyicong@hisilicon.com>
Add and whitelist *.topology blobs, prepares for the aarch64's ACPI
topology building test.
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
tests/data/acpi/virt/APIC.topology | 0
tests/data/acpi/virt/DSDT.topology | 0
tests/data/acpi/virt/PPTT.topology | 0
tests/qtest/bios-tables-test-allowed-diff.h | 3 +++
4 files changed, 3 insertions(+)
create mode 100644 tests/data/acpi/virt/APIC.topology
create mode 100644 tests/data/acpi/virt/DSDT.topology
create mode 100644 tests/data/acpi/virt/PPTT.topology
diff --git a/tests/data/acpi/virt/APIC.topology b/tests/data/acpi/virt/APIC.topology
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/data/acpi/virt/DSDT.topology b/tests/data/acpi/virt/DSDT.topology
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/data/acpi/virt/PPTT.topology b/tests/data/acpi/virt/PPTT.topology
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..90f53f9c1d 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,4 @@
/* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/virt/APIC.topology",
+"tests/data/acpi/virt/DSDT.topology",
+"tests/data/acpi/virt/PPTT.topology",
--
2.24.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 4/5] tests: acpi: aarch64: add topology test for aarch64
2022-10-31 9:05 [PATCH v3 0/5] Only generate cluster node in PPTT when specified Yicong Yang via
` (2 preceding siblings ...)
2022-10-31 9:05 ` [PATCH v3 3/5] tests: acpi: add and whitelist *.topology blobs Yicong Yang via
@ 2022-10-31 9:05 ` Yicong Yang via
2022-10-31 11:48 ` wangyanan (Y) via
2022-10-31 9:05 ` [PATCH v3 5/5] tests: acpi: aarch64: add *.topology tables Yicong Yang via
4 siblings, 1 reply; 14+ messages in thread
From: Yicong Yang via @ 2022-10-31 9:05 UTC (permalink / raw)
To: mst, peter.maydell, imammedo, ani, eduardo, marcel.apfelbaum,
f4bug, wangyanan55, qemu-devel
Cc: jonathan.cameron, linuxarm, yangyicong, prime.zeng,
hesham.almatary, ionela.voinescu, darren
From: Yicong Yang <yangyicong@hisilicon.com>
Add test for aarch64's ACPI topology building for all the supported
levels.
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
tests/qtest/bios-tables-test.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index e6096e7f73..099b723444 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1533,6 +1533,27 @@ static void test_acpi_virt_tcg(void)
free_test_data(&data);
}
+static void test_acpi_virt_tcg_topology(void)
+{
+ test_data data = {
+ .machine = "virt",
+ .variant = ".topology",
+ .tcg_only = true,
+ .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd",
+ .uefi_fl2 = "pc-bios/edk2-arm-vars.fd",
+ .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2",
+ .ram_start = 0x40000000ULL,
+ .scan_len = 128ULL * 1024 * 1024,
+ };
+
+ data.smbios_cpu_max_speed = 2900;
+ data.smbios_cpu_curr_speed = 2700;
+ test_acpi_one("-cpu cortex-a57 "
+ "-smbios type=4,max-speed=2900,current-speed=2700 "
+ "-smp sockets=1,clusters=2,cores=2,threads=2", &data);
+ free_test_data(&data);
+}
+
static void test_acpi_q35_viot(void)
{
test_data data = {
@@ -1864,6 +1885,7 @@ int main(int argc, char *argv[])
} else if (strcmp(arch, "aarch64") == 0) {
if (has_tcg) {
qtest_add_func("acpi/virt", test_acpi_virt_tcg);
+ qtest_add_func("acpi/virt/topology", test_acpi_virt_tcg_topology);
qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);
qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp);
qtest_add_func("acpi/virt/pxb", test_acpi_virt_tcg_pxb);
--
2.24.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 5/5] tests: acpi: aarch64: add *.topology tables
2022-10-31 9:05 [PATCH v3 0/5] Only generate cluster node in PPTT when specified Yicong Yang via
` (3 preceding siblings ...)
2022-10-31 9:05 ` [PATCH v3 4/5] tests: acpi: aarch64: add topology test for aarch64 Yicong Yang via
@ 2022-10-31 9:05 ` Yicong Yang via
4 siblings, 0 replies; 14+ messages in thread
From: Yicong Yang via @ 2022-10-31 9:05 UTC (permalink / raw)
To: mst, peter.maydell, imammedo, ani, eduardo, marcel.apfelbaum,
f4bug, wangyanan55, qemu-devel
Cc: jonathan.cameron, linuxarm, yangyicong, prime.zeng,
hesham.almatary, ionela.voinescu, darren
From: Yicong Yang <yangyicong@hisilicon.com>
Add *.topology tables for the aarch64's topology test.
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
tests/data/acpi/virt/APIC.topology | Bin 0 -> 700 bytes
tests/data/acpi/virt/DSDT.topology | Bin 0 -> 5398 bytes
tests/data/acpi/virt/PPTT.topology | Bin 0 -> 336 bytes
tests/qtest/bios-tables-test-allowed-diff.h | 3 ---
4 files changed, 3 deletions(-)
diff --git a/tests/data/acpi/virt/APIC.topology b/tests/data/acpi/virt/APIC.topology
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..558a1856981531b0e18e6d4aa12569cd80aa0015 100644
GIT binary patch
literal 700
zcmbV~OA5k35JW4P_<@+UE_0Na*(d}Y$i~BZHNk$8j<`%06xH*o$1LVr?)g<q={-L3
zZSKcs$-SwP#7w$Q7oT+W$*O86UrB!d{M)jrTJASXrnUcf%@(j=xH;d-@;AWZec1Q5
zvgjgM$r49dbP=q^5=U8d5v-jhj<V<?SbIwxWzj{j4wg8|qKmrq-__rL18-2#2XL1S
A0RR91
literal 0
HcmV?d00001
diff --git a/tests/data/acpi/virt/DSDT.topology b/tests/data/acpi/virt/DSDT.topology
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..501314c91be01d927fd125e0c72e919fdd85592e 100644
GIT binary patch
literal 5398
zcmZvg%WoT16o>EFlh__VVmr?J;S@^6vl`n?la>}@kDbINPK+mQkX*@?5QvgZB`Ty+
zA*ERq=#EBW9i&M78%V6!v17rS4gUZ;%(-)ClHYO9NEy$Wd(SuX%^YWrr|CEMr>B&P
zoiz5mZGWZlN!MGU#ZpS?ZT*>lwrAZR_>DpTc;0heH#yjDH?wuG+ooVmB?ougO=ZR^
z(wNmhUZA|HH0H$2U`-s1o55@1plt?M#lbN%cwHPEH-l^9V4{C~)7$Grmc7ol>sBhE
zWpd#4{KC95^E{>WrAev0Qa_9<%eq9-6S@lPn+M*e0e{@;+@&j2rCfi%?xZQ%t6K(9
zaB>C_OU;Ivb^Bf~y0|;Ly*)}@y*TW7=EcDs6$=mUA|kv89H9^U3L>U15S0+o&}R|e
zDvoes62k^Y6&c|j9bv>J#yBu)$Ov!z2*Z{bNnl(<Mpz#sj4_Gf0Am#yVHu4u#wA7u
z7}t>zR@(?8Au)2mSVP9TDXbAjQexzRv5t%zA|oX+iom!5j7s?B7	|Vw8Y!6B%Ne
z@-InL>eIk@9~p9;W~B3&1;#C8$aR{P81ulmjSRU?a}r|_7#|=*uG0yLu?&n4ks;S<
zUSg~OV*?p-ofag<Yrv=@L$1@J#JCKM1~TM2os<|?fZ+k7D%WXAV!R2ACNktYost-D
z1EYluxlX4g#=F4SM21|aGZNz}Ft(5(*XgXpaDlOn47pC{Bt{h&ZDh!GIxjJ<0pkub
z<T_oD7}tUE5i;aDU6dH>z}P{CT&GJC<0ddVz^KV}x-2nn0b>^#a-EhX#s|RI3mLn=
zbiH<X9^KupTX)x~`S7UGGf_=<F|93HHyXR=ZHd3%E0mqZuJTk{eWq5FOMgw;`dU3y
zpVFt&kf8DC_Vy=tzH*L=X*)d}sx80mDzk0Tc10C4dcPB+pc(~n3TmpDwKKz^rF0I>
z3nQIH6LV%P$fK!Is56Nl%%v{L%nc)*8BL`YNFR}=2ALG<%;+fbATv6HxYC)?)VRr{
zsX-=%I+M;QIEo!)MrU9LnbA~gnL^7TlS1?yW1eF{X5=|$GNY5H5Ix74CpD#XKG9Ta
zvCxx3^h_|%1oKRAPYTg9$vl(HlUg$Lq!2w*%#+$_bM=BtlH#5eqNl?=9p*_b9C}iS
zo@wTpW}a#8Ng;Y>m}iE0Qp<;)6ryLAd1jesmU~i&o;l{3W1iF^q9=vuIl(+9nCArd
zq!2yn=ZvZGpo;U%lUhpjq!2v|%(K8e3*3`J^ei&ZBJ-pc6g??K&q?Mv$vh{yCxz%)
zVxA@DNi8dSQiz`PW0|f{^dDl1c}{Up3ej_#c}_D=YH`t%LiC(ro-@pIhI>+op7i4q
z?&mD?q?Q;xDMZgX<~hea=eQ?@=sC|k=b0z9(CA4adM+@}1?IWHJt;)bMdrE4JgMbI
zPYTg<iFqzD&n50jA$l$|&t>LGEjoHqh@NHUS!SMP?n$BX>>syneJjn+H~mod+|Ba`
zahG08<eYTyD&qCvkxtLuSN4_02Y%0|_b~w~>=+n|-V-3|vVb!C&QW*tS%nQQL+SSg
z$a+IynSGoUHoBZe?+uW3MPQkIA*+-hc#XO`qyM2Qzd<W=Ikpqd<L|R7M*q%f8S0hw
z9eukp)LjHiemMM3|16_rc$G%14D|qJp{9kFA&pw<#XFD_3?Jz+y#&$4O7DN7lK$Op
zS0%mu-i|75rUrYyXTLa9Uh|-Gx}7-rqA=;?`<=gP|CSdwemZzu|Mm8tpT9VCY?@G|
z&m?`;9_c`H^hQmip6ZoT*6Y*!%ae!Jw=_}-W>-$9U!Fws%<jA%e55Dq{bz?i=gfY6
zkjmL%>AgYI@7Sl8%-Q_0_WR%d>NlMqXa4ET{pNK}Qzu`lvqIdm^om||b?jctXVs`*
zbm^L_IqoahC%6Z6b;=tTmqu^V^Txb4Yb5Sp)$bU$TFrqear1()q8m?o!I-6ikZ<Zd
zZoOqvk6JzIOX-d#Q;yw#me!%y@>@GArKLgZ-hS$l4j!E5Po6$-bien!d(dk*NB!eD
My@B5+&m2qr5A>`*Jpcdz
literal 0
HcmV?d00001
diff --git a/tests/data/acpi/virt/PPTT.topology b/tests/data/acpi/virt/PPTT.topology
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..3fbcae5ff08aaf16fedf4da45e941661d79c1174 100644
GIT binary patch
literal 336
zcmWFt2nh*bWME*baq@Te2v%^42yj*a0-z8Bhz+6{L>L&rG>8oYKrs+dflv?<DrSKu
z#s}p4;1GkGi=-D>45YUMh?!vef$Csl%t&G&Cde(wdO>1GKm-gx_1*yTS+Iz)B8h>R
aAic=uf$S9l3b27BK>%tVNQ@mK!T<mOd=3Es
literal 0
HcmV?d00001
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index 90f53f9c1d..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,4 +1 @@
/* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/virt/APIC.topology",
-"tests/data/acpi/virt/DSDT.topology",
-"tests/data/acpi/virt/PPTT.topology",
--
2.24.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/5] hw/acpi/aml-build: Only generate cluster node in PPTT when specified
2022-10-31 9:05 ` [PATCH v3 1/5] hw/acpi/aml-build: " Yicong Yang via
@ 2022-10-31 11:17 ` wangyanan (Y) via
2022-10-31 12:50 ` Yicong Yang via
0 siblings, 1 reply; 14+ messages in thread
From: wangyanan (Y) via @ 2022-10-31 11:17 UTC (permalink / raw)
To: Yicong Yang
Cc: jonathan.cameron, linuxarm, yangyicong, prime.zeng,
hesham.almatary, ionela.voinescu, darren, Michael S . Tsirkin,
Peter Maydell, Igor Mammedov, Ani Sinha, Eduardo Habkost,
Marcel Apfelbaum, Philippe Mathieu-Daudé,
qemu-devel@nongnu.org
On 2022/10/31 17:05, Yicong Yang wrote:
> From: Yicong Yang <yangyicong@hisilicon.com>
>
> Currently we'll always generate a cluster node no matter user has
> specified '-smp clusters=X' or not. Cluster is an optional level
> and will participant the building of Linux scheduling domains and
> only appears on a few platforms.
> It's unncessary to always build
> it which cannot reflect the real topology on platforms have no
> cluster and to avoid affecting the linux scheduling domains in the
> VM.
This sentence is a bit confusing, better to re-organize it.
> So only generate it when user specified explicitly.
"So only generate the cluster topology in ACPI PPTT when
the user has specified it explicitly in -smp."
Will this be more clear?
>
> Tested qemu-system-aarch64 with `-smp 8` and linux 6.1-rc1, without
> this patch:
> estuary:/sys/devices/system/cpu/cpu0/topology$ cat cluster_*
> ff # cluster_cpus
> 0-7 # cluster_cpus_list
> 56 # cluster_id
>
> with this patch:
> estuary:/sys/devices/system/cpu/cpu0/topology$ cat cluster_*
> ff # cluster_cpus
> 0-7 # cluster_cpus_list
> 36 # cluster_id, with no cluster node kernel will make it to
> physical package id
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> ---
> hw/acpi/aml-build.c | 2 +-
> hw/core/machine-smp.c | 3 +++
> include/hw/boards.h | 3 +++
> qemu-options.hx | 3 +++
> tests/qtest/bios-tables-test-allowed-diff.h | 1 +
> 5 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index e6bfac95c7..60c1acf3da 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -2030,7 +2030,7 @@ void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
> 0, socket_id, NULL, 0);
> }
>
> - if (mc->smp_props.clusters_supported) {
> + if (mc->smp_props.clusters_supported && mc->smp_props.has_clusters) {
> if (cpus->cpus[n].props.cluster_id != cluster_id) {
> assert(cpus->cpus[n].props.cluster_id > cluster_id);
> cluster_id = cpus->cpus[n].props.cluster_id;
> diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
> index b39ed21e65..9c8950b2b0 100644
> --- a/hw/core/machine-smp.c
> +++ b/hw/core/machine-smp.c
> @@ -158,6 +158,9 @@ void machine_parse_smp_config(MachineState *ms,
> ms->smp.threads = threads;
> ms->smp.max_cpus = maxcpus;
>
> + if (config->has_clusters)
> + mc->smp_props.has_clusters = true;
> +
why not "mc->smp_props.has_clusters = config->has_clusters"?
> /* sanity-check of the computed topology */
> if (sockets * dies * clusters * cores * threads != maxcpus) {
> g_autofree char *topo_msg = cpu_hierarchy_to_string(ms);
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 311ed17e18..06ed66453f 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -130,11 +130,14 @@ typedef struct {
> * @prefer_sockets - whether sockets are preferred over cores in smp parsing
> * @dies_supported - whether dies are supported by the machine
> * @clusters_supported - whether clusters are supported by the machine
> + * @has_clusters - whether clusters are explicitly specified in the user
> + * provided SMP configuration
> */
> typedef struct {
> bool prefer_sockets;
> bool dies_supported;
> bool clusters_supported;
> + bool has_clusters;
> } SMPCompatProps;
>
> /**
> diff --git a/qemu-options.hx b/qemu-options.hx
> index eb38e5dc40..bbdbdef0af 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -349,6 +349,9 @@ SRST
> ::
>
> -smp 2
> +
> + Note: The cluster topology will only be generated in ACPI and exposed
> + to guest if it's explicitly specified in -smp.
> ERST
>
> DEF("numa", HAS_ARG, QEMU_OPTION_numa,
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index dfb8523c8b..cb143a55a6 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1 +1,2 @@
> /* List of comma-separated changed AML files to ignore */
> +"tests/data/acpi/virt/PPTT",
This change in bios-tables-test-allowed-diff.h should be in a
standalone patch before this patch.
For your reference: see patch 4-6 in [1]:
https://patchew.org/QEMU/20220107083232.16256-1-wangyanan55@huawei.com/
Thanks,
Yanan
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/5] tests: virt: update expected ACPI tables for virt test
2022-10-31 9:05 ` [PATCH v3 2/5] tests: virt: update expected ACPI tables for virt test Yicong Yang via
@ 2022-10-31 11:21 ` wangyanan (Y) via
2022-10-31 11:24 ` Michael S. Tsirkin
2022-10-31 12:30 ` Michael S. Tsirkin
0 siblings, 2 replies; 14+ messages in thread
From: wangyanan (Y) via @ 2022-10-31 11:21 UTC (permalink / raw)
To: Yicong Yang
Cc: jonathan.cameron, linuxarm, yangyicong, prime.zeng,
hesham.almatary, ionela.voinescu, darren, Michael S . Tsirkin,
Peter Maydell, Igor Mammedov, Ani Sinha, Eduardo Habkost,
Marcel Apfelbaum, Philippe Mathieu-Daudé,
qemu-devel@nongnu.org
Hi Yicong,
On 2022/10/31 17:05, Yicong Yang wrote:
> From: Yicong Yang <yangyicong@hisilicon.com>
>
> Update the ACPI tables according to the acpi aml_build change.
We may also need the disassembled context of the table change
in the commit message, for review.
For your reference: see patch 6 in [1]:
https://patchew.org/QEMU/20220107083232.16256-1-wangyanan55@huawei.com/
Thanks,
Yanan
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> ---
> tests/data/acpi/virt/PPTT | Bin 96 -> 76 bytes
> tests/qtest/bios-tables-test-allowed-diff.h | 1 -
> 2 files changed, 1 deletion(-)
>
> diff --git a/tests/data/acpi/virt/PPTT b/tests/data/acpi/virt/PPTT
> index f56ea63b369a604877374ad696c396e796ab1c83..7a1258ecf123555b24462c98ccbb76b4ac1d0c2b 100644
> GIT binary patch
> delta 32
> fcmYfB;R*-{3GrcIU|?D?k;`ae01J-_kOKn%ZFdCM
>
> delta 53
> pcmeZC;0g!`2}xjJU|{l?$YrDgWH5jU5Ca567#O&Klm(arApowi1QY-O
>
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index cb143a55a6..dfb8523c8b 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1,2 +1 @@
> /* List of comma-separated changed AML files to ignore */
> -"tests/data/acpi/virt/PPTT",
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/5] tests: virt: update expected ACPI tables for virt test
2022-10-31 11:21 ` wangyanan (Y) via
@ 2022-10-31 11:24 ` Michael S. Tsirkin
2022-10-31 12:30 ` Michael S. Tsirkin
1 sibling, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2022-10-31 11:24 UTC (permalink / raw)
To: wangyanan (Y)
Cc: Yicong Yang, jonathan.cameron, linuxarm, yangyicong, prime.zeng,
hesham.almatary, ionela.voinescu, darren, Peter Maydell,
Igor Mammedov, Ani Sinha, Eduardo Habkost, Marcel Apfelbaum,
Philippe Mathieu-Daudé, qemu-devel@nongnu.org
On Mon, Oct 31, 2022 at 07:21:31PM +0800, wangyanan (Y) wrote:
> Hi Yicong,
>
> On 2022/10/31 17:05, Yicong Yang wrote:
> > From: Yicong Yang <yangyicong@hisilicon.com>
> >
> > Update the ACPI tables according to the acpi aml_build change.
> We may also need the disassembled context of the table change
> in the commit message, for review.
not just for review - in case it has to be rebased.
also pls make sure to
- edit the diff for clarify only retaining relevant parts
- prefix the diff with some prefix e.g. leading space in each line
to avoid confusing git
> For your reference: see patch 6 in [1]:
> https://patchew.org/QEMU/20220107083232.16256-1-wangyanan55@huawei.com/
>
> Thanks,
> Yanan
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> > ---
> > tests/data/acpi/virt/PPTT | Bin 96 -> 76 bytes
> > tests/qtest/bios-tables-test-allowed-diff.h | 1 -
> > 2 files changed, 1 deletion(-)
> >
> > diff --git a/tests/data/acpi/virt/PPTT b/tests/data/acpi/virt/PPTT
> > index f56ea63b369a604877374ad696c396e796ab1c83..7a1258ecf123555b24462c98ccbb76b4ac1d0c2b 100644
> > GIT binary patch
> > delta 32
> > fcmYfB;R*-{3GrcIU|?D?k;`ae01J-_kOKn%ZFdCM
> >
> > delta 53
> > pcmeZC;0g!`2}xjJU|{l?$YrDgWH5jU5Ca567#O&Klm(arApowi1QY-O
> >
> > diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> > index cb143a55a6..dfb8523c8b 100644
> > --- a/tests/qtest/bios-tables-test-allowed-diff.h
> > +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> > @@ -1,2 +1 @@
> > /* List of comma-separated changed AML files to ignore */
> > -"tests/data/acpi/virt/PPTT",
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 4/5] tests: acpi: aarch64: add topology test for aarch64
2022-10-31 9:05 ` [PATCH v3 4/5] tests: acpi: aarch64: add topology test for aarch64 Yicong Yang via
@ 2022-10-31 11:48 ` wangyanan (Y) via
2022-10-31 12:45 ` Yicong Yang via
0 siblings, 1 reply; 14+ messages in thread
From: wangyanan (Y) via @ 2022-10-31 11:48 UTC (permalink / raw)
To: Yicong Yang
Cc: jonathan.cameron, linuxarm, yangyicong, prime.zeng,
hesham.almatary, ionela.voinescu, darren, Michael S . Tsirkin,
Peter Maydell, Igor Mammedov, Ani Sinha, Eduardo Habkost,
Marcel Apfelbaum, Philippe Mathieu-Daudé,
qemu-devel@nongnu.org
Hi Yicong,
On 2022/10/31 17:05, Yicong Yang wrote:
> From: Yicong Yang <yangyicong@hisilicon.com>
>
> Add test for aarch64's ACPI topology building for all the supported
> levels.
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> ---
> tests/qtest/bios-tables-test.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index e6096e7f73..099b723444 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -1533,6 +1533,27 @@ static void test_acpi_virt_tcg(void)
> free_test_data(&data);
> }
>
> +static void test_acpi_virt_tcg_topology(void)
> +{
> + test_data data = {
> + .machine = "virt",
> + .variant = ".topology",
> + .tcg_only = true,
> + .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd",
> + .uefi_fl2 = "pc-bios/edk2-arm-vars.fd",
> + .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2",
> + .ram_start = 0x40000000ULL,
> + .scan_len = 128ULL * 1024 * 1024,
> + };
> +
> + data.smbios_cpu_max_speed = 2900;
> + data.smbios_cpu_curr_speed = 2700;
I'm not sure. But why do we need this two lines?
Can we keep the test as simple as test_acpi_virt_tcg_numamem
and avoid unrelated parts.
Thanks,
Yanan
> + test_acpi_one("-cpu cortex-a57 "
> + "-smbios type=4,max-speed=2900,current-speed=2700 "
> + "-smp sockets=1,clusters=2,cores=2,threads=2", &data);
> + free_test_data(&data);
> +}
> +
> static void test_acpi_q35_viot(void)
> {
> test_data data = {
> @@ -1864,6 +1885,7 @@ int main(int argc, char *argv[])
> } else if (strcmp(arch, "aarch64") == 0) {
> if (has_tcg) {
> qtest_add_func("acpi/virt", test_acpi_virt_tcg);
> + qtest_add_func("acpi/virt/topology", test_acpi_virt_tcg_topology);
> qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);
> qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp);
> qtest_add_func("acpi/virt/pxb", test_acpi_virt_tcg_pxb);
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/5] tests: virt: update expected ACPI tables for virt test
2022-10-31 11:21 ` wangyanan (Y) via
2022-10-31 11:24 ` Michael S. Tsirkin
@ 2022-10-31 12:30 ` Michael S. Tsirkin
2022-10-31 12:48 ` Yicong Yang via
1 sibling, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2022-10-31 12:30 UTC (permalink / raw)
To: wangyanan (Y)
Cc: Yicong Yang, jonathan.cameron, linuxarm, yangyicong, prime.zeng,
hesham.almatary, ionela.voinescu, darren, Peter Maydell,
Igor Mammedov, Ani Sinha, Eduardo Habkost, Marcel Apfelbaum,
Philippe Mathieu-Daudé, qemu-devel@nongnu.org
On Mon, Oct 31, 2022 at 07:21:31PM +0800, wangyanan (Y) wrote:
> Hi Yicong,
>
> On 2022/10/31 17:05, Yicong Yang wrote:
> > From: Yicong Yang <yangyicong@hisilicon.com>
> >
> > Update the ACPI tables according to the acpi aml_build change.
> We may also need the disassembled context of the table change
and it's not a "maybe need". We do need it.
> in the commit message, for review.
>
> For your reference: see patch 6 in [1]:
> https://patchew.org/QEMU/20220107083232.16256-1-wangyanan55@huawei.com/
>
> Thanks,
> Yanan
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> > ---
> > tests/data/acpi/virt/PPTT | Bin 96 -> 76 bytes
> > tests/qtest/bios-tables-test-allowed-diff.h | 1 -
> > 2 files changed, 1 deletion(-)
> >
> > diff --git a/tests/data/acpi/virt/PPTT b/tests/data/acpi/virt/PPTT
> > index f56ea63b369a604877374ad696c396e796ab1c83..7a1258ecf123555b24462c98ccbb76b4ac1d0c2b 100644
> > GIT binary patch
> > delta 32
> > fcmYfB;R*-{3GrcIU|?D?k;`ae01J-_kOKn%ZFdCM
> >
> > delta 53
> > pcmeZC;0g!`2}xjJU|{l?$YrDgWH5jU5Ca567#O&Klm(arApowi1QY-O
> >
> > diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> > index cb143a55a6..dfb8523c8b 100644
> > --- a/tests/qtest/bios-tables-test-allowed-diff.h
> > +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> > @@ -1,2 +1 @@
> > /* List of comma-separated changed AML files to ignore */
> > -"tests/data/acpi/virt/PPTT",
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 4/5] tests: acpi: aarch64: add topology test for aarch64
2022-10-31 11:48 ` wangyanan (Y) via
@ 2022-10-31 12:45 ` Yicong Yang via
0 siblings, 0 replies; 14+ messages in thread
From: Yicong Yang via @ 2022-10-31 12:45 UTC (permalink / raw)
To: wangyanan (Y)
Cc: yangyicong, jonathan.cameron, linuxarm, prime.zeng,
hesham.almatary, ionela.voinescu, darren, Michael S . Tsirkin,
Peter Maydell, Igor Mammedov, Ani Sinha, Eduardo Habkost,
Marcel Apfelbaum, Philippe Mathieu-Daudé,
qemu-devel@nongnu.org
On 2022/10/31 19:48, wangyanan (Y) wrote:
> Hi Yicong,
>
> On 2022/10/31 17:05, Yicong Yang wrote:
>> From: Yicong Yang <yangyicong@hisilicon.com>
>>
>> Add test for aarch64's ACPI topology building for all the supported
>> levels.
>>
>> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
>> ---
>> tests/qtest/bios-tables-test.c | 22 ++++++++++++++++++++++
>> 1 file changed, 22 insertions(+)
>>
>> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
>> index e6096e7f73..099b723444 100644
>> --- a/tests/qtest/bios-tables-test.c
>> +++ b/tests/qtest/bios-tables-test.c
>> @@ -1533,6 +1533,27 @@ static void test_acpi_virt_tcg(void)
>> free_test_data(&data);
>> }
>> +static void test_acpi_virt_tcg_topology(void)
>> +{
>> + test_data data = {
>> + .machine = "virt",
>> + .variant = ".topology",
>> + .tcg_only = true,
>> + .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd",
>> + .uefi_fl2 = "pc-bios/edk2-arm-vars.fd",
>> + .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2",
>> + .ram_start = 0x40000000ULL,
>> + .scan_len = 128ULL * 1024 * 1024,
>> + };
>> +
>> + data.smbios_cpu_max_speed = 2900;
>> + data.smbios_cpu_curr_speed = 2700;
> I'm not sure. But why do we need this two lines?
> Can we keep the test as simple as test_acpi_virt_tcg_numamem
> and avoid unrelated parts.
>
I guess it's because my silly copy and paste... will make it simpler.
> Thanks,
> Yanan
>> + test_acpi_one("-cpu cortex-a57 "
>> + "-smbios type=4,max-speed=2900,current-speed=2700 "
>> + "-smp sockets=1,clusters=2,cores=2,threads=2", &data);
>> + free_test_data(&data);
>> +}
>> +
>> static void test_acpi_q35_viot(void)
>> {
>> test_data data = {
>> @@ -1864,6 +1885,7 @@ int main(int argc, char *argv[])
>> } else if (strcmp(arch, "aarch64") == 0) {
>> if (has_tcg) {
>> qtest_add_func("acpi/virt", test_acpi_virt_tcg);
>> + qtest_add_func("acpi/virt/topology", test_acpi_virt_tcg_topology);
>> qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);
>> qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp);
>> qtest_add_func("acpi/virt/pxb", test_acpi_virt_tcg_pxb);
>
> .
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/5] tests: virt: update expected ACPI tables for virt test
2022-10-31 12:30 ` Michael S. Tsirkin
@ 2022-10-31 12:48 ` Yicong Yang via
0 siblings, 0 replies; 14+ messages in thread
From: Yicong Yang via @ 2022-10-31 12:48 UTC (permalink / raw)
To: Michael S. Tsirkin, wangyanan (Y)
Cc: yangyicong, jonathan.cameron, linuxarm, prime.zeng,
hesham.almatary, ionela.voinescu, darren, Peter Maydell,
Igor Mammedov, Ani Sinha, Eduardo Habkost, Marcel Apfelbaum,
Philippe Mathieu-Daudé, qemu-devel@nongnu.org
Hi Michael and Yanan,
On 2022/10/31 20:30, Michael S. Tsirkin wrote:
> On Mon, Oct 31, 2022 at 07:21:31PM +0800, wangyanan (Y) wrote:
>> Hi Yicong,
>>
>> On 2022/10/31 17:05, Yicong Yang wrote:
>>> From: Yicong Yang <yangyicong@hisilicon.com>
>>>
>>> Update the ACPI tables according to the acpi aml_build change.
>> We may also need the disassembled context of the table change
>
> and it's not a "maybe need". We do need it.
>
Got it. Let me check and attach these diff informations for all the
test table changes in this series. Thanks for the guidance!
Thanks.
>> in the commit message, for review.
>>
>> For your reference: see patch 6 in [1]:
>> https://patchew.org/QEMU/20220107083232.16256-1-wangyanan55@huawei.com/
>>
>> Thanks,
>> Yanan
>>> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>>> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
>>> ---
>>> tests/data/acpi/virt/PPTT | Bin 96 -> 76 bytes
>>> tests/qtest/bios-tables-test-allowed-diff.h | 1 -
>>> 2 files changed, 1 deletion(-)
>>>
>>> diff --git a/tests/data/acpi/virt/PPTT b/tests/data/acpi/virt/PPTT
>>> index f56ea63b369a604877374ad696c396e796ab1c83..7a1258ecf123555b24462c98ccbb76b4ac1d0c2b 100644
>>> GIT binary patch
>>> delta 32
>>> fcmYfB;R*-{3GrcIU|?D?k;`ae01J-_kOKn%ZFdCM
>>>
>>> delta 53
>>> pcmeZC;0g!`2}xjJU|{l?$YrDgWH5jU5Ca567#O&Klm(arApowi1QY-O
>>>
>>> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
>>> index cb143a55a6..dfb8523c8b 100644
>>> --- a/tests/qtest/bios-tables-test-allowed-diff.h
>>> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
>>> @@ -1,2 +1 @@
>>> /* List of comma-separated changed AML files to ignore */
>>> -"tests/data/acpi/virt/PPTT",
>
> .
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/5] hw/acpi/aml-build: Only generate cluster node in PPTT when specified
2022-10-31 11:17 ` wangyanan (Y) via
@ 2022-10-31 12:50 ` Yicong Yang via
0 siblings, 0 replies; 14+ messages in thread
From: Yicong Yang via @ 2022-10-31 12:50 UTC (permalink / raw)
To: wangyanan (Y)
Cc: yangyicong, jonathan.cameron, linuxarm, prime.zeng,
hesham.almatary, ionela.voinescu, darren, Michael S . Tsirkin,
Peter Maydell, Igor Mammedov, Ani Sinha, Eduardo Habkost,
Marcel Apfelbaum, Philippe Mathieu-Daudé,
qemu-devel@nongnu.org
On 2022/10/31 19:17, wangyanan (Y) wrote:
>
> On 2022/10/31 17:05, Yicong Yang wrote:
>> From: Yicong Yang <yangyicong@hisilicon.com>
>>
>> Currently we'll always generate a cluster node no matter user has
>> specified '-smp clusters=X' or not. Cluster is an optional level
>> and will participant the building of Linux scheduling domains and
>> only appears on a few platforms.
>> It's unncessary to always build
>> it which cannot reflect the real topology on platforms have no
>> cluster and to avoid affecting the linux scheduling domains in the
>> VM.
> This sentence is a bit confusing, better to re-organize it.
>> So only generate it when user specified explicitly.
> "So only generate the cluster topology in ACPI PPTT when
> the user has specified it explicitly in -smp."
> Will this be more clear?
ok. seems better.
will also address the comments below.
thanks.
>>
>> Tested qemu-system-aarch64 with `-smp 8` and linux 6.1-rc1, without
>> this patch:
>> estuary:/sys/devices/system/cpu/cpu0/topology$ cat cluster_*
>> ff # cluster_cpus
>> 0-7 # cluster_cpus_list
>> 56 # cluster_id
>>
>> with this patch:
>> estuary:/sys/devices/system/cpu/cpu0/topology$ cat cluster_*
>> ff # cluster_cpus
>> 0-7 # cluster_cpus_list
>> 36 # cluster_id, with no cluster node kernel will make it to
>> physical package id
>>
>> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
>> ---
>> hw/acpi/aml-build.c | 2 +-
>> hw/core/machine-smp.c | 3 +++
>> include/hw/boards.h | 3 +++
>> qemu-options.hx | 3 +++
>> tests/qtest/bios-tables-test-allowed-diff.h | 1 +
>> 5 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> index e6bfac95c7..60c1acf3da 100644
>> --- a/hw/acpi/aml-build.c
>> +++ b/hw/acpi/aml-build.c
>> @@ -2030,7 +2030,7 @@ void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
>> 0, socket_id, NULL, 0);
>> }
>> - if (mc->smp_props.clusters_supported) {
>> + if (mc->smp_props.clusters_supported && mc->smp_props.has_clusters) {
>> if (cpus->cpus[n].props.cluster_id != cluster_id) {
>> assert(cpus->cpus[n].props.cluster_id > cluster_id);
>> cluster_id = cpus->cpus[n].props.cluster_id;
>> diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
>> index b39ed21e65..9c8950b2b0 100644
>> --- a/hw/core/machine-smp.c
>> +++ b/hw/core/machine-smp.c
>> @@ -158,6 +158,9 @@ void machine_parse_smp_config(MachineState *ms,
>> ms->smp.threads = threads;
>> ms->smp.max_cpus = maxcpus;
>> + if (config->has_clusters)
>> + mc->smp_props.has_clusters = true;
>> +
> why not "mc->smp_props.has_clusters = config->has_clusters"?
>> /* sanity-check of the computed topology */
>> if (sockets * dies * clusters * cores * threads != maxcpus) {
>> g_autofree char *topo_msg = cpu_hierarchy_to_string(ms);
>> diff --git a/include/hw/boards.h b/include/hw/boards.h
>> index 311ed17e18..06ed66453f 100644
>> --- a/include/hw/boards.h
>> +++ b/include/hw/boards.h
>> @@ -130,11 +130,14 @@ typedef struct {
>> * @prefer_sockets - whether sockets are preferred over cores in smp parsing
>> * @dies_supported - whether dies are supported by the machine
>> * @clusters_supported - whether clusters are supported by the machine
>> + * @has_clusters - whether clusters are explicitly specified in the user
>> + * provided SMP configuration
>> */
>> typedef struct {
>> bool prefer_sockets;
>> bool dies_supported;
>> bool clusters_supported;
>> + bool has_clusters;
>> } SMPCompatProps;
>> /**
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index eb38e5dc40..bbdbdef0af 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -349,6 +349,9 @@ SRST
>> ::
>> -smp 2
>> +
>> + Note: The cluster topology will only be generated in ACPI and exposed
>> + to guest if it's explicitly specified in -smp.
>> ERST
>> DEF("numa", HAS_ARG, QEMU_OPTION_numa,
>> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
>> index dfb8523c8b..cb143a55a6 100644
>> --- a/tests/qtest/bios-tables-test-allowed-diff.h
>> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
>> @@ -1 +1,2 @@
>> /* List of comma-separated changed AML files to ignore */
>> +"tests/data/acpi/virt/PPTT",
> This change in bios-tables-test-allowed-diff.h should be in a
> standalone patch before this patch.
>
> For your reference: see patch 4-6 in [1]:
> https://patchew.org/QEMU/20220107083232.16256-1-wangyanan55@huawei.com/
>
> Thanks,
> Yanan
>
> .
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-10-31 12:50 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-31 9:05 [PATCH v3 0/5] Only generate cluster node in PPTT when specified Yicong Yang via
2022-10-31 9:05 ` [PATCH v3 1/5] hw/acpi/aml-build: " Yicong Yang via
2022-10-31 11:17 ` wangyanan (Y) via
2022-10-31 12:50 ` Yicong Yang via
2022-10-31 9:05 ` [PATCH v3 2/5] tests: virt: update expected ACPI tables for virt test Yicong Yang via
2022-10-31 11:21 ` wangyanan (Y) via
2022-10-31 11:24 ` Michael S. Tsirkin
2022-10-31 12:30 ` Michael S. Tsirkin
2022-10-31 12:48 ` Yicong Yang via
2022-10-31 9:05 ` [PATCH v3 3/5] tests: acpi: add and whitelist *.topology blobs Yicong Yang via
2022-10-31 9:05 ` [PATCH v3 4/5] tests: acpi: aarch64: add topology test for aarch64 Yicong Yang via
2022-10-31 11:48 ` wangyanan (Y) via
2022-10-31 12:45 ` Yicong Yang via
2022-10-31 9:05 ` [PATCH v3 5/5] tests: acpi: aarch64: add *.topology tables Yicong Yang via
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.