From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=66.187.233.73; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) (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 BC71B210EB4D4 for ; Fri, 10 Aug 2018 08:58:40 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 10022819700A; Fri, 10 Aug 2018 15:58:40 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-207.rdu2.redhat.com [10.10.120.207]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8865A10CD7F2; Fri, 10 Aug 2018 15:58:38 +0000 (UTC) To: Eric Dong , edk2-devel@lists.01.org Cc: =?UTF-8?Q?Marvin_H=c3=a4user?= , Fan Jeff , Ruiyu Ni References: <20180810041909.12776-1-eric.dong@intel.com> <20180810041909.12776-3-eric.dong@intel.com> From: Laszlo Ersek Message-ID: <0fb4d905-d895-34f3-6fab-92fd8bf29d61@redhat.com> Date: Fri, 10 Aug 2018 17:58:37 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20180810041909.12776-3-eric.dong@intel.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Fri, 10 Aug 2018 15:58:40 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Fri, 10 Aug 2018 15:58:40 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: [Patch v3 2/5] UefiCpuPkg/AcpiCpuData.h: Remove AcpiNVS and Below 4G limitation. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 10 Aug 2018 15:58:41 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit On 08/10/18 06:19, Eric Dong wrote: > ACPI_CPU_DATA structure first introduced to save data in > normal boot phase. Also this data will be used in S3 phase > by one PEI driver. So in first phase, this data is been > defined to use ACPI NVS memory type and must below 4G. > > Later in order to fix potential security issue, > PiSmmCpuDxeSmm driver added logic to copy ACPI_CPU_DATA > (except ResetVector and Stack buffer) to smram at smm > ready to lock point. ResetVector must below 1M and Stack > buffer is write only in S3 phase, so these two fields not > copy to smram. Also PiSmmCpuDxeSmm driver owned the task > to restore the CPU setting and it's a SMM driver. > > After above change, the acpi nvs memory type and below 4G > limitation is no longer needed. > > This change remove the limitation in the comments for > ACPI_CPU_DATA definition. > > Cc: Marvin Häuser > Cc: Fan Jeff > Cc: Laszlo Ersek > Cc: Ruiyu Ni > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Dong > --- > UefiCpuPkg/Include/AcpiCpuData.h | 34 ++++++++++++---------------------- > 1 file changed, 12 insertions(+), 22 deletions(-) > > diff --git a/UefiCpuPkg/Include/AcpiCpuData.h b/UefiCpuPkg/Include/AcpiCpuData.h > index ec092074ce..9e51145c08 100644 > --- a/UefiCpuPkg/Include/AcpiCpuData.h > +++ b/UefiCpuPkg/Include/AcpiCpuData.h > @@ -1,7 +1,7 @@ > /** @file > Definitions for CPU S3 data. > > -Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.
> +Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD License > which accompanies this distribution. The full text of the license may be found at > @@ -57,15 +57,13 @@ typedef struct { > // > UINT32 InitialApicId; > // > - // Physical address of CPU_REGISTER_TABLE_ENTRY structures. This buffer must be > - // allocated below 4GB from memory of type EfiACPIMemoryNVS. > + // Physical address of CPU_REGISTER_TABLE_ENTRY structures. > // > EFI_PHYSICAL_ADDRESS RegisterTableEntry; > } CPU_REGISTER_TABLE; > > // > -// Data structure that is required for ACPI S3 resume. This structure must be > -// allocated below 4GB from memory of type EfiACPIMemoryNVS. The PCD > +// Data structure that is required for ACPI S3 resume. The PCD > // PcdCpuS3DataAddress must be set to the physical address where this structure > // is allocated > // > @@ -78,21 +76,17 @@ typedef struct { > // > EFI_PHYSICAL_ADDRESS StartupVector; > // > - // Physical address of structure of type IA32_DESCRIPTOR. This structure must > - // be allocated below 4GB from memory of type EfiACPIMemoryNVS. The > + // Physical address of structure of type IA32_DESCRIPTOR. The > // IA32_DESCRIPTOR structure provides the base address and length of a GDT > - // The buffer for GDT must also be allocated below 4GB from memory of type > - // EfiACPIMemoryNVS. The GDT must be filled in with the GDT contents that are > + // The GDT must be filled in with the GDT contents that are > // used during an ACPI S3 resume. This is typically the contents of the GDT > // used by the boot processor when the platform is booted. > // > EFI_PHYSICAL_ADDRESS GdtrProfile; > // > - // Physical address of structure of type IA32_DESCRIPTOR. This structure must > - // be allocated below 4GB from memory of type EfiACPIMemoryNVS. The > + // Physical address of structure of type IA32_DESCRIPTOR. The > // IA32_DESCRIPTOR structure provides the base address and length of an IDT. > - // The buffer for IDT must also be allocated below 4GB from memory of type > - // EfiACPIMemoryNVS. The IDT must be filled in with the IDT contents that are > + // The IDT must be filled in with the IDT contents that are > // used during an ACPI S3 resume. This is typically the contents of the IDT > // used by the boot processor when the platform is booted. > // > @@ -100,7 +94,7 @@ typedef struct { > // > // Physical address of a buffer that is used as stacks during ACPI S3 resume. > // The total size of this buffer, in bytes, is NumberOfCpus * StackSize. This > - // structure must be allocated below 4GB from memory of type EfiACPIMemoryNVS. > + // structure must be allocated from memory of type EfiACPIMemoryNVS. > // > EFI_PHYSICAL_ADDRESS StackAddress; > // > @@ -118,14 +112,12 @@ typedef struct { > // Physical address of structure of type MTRR_SETTINGS that contains a copy > // of the MTRR settings that are compatible with the MTRR settings used by > // the boot processor when the platform was booted. These MTRR settings are > - // used during an ACPI S3 resume. This structure must be allocated below 4GB > - // from memory of type EfiACPIMemoryNVS. > + // used during an ACPI S3 resume. > // > EFI_PHYSICAL_ADDRESS MtrrTable; > // > // Physical address of an array of CPU_REGISTER_TABLE structures, with > - // NumberOfCpus entries. This array must be allocated below 4GB from memory > - // of type EfiACPIMemoryNVS. If a register table is not required, then the > + // NumberOfCpus entries. If a register table is not required, then the > // TableLength and AllocatedSize fields of CPU_REGISTER_TABLE are set to 0. > // If TableLength is > 0, then elements of RegisterTableEntry are used to > // initialize the CPU that matches InitialApicId, during an ACPI S3 resume, > @@ -134,8 +126,7 @@ typedef struct { > EFI_PHYSICAL_ADDRESS PreSmmInitRegisterTable; > // > // Physical address of an array of CPU_REGISTER_TABLE structures, with > - // NumberOfCpus entries. This array must be allocated below 4GB from memory > - // of type EfiACPIMemoryNVS. If a register table is not required, then the > + // NumberOfCpus entries. If a register table is not required, then the > // TableLength and AllocatedSize fields of CPU_REGISTER_TABLE are set to 0. > // If TableLength is > 0, then elements of RegisterTableEntry are used to > // initialize the CPU that matches InitialApicId, during an ACPI S3 resume, > @@ -144,8 +135,7 @@ typedef struct { > EFI_PHYSICAL_ADDRESS RegisterTable; > // > // Physical address of a buffer that contains the machine check handler that > - // is used during an ACPI S3 Resume. This buffer must be allocated below 4GB > - // from memory of type EfiACPIMemoryNVS. In order for this machine check > + // is used during an ACPI S3 Resume. In order for this machine check > // handler to be active on an AP during an ACPI S3 resume, the machine check > // vector in the IDT provided by IdtrProfile must be initialized to transfer > // control to this physical address. > Reviewed-by: Laszlo Ersek