From patchwork Thu Sep 19 11:48:09 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Beulich X-Patchwork-Id: 11152211 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 65B6114ED for ; Thu, 19 Sep 2019 11:49:56 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4B17D21907 for ; Thu, 19 Sep 2019 11:49:56 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4B17D21907 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iAuus-0004l4-MK; Thu, 19 Sep 2019 11:48:06 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iAuuq-0004kr-UF for xen-devel@lists.xenproject.org; Thu, 19 Sep 2019 11:48:04 +0000 X-Inumbo-ID: 55a2ffc8-dad3-11e9-9656-12813bfff9fa Received: from mx1.suse.de (unknown [195.135.220.15]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 55a2ffc8-dad3-11e9-9656-12813bfff9fa; Thu, 19 Sep 2019 11:48:02 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 70618AF7B; Thu, 19 Sep 2019 11:48:01 +0000 (UTC) To: "xen-devel@lists.xenproject.org" From: Jan Beulich Message-ID: Date: Thu, 19 Sep 2019 13:48:09 +0200 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 Content-Language: en-US Subject: [Xen-devel] [PATCH] libxc/x86: avoid overflow in CPUID APIC ID adjustments X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Juergen Gross , Andrew Cooper , Ian Jackson , Wei Liu , =?utf-8?q?Ro?= =?utf-8?q?ger_Pau_Monn=C3=A9?= Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" Recent AMD processors may report up to 128 logical processors in CPUID leaf 1. Doubling this value produces 0 (which OSes sincerely dislike), as the respective field is only 8 bits wide. Suppress doubling the value (and its leaf 0x80000008 counterpart) in such a case. Additionally don't even do any adjustment when the host topology already includes room for multiple threads per core. Furthermore don't double the Maximum Cores Per Package at all - by us introducing a fake HTT effect, the core count doesn't need to change. Instead adjust the Maximum Logical Processors Sharing Cache field, which so far was zapped altogether. Also zap leaf 4 (and at the same time leaf 2) EDX output for AMD. Signed-off-by: Jan Beulich --- TBD: Using xc_physinfo() output here needs a better solution. The threads_per_core value returned is the count of active siblings of CPU 0, rather than a system wide applicable value (and constant over the entire session). Using CPUID output (leaves 4 and 8000001e) doesn't look viable either, due to this not really being the host values on PVH. Judging from the host feature set's HTT flag also wouldn't tell us whether there actually are multiple threads per core. TBD: The adjustment of Maximum Logical Processors Sharing Cache should perhaps occur only if an adjustment to leaf 1 has occurred. --- a/tools/libxc/xc_cpuid_x86.c +++ b/tools/libxc/xc_cpuid_x86.c @@ -251,6 +251,8 @@ struct cpuid_domain_info uint32_t *featureset; unsigned int nr_features; + bool host_htt; + /* PV-only information. */ bool pv64; @@ -290,6 +292,7 @@ static int get_cpuid_domain_info(xc_inte xc_dominfo_t di; unsigned int in[2] = { 0, ~0U }, regs[4]; unsigned int i, host_nr_features = xc_get_cpu_featureset_size(); + xc_physinfo_t physinfo; int rc; cpuid(in, regs); @@ -343,6 +346,10 @@ static int get_cpuid_domain_info(xc_inte info->xfeature_mask = domctl.u.vcpuextstate.xfeature_mask; + rc = xc_physinfo(xch, &physinfo); + if ( !rc && physinfo.threads_per_core > 1 ) + info->host_htt = true; + if ( di.hvm ) { uint64_t val; @@ -385,7 +392,7 @@ static void amd_xc_cpuid_policy(const st { case 0x00000002: case 0x00000004: - regs[0] = regs[1] = regs[2] = 0; + regs[0] = regs[1] = regs[2] = regs[3] = 0; break; case 0x80000000: @@ -395,11 +402,25 @@ static void amd_xc_cpuid_policy(const st case 0x80000008: /* - * ECX[15:12] is ApicIdCoreSize: ECX[7:0] is NumberOfCores (minus one). - * Update to reflect vLAPIC_ID = vCPU_ID * 2. + * ECX[15:12] is ApicIdCoreSize. + * ECX[7:0] is NumberOfCores (minus one). + */ + if ( info->host_htt ) + { + regs[2] &= 0xf0ffu; + break; + } + /* + * Update to reflect vLAPIC_ID = vCPU_ID * 2. But make sure to avoid + * - overflow, + * - going out of sync with leaf 1 EBX[23:16], + * - incrementing ApicIdCoreSize when it's zero (which changes the + * meaning of bits 7:0). */ - regs[2] = ((regs[2] + (1u << 12)) & 0xf000u) | - ((regs[2] & 0xffu) << 1) | 1u; + if ( (regs[2] & 0xf000u) && (regs[2] & 0xf000u) != 0xf000u ) + regs[2] = ((regs[2] + (1u << 12)) & 0xf000u) | (regs[2] & 0xffu); + if ( (regs[2] & 0x7fu) < 0x7fu ) + regs[2] = (regs[2] & 0xf000u) | ((regs[2] & 0x7fu) << 1) | 1u; break; case 0x8000000a: { @@ -442,10 +463,19 @@ static void intel_xc_cpuid_policy(const case 0x00000004: /* * EAX[31:26] is Maximum Cores Per Package (minus one). - * Update to reflect vLAPIC_ID = vCPU_ID * 2. + * EAX[25:14] is Maximum Logical Processors Sharing Cache (minus one). */ - regs[0] = (((regs[0] & 0x7c000000u) << 1) | 0x04000000u | - (regs[0] & 0x3ffu)); + if ( info->host_htt ) + regs[0] &= 0xffffc3ffu; + else + { + /* + * Update to reflect vLAPIC_ID = vCPU_ID * 2. Note that overflow + * is sufficiently benign here. + */ + regs[0] = (((regs[0] | 0x00002000u) << 1) & 0x03ffc000u) | + (regs[0] & 0xfc0003ffu); + } regs[3] &= 0x3ffu; break; @@ -478,9 +508,13 @@ static void xc_cpuid_hvm_policy(const st case 0x00000001: /* * EBX[23:16] is Maximum Logical Processors Per Package. - * Update to reflect vLAPIC_ID = vCPU_ID * 2. + * Update to reflect vLAPIC_ID = vCPU_ID * 2, but make sure to avoid + * overflow. */ - regs[1] = (regs[1] & 0x0000ffffu) | ((regs[1] & 0x007f0000u) << 1); + if ( !info->host_htt && !(regs[1] & 0x00800000u) ) + regs[1] = (regs[1] & 0x0000ffffu) | ((regs[1] & 0x007f0000u) << 1); + else + regs[1] &= 0x00ffffffu; regs[2] = info->featureset[featureword_of(X86_FEATURE_SSE3)]; regs[3] = (info->featureset[featureword_of(X86_FEATURE_FPU)] |