From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.zytor.com (terminus.zytor.com [65.50.211.136]) (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 B973121E43B5B for ; Fri, 22 Sep 2017 06:53:53 -0700 (PDT) Received: from [10.26.0.110] (corporativo.static.gvt.net.br [177.135.97.54] (may be forged)) (authenticated bits=0) by mail.zytor.com (8.15.2/8.15.2) with ESMTPSA id v8MDsjI3030012 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Fri, 22 Sep 2017 06:54:47 -0700 To: "Ni, Ruiyu" , "edk2-devel@lists.01.org" Cc: "Kinney, Michael D" , "Gao, Liming" , Laszlo Ersek References: <734D49CCEBEEF84792F5B80ED585239D5BA65B1C@SHSMSX103.ccr.corp.intel.com> From: Paulo Alcantara Message-ID: <43aa7356-8a0b-6ef3-702c-21034bf5c1e0@zytor.com> Date: Fri, 22 Sep 2017 10:54:39 -0300 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5BA65B1C@SHSMSX103.ccr.corp.intel.com> Subject: Re: [PATCH v3 1/2] MdePkg: Add UDF volume structure definitions X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 22 Sep 2017 13:53:53 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Ruiyu, On 9/21/2017 11:50 PM, Ni, Ruiyu wrote: > Paulo, > Comments below: > > #define UDF_TAG_ID(_Tag) \ > (UDF_VOLUME_DESCRIPTOR_ID)((_Tag)->TagIdentifier) > 1. I prefer to remove the UDF_TAG_ID macro. Adding type-cast to get TAG_ID is very straightforward. Right. I'll remove it. > > #define UDF_LVD_REVISION(_Lv) \ > *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix > typedef struct { > UINT8 Flags; > UINT8 Identifier[23]; > UINT8 IdentifierSuffix[8]; > } UDF_ENTITY_ID; > 2. Entity structure is defined by ECMA-167 spec and re-used by UDF spec. > I think since we are creating UDF structure in udf.h, how about using the below > structure layout and removing the UDF_LVD_REVISION macro: > Typedef struct { > UINT8 Flags; > UINT8 Identifier[23]; > UINT16 UdfRevision; > UINT8 DomainFlags; > UINT8 Reserved[5]; > } UDF_ENTITY_ID; Much better! Thanks for looking into that. The UDF 2.60 specification says: > UDF classifies Entity Identifiers into 4 separate types. Each type has > its own Suffix Type for the Identifier Suffix field. The 4 types are: > > Domain Entity Identifiers with a Domain Identifier Suffix > UDF Entity Identifiers with a UDF Identifier Suffix > Implementation Entity Identifiers with an Implementation Identifier Suffix > Application Entity Identifiers with an Application Identifier Suffix Given that, I think an union would be more appropriate. E.g., typedef struct { UINT8 Flags; UINT8 Identifier[23]; union { struct { UINT16 UdfRevision; UINT8 DomainFlags; UINT8 Reversed[5]; } DomainIdentifier; struct { UINT16 UdfRevision; UINT8 OSClass; UINT8 OSIdentifier; UINT8 Reserved[4]; } EntityIdentifier; struct { UINT8 OSClass; UINT8 OSIdentifier; UINT8 ImplementationUseArea[6]; } ImplementationEntityIdentifier; struct { UINT8 ApplicationUseArea[8]; } ApplicationEntityIdentifier; UINT8 IdentifierSuffix[8]; }; } UDF_ENTITY_ID; Do you agree with me? Or that wouldn't be necessary? Thanks! Paulo > > I am not sure if there are other structures that are defined in ECMA spec and > re-used by UDF spec. I think we can apply the similar rules to those structures > as well. > > Thanks/Ray > >> -----Original Message----- >> From: Paulo Alcantara [mailto:pcacjr@zytor.com] >> Sent: Thursday, September 21, 2017 2:16 AM >> To: edk2-devel@lists.01.org >> Cc: Paulo Alcantara ; Kinney, Michael D >> ; Gao, Liming ; Laszlo >> Ersek ; Ni, Ruiyu >> Subject: [PATCH v3 1/2] MdePkg: Add UDF volume structure definitions >> >> This patch adds a few more UDF volume structures in order to detect an UDF >> file system which is supported by current EDK2 UDF file system >> implementation in Partition driver. >> >> Cc: Michael D Kinney >> Cc: Liming Gao >> Cc: Laszlo Ersek >> Cc: Ruiyu Ni >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Paulo Alcantara >> --- >> MdePkg/Include/IndustryStandard/Udf.h | 63 ++++++++++++++++++-- >> 1 file changed, 59 insertions(+), 4 deletions(-) >> >> diff --git a/MdePkg/Include/IndustryStandard/Udf.h >> b/MdePkg/Include/IndustryStandard/Udf.h >> index 0febb4bcda..002e87e150 100644 >> --- a/MdePkg/Include/IndustryStandard/Udf.h >> +++ b/MdePkg/Include/IndustryStandard/Udf.h >> @@ -24,11 +24,28 @@ >> #define UDF_LOGICAL_SECTOR_SIZE ((UINT64)(1ULL << >> UDF_LOGICAL_SECTOR_SHIFT)) >> #define UDF_VRS_START_OFFSET ((UINT64)(16ULL << >> UDF_LOGICAL_SECTOR_SHIFT)) >> >> -#define _GET_TAG_ID(_Pointer) \ >> - (((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier) >> +typedef enum { >> + UdfPrimaryVolumeDescriptor = 1, >> + UdfAnchorVolumeDescriptorPointer = 2, >> + UdfVolumeDescriptorPointer = 3, >> + UdfImplemenationUseVolumeDescriptor = 4, >> + UdfPartitionDescriptor = 5, >> + UdfLogicalVolumeDescriptor = 6, >> + UdfUnallocatedSpaceDescriptor = 7, >> + UdfTerminatingDescriptor = 8, >> + UdfLogicalVolumeIntegrityDescriptor = 9, >> + UdfFileSetDescriptor = 256, >> + UdfFileIdentifierDescriptor = 257, >> + UdfAllocationExtentDescriptor = 258, >> + UdfFileEntry = 261, >> + UdfExtendedFileEntry = 266, >> +} UDF_VOLUME_DESCRIPTOR_ID; >> >> -#define IS_AVDP(_Pointer) \ >> - ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2)) >> +#define UDF_TAG_ID(_Tag) \ >> + (UDF_VOLUME_DESCRIPTOR_ID)((_Tag)->TagIdentifier) >> + >> +#define UDF_LVD_REVISION(_Lv) \ >> + *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix >> >> #pragma pack(1) >> >> @@ -49,12 +66,50 @@ typedef struct { >> } UDF_EXTENT_AD; >> >> typedef struct { >> + UINT8 CharacterSetType; >> + UINT8 CharacterSetInfo[63]; >> +} UDF_CHAR_SPEC; >> + >> +typedef struct { >> + UINT8 Flags; >> + UINT8 Identifier[23]; >> + UINT8 IdentifierSuffix[8]; >> +} UDF_ENTITY_ID; >> + >> +typedef struct { >> + UINT32 LogicalBlockNumber; >> + UINT16 PartitionReferenceNumber; >> +} UDF_LB_ADDR; >> + >> +typedef struct { >> + UINT32 ExtentLength; >> + UDF_LB_ADDR ExtentLocation; >> + UINT8 ImplementationUse[6]; >> +} UDF_LONG_ALLOCATION_DESCRIPTOR; >> + >> +typedef struct { >> UDF_DESCRIPTOR_TAG DescriptorTag; >> UDF_EXTENT_AD MainVolumeDescriptorSequenceExtent; >> UDF_EXTENT_AD ReserveVolumeDescriptorSequenceExtent; >> UINT8 Reserved[480]; >> } UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER; >> >> +typedef struct { >> + UDF_DESCRIPTOR_TAG DescriptorTag; >> + UINT32 VolumeDescriptorSequenceNumber; >> + UDF_CHAR_SPEC DescriptorCharacterSet; >> + UINT8 LogicalVolumeIdentifier[128]; >> + UINT32 LogicalBlockSize; >> + UDF_ENTITY_ID DomainIdentifier; >> + UDF_LONG_ALLOCATION_DESCRIPTOR LogicalVolumeContentsUse; >> + UINT32 MapTableLength; >> + UINT32 NumberOfPartitionMaps; >> + UDF_ENTITY_ID ImplementationIdentifier; >> + UINT8 ImplementationUse[128]; >> + UDF_EXTENT_AD IntegritySequenceExtent; >> + UINT8 PartitionMaps[6]; >> +} UDF_LOGICAL_VOLUME_DESCRIPTOR; >> + >> #pragma pack() >> >> #endif >> -- >> 2.11.0 > >