From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.88; helo=mga01.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id C13E521B02822 for ; Mon, 15 Oct 2018 19:26:02 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Oct 2018 19:26:02 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,386,1534834800"; d="scan'208";a="100546896" Received: from ray-dev.ccr.corp.intel.com (HELO [10.239.9.11]) ([10.239.9.11]) by orsmga002.jf.intel.com with ESMTP; 15 Oct 2018 19:26:01 -0700 To: Eric Dong , edk2-devel@lists.01.org Cc: Laszlo Ersek References: <20181015024948.228-1-eric.dong@intel.com> <20181015024948.228-2-eric.dong@intel.com> From: "Ni, Ruiyu" Message-ID: Date: Tue, 16 Oct 2018 10:27:11 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20181015024948.228-2-eric.dong@intel.com> Subject: Re: [Patch 1/4] UefiCpuPkg/Include/AcpiCpuData.h: Add Semaphore related Information. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Oct 2018 02:26:03 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 10/15/2018 10:49 AM, Eric Dong wrote: > In order to support semaphore related logic, add new definition for it. > > Cc: Ruiyu Ni > Cc: Laszlo Ersek > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Dong > --- > UefiCpuPkg/Include/AcpiCpuData.h | 23 ++++++++++++++++++++++- > 1 file changed, 22 insertions(+), 1 deletion(-) > > diff --git a/UefiCpuPkg/Include/AcpiCpuData.h b/UefiCpuPkg/Include/AcpiCpuData.h > index 9e51145c08..b3cf2f664a 100644 > --- a/UefiCpuPkg/Include/AcpiCpuData.h > +++ b/UefiCpuPkg/Include/AcpiCpuData.h > @@ -15,6 +15,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > #ifndef _ACPI_CPU_DATA_H_ > #define _ACPI_CPU_DATA_H_ > > +#include > + > // > // Register types in register table > // > @@ -22,9 +24,20 @@ typedef enum { > Msr, > ControlRegister, > MemoryMapped, > - CacheControl > + CacheControl, > + Semaphore I assume the REGISTER_TYPE definition will be move to internal (non-public) in phase 2. > } REGISTER_TYPE; > > +// > +// CPU information. > +// > +typedef struct { > + UINT32 PackageCount; // Packages in this CPU. > + UINT32 CoreCount; // Max Core count in the packages. > + UINT32 ThreadCount; // MAx thread count in the cores. > + UINT32 *ValidCoresInPackages; // Valid cores in each package. Can you please add more comments to describe each field above? PackageCount is easy to understand. But CoreCount is not. Maybe different packages have different number of cores. In this case, what value will CoreCount be? Similar question to ThreadCount. What does ValidCoresInPackages mean? Does it hold the valid (non-dead) core numbers for each package? So it's a UINT32 array with PackageCount elements? How about using name ValidCoreCountPerPackage? How about using MaxCoreCount/MaxThreadCount for CoreCount and ThreadCount? > +} CPU_STATUS_INFORMATION; > + > // > // Element of register table entry > // > @@ -147,6 +160,14 @@ typedef struct { > // provided. > // > UINT32 ApMachineCheckHandlerSize; > + // > + // CPU information which is required when set the register table. > + // > + CPU_STATUS_INFORMATION CpuStatus; > + // > + // Location info for each ap. > + // > + EFI_CPU_PHYSICAL_LOCATION *ApLocation; Please use EFI_PHYSICAL_ADDRESS for ApLocation. It's ok now. But if there are more fields below ApLocation, the offset of those fields differs between PEI and DXE. That will cause bugs. > } ACPI_CPU_DATA; > > #endif > -- Thanks, Ray