From nobody Sat Nov 23 16:09:46 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.libvirt.org designates 8.43.85.245 as permitted sender) client-ip=8.43.85.245; envelope-from=devel-bounces@lists.libvirt.org; helo=lists.libvirt.org; Authentication-Results: mx.zohomail.com; spf=pass (zohomail.com: domain of lists.libvirt.org designates 8.43.85.245 as permitted sender) smtp.mailfrom=devel-bounces@lists.libvirt.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.libvirt.org (lists.libvirt.org [8.43.85.245]) by mx.zohomail.com with SMTPS id 1715603741871199.00030989899915; Mon, 13 May 2024 05:35:41 -0700 (PDT) Received: by lists.libvirt.org (Postfix, from userid 996) id C8E9E1BBA; Mon, 13 May 2024 08:35:40 -0400 (EDT) Received: from lists.libvirt.org (localhost [IPv6:::1]) by lists.libvirt.org (Postfix) with ESMTP id F1BC01B28; Mon, 13 May 2024 08:34:11 -0400 (EDT) Received: by lists.libvirt.org (Postfix, from userid 996) id 6ADCC1AD2; Mon, 13 May 2024 08:34:06 -0400 (EDT) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.libvirt.org (Postfix) with ESMTPS id AD0211AD7 for ; Mon, 13 May 2024 08:34:05 -0400 (EDT) Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-663-OiADxjuAP6KTEO9ciwVKrg-1; Mon, 13 May 2024 08:34:02 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 219C6857A85; Mon, 13 May 2024 12:34:02 +0000 (UTC) Received: from toolbox.default.com (unknown [10.42.28.55]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9977C51BF; Mon, 13 May 2024 12:34:00 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on lists.libvirt.org X-Spam-Level: X-Spam-Status: No, score=-0.7 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL,SPF_HELO_NONE autolearn=unavailable autolearn_force=no version=3.4.4 X-MC-Unique: OiADxjuAP6KTEO9ciwVKrg-1 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= To: qemu-devel@nongnu.org Subject: [PATCH 1/2] hw/core: allow parameter=1 for SMP topology on any machine Date: Mon, 13 May 2024 13:33:57 +0100 Message-ID: <20240513123358.612355-2-berrange@redhat.com> In-Reply-To: <20240513123358.612355-1-berrange@redhat.com> References: <20240513123358.612355-1-berrange@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.5 Message-ID-Hash: TAZRXUCON6D7RM6J65TIS55HS5K7WPGY X-Message-ID-Hash: TAZRXUCON6D7RM6J65TIS55HS5K7WPGY X-MailFrom: berrange@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-config-1; header-match-config-2; header-match-config-3; header-match-devel.lists.libvirt.org-0; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; suspicious-header CC: Thomas Huth , devel@lists.libvirt.org, =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= , Eduardo Habkost , Marcel Apfelbaum , Peter Krempa , Zhao Liu , Yanan Wang X-Mailman-Version: 3.2.2 Precedence: list List-Id: Development discussions about the libvirt library & tools Archived-At: List-Archive: List-Help: List-Post: List-Subscribe: List-Unsubscribe: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-ZM-MESSAGEID: 1715603864655100001 This effectively reverts commit 54c4ea8f3ae614054079395842128a856a73dbf9 Author: Zhao Liu Date: Sat Mar 9 00:01:37 2024 +0800 hw/core/machine-smp: Deprecate unsupported "parameter=3D1" SMP configur= ations 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=3D1,books=3D1,clusters=3D1". 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=3D1' 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=3D1' 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=3D1' 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=C3=A9 Reviewed-by: Zhao Liu --- docs/about/deprecated.rst | 14 ------- hw/core/machine-smp.c | 82 ++++++++++++------------------------- tests/unit/test-smp-parse.c | 8 ++-- 3 files changed, 30 insertions(+), 74 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index e22acb17f2..5b551b12a6 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -47,20 +47,6 @@ as short-form boolean values, and passed to plugins as `= `arg_name=3Don``. However, short-form booleans are deprecated and full explicit ``arg_name= =3Don`` form is preferred. =20 -``-smp`` (Unsupported "parameter=3D1" SMP configurations) (since 9.0) -''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' - -Specified CPU topology parameters must be supported by the machine. - -In the SMP configuration, users should provide the CPU topology parameters= that -are supported by the target machine. - -However, historically it was allowed for users to specify the unsupported -topology parameter as "1", which is meaningless. So support for this kind = of -configurations (e.g. -smp drawers=3D1,books=3D1,clusters=3D1 for x86 PC ma= chine) is -marked deprecated since 9.0, users have to ensure that all the topology me= mbers -described with -smp are supported by the target machine. - User-mode emulator command line arguments ----------------------------------------- =20 diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c index 2b93fa99c9..eb43caca9b 100644 --- a/hw/core/machine-smp.c +++ b/hw/core/machine-smp.c @@ -119,75 +119,45 @@ void machine_parse_smp_config(MachineState *ms, =20 /* * If not supported by the machine, a topology parameter must be - * omitted. + * not be set to a value greater than 1. */ - if (!mc->smp_props.modules_supported && config->has_modules) { - if (config->modules > 1) { - error_setg(errp, "modules not supported by this " - "machine's CPU topology"); - return; - } else { - /* Here modules only equals 1 since we've checked zero case. */ - warn_report("Deprecated CPU topology (considered invalid): " - "Unsupported modules parameter mustn't be " - "specified as 1"); - } + if (!mc->smp_props.modules_supported && + config->has_modules && config->modules > 1) { + error_setg(errp, + "modules > 1 not supported by this machine's CPU topolo= gy"); + return; } modules =3D modules > 0 ? modules : 1; =20 - if (!mc->smp_props.clusters_supported && config->has_clusters) { - if (config->clusters > 1) { - error_setg(errp, "clusters not supported by this " - "machine's CPU topology"); - return; - } else { - /* Here clusters only equals 1 since we've checked zero case. = */ - warn_report("Deprecated CPU topology (considered invalid): " - "Unsupported clusters parameter mustn't be " - "specified as 1"); - } + if (!mc->smp_props.clusters_supported && + config->has_clusters && config->clusters > 1) { + error_setg(errp, + "clusters > 1 not supported by this machine's CPU topol= ogy"); + return; } clusters =3D clusters > 0 ? clusters : 1; =20 - if (!mc->smp_props.dies_supported && config->has_dies) { - if (config->dies > 1) { - error_setg(errp, "dies not supported by this " - "machine's CPU topology"); - return; - } else { - /* Here dies only equals 1 since we've checked zero case. */ - warn_report("Deprecated CPU topology (considered invalid): " - "Unsupported dies parameter mustn't be " - "specified as 1"); - } + if (!mc->smp_props.dies_supported && + config->has_dies && config->dies > 1) { + error_setg(errp, + "dies > 1 not supported by this machine's CPU topology"= ); + return; } dies =3D dies > 0 ? dies : 1; =20 - if (!mc->smp_props.books_supported && config->has_books) { - if (config->books > 1) { - error_setg(errp, "books not supported by this " - "machine's CPU topology"); - return; - } else { - /* Here books only equals 1 since we've checked zero case. */ - warn_report("Deprecated CPU topology (considered invalid): " - "Unsupported books parameter mustn't be " - "specified as 1"); - } + if (!mc->smp_props.books_supported && + config->has_books && config->books > 1) { + error_setg(errp, + "books > 1 not supported by this machine's CPU topology= "); + return; } books =3D books > 0 ? books : 1; =20 - if (!mc->smp_props.drawers_supported && config->has_drawers) { - if (config->drawers > 1) { - error_setg(errp, "drawers not supported by this " - "machine's CPU topology"); - return; - } else { - /* Here drawers only equals 1 since we've checked zero case. */ - warn_report("Deprecated CPU topology (considered invalid): " - "Unsupported drawers parameter mustn't be " - "specified as 1"); - } + if (!mc->smp_props.drawers_supported && + config->has_drawers && config->drawers > 1) { + error_setg(errp, + "drawers > 1 not supported by this machine's CPU topolo= gy"); + return; } drawers =3D drawers > 0 ? drawers : 1; =20 diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c index 8994337e12..56165e6644 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[= ] =3D { { /* config: -smp 2,dies=3D2 */ .config =3D SMP_CONFIG_WITH_DIES(T, 2, F, 0, T, 2, F, 0, F, 0, F, = 0), - .expect_error =3D "dies not supported by this machine's CPU topolo= gy", + .expect_error =3D "dies > 1 not supported by this machine's CPU to= pology", }, { /* config: -smp 2,clusters=3D2 */ .config =3D SMP_CONFIG_WITH_CLUSTERS(T, 2, F, 0, T, 2, F, 0, F, 0,= F, 0), - .expect_error =3D "clusters not supported by this machine's CPU to= pology", + .expect_error =3D "clusters > 1 not supported by this machine's CP= U topology", }, { /* config: -smp 2,books=3D2 */ .config =3D SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 2, F, 0, T, 2, F, 0, F, 0, F, 0, F, 0), - .expect_error =3D "books not supported by this machine's CPU topol= ogy", + .expect_error =3D "books > 1 not supported by this machine's CPU t= opology", }, { /* config: -smp 2,drawers=3D2 */ .config =3D SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 2, T, 2, F, 0, F, 0, F, 0, F, 0, F, 0), - .expect_error =3D "drawers not supported by this machine's CPU top= ology", + .expect_error =3D "drawers > 1 not supported by this machine's CPU= topology", }, { /* config: -smp 8,sockets=3D2,cores=3D4,threads=3D2,maxcpus=3D8 */ .config =3D SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, T, 2, T, 8), --=20 2.43.0 _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org