From 9d7950edb0cdf8f4e5746e220e6e8a9e713bad16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Mon, 13 May 2024 13:33:57 +0100 Subject: hw/core: allow parameter=1 for SMP topology on any machine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This effectively reverts commit 54c4ea8f3ae614054079395842128a856a73dbf9 Author: Zhao Liu Date: Sat Mar 9 00:01:37 2024 +0800 hw/core/machine-smp: Deprecate unsupported "parameter=1" SMP configurations but is not done as a 'git revert' since the part of the changes to the file hw/core/machine-smp.c which add 'has_XXX' checks remain desirable. Furthermore, we have to tweak the subsequently added unit test to account for differing warning message. The rationale for the original deprecation was: "Currently, it was allowed for users to specify the unsupported topology parameter as "1". For example, x86 PC machine doesn't support drawer/book/cluster topology levels, but user could specify "-smp drawers=1,books=1,clusters=1". This is meaningless and confusing, so that the support for this kind of configurations is marked deprecated since 9.0." There are varying POVs on the topic of 'unsupported' topology levels. It is common to say that on a system without hyperthreading, that there is always 1 thread. Likewise when new CPUs introduced a concept of multiple "dies', it was reasonable to say that all historical CPUs before that implicitly had 1 'die'. Likewise for the more recently introduced 'modules' and 'clusters' parameter'. From this POV, it is valid to set 'parameter=1' on the -smp command line for any machine, only a value > 1 is strictly an error condition. It doesn't cause any functional difficulty for QEMU, because internally the QEMU code is itself assuming that all "unsupported" parameters implicitly have a value of '1'. At the libvirt level, we've allowed applications to set 'parameter=1' when configuring a guest, and pass that through to QEMU. Deprecating this creates extra difficulty for because there's no info exposed from QEMU about which machine types "support" which parameters. Thus, libvirt can't know whether it is valid to pass 'parameter=1' for a given machine type, or whether it will trigger deprecation messages. Since there's no apparent functional benefit to deleting this deprecated behaviour from QEMU, and it creates problems for consumers of QEMU, remove this deprecation. Signed-off-by: Daniel P. Berrangé Reviewed-by: Zhao Liu Reviewed-by: Ján Tomko Message-ID: <20240513123358.612355-2-berrange@redhat.com> Signed-off-by: Philippe Mathieu-Daudé --- tests/unit/test-smp-parse.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'tests') diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c index 8994337..56165e6 100644 --- a/tests/unit/test-smp-parse.c +++ b/tests/unit/test-smp-parse.c @@ -337,21 +337,21 @@ static const struct SMPTestData data_generic_invalid[] = { { /* config: -smp 2,dies=2 */ .config = SMP_CONFIG_WITH_DIES(T, 2, F, 0, T, 2, F, 0, F, 0, F, 0), - .expect_error = "dies not supported by this machine's CPU topology", + .expect_error = "dies > 1 not supported by this machine's CPU topology", }, { /* config: -smp 2,clusters=2 */ .config = SMP_CONFIG_WITH_CLUSTERS(T, 2, F, 0, T, 2, F, 0, F, 0, F, 0), - .expect_error = "clusters not supported by this machine's CPU topology", + .expect_error = "clusters > 1 not supported by this machine's CPU topology", }, { /* config: -smp 2,books=2 */ .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 2, F, 0, T, 2, F, 0, F, 0, F, 0, F, 0), - .expect_error = "books not supported by this machine's CPU topology", + .expect_error = "books > 1 not supported by this machine's CPU topology", }, { /* config: -smp 2,drawers=2 */ .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 2, T, 2, F, 0, F, 0, F, 0, F, 0, F, 0), - .expect_error = "drawers not supported by this machine's CPU topology", + .expect_error = "drawers > 1 not supported by this machine's CPU topology", }, { /* config: -smp 8,sockets=2,cores=4,threads=2,maxcpus=8 */ .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, T, 2, T, 8), -- cgit v1.1 From e68dcbb07923df0886802727edc3b21a10b0d342 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Mon, 13 May 2024 13:33:58 +0100 Subject: tests: add testing of parameter=1 for SMP topology MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validate that it is possible to pass 'parameter=1' for any SMP topology parameter, since unsupported parameters are implicitly considered to always have a value of 1. Signed-off-by: Daniel P. Berrangé Reviewed-by: Zhao Liu Reviewed-by: Ján Tomko Message-ID: <20240513123358.612355-3-berrange@redhat.com> Signed-off-by: Philippe Mathieu-Daudé --- tests/unit/test-smp-parse.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'tests') diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c index 56165e6..9fdba24 100644 --- a/tests/unit/test-smp-parse.c +++ b/tests/unit/test-smp-parse.c @@ -330,6 +330,14 @@ static const struct SMPTestData data_generic_valid[] = { .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, T, 2, T, 16), .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16), .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16), + }, { + /* + * Unsupported parameters are always allowed to be set to '1' + * config: -smp 8,books=1,drawers=1,sockets=2,modules=1,dies=1,cores=2,threads=2,maxcpus=8 + * expect: cpus=8,sockets=2,cores=2,threads=2,maxcpus=8 */ + .config = SMP_CONFIG_WITH_FULL_TOPO(8, 1, 1, 2, 1, 1, 2, 2, 8), + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 2, 2, 8), + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 2, 2, 8), }, }; -- cgit v1.1 From 93a3048dcf4565c73f2aa1d751f7197e296f1f1f Mon Sep 17 00:00:00 2001 From: Gustavo Romero Date: Wed, 15 May 2024 17:31:31 +0000 Subject: tests: Gently exit from GDB when tests complete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GDB commit a207f6b3a38 ('Rewrite "python" command exception handling') changed how exit() called from Python scripts loaded by GDB behave, turning it into an exception instead of a generic error code that is returned. This change caused several QEMU tests to crash with the following exception: Python Exception : 0 Error occurred in Python: 0 This happens because in tests/guest-debug/test_gdbstub.py exit is called after the tests have completed. This commit fixes it by politely asking GDB to exit via gdb.execute, passing the proper fail_count to be reported to 'make', instead of abruptly calling exit() from the Python script. Signed-off-by: Gustavo Romero Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20240515173132.2462201-4-gustavo.romero@linaro.org> Signed-off-by: Philippe Mathieu-Daudé --- tests/guest-debug/test_gdbstub.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/guest-debug/test_gdbstub.py b/tests/guest-debug/test_gdbstub.py index 7f71d34..46fbf98 100644 --- a/tests/guest-debug/test_gdbstub.py +++ b/tests/guest-debug/test_gdbstub.py @@ -57,4 +57,4 @@ def main(test, expected_arch=None): pass print("All tests complete: {} failures".format(fail_count)) - exit(fail_count) + gdb.execute(f"exit {fail_count}") -- cgit v1.1