From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id DF09E7803CC for ; Tue, 21 Nov 2023 16:12:23 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=dJYNi9mWNjZ+j00BbfHo18TAFbmHgwwlSo1dJyO9/Ec=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1700583142; v=1; b=gjFZas0iJl8wYitfz/sUnINTm12d+SVW5wQzZCjMKWj6CvhUqkP/DnLtKOb+XKPvn2BMK/BS 33MSd5JrE9vQJHJeDzli9LkwqCagqk8LkZyB3ULTZ5JanOurxwpKKQijfiEAPVX8R0bhwA+GpHX TrQ80/2GAyYBWNEpiefRnFkc= X-Received: by 127.0.0.2 with SMTP id GzmdYY7687511xpEqtL9NTSp; Tue, 21 Nov 2023 08:12:22 -0800 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web10.45460.1700583141629875695 for ; Tue, 21 Nov 2023 08:12:21 -0800 X-Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-230-CZIXhsNIPJOpsL0WMUXR4w-1; Tue, 21 Nov 2023 11:12:17 -0500 X-MC-Unique: CZIXhsNIPJOpsL0WMUXR4w-1 X-Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (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 E056D3822540; Tue, 21 Nov 2023 16:12:16 +0000 (UTC) X-Received: from [10.39.194.228] (unknown [10.39.194.228]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 83EAA1C060AE; Tue, 21 Nov 2023 16:12:15 +0000 (UTC) Message-ID: <815d7db5-909f-e5e4-4673-9119e38c8c0e@redhat.com> Date: Tue, 21 Nov 2023 17:12:14 +0100 MIME-Version: 1.0 Subject: Re: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm: Use processor extended information To: "Wu, Jiaxin" , "Ni, Ray" , "devel@edk2.groups.io" Cc: "Dong, Eric" , "Kumar, Rahul R" , Gerd Hoffmann , "Zeng, Star" References: <20231115111553.6592-1-jiaxin.wu@intel.com> <20231115111553.6592-4-jiaxin.wu@intel.com> <8626fc57-1956-e9f1-f0ab-6c5a3ba45059@redhat.com> From: "Laszlo Ersek" In-Reply-To: X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.7 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,lersek@redhat.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: ok3h5UBhRFtQ3iFpKGR7yX5yx7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=gjFZas0i; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On 11/20/23 13:42, Wu, Jiaxin wrote: > For core id in cpu features library, I agree it should be not easy or > simple change to 0x1f. > >   > > But in SMM CPU, there is no usage case depends on the number of cores > retrieved from cupid 0x0b return value, only PackageId will be used. So, > this patch doesn’t do bad things, should no regression issue. I agree > with Ray’s explanation that  “CPUID.0B.PackageId == CPUID.1F.PackageId”, > thus no need update for the PackageId update. > >   > > I checked the latest SDM: > >   > > “The sub-leaves of CPUID leaf 0BH describe an ordered hierarchy of > logical processors starting from the smallest-scoped domain of a Logical > Processor (sub-leaf index 0) to the Core domain (sub-leaf index 1) to > the largest-scoped domain (the last valid sub-leaf index) **that is > implicitly subordinate to the unenumerated highest-scoped domain of the > processor package (socket)**” > >   > > Looks it already updated to indicate the largest-scoped domain is package. > >   > > With all above, I agree to drop this path, but other 2 patches in this > set should be ok. Thanks Ray help clarify this. This is precisely the reason why I originally requested the now-last patch to be split off from the rest. I certainly didn't / couldn't go into such depths of CPUID.0B versus CPUID.1F discussion, but still that change looked very distinct from *populating* Location2 in the SMM-add-processor protocol member function (upon CPU hotplug). So, FWIW, I'm fine if the last patch in the series gets dropped. Thanks Laszlo > >   > > Thanks, > > Jiaxin > >   > > *From:* Ni, Ray > *Sent:* Monday, November 20, 2023 9:45 AM > *To:* Laszlo Ersek ; devel@edk2.groups.io; Wu, Jiaxin > > *Cc:* Dong, Eric ; Kumar, Rahul R > ; Gerd Hoffmann ; Zeng, Star > > *Subject:* Re: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm: > Use processor extended information > >   > > let me add more to explain: > >   > > 1. CPUID.0B.PackageId == CPUID.1F.PackageId > >   > > SDM clearly states the scope of every MSR (public): package, core, or > thread. > > But SDM doesn't emphasize that if a MSR is package scope, it's within > the package defined by CPUID.0B or CPUID.1F. > > That implies, CPUID.0B and CPUID.1F should return the same value for > package ID. > >   > > Also, SDM has following statement to explain result of EAX for CPUID.0B > and CPUID.1F: > >     Bits 04-00: The number of bits that the x2APIC ID must be shifted to > the right to address instances of the "*next higher-scoped"*​ domain. > >   > > That means when CPUID.0B returns the EAX[04:00], it returns the total > bits of "thread", "core", "module", "tie", "die" because "package" is > the next higher-scoped domain. > >   > > That also supports the idea: CPUID.0B.PackageId == CPUID.1F.PackageId. > >   > > 2. CPU Feature Initialization > >   > > In UefiCpuPkg/Include/RegisterCpuFeaturesLib.h, the following macros > were added to support consumers of RegisterCpuFeaturesLib specify > dependencies among different features. > > For example, when feature #a PACKAGE_BEFORE feature #b, #b is performed > in one thread of a package and after all threads have performed #a. > > That means internally multi-thread-sync is used to guarantee the > dependencies. > > #define CPU_FEATURE_THREAD_BEFORE   BIT25 > > #define CPU_FEATURE_THREAD_AFTER    BIT26 > > #define CPU_FEATURE_CORE_BEFORE     BIT27 > > #define CPU_FEATURE_CORE_AFTER      BIT28 > > #define CPU_FEATURE_PACKAGE_BEFORE  BIT29 > > #define CPU_FEATURE_PACKAGE_AFTER   BIT30 > >   > > But above 3 sets of macro only define the dependencies between 3 scopes: > thread, core and package. > > Other scopes were not supported as there is no MSR which belongs to > other scopes at that moment, even today. > > So, the cpu features library implementation also only depends on CPUID.0B. > > If we update the code to get package id from CPUID.1F, to be consistent, > we should also get the core id from CPUID.1F. > > But if we do that, the number of cores which belong to the same domain > could be less in CPUID.1F. As CPUID.1F returns > the number of cores per module, instead of per package. > > That will break the MP sync logic which depends on the number of cores > per each domain. > >   > > Conclusion: we should not update code to use CPUID.1F as it will break > the MP-sync logic in RegisterCpuFeaturesLib which is not aware of more > than 3 layers of scopes. > >   > > Thanks, > > Ray > >   > > ------------------------------------------------------------------------ > > *From:* Laszlo Ersek > > *Sent:* Saturday, November 18, 2023 5:05 AM > *To:* devel@edk2.groups.io > >; Ni, Ray >; Wu, Jiaxin > > *Cc:* Dong, Eric >; > Kumar, Rahul R >; Gerd Hoffmann >; Zeng, Star > > *Subject:* Re: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm: > Use processor extended information > >   > > On 11/16/23 02:30, Ni, Ray wrote: >> I cannot remember if CPUID.0B and CPUID.1F return the same value for >> package ID. >> >> And I am not sure about the benefit if we get the package id from > location2. > > Isn't the benefit that Location2 / CPUID leaf 1F is fully specified, > while leaf 0B isn't? From the commit message it seems we should always > prefer leaf 1F and Location2, even if we're not aware of concrete > problems with leaf 0B. > > Do you think we should only merge patches #1 and #2? > > Thanks, > Laszlo > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#111557): https://edk2.groups.io/g/devel/message/111557 Mute This Topic: https://groups.io/mt/102602853/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-