[edk2] [PATCH v2 1/2] UefiCpuPkg/Include: Expand description of AcpiCpuData.h structures

[Michael Kinney]

Provide a more detailed description of each field of the
ACPI_CPU_DATA and CPU_REGISTER_TABLE structures.

Cc: Laszlo Ersek 
Cc: "Yao, Jiewen" 
Cc: "Fan, Jeff" 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael Kinney 
---
 UefiCpuPkg/Include/AcpiCpuData.h | 115 ++-
 1 file changed, 102 insertions(+), 13 deletions(-)

diff --git a/UefiCpuPkg/Include/AcpiCpuData.h b/UefiCpuPkg/Include/AcpiCpuData.h
index a367257..d6acc81 100644
--- a/UefiCpuPkg/Include/AcpiCpuData.h
+++ b/UefiCpuPkg/Include/AcpiCpuData.h
@@ -18,7 +18,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 //
 // Register types in register table
 //
-typedef enum _REGISTER_TYPE {
+typedef enum {
   Msr,
   ControlRegister,
   MemoryMapped,
@@ -29,11 +29,11 @@ typedef enum _REGISTER_TYPE {
 // Element of register table entry
 //
 typedef struct {
-  REGISTER_TYPE RegisterType;
-  UINT32Index;
-  UINT8 ValidBitStart;
-  UINT8 ValidBitLength;
-  UINT64Value;
+  REGISTER_TYPE  RegisterType;
+  UINT32 Index;
+  UINT8  ValidBitStart;
+  UINT8  ValidBitLength;
+  UINT64 Value;
 } CPU_REGISTER_TABLE_ENTRY;
 
 //
@@ -41,30 +41,119 @@ typedef struct {
 // allocated size of this table, and pointer to the list of table entries.
 //
 typedef struct {
-  UINT32   TableLength;
-  UINT32   NumberBeforeReset;
-  UINT32   AllocatedSize;
-  UINT32   InitialApicId;
-  CPU_REGISTER_TABLE_ENTRY *RegisterTableEntry;
+  //
+  // The number of valid entries in the RegisterTableEntry buffer
+  //
+  UINT32TableLength;
+  UINT32NumberBeforeReset;
+  //
+  // The size, in bytes, of the RegisterTableEntry buffer
+  //
+  UINT32AllocatedSize;
+  //
+  // The initial APIC ID of the CPU this register table applies to
+  //
+  UINT32InitialApicId;
+  //
+  // Buffer of CPU_REGISTER_TABLE_ENTRY structures.  This buffer must be
+  // allocated below 4GB from memory of type EfiACPIMemoryNVS.
+  //
+  CPU_REGISTER_TABLE_ENTRY  *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
+// PcdCpuS3DataAddress must be set to the physical address where this structure
+// is allocated
+//
 typedef struct {
+  //
+  // Physical address of 4KB buffer allocated below 1MB from memory of type
+  // EfiReservedMemoryType.  The buffer is not required to be initialized, but
+  // it is recommended that the buffer be zero-filled.  This buffer used to
+  // wake APs during an ACPI S3 resume.
+  //
   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
+  // 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
+  // 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
+  // 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
+  // used during an ACPI S3 resume.  This is typically the contents of the IDT
+  // used by the boot processor when the platform is booted.
+  //
   EFI_PHYSICAL_ADDRESS  IdtrProfile;
+  //
+  // 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.
+  //
   EFI_PHYSICAL_ADDRESS  StackAddress;
+  //
+  // The size, in bytes, of the stack provided to each CPU during ACPI S3 
resume.
+  //
   UINT32StackSize;
+  //
+  // The number of CPUs.  If a platform does not support hot plug CPUs, then
+  // this is the number of CPUs detected when the platform is booted, 
regardless
+  // of being enabled or disabled.  If a platform does support hot plug CPUs,
+  // then this is the maximum number of CPUs that the platform supports.
+  //
   UINT32NumberOfCpus;
+  //
+  // Physical address of structure of type MTRR_SETTINGS that contains a copy
+  // of the MTRR 

Re: [edk2] [PATCH v2 1/2] UefiCpuPkg/Include: Expand description of AcpiCpuData.h structures

[Laszlo Ersek]

On 11/25/15 00:47, Michael Kinney wrote:

[snip]

> +//
> +// Data structure that is required for ACPI S3 resume.  This structure must 
> be
> +// allocated below 4GB from memory of type EfiACPIMemoryNVS.  The PCD
> +// PcdCpuS3DataAddress must be set to the physical address where this 
> structure
> +// is allocated
> +//
>  typedef struct {
> +  //
> +  // Physical address of 4KB buffer allocated below 1MB from memory of type
> +  // EfiReservedMemoryType.  The buffer is not required to be initialized, 
> but
> +  // it is recommended that the buffer be zero-filled.  This buffer used to

(1) "This buffer [is] used to"

> +  // wake APs during an ACPI S3 resume.
> +  //
>EFI_PHYSICAL_ADDRESS  StartupVector;

With that change -- and there is no need to submit a v3 just because of
it; it can be fixed at commit time --

Reviewed-by: Laszlo Ersek 

The documentation looks great, many thanks for writing it up!

Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel