From patchwork Mon Nov 6 11:03:17 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Philippe_Mathieu-Daud=C3=A9?= X-Patchwork-Id: 741352 Delivered-To: patch@linaro.org Received: by 2002:adf:fd90:0:b0:32d:baff:b0ca with SMTP id d16csp1029103wrr; Mon, 6 Nov 2023 03:23:05 -0800 (PST) X-Google-Smtp-Source: AGHT+IF52YeOsc7BXpmyH0GWyVF924mf9ybLuZTB2OOphNXzjIgrivqibV+OGmkDcwUpMFT/axgf X-Received: by 2002:a9d:6ac3:0:b0:6bb:1071:ea72 with SMTP id m3-20020a9d6ac3000000b006bb1071ea72mr27274221otq.36.1699269784767; Mon, 06 Nov 2023 03:23:04 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1699269784; cv=none; d=google.com; s=arc-20160816; b=uSq0iae1myc68Il+l/1j0nJk3KaY5rGY867zspDFEde2b4pPMre3OWbEVMmygHTZdI 4SLHWyIzO6pMUbPxO/ZquFdhoASvhBp0BINktN/yJQy4P+ySY4TCFMXxBh1Z/agU1wwp d6idwwgBMAXgP+NlsWfHWamK//0r9o4vflh+zcl0Bd30iIQQBGaIFVd7cIqh98HKCou7 RAxVFriqGC1U3WNUbloPtZZsgwvRNfHo1JaVJopvUU3tTfefPjq5NCD6a/S4SnL4gLuw WLAAX2HmX8du3DwfXhbnnEBf/GeI7R7rUseld3tXzOmro5WACC7ZJMeBD0OqYqT96v7F RgEw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:content-transfer-encoding :mime-version:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=0yFelocR66jYXowauowrhR0mUeB3U99cidSOqtYw+Cg=; fh=uPeuKhg1mH9zdni2B1WgSqdekR1mTgg7x5s27WSu2zA=; b=MrlSw8iyDvRPRDCzjdNRjrUKp2vEIQKdLNvHOv8xvLbkZGTO+AHqYgfccoBO+F+mx6 +jZ7O0XdliUV4oYK+zGRtFA37lUy/61zEd9Zrwc8ejg0HV0oLd3QOO6vVzqFAvlxRNoe EPUU7mRY87QwvEHGVrUb9aDtnN8lvBOU8m+e6bE259jDVQDs5X7b89vQyscjB72B2O87 uxMbpvW2KQF8g96GU6gFdUmQeCypXl5dj74op5BSm0Lw2QVHAWgUa5P6zuMAqmX9UXsk UU08Z3XaNnyy7Xbd0RjN+hXyfYVIkPNu8gBFRJJ9VRFAI9kKRsCk2v1NNzD95YkL7LFH 9pBg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=VMAjPMAS; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from lists.gnu.org (lists.gnu.org. [209.51.188.17]) by mx.google.com with ESMTPS id v6-20020a05622a144600b0041b826198bdsi5348627qtx.270.2023.11.06.03.23.04 for (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Mon, 06 Nov 2023 03:23:04 -0800 (PST) Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=VMAjPMAS; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qzxTX-0007Oq-2y; Mon, 06 Nov 2023 06:08:59 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qzxTR-00072m-OK for qemu-devel@nongnu.org; Mon, 06 Nov 2023 06:08:53 -0500 Received: from mail-wr1-x42d.google.com ([2a00:1450:4864:20::42d]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qzxTM-0005FO-M0 for qemu-devel@nongnu.org; Mon, 06 Nov 2023 06:08:53 -0500 Received: by mail-wr1-x42d.google.com with SMTP id ffacd0b85a97d-32f7abbb8b4so2789749f8f.0 for ; Mon, 06 Nov 2023 03:08:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1699268927; x=1699873727; darn=nongnu.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=0yFelocR66jYXowauowrhR0mUeB3U99cidSOqtYw+Cg=; b=VMAjPMAS6edt0sctsVploARAsqA0Ru+q2aYkZUJg8afRfE8EWtEuWrDGq26/dzGwFz VDUiJtUOoXnBUWlSvPQ87n7RBWM6JiyvZYicIeFVDzkxeC02d+KM6nbmAImbDcPtxW15 C915m5/KkYkYOyKMnM9k0wrwz9e21qKQ2lUZdIXf56+hS4SUXb8dxatM6ba6ts3HIWbt s6Hul5Eg6veHwB0X1UqX3e99NPRxh8W45tTMJS33yp+3NhqUmpe/oY/95Dywhk/FLUct Ci/HqLNYfghAa2H3IE8JlfqTGBfesvG++hT/bmF4YyieHKfi0/Jyq5r6/666FbHjxucx lAsw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699268927; x=1699873727; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=0yFelocR66jYXowauowrhR0mUeB3U99cidSOqtYw+Cg=; b=ri20xWejzXsPNqkK2QgV6jGmGB+I0J8Gl79z1wOwGXuMkflLixzQqvEMAzl64rmofG H9alNNedBuupSA6zdFqMl/mMTkf8jOjBrvKF/KKwfiuJKfZxFHgvs+0CYmO48XwJ2jY+ iQHsDPc+PsatV71oFO8miT4f2lnM2vRSodS0ir5xAIb6R0Lpbxpu23PDl9FurnAPOabk J504SzBnkHDP3fFCPJbEoSoh+ArqXoWkuDjN4LpTu3N7pC24W60YF0wqxFi1/GR9TL3K EJ2Sl4EKlo9eXoSn1a3xpzKxw0T8mWTTk8D45dUgx2bDzWp5FsiW3XFxu1cOmFsVCuJz JCwA== X-Gm-Message-State: AOJu0YzfYa0h4Cm5qrF+H1Rtnj4LVXf0fza004o8Uv+oUOnYlu/QxZXB udSxQR/7acAsPOZiwoQLHGtlzfXW3FESNKN59kk= X-Received: by 2002:a5d:6c6f:0:b0:32f:8085:7411 with SMTP id r15-20020a5d6c6f000000b0032f80857411mr21345810wrz.24.1699268927044; Mon, 06 Nov 2023 03:08:47 -0800 (PST) Received: from m1x-phil.lan (176-131-220-199.abo.bbox.fr. [176.131.220.199]) by smtp.gmail.com with ESMTPSA id g8-20020a5d4888000000b0032f7cc56509sm1544640wrq.98.2023.11.06.03.08.44 (version=TLS1_3 cipher=TLS_CHACHA20_POLY1305_SHA256 bits=256/256); Mon, 06 Nov 2023 03:08:46 -0800 (PST) From: =?utf-8?q?Philippe_Mathieu-Daud=C3=A9?= To: qemu-devel@nongnu.org Cc: kvm@vger.kernel.org, qemu-s390x@nongnu.org, qemu-block@nongnu.org, qemu-riscv@nongnu.org, qemu-ppc@nongnu.org, qemu-arm@nongnu.org, Zhuocheng Ding , Zhao Liu , Babu Moger , Yongwei Ma , "Michael S . Tsirkin" , =?utf-8?q?Philippe_Mathieu-Daud?= =?utf-8?q?=C3=A9?= , Paolo Bonzini , Richard Henderson Subject: [PULL 45/60] system/cpus: Fix CPUState.nr_cores' calculation Date: Mon, 6 Nov 2023 12:03:17 +0100 Message-ID: <20231106110336.358-46-philmd@linaro.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20231106110336.358-1-philmd@linaro.org> References: <20231106110336.358-1-philmd@linaro.org> MIME-Version: 1.0 Received-SPF: pass client-ip=2a00:1450:4864:20::42d; envelope-from=philmd@linaro.org; helo=mail-wr1-x42d.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: qemu-devel-bounces+patch=linaro.org@nongnu.org From: Zhuocheng Ding >From CPUState.nr_cores' comment, it represents "number of cores within this CPU package". After 003f230e37d7 ("machine: Tweak the order of topology members in struct CpuTopology"), the meaning of smp.cores changed to "the number of cores in one die", but this commit missed to change CPUState.nr_cores' calculation, so that CPUState.nr_cores became wrong and now it misses to consider numbers of clusters and dies. At present, only i386 is using CPUState.nr_cores. But as for i386, which supports die level, the uses of CPUState.nr_cores are very confusing: Early uses are based on the meaning of "cores per package" (before die is introduced into i386), and later uses are based on "cores per die" (after die's introduction). This difference is due to that commit a94e1428991f ("target/i386: Add CPUID.1F generation support for multi-dies PCMachine") misunderstood that CPUState.nr_cores means "cores per die" when calculated CPUID.1FH.01H:EBX. After that, the changes in i386 all followed this wrong understanding. With the influence of 003f230e37d7 and a94e1428991f, for i386 currently the result of CPUState.nr_cores is "cores per die", thus the original uses of CPUState.cores based on the meaning of "cores per package" are wrong when multiple dies exist: 1. In cpu_x86_cpuid() of target/i386/cpu.c, CPUID.01H:EBX[bits 23:16] is incorrect because it expects "cpus per package" but now the result is "cpus per die". 2. In cpu_x86_cpuid() of target/i386/cpu.c, for all leaves of CPUID.04H: EAX[bits 31:26] is incorrect because they expect "cpus per package" but now the result is "cpus per die". The error not only impacts the EAX calculation in cache_info_passthrough case, but also impacts other cases of setting cache topology for Intel CPU according to cpu topology (specifically, the incoming parameter "num_cores" expects "cores per package" in encode_cache_cpuid4()). 3. In cpu_x86_cpuid() of target/i386/cpu.c, CPUID.0BH.01H:EBX[bits 15:00] is incorrect because the EBX of 0BH.01H (core level) expects "cpus per package", which may be different with 1FH.01H (The reason is 1FH can support more levels. For QEMU, 1FH also supports die, 1FH.01H:EBX[bits 15:00] expects "cpus per die"). 4. In cpu_x86_cpuid() of target/i386/cpu.c, when CPUID.80000001H is calculated, here "cpus per package" is expected to be checked, but in fact, now it checks "cpus per die". Though "cpus per die" also works for this code logic, this isn't consistent with AMD's APM. 5. In cpu_x86_cpuid() of target/i386/cpu.c, CPUID.80000008H:ECX expects "cpus per package" but it obtains "cpus per die". 6. In simulate_rdmsr() of target/i386/hvf/x86_emu.c, in kvm_rdmsr_core_thread_count() of target/i386/kvm/kvm.c, and in helper_rdmsr() of target/i386/tcg/sysemu/misc_helper.c, MSR_CORE_THREAD_COUNT expects "cpus per package" and "cores per package", but in these functions, it obtains "cpus per die" and "cores per die". On the other hand, these uses are correct now (they are added in/after a94e1428991f): 1. In cpu_x86_cpuid() of target/i386/cpu.c, topo_info.cores_per_die meets the actual meaning of CPUState.nr_cores ("cores per die"). 2. In cpu_x86_cpuid() of target/i386/cpu.c, vcpus_per_socket (in CPUID. 04H's calculation) considers number of dies, so it's correct. 3. In cpu_x86_cpuid() of target/i386/cpu.c, CPUID.1FH.01H:EBX[bits 15:00] needs "cpus per die" and it gets the correct result, and CPUID.1FH.02H:EBX[bits 15:00] gets correct "cpus per package". When CPUState.nr_cores is correctly changed to "cores per package" again , the above errors will be fixed without extra work, but the "currently" correct cases will go wrong and need special handling to pass correct "cpus/cores per die" they want. Fix CPUState.nr_cores' calculation to fit the original meaning "cores per package", as well as changing calculation of topo_info.cores_per_die, vcpus_per_socket and CPUID.1FH. Fixes: a94e1428991f ("target/i386: Add CPUID.1F generation support for multi-dies PCMachine") Fixes: 003f230e37d7 ("machine: Tweak the order of topology members in struct CpuTopology") Signed-off-by: Zhuocheng Ding Co-developed-by: Zhao Liu Signed-off-by: Zhao Liu Tested-by: Babu Moger Tested-by: Yongwei Ma Acked-by: Michael S. Tsirkin Message-ID: <20231024090323.1859210-4-zhao1.liu@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé --- system/cpus.c | 2 +- target/i386/cpu.c | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/system/cpus.c b/system/cpus.c index 952f15868c..a444a747f0 100644 --- a/system/cpus.c +++ b/system/cpus.c @@ -631,7 +631,7 @@ void qemu_init_vcpu(CPUState *cpu) { MachineState *ms = MACHINE(qdev_get_machine()); - cpu->nr_cores = ms->smp.cores; + cpu->nr_cores = machine_topo_get_cores_per_socket(ms); cpu->nr_threads = ms->smp.threads; cpu->stopped = true; cpu->random_seed = qemu_guest_random_seed_thread_part1(); diff --git a/target/i386/cpu.c b/target/i386/cpu.c index fc8484cb5e..358d9c0a65 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -6019,7 +6019,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, X86CPUTopoInfo topo_info; topo_info.dies_per_pkg = env->nr_dies; - topo_info.cores_per_die = cs->nr_cores; + topo_info.cores_per_die = cs->nr_cores / env->nr_dies; topo_info.threads_per_core = cs->nr_threads; /* Calculate & apply limits for different index ranges */ @@ -6095,8 +6095,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, */ if (*eax & 31) { int host_vcpus_per_cache = 1 + ((*eax & 0x3FFC000) >> 14); - int vcpus_per_socket = env->nr_dies * cs->nr_cores * - cs->nr_threads; + int vcpus_per_socket = cs->nr_cores * cs->nr_threads; if (cs->nr_cores > 1) { *eax &= ~0xFC000000; *eax |= (pow2ceil(cs->nr_cores) - 1) << 26; @@ -6273,12 +6272,12 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, break; case 1: *eax = apicid_die_offset(&topo_info); - *ebx = cs->nr_cores * cs->nr_threads; + *ebx = topo_info.cores_per_die * topo_info.threads_per_core; *ecx |= CPUID_TOPOLOGY_LEVEL_CORE; break; case 2: *eax = apicid_pkg_offset(&topo_info); - *ebx = env->nr_dies * cs->nr_cores * cs->nr_threads; + *ebx = cs->nr_cores * cs->nr_threads; *ecx |= CPUID_TOPOLOGY_LEVEL_DIE; break; default: