Re: [PATCH 2/2] MdePkg/UefiGpt.h: Add new definition for enable GPT support

["Chen, Chen A" <chen.a.chen@intel.com>]

-----Original Message-----
From: Wu, Hao A 
Sent: Friday, January 25, 2019 11:04 AM
To: Chen, Chen A <chen.a.chen@intel.com>; edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: RE: [edk2] [PATCH 2/2] MdePkg/UefiGpt.h: Add new definition for enable GPT support

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of 
> Chen A Chen
> Sent: Thursday, January 17, 2019 10:03 AM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D; Zhang, Chao B; Gao, Liming
> Subject: [edk2] [PATCH 2/2] MdePkg/UefiGpt.h: Add new definition for 
> enable GPT support
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1470
> This two new definitions are defined for GPT in FatPei diver.
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Zhang Chao B <chao.b.zhang@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chen A Chen <chen.a.chen@intel.com>
> ---
>  MdePkg/Include/Uefi/UefiGpt.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/MdePkg/Include/Uefi/UefiGpt.h 
> b/MdePkg/Include/Uefi/UefiGpt.h index f635b05390..8665c8cbc9 100644
> --- a/MdePkg/Include/Uefi/UefiGpt.h
> +++ b/MdePkg/Include/Uefi/UefiGpt.h
> @@ -24,9 +24,25 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, 
> EITHER EXPRESS OR IMPLIED.
>  /// EFI Partition Table Signature: "EFI PART".
>  ///
>  #define EFI_PTAB_HEADER_ID      SIGNATURE_64 ('E','F','I',' ','P','A','R','T')
> +///
> +/// Minimum bytes reserve for EFI entry array buffer.
> +///
> +#define GPT_PART_ENTRY_MIN_SIZE 16384

May I know where this definition comes from?
Does it come from the UEFI spec?

Chen: The MACRO is not explicitly defined in UEFI Spec, But UEFI Spec specifies a minimum value for GPT entry araray.

> 
>  #pragma pack(1)
> 
> +///
> +/// MBR Partition Entry
> +///
> +typedef struct {
> +  UINT8   BootIndicator;
> +  UINT8   StartingCHS[3];
> +  UINT8   OSType;
> +  UINT8   EndingCHS[3];
> +  UINT32  StartingLBA;
> +  UINT32  SizeInLBA;
> +} MBR_PARTITION_ENTRY;
> +

What about using the 'MBR_PARTITION_RECORD' definition within edk2/MdePkg/Include/IndustryStandard/Mbr.h

and thus get rid of adding this one?

Chen: This structure defined as the following format in Mbr.h

typedef struct {
  ..
  UINT8 StartingLBA[4];
  UINT8 SizeInLBA[4];
} MBR_PARTITION_RECORD;

For StartingLBA, this field is represented the LBA, so I think it should be UINT32 type not an array type.

The same meaning is also for SizeInLba.

Best Regards,
Hao Wu

>  ///
>  /// GPT Partition Table Header.
>  ///
> --
> 2.16.2.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel