From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web10.1158.1571931918737192777 for ; Thu, 24 Oct 2019 08:45:19 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=O8UvSPiD; spf=pass (domain: redhat.com, ip: 205.139.110.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1571931917; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=7woa4OPVWD+uqc3J6SBvKSx387SliMi4EqxY5XASUYA=; b=O8UvSPiD28F88bLzOUoi4oSYh08AvNcZAzgI3DAn7l1gatHtuL4hqKaC+1OCKsAgi1HZxj nIlZqVRX+u3TNcKuxYyzB3GGaKHn6i6SrLx9XrrAQDSsQQK9amw/gFz7QLYRrt9CpDiQwx RzXMNTns2yHisbBIIg4sM93wOUYvzgw= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-100-t6JPmtyMNzSkJoQcApSEvg-1; Thu, 24 Oct 2019 11:45:09 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A7D918B7CE2; Thu, 24 Oct 2019 15:28:55 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (unknown [10.36.118.161]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1CDAC5ED48; Thu, 24 Oct 2019 15:28:53 +0000 (UTC) Subject: Re: [PATCH v2 3/3] OvmfPkg/PlatformPei: rewrite MaxCpuCountInitialization() for CPU hotplug To: Anthony PERARD Cc: edk2-devel-groups-io , Ard Biesheuvel , Igor Mammedov , Jordan Justen , Julien Grall References: <20191022221554.14963-1-lersek@redhat.com> <20191022221554.14963-4-lersek@redhat.com> <20191024142757.GN1138@perard.uk.xensource.com> From: "Laszlo Ersek" Message-ID: <8e371b4e-2231-b436-5ffc-7b9ca761c124@redhat.com> Date: Thu, 24 Oct 2019 17:28:53 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20191024142757.GN1138@perard.uk.xensource.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-MC-Unique: t6JPmtyMNzSkJoQcApSEvg-1 X-Mimecast-Spam-Score: 0 Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 10/24/19 16:27, Anthony PERARD wrote: > On Wed, Oct 23, 2019 at 12:15:54AM +0200, Laszlo Ersek wrote: >> MaxCpuCountInitialization() currently handles the following options: >> >> (1) QEMU does not report the boot CPU count (FW_CFG_NB_CPUS is 0) >> >> In this case, PlatformPei makes MpInitLib enumerate APs up to the >> default PcdCpuMaxLogicalProcessorNumber value (64) minus 1, or until >> the default PcdCpuApInitTimeOutInMicroSeconds (50,000) elapses. >> (Whichever is reached first.) >> >> Time-limited AP enumeration had never been reliable on QEMU/KVM, whi= ch >> is why commit 45a70db3c3a5 strated handling case (2) below, in OVMF. >> >> (2) QEMU reports the boot CPU count (FW_CFG_NB_CPUS is nonzero) >> >> In this case, PlatformPei sets >> >> - PcdCpuMaxLogicalProcessorNumber to the reported boot CPU count >> (FW_CFG_NB_CPUS, which exports "PCMachineState.boot_cpus"), >> >> - and PcdCpuApInitTimeOutInMicroSeconds to practically "infinity" >> (MAX_UINT32, ~71 minutes). >> >> That causes MpInitLib to enumerate exactly the present (boot) APs. >> >> With CPU hotplug in mind, this method is not good enough. Because, >> using QEMU terminology, UefiCpuPkg expects >> PcdCpuMaxLogicalProcessorNumber to provide the "possible CPUs" count >> ("MachineState.smp.max_cpus"), which includes present and not presen= t >> CPUs both (with not present CPUs being subject for hot-plugging). >> FW_CFG_NB_CPUS does not include not present CPUs. >> >> Rewrite MaxCpuCountInitialization() for handling the following cases: >> >> (1) The behavior of case (1) does not change. (No UefiCpuPkg PCDs are se= t >> to values different from the defaults.) >> >> (2) QEMU reports the boot CPU count ("PCMachineState.boot_cpus", via >> FW_CFG_NB_CPUS), but not the possible CPUs count >> ("MachineState.smp.max_cpus"). >> >> In this case, the behavior remains unchanged. >> >> The way MpInitLib is instructed to do the same differs however: we n= ow >> set the new PcdCpuBootLogicalProcessorNumber to the boot CPU count >> (while continuing to set PcdCpuMaxLogicalProcessorNumber identically= ). >> PcdCpuApInitTimeOutInMicroSeconds becomes irrelevant. >> >> (3) QEMU reports both the boot CPU count ("PCMachineState.boot_cpus", vi= a >> FW_CFG_NB_CPUS), and the possible CPUs count >> ("MachineState.smp.max_cpus"). >> >> We tell UefiCpuPkg about the possible CPUs count through >> PcdCpuMaxLogicalProcessorNumber. We also tell MpInitLib the boot CPU >> count for precise and quick AP enumeration, via >> PcdCpuBootLogicalProcessorNumber. PcdCpuApInitTimeOutInMicroSeconds = is >> irrelevant again. >> >> This patch is a pre-requisite for enabling CPU hotplug with SMM_REQUIRE. >> As a side effect, the patch also enables S3 to work with CPU hotplug at >> once, *without* SMM_REQUIRE. >> >> (Without the patch, S3 resume fails, if a CPU is hot-plugged at OS >> runtime, prior to suspend: the FW_CFG_NB_CPUS increase seen during resum= e >> causes PcdCpuMaxLogicalProcessorNumber to increase as well, which is not >> permitted. >> >> With the patch, PcdCpuMaxLogicalProcessorNumber stays the same, namely >> "MachineState.smp.max_cpus". Therefore, the CPU structures allocated >> during normal boot can accommodate the CPUs at S3 resume that have been >> hotplugged prior to S3 suspend.) >> >> Cc: Anthony Perard >> Cc: Ard Biesheuvel >> Cc: Igor Mammedov >> Cc: Jordan Justen >> Cc: Julien Grall >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1515 >> Signed-off-by: Laszlo Ersek >=20 > Acked-by: Anthony PERARD >=20 > Xen falls into case (1) and OVMF still boots fine with the series > applied. Awesome, thank you very much for checking! Laszlo