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:pca...@zytor.com]
Sent: Thursday, September 21, 2017 2:16 AM
To: edk2-devel@lists.01.org
Cc: Paulo Alcantara <pca...@zytor.com>; Kinney, Michael D
<michael.d.kin...@intel.com>; Gao, Liming <liming....@intel.com>; Laszlo
Ersek <ler...@redhat.com>; Ni, Ruiyu <ruiyu...@intel.com>
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 <michael.d.kin...@intel.com>
Cc: Liming Gao <liming....@intel.com>
Cc: Laszlo Ersek <ler...@redhat.com>
Cc: Ruiyu Ni <ruiyu...@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara <pca...@zytor.com>
---
  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


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

Reply via email to