[PATCH v2 0/3] UDF partition driver fix

[PATCH v2 0/3] UDF partition driver fix

[Paulo Alcantara]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=707

Hi,

This patchset fixes a bug in Partition driver that UDF partitions
occupied the entire disk space instead of using LVD space only.

BTW, I've only tested it under OVMF and built it with GCC only. That
would be appreciable if someone could build with other toolchains and
see if this doesn't break.

I used a Windows 10 ISO image with UdfDxe disabled and enabled. The
`map -r` output seemed OK. No breakage when booting an OS off an
ElTorito partition from an UDF bridge disk.

v1->v2:
 - Followed Laszlo's suggestions to submit a proper patchset. Thanks!
 - As I'm still waiting for Ruiyu and Star to test this fix, I took
   advantage of it and did some code cleanups :-)

Repo:   https://github.com/pcacjr/edk2.git
Branch: udf-partition-fix-v2

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Reported-by: Ruiyu Ni <ruiyu.ni@intel.com>
Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
---

Paulo Alcantara (3):
  MdePkg: Add UDF volume structure definitions
  MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition
  MdeModulePkg/UdfDxe: Rework driver to support PartitionDxe changes

 MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c            | 323 +++++++++++-
 MdeModulePkg/Universal/Disk/UdfDxe/File.c                 |  13 +-
 MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 515 ++++++++------------
 MdeModulePkg/Universal/Disk/UdfDxe/Udf.c                  |   7 -
 MdeModulePkg/Universal/Disk/UdfDxe/Udf.h                  |  88 +---
 MdePkg/Include/IndustryStandard/Udf.h                     |  63 +++
 6 files changed, 566 insertions(+), 443 deletions(-)

-- 
2.11.0

[PATCH v2 1/3] MdePkg: Add UDF volume structure definitions

[Paulo Alcantara]

This patch adds a fewe more UDF structures in order to detect Logical
Volume and Partition descriptors during Main Volume Descriptor Sequence
in Partition driver.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
---
 MdePkg/Include/IndustryStandard/Udf.h | 63 ++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/MdePkg/Include/IndustryStandard/Udf.h b/MdePkg/Include/IndustryStandard/Udf.h
index 0febb4bcda..6cc42f8543 100644
--- a/MdePkg/Include/IndustryStandard/Udf.h
+++ b/MdePkg/Include/IndustryStandard/Udf.h
@@ -27,9 +27,19 @@
 #define _GET_TAG_ID(_Pointer) \
   (((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)
 
+#define IS_PD(_Pointer) \
+  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5))
+#define IS_LVD(_Pointer) \
+  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6))
+#define IS_TD(_Pointer) \
+  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
+
 #define IS_AVDP(_Pointer) \
   ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))
 
+#define LV_UDF_REVISION(_Lv) \
+  *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix
+
 #pragma pack(1)
 
 typedef struct {
@@ -55,6 +65,59 @@ typedef struct {
   UINT8               Reserved[480];
 } UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER;
 
+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;
+  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;
+
+typedef struct {
+  UDF_DESCRIPTOR_TAG         DescriptorTag;
+  UINT32                     VolumeDescriptorSequenceNumber;
+  UINT16                     PartitionFlags;
+  UINT16                     PartitionNumber;
+  UDF_ENTITY_ID              PartitionContents;
+  UINT8                      PartitionContentsUse[128];
+  UINT32                     AccessType;
+  UINT32                     PartitionStartingLocation;
+  UINT32                     PartitionLength;
+  UDF_ENTITY_ID              ImplementationIdentifier;
+  UINT8                      ImplementationUse[128];
+  UINT8                      Reserved[156];
+} UDF_PARTITION_DESCRIPTOR;
+
 #pragma pack()
 
 #endif
-- 
2.11.0

Re: [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions

[Ni, Ruiyu]

#define _GET_TAG_ID(_Pointer) \
  (((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)

#define IS_PD(_Pointer) \
  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5))
#define IS_LVD(_Pointer) \
  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6))
#define IS_TD(_Pointer) \
  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))

#define IS_AVDP(_Pointer) \
  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))

Paulo,
If you take a look at Pci22.h in the same directory, all macros are started
as "IS_PCI_".
How about changing the above IS_xxx to IS_UDF_xxx?
Shall we change AVDP to AVD, following the same naming style as PD, LVD and TD?
How about changing _Pointer to _Tag or DescriptorTag?


Thanks/Ray

> -----Original Message-----
> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
> Sent: Sunday, September 17, 2017 9:13 PM
> To: edk2-devel@lists.01.org
> Cc: Paulo Alcantara <pcacjr@zytor.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Laszlo
> Ersek <lersek@redhat.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions
> 
> This patch adds a fewe more UDF structures in order to detect Logical
> Volume and Partition descriptors during Main Volume Descriptor Sequence
> in Partition driver.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
> ---
>  MdePkg/Include/IndustryStandard/Udf.h | 63 ++++++++++++++++++++
>  1 file changed, 63 insertions(+)
> 
> diff --git a/MdePkg/Include/IndustryStandard/Udf.h
> b/MdePkg/Include/IndustryStandard/Udf.h
> index 0febb4bcda..6cc42f8543 100644
> --- a/MdePkg/Include/IndustryStandard/Udf.h
> +++ b/MdePkg/Include/IndustryStandard/Udf.h
> @@ -27,9 +27,19 @@
>  #define _GET_TAG_ID(_Pointer) \
>    (((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)
> 
> +#define IS_PD(_Pointer) \
> +  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5)) #define IS_LVD(_Pointer) \
> +  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6)) #define IS_TD(_Pointer) \
> +  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
> +
>  #define IS_AVDP(_Pointer) \
>    ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))
> 
> +#define LV_UDF_REVISION(_Lv) \
> +  *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix
> +
>  #pragma pack(1)
> 
>  typedef struct {
> @@ -55,6 +65,59 @@ typedef struct {
>    UINT8               Reserved[480];
>  } UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER;
> 
> +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;
> +  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;
> +
> +typedef struct {
> +  UDF_DESCRIPTOR_TAG         DescriptorTag;
> +  UINT32                     VolumeDescriptorSequenceNumber;
> +  UINT16                     PartitionFlags;
> +  UINT16                     PartitionNumber;
> +  UDF_ENTITY_ID              PartitionContents;
> +  UINT8                      PartitionContentsUse[128];
> +  UINT32                     AccessType;
> +  UINT32                     PartitionStartingLocation;
> +  UINT32                     PartitionLength;
> +  UDF_ENTITY_ID              ImplementationIdentifier;
> +  UINT8                      ImplementationUse[128];
> +  UINT8                      Reserved[156];
> +} UDF_PARTITION_DESCRIPTOR;
> +
>  #pragma pack()
> 
>  #endif
> --
> 2.11.0

Re: [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions

[Ni, Ruiyu]

Continue reading the patch #2, I think we can change IS_PD to:
#define IS_UDF_PD(Tag) ((Tag)->TagIdentifier == 5)

Using the above way, we can avoid caller to supply an invalid buffer.

Thanks/Ray

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ni,
> Ruiyu
> Sent: Monday, September 18, 2017 8:29 AM
> To: Paulo Alcantara <pcacjr@zytor.com>; edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2] [PATCH v2 1/3] MdePkg: Add UDF volume structure
> definitions
> 
> #define _GET_TAG_ID(_Pointer) \
>   (((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)
> 
> #define IS_PD(_Pointer) \
>   ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5))
> #define IS_LVD(_Pointer) \
>   ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6))
> #define IS_TD(_Pointer) \
>   ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
> 
> #define IS_AVDP(_Pointer) \
>   ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))
> 
> Paulo,
> If you take a look at Pci22.h in the same directory, all macros are started
> as "IS_PCI_".
> How about changing the above IS_xxx to IS_UDF_xxx?
> Shall we change AVDP to AVD, following the same naming style as PD, LVD
> and TD?
> How about changing _Pointer to _Tag or DescriptorTag?
> 
> 
> Thanks/Ray
> 
> > -----Original Message-----
> > From: Paulo Alcantara [mailto:pcacjr@zytor.com]
> > Sent: Sunday, September 17, 2017 9:13 PM
> > To: edk2-devel@lists.01.org
> > Cc: Paulo Alcantara <pcacjr@zytor.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>;
> Laszlo
> > Ersek <lersek@redhat.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> > Subject: [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions
> >
> > This patch adds a fewe more UDF structures in order to detect Logical
> > Volume and Partition descriptors during Main Volume Descriptor Sequence
> > in Partition driver.
> >
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
> > ---
> >  MdePkg/Include/IndustryStandard/Udf.h | 63 ++++++++++++++++++++
> >  1 file changed, 63 insertions(+)
> >
> > diff --git a/MdePkg/Include/IndustryStandard/Udf.h
> > b/MdePkg/Include/IndustryStandard/Udf.h
> > index 0febb4bcda..6cc42f8543 100644
> > --- a/MdePkg/Include/IndustryStandard/Udf.h
> > +++ b/MdePkg/Include/IndustryStandard/Udf.h
> > @@ -27,9 +27,19 @@
> >  #define _GET_TAG_ID(_Pointer) \
> >    (((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)
> >
> > +#define IS_PD(_Pointer) \
> > +  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5)) #define IS_LVD(_Pointer) \
> > +  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6)) #define IS_TD(_Pointer) \
> > +  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
> > +
> >  #define IS_AVDP(_Pointer) \
> >    ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))
> >
> > +#define LV_UDF_REVISION(_Lv) \
> > +  *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix
> > +
> >  #pragma pack(1)
> >
> >  typedef struct {
> > @@ -55,6 +65,59 @@ typedef struct {
> >    UINT8               Reserved[480];
> >  } UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER;
> >
> > +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;
> > +  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;
> > +
> > +typedef struct {
> > +  UDF_DESCRIPTOR_TAG         DescriptorTag;
> > +  UINT32                     VolumeDescriptorSequenceNumber;
> > +  UINT16                     PartitionFlags;
> > +  UINT16                     PartitionNumber;
> > +  UDF_ENTITY_ID              PartitionContents;
> > +  UINT8                      PartitionContentsUse[128];
> > +  UINT32                     AccessType;
> > +  UINT32                     PartitionStartingLocation;
> > +  UINT32                     PartitionLength;
> > +  UDF_ENTITY_ID              ImplementationIdentifier;
> > +  UINT8                      ImplementationUse[128];
> > +  UINT8                      Reserved[156];
> > +} UDF_PARTITION_DESCRIPTOR;
> > +
> >  #pragma pack()
> >
> >  #endif
> > --
> > 2.11.0
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions

[Paulo Alcantara]

Hi,

On 9/17/2017 9:42 PM, Ni, Ruiyu wrote:
> Continue reading the patch #2, I think we can change IS_PD to:
> #define IS_UDF_PD(Tag) ((Tag)->TagIdentifier == 5)
> 
> Using the above way, we can avoid caller to supply an invalid buffer.

Good point. I'll fix it.

Thanks!
Paulo

> 
> Thanks/Ray
> 
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ni,
>> Ruiyu
>> Sent: Monday, September 18, 2017 8:29 AM
>> To: Paulo Alcantara <pcacjr@zytor.com>; edk2-devel@lists.01.org
>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek
>> <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>
>> Subject: Re: [edk2] [PATCH v2 1/3] MdePkg: Add UDF volume structure
>> definitions
>>
>> #define _GET_TAG_ID(_Pointer) \
>>    (((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)
>>
>> #define IS_PD(_Pointer) \
>>    ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5))
>> #define IS_LVD(_Pointer) \
>>    ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6))
>> #define IS_TD(_Pointer) \
>>    ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
>>
>> #define IS_AVDP(_Pointer) \
>>    ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))
>>
>> Paulo,
>> If you take a look at Pci22.h in the same directory, all macros are started
>> as "IS_PCI_".
>> How about changing the above IS_xxx to IS_UDF_xxx?
>> Shall we change AVDP to AVD, following the same naming style as PD, LVD
>> and TD?
>> How about changing _Pointer to _Tag or DescriptorTag?
>>
>>
>> Thanks/Ray
>>
>>> -----Original Message-----
>>> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
>>> Sent: Sunday, September 17, 2017 9:13 PM
>>> To: edk2-devel@lists.01.org
>>> Cc: Paulo Alcantara <pcacjr@zytor.com>; Kinney, Michael D
>>> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>;
>> Laszlo
>>> Ersek <lersek@redhat.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
>>> Subject: [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions
>>>
>>> This patch adds a fewe more UDF structures in order to detect Logical
>>> Volume and Partition descriptors during Main Volume Descriptor Sequence
>>> in Partition driver.
>>>
>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>> Cc: Liming Gao <liming.gao@intel.com>
>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
>>> ---
>>>   MdePkg/Include/IndustryStandard/Udf.h | 63 ++++++++++++++++++++
>>>   1 file changed, 63 insertions(+)
>>>
>>> diff --git a/MdePkg/Include/IndustryStandard/Udf.h
>>> b/MdePkg/Include/IndustryStandard/Udf.h
>>> index 0febb4bcda..6cc42f8543 100644
>>> --- a/MdePkg/Include/IndustryStandard/Udf.h
>>> +++ b/MdePkg/Include/IndustryStandard/Udf.h
>>> @@ -27,9 +27,19 @@
>>>   #define _GET_TAG_ID(_Pointer) \
>>>     (((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)
>>>
>>> +#define IS_PD(_Pointer) \
>>> +  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5)) #define IS_LVD(_Pointer) \
>>> +  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6)) #define IS_TD(_Pointer) \
>>> +  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
>>> +
>>>   #define IS_AVDP(_Pointer) \
>>>     ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))
>>>
>>> +#define LV_UDF_REVISION(_Lv) \
>>> +  *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix
>>> +
>>>   #pragma pack(1)
>>>
>>>   typedef struct {
>>> @@ -55,6 +65,59 @@ typedef struct {
>>>     UINT8               Reserved[480];
>>>   } UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER;
>>>
>>> +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;
>>> +  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;
>>> +
>>> +typedef struct {
>>> +  UDF_DESCRIPTOR_TAG         DescriptorTag;
>>> +  UINT32                     VolumeDescriptorSequenceNumber;
>>> +  UINT16                     PartitionFlags;
>>> +  UINT16                     PartitionNumber;
>>> +  UDF_ENTITY_ID              PartitionContents;
>>> +  UINT8                      PartitionContentsUse[128];
>>> +  UINT32                     AccessType;
>>> +  UINT32                     PartitionStartingLocation;
>>> +  UINT32                     PartitionLength;
>>> +  UDF_ENTITY_ID              ImplementationIdentifier;
>>> +  UINT8                      ImplementationUse[128];
>>> +  UINT8                      Reserved[156];
>>> +} UDF_PARTITION_DESCRIPTOR;
>>> +
>>>   #pragma pack()
>>>
>>>   #endif
>>> --
>>> 2.11.0
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> 

Re: [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions

[Paulo Alcantara]

Hi Ruiyu,

On 9/17/2017 9:28 PM, Ni, Ruiyu wrote:
> #define _GET_TAG_ID(_Pointer) \
>    (((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)
> 
> #define IS_PD(_Pointer) \
>    ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5))
> #define IS_LVD(_Pointer) \
>    ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6))
> #define IS_TD(_Pointer) \
>    ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
> 
> #define IS_AVDP(_Pointer) \
>    ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))
> 
> Paulo,
> If you take a look at Pci22.h in the same directory, all macros are started
> as "IS_PCI_".
> How about changing the above IS_xxx to IS_UDF_xxx?

Looks good to me. I'll change them.

> Shall we change AVDP to AVD, following the same naming style as PD, LVD and TD?

There is no "Anchor Volume Descriptor" but "Anchor Volume Descriptor 
Pointer", so we still need to keep 'P'.

> How about changing _Pointer to _Tag or DescriptorTag?

I will change it to _Tag. _Pointer is really a bad name. Thanks! :-)

Paulo

> 
> 
> Thanks/Ray
> 
>> -----Original Message-----
>> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
>> Sent: Sunday, September 17, 2017 9:13 PM
>> To: edk2-devel@lists.01.org
>> Cc: Paulo Alcantara <pcacjr@zytor.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Laszlo
>> Ersek <lersek@redhat.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
>> Subject: [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions
>>
>> This patch adds a fewe more UDF structures in order to detect Logical
>> Volume and Partition descriptors during Main Volume Descriptor Sequence
>> in Partition driver.
>>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
>> ---
>>   MdePkg/Include/IndustryStandard/Udf.h | 63 ++++++++++++++++++++
>>   1 file changed, 63 insertions(+)
>>
>> diff --git a/MdePkg/Include/IndustryStandard/Udf.h
>> b/MdePkg/Include/IndustryStandard/Udf.h
>> index 0febb4bcda..6cc42f8543 100644
>> --- a/MdePkg/Include/IndustryStandard/Udf.h
>> +++ b/MdePkg/Include/IndustryStandard/Udf.h
>> @@ -27,9 +27,19 @@
>>   #define _GET_TAG_ID(_Pointer) \
>>     (((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)
>>
>> +#define IS_PD(_Pointer) \
>> +  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5)) #define IS_LVD(_Pointer) \
>> +  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6)) #define IS_TD(_Pointer) \
>> +  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
>> +
>>   #define IS_AVDP(_Pointer) \
>>     ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))
>>
>> +#define LV_UDF_REVISION(_Lv) \
>> +  *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix
>> +
>>   #pragma pack(1)
>>
>>   typedef struct {
>> @@ -55,6 +65,59 @@ typedef struct {
>>     UINT8               Reserved[480];
>>   } UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER;
>>
>> +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;
>> +  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;
>> +
>> +typedef struct {
>> +  UDF_DESCRIPTOR_TAG         DescriptorTag;
>> +  UINT32                     VolumeDescriptorSequenceNumber;
>> +  UINT16                     PartitionFlags;
>> +  UINT16                     PartitionNumber;
>> +  UDF_ENTITY_ID              PartitionContents;
>> +  UINT8                      PartitionContentsUse[128];
>> +  UINT32                     AccessType;
>> +  UINT32                     PartitionStartingLocation;
>> +  UINT32                     PartitionLength;
>> +  UDF_ENTITY_ID              ImplementationIdentifier;
>> +  UINT8                      ImplementationUse[128];
>> +  UINT8                      Reserved[156];
>> +} UDF_PARTITION_DESCRIPTOR;
>> +
>>   #pragma pack()
>>
>>   #endif
>> --
>> 2.11.0
> 
> 

[PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition

[Paulo Alcantara]

Do not use entire block device size for the UDF logical partition,
instead reserve the appropriate space (LVD space) for it.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Reported-by: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
---
 MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 323 ++++++++++++++++++--
 1 file changed, 299 insertions(+), 24 deletions(-)

diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
index 7856b5dfc1..3e84882922 100644
--- a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
+++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
@@ -84,24 +84,8 @@ FindAnchorVolumeDescriptorPointer (
   return EFI_VOLUME_CORRUPTED;
 }
 
-/**
-  Check if block device supports a valid UDF file system as specified by OSTA
-  Universal Disk Format Specification 2.60.
-
-  @param[in]   BlockIo  BlockIo interface.
-  @param[in]   DiskIo   DiskIo interface.
-
-  @retval EFI_SUCCESS          UDF file system found.
-  @retval EFI_UNSUPPORTED      UDF file system not found.
-  @retval EFI_NO_MEDIA         The device has no media.
-  @retval EFI_DEVICE_ERROR     The device reported an error.
-  @retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.
-  @retval EFI_OUT_OF_RESOURCES The scan was not successful due to lack of
-                               resources.
-
-**/
 EFI_STATUS
-SupportUdfFileSystem (
+FindUdfVolumeIdentifiers (
   IN EFI_BLOCK_IO_PROTOCOL  *BlockIo,
   IN EFI_DISK_IO_PROTOCOL   *DiskIo
   )
@@ -111,7 +95,6 @@ SupportUdfFileSystem (
   UINT64                                EndDiskOffset;
   CDROM_VOLUME_DESCRIPTOR               VolDescriptor;
   CDROM_VOLUME_DESCRIPTOR               TerminatingVolDescriptor;
-  UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  AnchorPoint;
 
   ZeroMem ((VOID *)&TerminatingVolDescriptor, sizeof (CDROM_VOLUME_DESCRIPTOR));
 
@@ -207,12 +190,302 @@ SupportUdfFileSystem (
     return EFI_UNSUPPORTED;
   }
 
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+GetPartitionNumber (
+  IN   UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc,
+  OUT  UINT16                         *PartitionNum
+  )
+{
+  EFI_STATUS                      Status;
+  UDF_LONG_ALLOCATION_DESCRIPTOR  *LongAd;
+
+  Status = EFI_SUCCESS;
+
+  switch (LV_UDF_REVISION (LogicalVolDesc)) {
+  case 0x0102:
+    //
+    // UDF 1.20 only supports Type 1 Partition
+    //
+    *PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc->PartitionMaps[4]);
+    break;
+  case 0x0150:
+    //
+    // Ensure Type 1 Partition map
+    //
+    ASSERT (LogicalVolDesc->PartitionMaps[0] == 1 &&
+            LogicalVolDesc->PartitionMaps[1] == 6);
+    *PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc->PartitionMaps[4]);
+    break;
+  case 0x0260:
+    LongAd = &LogicalVolDesc->LogicalVolumeContentsUse;
+    *PartitionNum = LongAd->ExtentLocation.PartitionReferenceNumber;
+    break;
+  default:
+    //
+    // Unhandled UDF revision
+    //
+    Status = EFI_VOLUME_CORRUPTED;
+    break;
+  }
+
+  return Status;
+}
+
+EFI_STATUS
+FindLogicalVolumeLocation (
+  IN   EFI_BLOCK_IO_PROTOCOL                 *BlockIo,
+  IN   EFI_DISK_IO_PROTOCOL                  *DiskIo,
+  IN   UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  *AnchorPoint,
+  OUT  UINT64                                *MainVdsStartLsn,
+  OUT  UINT64                                *LogicalVolEndLsn
+  )
+{
+  EFI_STATUS                     Status;
+  UINT32                         BlockSize;
+  EFI_LBA                        LastBlock;
+  UDF_EXTENT_AD                  *ExtentAd;
+  UINT64                         StartingLsn;
+  UINT64                         EndingLsn;
+  VOID                           *Buffer;
+  UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc;
+  UDF_PARTITION_DESCRIPTOR       *PartitionDesc;
+  UINT64                         GuardMainVdsStartLsn;
+  UINT16                         PartitionNum;
+
+  BlockSize             = BlockIo->Media->BlockSize;
+  LastBlock             = BlockIo->Media->LastBlock;
+  ExtentAd              = &AnchorPoint->MainVolumeDescriptorSequenceExtent;
+  StartingLsn           = (UINT64)ExtentAd->ExtentLocation;
+  EndingLsn             =
+    StartingLsn + DivU64x32 ((UINT64)ExtentAd->ExtentLength, BlockSize);
+
+  LogicalVolDesc        = NULL;
+  PartitionDesc         = NULL;
+  GuardMainVdsStartLsn  = StartingLsn;
+
+  //
+  // Allocate buffer for reading disk blocks
+  //
+  Buffer = AllocateZeroPool (BlockSize);
+  if (Buffer == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  Status = EFI_VOLUME_CORRUPTED;
+
+  //
+  // As per UDF 2.60 specification:
+  //
+  // There shall be exactly one prevailing Logical Volume Descriptor
+  // recorded per Volume Set.
+  //
+  // Start Main Volume Descriptor Sequence and find Logical Volume Descriptor
+  //
+  while (StartingLsn <= EndingLsn) {
+    //
+    // Read disk block
+    //
+    Status = DiskIo->ReadDisk (
+      DiskIo,
+      BlockIo->Media->MediaId,
+      MultU64x32 (StartingLsn, BlockSize),
+      BlockSize,
+      Buffer
+      );
+    if (EFI_ERROR (Status)) {
+      goto Out_Free;
+    }
+
+    //
+    // Check if read block is a Terminating Descriptor
+    //
+    if (IS_TD (Buffer)) {
+      //
+      // Stop Main Volume Descriptor Sequence
+      //
+      break;
+    }
+
+    //
+    // Check if read block is a Logical Volume Descriptor
+    //
+    if (IS_LVD (Buffer)) {
+      //
+      // Ensure only one LVD (Logical Volume Descriptor) is handled
+      //
+      if (LogicalVolDesc != NULL) {
+        Status = EFI_UNSUPPORTED;
+        goto Out_Free;
+      }
+
+      //
+      // As per UDF 2.60 specification:
+      //
+      // For the purpose of interchange, Partition Maps shall be limited to
+      // Partition Map type 1, except type 2 maps.
+      //
+      // NOTE: Type 1 Partitions are the only supported in this implementation.
+      //
+      LogicalVolDesc = AllocateZeroPool (sizeof (*LogicalVolDesc));
+      if (LogicalVolDesc == NULL) {
+        Status = EFI_OUT_OF_RESOURCES;
+        goto Out_Free;
+      }
+
+      //
+      // Save Logical Volume Descriptor
+      //
+      CopyMem (LogicalVolDesc, Buffer, sizeof (*LogicalVolDesc));
+    } else if (IS_PD (Buffer)) {
+      //
+      // Ensure only one PD (Partition Descriptor) is handled
+      //
+      if (PartitionDesc != NULL) {
+        Status = EFI_UNSUPPORTED;
+        goto Out_Free;
+      }
+
+      //
+      // Found a Partition Descriptor.
+      //
+      // As per UDF 2.60 specification:
+      //
+      // A Partition Descriptor Access Type of read-only, rewritable,
+      // overwritable, write-once and pseudo-overwritable shall be
+      // supported. There shall be exactly one prevailing Partition
+      // Descriptor recorded per volume, with one exception. For Volume
+      // Sets that consist of single volume, the volume may contain 2 non-
+      // overlapping Partitions with 2 prevailing Partition Descriptors only
+      // if one has an Access Type of read-only and the other has an
+      // Access Type of rewritable, overwritable, or write-once. The
+      // Logical Volume for this volume would consist of the contents of
+      // both partitions.
+      //
+      PartitionDesc = AllocateZeroPool (sizeof (*PartitionDesc));
+      if (PartitionDesc == NULL) {
+        Status = EFI_OUT_OF_RESOURCES;
+        goto Out_Free;
+      }
+
+      //
+      // Save Partition Descriptor
+      //
+      CopyMem (PartitionDesc, Buffer, sizeof (*PartitionDesc));
+    }
+    //
+    // Go to next disk block
+    //
+    StartingLsn++;
+  }
+
+  Status = EFI_VOLUME_CORRUPTED;
+
+  //
+  // Check if LVD and PD were found
+  //
+  if (LogicalVolDesc != NULL && PartitionDesc != NULL) {
+    //
+    // Get partition number from Partition map in LVD descriptor
+    //
+    Status = GetPartitionNumber (LogicalVolDesc, &PartitionNum);
+    if (EFI_ERROR (Status)) {
+      goto Out_Free;
+    }
+
+    //
+    // Make sure we're handling expected Partition Descriptor
+    //
+    if (PartitionDesc->PartitionNumber != PartitionNum) {
+      Status = EFI_VOLUME_CORRUPTED;
+      goto Out_Free;
+    }
+
+    //
+    // Cover the main VDS area so UdfDxe driver will also be able to get LVD and
+    // PD descriptors out from the file system.
+    //
+    *MainVdsStartLsn = GuardMainVdsStartLsn;
+    *LogicalVolEndLsn = *MainVdsStartLsn + (UINT64)ExtentAd->ExtentLength;
+
+    //
+    // Cover UDF partition area
+    //
+    *LogicalVolEndLsn +=
+      ((UINT64)PartitionDesc->PartitionStartingLocation -
+       *LogicalVolEndLsn) + PartitionDesc->PartitionLength - 1;
+    //
+    // Ensure to not attempt reading past end of device
+    //
+    if (*LogicalVolEndLsn > LastBlock) {
+      Status = EFI_VOLUME_CORRUPTED;
+    } else {
+      Status = EFI_SUCCESS;
+    }
+  }
+
+Out_Free:
+  //
+  // Free block read buffer
+  //
+  FreePool (Buffer);
+  //
+  // Free Logical Volume Descriptor
+  //
+  if (LogicalVolDesc != NULL) {
+    FreePool (LogicalVolDesc);
+  }
+  //
+  // Free Partition Descriptor
+  //
+  if (PartitionDesc != NULL) {
+    FreePool (PartitionDesc);
+  }
+
+  return Status;
+}
+
+EFI_STATUS
+FindUdfLogicalVolume (
+  IN EFI_BLOCK_IO_PROTOCOL  *BlockIo,
+  IN EFI_DISK_IO_PROTOCOL   *DiskIo,
+  OUT EFI_LBA               *StartingLBA,
+  OUT EFI_LBA               *EndingLBA
+  )
+{
+  EFI_STATUS Status;
+  UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  AnchorPoint;
+
+  //
+  // Find UDF volume identifiers
+  //
+  Status = FindUdfVolumeIdentifiers (BlockIo, DiskIo);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  //
+  // Find Anchor Volume Descriptor Pointer
+  //
   Status = FindAnchorVolumeDescriptorPointer (BlockIo, DiskIo, &AnchorPoint);
   if (EFI_ERROR (Status)) {
-    return EFI_UNSUPPORTED;
+    return Status;
   }
 
-  return EFI_SUCCESS;
+  //
+  // Find Logical Volume location
+  //
+  Status = FindLogicalVolumeLocation (
+    BlockIo,
+    DiskIo,
+    &AnchorPoint,
+    (UINT64 *)StartingLBA,
+    (UINT64 *)EndingLBA
+    );
+
+  return Status;
 }
 
 /**
@@ -250,6 +523,8 @@ PartitionInstallUdfChildHandles (
   EFI_GUID                     *VendorDefinedGuid;
   EFI_GUID                     UdfDevPathGuid = EFI_UDF_DEVICE_PATH_GUID;
   EFI_PARTITION_INFO_PROTOCOL  PartitionInfo;
+  EFI_LBA                      StartingLBA;
+  EFI_LBA                      EndingLBA;
 
   Media = BlockIo->Media;
 
@@ -291,9 +566,9 @@ PartitionInstallUdfChildHandles (
   }
 
   //
-  // Check if block device supports an UDF file system
+  // Find UDF logical volume on block device
   //
-  Status = SupportUdfFileSystem (BlockIo, DiskIo);
+  Status = FindUdfLogicalVolume (BlockIo, DiskIo, &StartingLBA, &EndingLBA);
   if (EFI_ERROR (Status)) {
     return EFI_NOT_FOUND;
   }
@@ -318,8 +593,8 @@ PartitionInstallUdfChildHandles (
     DevicePath,
     (EFI_DEVICE_PATH_PROTOCOL *)&gUdfDevicePath,
     &PartitionInfo,
-    0,
-    Media->LastBlock,
+    StartingLBA,
+    EndingLBA,
     Media->BlockSize
     );
   if (!EFI_ERROR (Status)) {
-- 
2.11.0

Re: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition

[Ni, Ruiyu]

Paulo,
Several comments:
1. Can below logic be removed in PartitionDxe/Udf.c?  
while (!IsDevicePathEnd (DevicePathNode)) {
    //
    // Do not allow checking for UDF file systems in CDROM "El Torito"
    // partitions, and skip duplicate installation of UDF file system child
    // nodes.
    //
    if (DevicePathType (DevicePathNode) == MEDIA_DEVICE_PATH) {
      if (DevicePathSubType (DevicePathNode) == MEDIA_CDROM_DP) {
        return EFI_NOT_FOUND;
      }
      if (DevicePathSubType (DevicePathNode) == MEDIA_VENDOR_DP) {
        VendorDefinedGuid = (EFI_GUID *)((UINTN)DevicePathNode +
                                         OFFSET_OF (VENDOR_DEVICE_PATH, Guid));
        if (CompareGuid (VendorDefinedGuid, &UdfDevPathGuid)) {
          return EFI_NOT_FOUND;
        }
      }
    }
    //
    // Try next device path node
    //
    DevicePathNode = NextDevicePathNode (DevicePathNode);
  }

2. Can you add function header comments for the newly added functions?

3. Change the consuming code of IS_UDF_XXX macro after patch #1 is changed.

4. Perhaps to add more checks for these numbers read from the UDF CDROM.
As Jiewen mentioned in another mail, secure code needs to validate all external inputs.
We shouldn't assume the UDF CDROM is a well-formatted CDROM.
A hacker may create a UDF CDROM to hack the system by using mechanism like
buffer overflow, integer overflow, etc.


Thanks/Ray

> -----Original Message-----
> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
> Sent: Sunday, September 17, 2017 9:13 PM
> To: edk2-devel@lists.01.org
> Cc: Paulo Alcantara <pcacjr@zytor.com>; Dong, Eric <eric.dong@intel.com>;
> Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Laszlo
> Ersek <lersek@redhat.com>
> Subject: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF
> logical partition
> 
> Do not use entire block device size for the UDF logical partition, instead
> reserve the appropriate space (LVD space) for it.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Reported-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
> ---
>  MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 323
> ++++++++++++++++++--
>  1 file changed, 299 insertions(+), 24 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> index 7856b5dfc1..3e84882922 100644
> --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> @@ -84,24 +84,8 @@ FindAnchorVolumeDescriptorPointer (
>    return EFI_VOLUME_CORRUPTED;
>  }
> 
> -/**
> -  Check if block device supports a valid UDF file system as specified by OSTA
> -  Universal Disk Format Specification 2.60.
> -
> -  @param[in]   BlockIo  BlockIo interface.
> -  @param[in]   DiskIo   DiskIo interface.
> -
> -  @retval EFI_SUCCESS          UDF file system found.
> -  @retval EFI_UNSUPPORTED      UDF file system not found.
> -  @retval EFI_NO_MEDIA         The device has no media.
> -  @retval EFI_DEVICE_ERROR     The device reported an error.
> -  @retval EFI_VOLUME_CORRUPTED The file system structures are
> corrupted.
> -  @retval EFI_OUT_OF_RESOURCES The scan was not successful due to lack
> of
> -                               resources.
> -
> -**/
>  EFI_STATUS
> -SupportUdfFileSystem (
> +FindUdfVolumeIdentifiers (
>    IN EFI_BLOCK_IO_PROTOCOL  *BlockIo,
>    IN EFI_DISK_IO_PROTOCOL   *DiskIo
>    )
> @@ -111,7 +95,6 @@ SupportUdfFileSystem (
>    UINT64                                EndDiskOffset;
>    CDROM_VOLUME_DESCRIPTOR               VolDescriptor;
>    CDROM_VOLUME_DESCRIPTOR               TerminatingVolDescriptor;
> -  UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  AnchorPoint;
> 
>    ZeroMem ((VOID *)&TerminatingVolDescriptor, sizeof
> (CDROM_VOLUME_DESCRIPTOR));
> 
> @@ -207,12 +190,302 @@ SupportUdfFileSystem (
>      return EFI_UNSUPPORTED;
>    }
> 
> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +GetPartitionNumber (
> +  IN   UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc,
> +  OUT  UINT16                         *PartitionNum
> +  )
> +{
> +  EFI_STATUS                      Status;
> +  UDF_LONG_ALLOCATION_DESCRIPTOR  *LongAd;
> +
> +  Status = EFI_SUCCESS;
> +
> +  switch (LV_UDF_REVISION (LogicalVolDesc)) {  case 0x0102:
> +    //
> +    // UDF 1.20 only supports Type 1 Partition
> +    //
> +    *PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc-
> >PartitionMaps[4]);
> +    break;
> +  case 0x0150:
> +    //
> +    // Ensure Type 1 Partition map
> +    //
> +    ASSERT (LogicalVolDesc->PartitionMaps[0] == 1 &&
> +            LogicalVolDesc->PartitionMaps[1] == 6);
> +    *PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc-
> >PartitionMaps[4]);
> +    break;
> +  case 0x0260:
> +    LongAd = &LogicalVolDesc->LogicalVolumeContentsUse;
> +    *PartitionNum = LongAd->ExtentLocation.PartitionReferenceNumber;
> +    break;
> +  default:
> +    //
> +    // Unhandled UDF revision
> +    //
> +    Status = EFI_VOLUME_CORRUPTED;
> +    break;
> +  }
> +
> +  return Status;
> +}
> +
> +EFI_STATUS
> +FindLogicalVolumeLocation (
> +  IN   EFI_BLOCK_IO_PROTOCOL                 *BlockIo,
> +  IN   EFI_DISK_IO_PROTOCOL                  *DiskIo,
> +  IN   UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  *AnchorPoint,
> +  OUT  UINT64                                *MainVdsStartLsn,
> +  OUT  UINT64                                *LogicalVolEndLsn
> +  )
> +{
> +  EFI_STATUS                     Status;
> +  UINT32                         BlockSize;
> +  EFI_LBA                        LastBlock;
> +  UDF_EXTENT_AD                  *ExtentAd;
> +  UINT64                         StartingLsn;
> +  UINT64                         EndingLsn;
> +  VOID                           *Buffer;
> +  UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc;
> +  UDF_PARTITION_DESCRIPTOR       *PartitionDesc;
> +  UINT64                         GuardMainVdsStartLsn;
> +  UINT16                         PartitionNum;
> +
> +  BlockSize             = BlockIo->Media->BlockSize;
> +  LastBlock             = BlockIo->Media->LastBlock;
> +  ExtentAd              = &AnchorPoint->MainVolumeDescriptorSequenceExtent;
> +  StartingLsn           = (UINT64)ExtentAd->ExtentLocation;
> +  EndingLsn             =
> +    StartingLsn + DivU64x32 ((UINT64)ExtentAd->ExtentLength,
> + BlockSize);
> +
> +  LogicalVolDesc        = NULL;
> +  PartitionDesc         = NULL;
> +  GuardMainVdsStartLsn  = StartingLsn;
> +
> +  //
> +  // Allocate buffer for reading disk blocks  //  Buffer =
> + AllocateZeroPool (BlockSize);  if (Buffer == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  Status = EFI_VOLUME_CORRUPTED;
> +
> +  //
> +  // As per UDF 2.60 specification:
> +  //
> +  // There shall be exactly one prevailing Logical Volume Descriptor
> + // recorded per Volume Set.
> +  //
> +  // Start Main Volume Descriptor Sequence and find Logical Volume
> + Descriptor  //  while (StartingLsn <= EndingLsn) {
> +    //
> +    // Read disk block
> +    //
> +    Status = DiskIo->ReadDisk (
> +      DiskIo,
> +      BlockIo->Media->MediaId,
> +      MultU64x32 (StartingLsn, BlockSize),
> +      BlockSize,
> +      Buffer
> +      );
> +    if (EFI_ERROR (Status)) {
> +      goto Out_Free;
> +    }
> +
> +    //
> +    // Check if read block is a Terminating Descriptor
> +    //
> +    if (IS_TD (Buffer)) {
> +      //
> +      // Stop Main Volume Descriptor Sequence
> +      //
> +      break;
> +    }
> +
> +    //
> +    // Check if read block is a Logical Volume Descriptor
> +    //
> +    if (IS_LVD (Buffer)) {
> +      //
> +      // Ensure only one LVD (Logical Volume Descriptor) is handled
> +      //
> +      if (LogicalVolDesc != NULL) {
> +        Status = EFI_UNSUPPORTED;
> +        goto Out_Free;
> +      }
> +
> +      //
> +      // As per UDF 2.60 specification:
> +      //
> +      // For the purpose of interchange, Partition Maps shall be limited to
> +      // Partition Map type 1, except type 2 maps.
> +      //
> +      // NOTE: Type 1 Partitions are the only supported in this implementation.
> +      //
> +      LogicalVolDesc = AllocateZeroPool (sizeof (*LogicalVolDesc));
> +      if (LogicalVolDesc == NULL) {
> +        Status = EFI_OUT_OF_RESOURCES;
> +        goto Out_Free;
> +      }
> +
> +      //
> +      // Save Logical Volume Descriptor
> +      //
> +      CopyMem (LogicalVolDesc, Buffer, sizeof (*LogicalVolDesc));
> +    } else if (IS_PD (Buffer)) {
> +      //
> +      // Ensure only one PD (Partition Descriptor) is handled
> +      //
> +      if (PartitionDesc != NULL) {
> +        Status = EFI_UNSUPPORTED;
> +        goto Out_Free;
> +      }
> +
> +      //
> +      // Found a Partition Descriptor.
> +      //
> +      // As per UDF 2.60 specification:
> +      //
> +      // A Partition Descriptor Access Type of read-only, rewritable,
> +      // overwritable, write-once and pseudo-overwritable shall be
> +      // supported. There shall be exactly one prevailing Partition
> +      // Descriptor recorded per volume, with one exception. For Volume
> +      // Sets that consist of single volume, the volume may contain 2 non-
> +      // overlapping Partitions with 2 prevailing Partition Descriptors only
> +      // if one has an Access Type of read-only and the other has an
> +      // Access Type of rewritable, overwritable, or write-once. The
> +      // Logical Volume for this volume would consist of the contents of
> +      // both partitions.
> +      //
> +      PartitionDesc = AllocateZeroPool (sizeof (*PartitionDesc));
> +      if (PartitionDesc == NULL) {
> +        Status = EFI_OUT_OF_RESOURCES;
> +        goto Out_Free;
> +      }
> +
> +      //
> +      // Save Partition Descriptor
> +      //
> +      CopyMem (PartitionDesc, Buffer, sizeof (*PartitionDesc));
> +    }
> +    //
> +    // Go to next disk block
> +    //
> +    StartingLsn++;
> +  }
> +
> +  Status = EFI_VOLUME_CORRUPTED;
> +
> +  //
> +  // Check if LVD and PD were found
> +  //
> +  if (LogicalVolDesc != NULL && PartitionDesc != NULL) {
> +    //
> +    // Get partition number from Partition map in LVD descriptor
> +    //
> +    Status = GetPartitionNumber (LogicalVolDesc, &PartitionNum);
> +    if (EFI_ERROR (Status)) {
> +      goto Out_Free;
> +    }
> +
> +    //
> +    // Make sure we're handling expected Partition Descriptor
> +    //
> +    if (PartitionDesc->PartitionNumber != PartitionNum) {
> +      Status = EFI_VOLUME_CORRUPTED;
> +      goto Out_Free;
> +    }
> +
> +    //
> +    // Cover the main VDS area so UdfDxe driver will also be able to get LVD
> and
> +    // PD descriptors out from the file system.
> +    //
> +    *MainVdsStartLsn = GuardMainVdsStartLsn;
> +    *LogicalVolEndLsn = *MainVdsStartLsn +
> + (UINT64)ExtentAd->ExtentLength;
> +
> +    //
> +    // Cover UDF partition area
> +    //
> +    *LogicalVolEndLsn +=
> +      ((UINT64)PartitionDesc->PartitionStartingLocation -
> +       *LogicalVolEndLsn) + PartitionDesc->PartitionLength - 1;
> +    //
> +    // Ensure to not attempt reading past end of device
> +    //
> +    if (*LogicalVolEndLsn > LastBlock) {
> +      Status = EFI_VOLUME_CORRUPTED;
> +    } else {
> +      Status = EFI_SUCCESS;
> +    }
> +  }
> +
> +Out_Free:
> +  //
> +  // Free block read buffer
> +  //
> +  FreePool (Buffer);
> +  //
> +  // Free Logical Volume Descriptor
> +  //
> +  if (LogicalVolDesc != NULL) {
> +    FreePool (LogicalVolDesc);
> +  }
> +  //
> +  // Free Partition Descriptor
> +  //
> +  if (PartitionDesc != NULL) {
> +    FreePool (PartitionDesc);
> +  }
> +
> +  return Status;
> +}
> +
> +EFI_STATUS
> +FindUdfLogicalVolume (
> +  IN EFI_BLOCK_IO_PROTOCOL  *BlockIo,
> +  IN EFI_DISK_IO_PROTOCOL   *DiskIo,
> +  OUT EFI_LBA               *StartingLBA,
> +  OUT EFI_LBA               *EndingLBA
> +  )
> +{
> +  EFI_STATUS Status;
> +  UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  AnchorPoint;
> +
> +  //
> +  // Find UDF volume identifiers
> +  //
> +  Status = FindUdfVolumeIdentifiers (BlockIo, DiskIo);  if (EFI_ERROR
> + (Status)) {
> +    return Status;
> +  }
> +
> +  //
> +  // Find Anchor Volume Descriptor Pointer  //
>    Status = FindAnchorVolumeDescriptorPointer (BlockIo, DiskIo,
> &AnchorPoint);
>    if (EFI_ERROR (Status)) {
> -    return EFI_UNSUPPORTED;
> +    return Status;
>    }
> 
> -  return EFI_SUCCESS;
> +  //
> +  // Find Logical Volume location
> +  //
> +  Status = FindLogicalVolumeLocation (
> +    BlockIo,
> +    DiskIo,
> +    &AnchorPoint,
> +    (UINT64 *)StartingLBA,
> +    (UINT64 *)EndingLBA
> +    );
> +
> +  return Status;
>  }
> 
>  /**
> @@ -250,6 +523,8 @@ PartitionInstallUdfChildHandles (
>    EFI_GUID                     *VendorDefinedGuid;
>    EFI_GUID                     UdfDevPathGuid = EFI_UDF_DEVICE_PATH_GUID;
>    EFI_PARTITION_INFO_PROTOCOL  PartitionInfo;
> +  EFI_LBA                      StartingLBA;
> +  EFI_LBA                      EndingLBA;
> 
>    Media = BlockIo->Media;
> 
> @@ -291,9 +566,9 @@ PartitionInstallUdfChildHandles (
>    }
> 
>    //
> -  // Check if block device supports an UDF file system
> +  // Find UDF logical volume on block device
>    //
> -  Status = SupportUdfFileSystem (BlockIo, DiskIo);
> +  Status = FindUdfLogicalVolume (BlockIo, DiskIo, &StartingLBA,
> + &EndingLBA);
>    if (EFI_ERROR (Status)) {
>      return EFI_NOT_FOUND;
>    }
> @@ -318,8 +593,8 @@ PartitionInstallUdfChildHandles (
>      DevicePath,
>      (EFI_DEVICE_PATH_PROTOCOL *)&gUdfDevicePath,
>      &PartitionInfo,
> -    0,
> -    Media->LastBlock,
> +    StartingLBA,
> +    EndingLBA,
>      Media->BlockSize
>      );
>    if (!EFI_ERROR (Status)) {
> --
> 2.11.0

Re: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition

[Paulo Alcantara]

Hi,

On 9/17/2017 9:54 PM, Ni, Ruiyu wrote:
> Paulo,
> Several comments:
> 1. Can below logic be removed in PartitionDxe/Udf.c?
> while (!IsDevicePathEnd (DevicePathNode)) {
>      //
>      // Do not allow checking for UDF file systems in CDROM "El Torito"
>      // partitions, and skip duplicate installation of UDF file system child
>      // nodes.
>      //
>      if (DevicePathType (DevicePathNode) == MEDIA_DEVICE_PATH) {
>        if (DevicePathSubType (DevicePathNode) == MEDIA_CDROM_DP) {
>          return EFI_NOT_FOUND;
>        }
>        if (DevicePathSubType (DevicePathNode) == MEDIA_VENDOR_DP) {
>          VendorDefinedGuid = (EFI_GUID *)((UINTN)DevicePathNode +
>                                           OFFSET_OF (VENDOR_DEVICE_PATH, Guid));
>          if (CompareGuid (VendorDefinedGuid, &UdfDevPathGuid)) {
>            return EFI_NOT_FOUND;
>          }
>        }
>      }
>      //
>      // Try next device path node
>      //
>      DevicePathNode = NextDevicePathNode (DevicePathNode);
>    }

I think it's no longer necessary, so I'll remove it.

> 
> 2. Can you add function header comments for the newly added functions?

Sure.

> 
> 3. Change the consuming code of IS_UDF_XXX macro after patch #1 is changed.

ACK.

> 
> 4. Perhaps to add more checks for these numbers read from the UDF CDROM.
> As Jiewen mentioned in another mail, secure code needs to validate all external inputs.
> We shouldn't assume the UDF CDROM is a well-formatted CDROM.
> A hacker may create a UDF CDROM to hack the system by using mechanism like
> buffer overflow, integer overflow, etc.

OK, when we start the Main Volume Descriptor Sequence (MVDS), we rely on 
"ExtentAd->ExtentLocation" to know the LBA
where to start the sequence, and on "ExtentAd->ExtentLength" to know 
many LBAs the sequence has. We only care about
LVD, PD and TD descriptors. We currently check whether a TD descriptor 
exists, or the search exceed ExtentAd->ExtentLength blocks
to determine when stopping the sequence. That doesn't seem to be safe 
enough, but here are some thoughts:

a. "ExtentAd->ExtentLength" may point to a bad value. If it's greater 
than or equal to BlockIo->Media->LastBlock, and there is no
TD descriptor, we'll have to walk the entire disk and there will too 
much I/O overhead and then impacting bootup. Obviously,
when it's greater than LastBlock, DiksIo->ReadDisk() should not allow to 
read a non-existent LBA and then return
EFI_INVALID_PARAMETER, as per UEFI spec.

b. "ExtentAd->ExtentLocation" might point to a invalid LBA, although 
DiskIo->ReadDisk() would not allow it.

So, it's time to look at UDF 2.60 spec again:

 > Volume Descriptor Sequence Extent
 >
 > Both the main and reserve volume descriptor sequence extents
 > shall each have a minimum length of 16 logical sectors. The VDS
 > Extent may be terminated by the extent length.

It doesn't tell us either a limit of logical blocks, or a maximum number.

Next:

 > 6.9.2.3 Step 3. Volume Descriptor Sequence
 > Read logical sectors:
 > MVDS_Location through MVDS_Location + (MVDS_Length - 1) / SectorSize
 > ...

MVDS_Length == ExtentAd->ExtentLength, so still insufficient.

Next:

 > The data located in bytes 0-3 and 5 of the Anchor Volume Descriptor 
Pointer may be
 > used for format verification if desired. Verifying the Tag Checksum 
in byte 4 and
 > Descriptor CRC in bytes 8-11 are good additional verifications to 
perform.
 > MVDS_Location and MVDS_Length are read from this structure.

Seems like a great check prior to reading ExtentAd->ExtentLocation and 
ExtentAd->ExtentLength contents. Would that be sufficient to you?

I haven't found a way to limit the number of blocks a Volume Descriptor 
Sequence might have.

Thanks,
Paulo

> 
> 
> Thanks/Ray
> 
>> -----Original Message-----
>> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
>> Sent: Sunday, September 17, 2017 9:13 PM
>> To: edk2-devel@lists.01.org
>> Cc: Paulo Alcantara <pcacjr@zytor.com>; Dong, Eric <eric.dong@intel.com>;
>> Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Laszlo
>> Ersek <lersek@redhat.com>
>> Subject: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF
>> logical partition
>>
>> Do not use entire block device size for the UDF logical partition, instead
>> reserve the appropriate space (LVD space) for it.
>>
>> Cc: Eric Dong <eric.dong@intel.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Cc: Star Zeng <star.zeng@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Reported-by: Ruiyu Ni <ruiyu.ni@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
>> ---
>>   MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 323
>> ++++++++++++++++++--
>>   1 file changed, 299 insertions(+), 24 deletions(-)
>>
>> diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
>> b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
>> index 7856b5dfc1..3e84882922 100644
>> --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
>> +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
>> @@ -84,24 +84,8 @@ FindAnchorVolumeDescriptorPointer (
>>     return EFI_VOLUME_CORRUPTED;
>>   }
>>
>> -/**
>> -  Check if block device supports a valid UDF file system as specified by OSTA
>> -  Universal Disk Format Specification 2.60.
>> -
>> -  @param[in]   BlockIo  BlockIo interface.
>> -  @param[in]   DiskIo   DiskIo interface.
>> -
>> -  @retval EFI_SUCCESS          UDF file system found.
>> -  @retval EFI_UNSUPPORTED      UDF file system not found.
>> -  @retval EFI_NO_MEDIA         The device has no media.
>> -  @retval EFI_DEVICE_ERROR     The device reported an error.
>> -  @retval EFI_VOLUME_CORRUPTED The file system structures are
>> corrupted.
>> -  @retval EFI_OUT_OF_RESOURCES The scan was not successful due to lack
>> of
>> -                               resources.
>> -
>> -**/
>>   EFI_STATUS
>> -SupportUdfFileSystem (
>> +FindUdfVolumeIdentifiers (
>>     IN EFI_BLOCK_IO_PROTOCOL  *BlockIo,
>>     IN EFI_DISK_IO_PROTOCOL   *DiskIo
>>     )
>> @@ -111,7 +95,6 @@ SupportUdfFileSystem (
>>     UINT64                                EndDiskOffset;
>>     CDROM_VOLUME_DESCRIPTOR               VolDescriptor;
>>     CDROM_VOLUME_DESCRIPTOR               TerminatingVolDescriptor;
>> -  UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  AnchorPoint;
>>
>>     ZeroMem ((VOID *)&TerminatingVolDescriptor, sizeof
>> (CDROM_VOLUME_DESCRIPTOR));
>>
>> @@ -207,12 +190,302 @@ SupportUdfFileSystem (
>>       return EFI_UNSUPPORTED;
>>     }
>>
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +EFI_STATUS
>> +GetPartitionNumber (
>> +  IN   UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc,
>> +  OUT  UINT16                         *PartitionNum
>> +  )
>> +{
>> +  EFI_STATUS                      Status;
>> +  UDF_LONG_ALLOCATION_DESCRIPTOR  *LongAd;
>> +
>> +  Status = EFI_SUCCESS;
>> +
>> +  switch (LV_UDF_REVISION (LogicalVolDesc)) {  case 0x0102:
>> +    //
>> +    // UDF 1.20 only supports Type 1 Partition
>> +    //
>> +    *PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc-
>>> PartitionMaps[4]);
>> +    break;
>> +  case 0x0150:
>> +    //
>> +    // Ensure Type 1 Partition map
>> +    //
>> +    ASSERT (LogicalVolDesc->PartitionMaps[0] == 1 &&
>> +            LogicalVolDesc->PartitionMaps[1] == 6);
>> +    *PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc-
>>> PartitionMaps[4]);
>> +    break;
>> +  case 0x0260:
>> +    LongAd = &LogicalVolDesc->LogicalVolumeContentsUse;
>> +    *PartitionNum = LongAd->ExtentLocation.PartitionReferenceNumber;
>> +    break;
>> +  default:
>> +    //
>> +    // Unhandled UDF revision
>> +    //
>> +    Status = EFI_VOLUME_CORRUPTED;
>> +    break;
>> +  }
>> +
>> +  return Status;
>> +}
>> +
>> +EFI_STATUS
>> +FindLogicalVolumeLocation (
>> +  IN   EFI_BLOCK_IO_PROTOCOL                 *BlockIo,
>> +  IN   EFI_DISK_IO_PROTOCOL                  *DiskIo,
>> +  IN   UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  *AnchorPoint,
>> +  OUT  UINT64                                *MainVdsStartLsn,
>> +  OUT  UINT64                                *LogicalVolEndLsn
>> +  )
>> +{
>> +  EFI_STATUS                     Status;
>> +  UINT32                         BlockSize;
>> +  EFI_LBA                        LastBlock;
>> +  UDF_EXTENT_AD                  *ExtentAd;
>> +  UINT64                         StartingLsn;
>> +  UINT64                         EndingLsn;
>> +  VOID                           *Buffer;
>> +  UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc;
>> +  UDF_PARTITION_DESCRIPTOR       *PartitionDesc;
>> +  UINT64                         GuardMainVdsStartLsn;
>> +  UINT16                         PartitionNum;
>> +
>> +  BlockSize             = BlockIo->Media->BlockSize;
>> +  LastBlock             = BlockIo->Media->LastBlock;
>> +  ExtentAd              = &AnchorPoint->MainVolumeDescriptorSequenceExtent;
>> +  StartingLsn           = (UINT64)ExtentAd->ExtentLocation;
>> +  EndingLsn             =
>> +    StartingLsn + DivU64x32 ((UINT64)ExtentAd->ExtentLength,
>> + BlockSize);
>> +
>> +  LogicalVolDesc        = NULL;
>> +  PartitionDesc         = NULL;
>> +  GuardMainVdsStartLsn  = StartingLsn;
>> +
>> +  //
>> +  // Allocate buffer for reading disk blocks  //  Buffer =
>> + AllocateZeroPool (BlockSize);  if (Buffer == NULL) {
>> +    return EFI_OUT_OF_RESOURCES;
>> +  }
>> +
>> +  Status = EFI_VOLUME_CORRUPTED;
>> +
>> +  //
>> +  // As per UDF 2.60 specification:
>> +  //
>> +  // There shall be exactly one prevailing Logical Volume Descriptor
>> + // recorded per Volume Set.
>> +  //
>> +  // Start Main Volume Descriptor Sequence and find Logical Volume
>> + Descriptor  //  while (StartingLsn <= EndingLsn) {
>> +    //
>> +    // Read disk block
>> +    //
>> +    Status = DiskIo->ReadDisk (
>> +      DiskIo,
>> +      BlockIo->Media->MediaId,
>> +      MultU64x32 (StartingLsn, BlockSize),
>> +      BlockSize,
>> +      Buffer
>> +      );
>> +    if (EFI_ERROR (Status)) {
>> +      goto Out_Free;
>> +    }
>> +
>> +    //
>> +    // Check if read block is a Terminating Descriptor
>> +    //
>> +    if (IS_TD (Buffer)) {
>> +      //
>> +      // Stop Main Volume Descriptor Sequence
>> +      //
>> +      break;
>> +    }
>> +
>> +    //
>> +    // Check if read block is a Logical Volume Descriptor
>> +    //
>> +    if (IS_LVD (Buffer)) {
>> +      //
>> +      // Ensure only one LVD (Logical Volume Descriptor) is handled
>> +      //
>> +      if (LogicalVolDesc != NULL) {
>> +        Status = EFI_UNSUPPORTED;
>> +        goto Out_Free;
>> +      }
>> +
>> +      //
>> +      // As per UDF 2.60 specification:
>> +      //
>> +      // For the purpose of interchange, Partition Maps shall be limited to
>> +      // Partition Map type 1, except type 2 maps.
>> +      //
>> +      // NOTE: Type 1 Partitions are the only supported in this implementation.
>> +      //
>> +      LogicalVolDesc = AllocateZeroPool (sizeof (*LogicalVolDesc));
>> +      if (LogicalVolDesc == NULL) {
>> +        Status = EFI_OUT_OF_RESOURCES;
>> +        goto Out_Free;
>> +      }
>> +
>> +      //
>> +      // Save Logical Volume Descriptor
>> +      //
>> +      CopyMem (LogicalVolDesc, Buffer, sizeof (*LogicalVolDesc));
>> +    } else if (IS_PD (Buffer)) {
>> +      //
>> +      // Ensure only one PD (Partition Descriptor) is handled
>> +      //
>> +      if (PartitionDesc != NULL) {
>> +        Status = EFI_UNSUPPORTED;
>> +        goto Out_Free;
>> +      }
>> +
>> +      //
>> +      // Found a Partition Descriptor.
>> +      //
>> +      // As per UDF 2.60 specification:
>> +      //
>> +      // A Partition Descriptor Access Type of read-only, rewritable,
>> +      // overwritable, write-once and pseudo-overwritable shall be
>> +      // supported. There shall be exactly one prevailing Partition
>> +      // Descriptor recorded per volume, with one exception. For Volume
>> +      // Sets that consist of single volume, the volume may contain 2 non-
>> +      // overlapping Partitions with 2 prevailing Partition Descriptors only
>> +      // if one has an Access Type of read-only and the other has an
>> +      // Access Type of rewritable, overwritable, or write-once. The
>> +      // Logical Volume for this volume would consist of the contents of
>> +      // both partitions.
>> +      //
>> +      PartitionDesc = AllocateZeroPool (sizeof (*PartitionDesc));
>> +      if (PartitionDesc == NULL) {
>> +        Status = EFI_OUT_OF_RESOURCES;
>> +        goto Out_Free;
>> +      }
>> +
>> +      //
>> +      // Save Partition Descriptor
>> +      //
>> +      CopyMem (PartitionDesc, Buffer, sizeof (*PartitionDesc));
>> +    }
>> +    //
>> +    // Go to next disk block
>> +    //
>> +    StartingLsn++;
>> +  }
>> +
>> +  Status = EFI_VOLUME_CORRUPTED;
>> +
>> +  //
>> +  // Check if LVD and PD were found
>> +  //
>> +  if (LogicalVolDesc != NULL && PartitionDesc != NULL) {
>> +    //
>> +    // Get partition number from Partition map in LVD descriptor
>> +    //
>> +    Status = GetPartitionNumber (LogicalVolDesc, &PartitionNum);
>> +    if (EFI_ERROR (Status)) {
>> +      goto Out_Free;
>> +    }
>> +
>> +    //
>> +    // Make sure we're handling expected Partition Descriptor
>> +    //
>> +    if (PartitionDesc->PartitionNumber != PartitionNum) {
>> +      Status = EFI_VOLUME_CORRUPTED;
>> +      goto Out_Free;
>> +    }
>> +
>> +    //
>> +    // Cover the main VDS area so UdfDxe driver will also be able to get LVD
>> and
>> +    // PD descriptors out from the file system.
>> +    //
>> +    *MainVdsStartLsn = GuardMainVdsStartLsn;
>> +    *LogicalVolEndLsn = *MainVdsStartLsn +
>> + (UINT64)ExtentAd->ExtentLength;
>> +
>> +    //
>> +    // Cover UDF partition area
>> +    //
>> +    *LogicalVolEndLsn +=
>> +      ((UINT64)PartitionDesc->PartitionStartingLocation -
>> +       *LogicalVolEndLsn) + PartitionDesc->PartitionLength - 1;
>> +    //
>> +    // Ensure to not attempt reading past end of device
>> +    //
>> +    if (*LogicalVolEndLsn > LastBlock) {
>> +      Status = EFI_VOLUME_CORRUPTED;
>> +    } else {
>> +      Status = EFI_SUCCESS;
>> +    }
>> +  }
>> +
>> +Out_Free:
>> +  //
>> +  // Free block read buffer
>> +  //
>> +  FreePool (Buffer);
>> +  //
>> +  // Free Logical Volume Descriptor
>> +  //
>> +  if (LogicalVolDesc != NULL) {
>> +    FreePool (LogicalVolDesc);
>> +  }
>> +  //
>> +  // Free Partition Descriptor
>> +  //
>> +  if (PartitionDesc != NULL) {
>> +    FreePool (PartitionDesc);
>> +  }
>> +
>> +  return Status;
>> +}
>> +
>> +EFI_STATUS
>> +FindUdfLogicalVolume (
>> +  IN EFI_BLOCK_IO_PROTOCOL  *BlockIo,
>> +  IN EFI_DISK_IO_PROTOCOL   *DiskIo,
>> +  OUT EFI_LBA               *StartingLBA,
>> +  OUT EFI_LBA               *EndingLBA
>> +  )
>> +{
>> +  EFI_STATUS Status;
>> +  UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  AnchorPoint;
>> +
>> +  //
>> +  // Find UDF volume identifiers
>> +  //
>> +  Status = FindUdfVolumeIdentifiers (BlockIo, DiskIo);  if (EFI_ERROR
>> + (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  //
>> +  // Find Anchor Volume Descriptor Pointer  //
>>     Status = FindAnchorVolumeDescriptorPointer (BlockIo, DiskIo,
>> &AnchorPoint);
>>     if (EFI_ERROR (Status)) {
>> -    return EFI_UNSUPPORTED;
>> +    return Status;
>>     }
>>
>> -  return EFI_SUCCESS;
>> +  //
>> +  // Find Logical Volume location
>> +  //
>> +  Status = FindLogicalVolumeLocation (
>> +    BlockIo,
>> +    DiskIo,
>> +    &AnchorPoint,
>> +    (UINT64 *)StartingLBA,
>> +    (UINT64 *)EndingLBA
>> +    );
>> +
>> +  return Status;
>>   }
>>
>>   /**
>> @@ -250,6 +523,8 @@ PartitionInstallUdfChildHandles (
>>     EFI_GUID                     *VendorDefinedGuid;
>>     EFI_GUID                     UdfDevPathGuid = EFI_UDF_DEVICE_PATH_GUID;
>>     EFI_PARTITION_INFO_PROTOCOL  PartitionInfo;
>> +  EFI_LBA                      StartingLBA;
>> +  EFI_LBA                      EndingLBA;
>>
>>     Media = BlockIo->Media;
>>
>> @@ -291,9 +566,9 @@ PartitionInstallUdfChildHandles (
>>     }
>>
>>     //
>> -  // Check if block device supports an UDF file system
>> +  // Find UDF logical volume on block device
>>     //
>> -  Status = SupportUdfFileSystem (BlockIo, DiskIo);
>> +  Status = FindUdfLogicalVolume (BlockIo, DiskIo, &StartingLBA,
>> + &EndingLBA);
>>     if (EFI_ERROR (Status)) {
>>       return EFI_NOT_FOUND;
>>     }
>> @@ -318,8 +593,8 @@ PartitionInstallUdfChildHandles (
>>       DevicePath,
>>       (EFI_DEVICE_PATH_PROTOCOL *)&gUdfDevicePath,
>>       &PartitionInfo,
>> -    0,
>> -    Media->LastBlock,
>> +    StartingLBA,
>> +    EndingLBA,
>>       Media->BlockSize
>>       );
>>     if (!EFI_ERROR (Status)) {
>> --
>> 2.11.0
> 
> 

Re: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition

[Ni, Ruiyu]

Thanks/Ray

> -----Original Message-----
> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
> Sent: Tuesday, September 19, 2017 12:38 AM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>;
> Laszlo Ersek <lersek@redhat.com>
> Subject: Re: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of
> UDF logical partition
> 
> Hi,
> 
> On 9/17/2017 9:54 PM, Ni, Ruiyu wrote:
> > Paulo,
> > Several comments:
> > 1. Can below logic be removed in PartitionDxe/Udf.c?
> > while (!IsDevicePathEnd (DevicePathNode)) {
> >      //
> >      // Do not allow checking for UDF file systems in CDROM "El Torito"
> >      // partitions, and skip duplicate installation of UDF file system child
> >      // nodes.
> >      //
> >      if (DevicePathType (DevicePathNode) == MEDIA_DEVICE_PATH) {
> >        if (DevicePathSubType (DevicePathNode) == MEDIA_CDROM_DP) {
> >          return EFI_NOT_FOUND;
> >        }
> >        if (DevicePathSubType (DevicePathNode) == MEDIA_VENDOR_DP) {
> >          VendorDefinedGuid = (EFI_GUID *)((UINTN)DevicePathNode +
> >                                           OFFSET_OF (VENDOR_DEVICE_PATH, Guid));
> >          if (CompareGuid (VendorDefinedGuid, &UdfDevPathGuid)) {
> >            return EFI_NOT_FOUND;
> >          }
> >        }
> >      }
> >      //
> >      // Try next device path node
> >      //
> >      DevicePathNode = NextDevicePathNode (DevicePathNode);
> >    }
> 
> I think it's no longer necessary, so I'll remove it.
> 
> >
> > 2. Can you add function header comments for the newly added functions?
> 
> Sure.

When you add the header comments, please run
Python BaseTools/Source/Python/Ecc/Ecc.py -t MdeModulePkg/Universal/Disk/PartitionDxe -s
To make sure there is no other coding style issue.

> 
> >
> > 3. Change the consuming code of IS_UDF_XXX macro after patch #1 is
> changed.
> 
> ACK.

I think you may also need to declare the Buffer as UDF_DESCRIPTOR_TAG.

> 
> >
> > 4. Perhaps to add more checks for these numbers read from the UDF
> CDROM.
> > As Jiewen mentioned in another mail, secure code needs to validate all
> external inputs.
> > We shouldn't assume the UDF CDROM is a well-formatted CDROM.
> > A hacker may create a UDF CDROM to hack the system by using mechanism
> > like buffer overflow, integer overflow, etc.
> 
> OK, when we start the Main Volume Descriptor Sequence (MVDS), we rely
> on "ExtentAd->ExtentLocation" to know the LBA where to start the
> sequence, and on "ExtentAd->ExtentLength" to know many LBAs the
> sequence has. We only care about LVD, PD and TD descriptors. We currently
> check whether a TD descriptor exists, or the search exceed ExtentAd-
> >ExtentLength blocks to determine when stopping the sequence. That
> doesn't seem to be safe enough, but here are some thoughts:
> 
> a. "ExtentAd->ExtentLength" may point to a bad value. If it's greater than or
> equal to BlockIo->Media->LastBlock, and there is no TD descriptor, we'll have
> to walk the entire disk and there will too much I/O overhead and then
> impacting bootup. Obviously, when it's greater than LastBlock, DiksIo-
> >ReadDisk() should not allow to read a non-existent LBA and then return
> EFI_INVALID_PARAMETER, as per UEFI spec.
> 
> b. "ExtentAd->ExtentLocation" might point to a invalid LBA, although
> DiskIo->ReadDisk() would not allow it.
> 
> So, it's time to look at UDF 2.60 spec again:
> 
>  > Volume Descriptor Sequence Extent
>  >
>  > Both the main and reserve volume descriptor sequence extents  > shall
> each have a minimum length of 16 logical sectors. The VDS  > Extent may be
> terminated by the extent length.
> 
> It doesn't tell us either a limit of logical blocks, or a maximum number.
> 
> Next:
> 
>  > 6.9.2.3 Step 3. Volume Descriptor Sequence  > Read logical sectors:
>  > MVDS_Location through MVDS_Location + (MVDS_Length - 1) /
> SectorSize  > ...
> 
> MVDS_Length == ExtentAd->ExtentLength, so still insufficient.
> 
> Next:
> 
>  > The data located in bytes 0-3 and 5 of the Anchor Volume Descriptor
> Pointer may be  > used for format verification if desired. Verifying the Tag
> Checksum in byte 4 and  > Descriptor CRC in bytes 8-11 are good additional
> verifications to perform.
>  > MVDS_Location and MVDS_Length are read from this structure.
> 
> Seems like a great check prior to reading ExtentAd->ExtentLocation and
> ExtentAd->ExtentLength contents. Would that be sufficient to you?


OK to me.

> 
> I haven't found a way to limit the number of blocks a Volume Descriptor
> Sequence might have.
> 
> Thanks,
> Paulo
> 
> >
> >
> > Thanks/Ray
> >
> >> -----Original Message-----
> >> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
> >> Sent: Sunday, September 17, 2017 9:13 PM
> >> To: edk2-devel@lists.01.org
> >> Cc: Paulo Alcantara <pcacjr@zytor.com>; Dong, Eric
> >> <eric.dong@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star
> >> <star.zeng@intel.com>; Laszlo Ersek <lersek@redhat.com>
> >> Subject: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of
> >> UDF logical partition
> >>
> >> Do not use entire block device size for the UDF logical partition,
> >> instead reserve the appropriate space (LVD space) for it.
> >>
> >> Cc: Eric Dong <eric.dong@intel.com>
> >> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> >> Cc: Star Zeng <star.zeng@intel.com>
> >> Cc: Laszlo Ersek <lersek@redhat.com>
> >> Reported-by: Ruiyu Ni <ruiyu.ni@intel.com>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
> >> ---
> >>   MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 323
> >> ++++++++++++++++++--
> >>   1 file changed, 299 insertions(+), 24 deletions(-)
> >>
> >> diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> >> b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> >> index 7856b5dfc1..3e84882922 100644
> >> --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> >> +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> >> @@ -84,24 +84,8 @@ FindAnchorVolumeDescriptorPointer (
> >>     return EFI_VOLUME_CORRUPTED;
> >>   }
> >>
> >> -/**
> >> -  Check if block device supports a valid UDF file system as
> >> specified by OSTA
> >> -  Universal Disk Format Specification 2.60.
> >> -
> >> -  @param[in]   BlockIo  BlockIo interface.
> >> -  @param[in]   DiskIo   DiskIo interface.
> >> -
> >> -  @retval EFI_SUCCESS          UDF file system found.
> >> -  @retval EFI_UNSUPPORTED      UDF file system not found.
> >> -  @retval EFI_NO_MEDIA         The device has no media.
> >> -  @retval EFI_DEVICE_ERROR     The device reported an error.
> >> -  @retval EFI_VOLUME_CORRUPTED The file system structures are
> >> corrupted.
> >> -  @retval EFI_OUT_OF_RESOURCES The scan was not successful due to
> >> lack of
> >> -                               resources.
> >> -
> >> -**/
> >>   EFI_STATUS
> >> -SupportUdfFileSystem (
> >> +FindUdfVolumeIdentifiers (
> >>     IN EFI_BLOCK_IO_PROTOCOL  *BlockIo,
> >>     IN EFI_DISK_IO_PROTOCOL   *DiskIo
> >>     )
> >> @@ -111,7 +95,6 @@ SupportUdfFileSystem (
> >>     UINT64                                EndDiskOffset;
> >>     CDROM_VOLUME_DESCRIPTOR               VolDescriptor;
> >>     CDROM_VOLUME_DESCRIPTOR               TerminatingVolDescriptor;
> >> -  UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  AnchorPoint;
> >>
> >>     ZeroMem ((VOID *)&TerminatingVolDescriptor, sizeof
> >> (CDROM_VOLUME_DESCRIPTOR));
> >>
> >> @@ -207,12 +190,302 @@ SupportUdfFileSystem (
> >>       return EFI_UNSUPPORTED;
> >>     }
> >>
> >> +  return EFI_SUCCESS;
> >> +}
> >> +
> >> +EFI_STATUS
> >> +GetPartitionNumber (
> >> +  IN   UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc,
> >> +  OUT  UINT16                         *PartitionNum
> >> +  )
> >> +{
> >> +  EFI_STATUS                      Status;
> >> +  UDF_LONG_ALLOCATION_DESCRIPTOR  *LongAd;
> >> +
> >> +  Status = EFI_SUCCESS;
> >> +
> >> +  switch (LV_UDF_REVISION (LogicalVolDesc)) {  case 0x0102:
> >> +    //
> >> +    // UDF 1.20 only supports Type 1 Partition
> >> +    //
> >> +    *PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc-
> >>> PartitionMaps[4]);
> >> +    break;
> >> +  case 0x0150:
> >> +    //
> >> +    // Ensure Type 1 Partition map
> >> +    //
> >> +    ASSERT (LogicalVolDesc->PartitionMaps[0] == 1 &&
> >> +            LogicalVolDesc->PartitionMaps[1] == 6);
> >> +    *PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc-
> >>> PartitionMaps[4]);
> >> +    break;
> >> +  case 0x0260:
> >> +    LongAd = &LogicalVolDesc->LogicalVolumeContentsUse;
> >> +    *PartitionNum = LongAd->ExtentLocation.PartitionReferenceNumber;
> >> +    break;
> >> +  default:
> >> +    //
> >> +    // Unhandled UDF revision
> >> +    //
> >> +    Status = EFI_VOLUME_CORRUPTED;
> >> +    break;
> >> +  }
> >> +
> >> +  return Status;
> >> +}
> >> +
> >> +EFI_STATUS
> >> +FindLogicalVolumeLocation (
> >> +  IN   EFI_BLOCK_IO_PROTOCOL                 *BlockIo,
> >> +  IN   EFI_DISK_IO_PROTOCOL                  *DiskIo,
> >> +  IN   UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  *AnchorPoint,
> >> +  OUT  UINT64                                *MainVdsStartLsn,
> >> +  OUT  UINT64                                *LogicalVolEndLsn
> >> +  )
> >> +{
> >> +  EFI_STATUS                     Status;
> >> +  UINT32                         BlockSize;
> >> +  EFI_LBA                        LastBlock;
> >> +  UDF_EXTENT_AD                  *ExtentAd;
> >> +  UINT64                         StartingLsn;
> >> +  UINT64                         EndingLsn;
> >> +  VOID                           *Buffer;
> >> +  UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc;
> >> +  UDF_PARTITION_DESCRIPTOR       *PartitionDesc;
> >> +  UINT64                         GuardMainVdsStartLsn;
> >> +  UINT16                         PartitionNum;
> >> +
> >> +  BlockSize             = BlockIo->Media->BlockSize;
> >> +  LastBlock             = BlockIo->Media->LastBlock;
> >> +  ExtentAd              = &AnchorPoint-
> >MainVolumeDescriptorSequenceExtent;
> >> +  StartingLsn           = (UINT64)ExtentAd->ExtentLocation;
> >> +  EndingLsn             =
> >> +    StartingLsn + DivU64x32 ((UINT64)ExtentAd->ExtentLength,
> >> + BlockSize);
> >> +
> >> +  LogicalVolDesc        = NULL;
> >> +  PartitionDesc         = NULL;
> >> +  GuardMainVdsStartLsn  = StartingLsn;
> >> +
> >> +  //
> >> +  // Allocate buffer for reading disk blocks  //  Buffer =
> >> + AllocateZeroPool (BlockSize);  if (Buffer == NULL) {
> >> +    return EFI_OUT_OF_RESOURCES;
> >> +  }
> >> +
> >> +  Status = EFI_VOLUME_CORRUPTED;
> >> +
> >> +  //
> >> +  // As per UDF 2.60 specification:
> >> +  //
> >> +  // There shall be exactly one prevailing Logical Volume Descriptor
> >> + // recorded per Volume Set.
> >> +  //
> >> +  // Start Main Volume Descriptor Sequence and find Logical Volume
> >> + Descriptor  //  while (StartingLsn <= EndingLsn) {
> >> +    //
> >> +    // Read disk block
> >> +    //
> >> +    Status = DiskIo->ReadDisk (
> >> +      DiskIo,
> >> +      BlockIo->Media->MediaId,
> >> +      MultU64x32 (StartingLsn, BlockSize),
> >> +      BlockSize,
> >> +      Buffer
> >> +      );
> >> +    if (EFI_ERROR (Status)) {
> >> +      goto Out_Free;
> >> +    }
> >> +
> >> +    //
> >> +    // Check if read block is a Terminating Descriptor
> >> +    //
> >> +    if (IS_TD (Buffer)) {
> >> +      //
> >> +      // Stop Main Volume Descriptor Sequence
> >> +      //
> >> +      break;
> >> +    }
> >> +
> >> +    //
> >> +    // Check if read block is a Logical Volume Descriptor
> >> +    //
> >> +    if (IS_LVD (Buffer)) {
> >> +      //
> >> +      // Ensure only one LVD (Logical Volume Descriptor) is handled
> >> +      //
> >> +      if (LogicalVolDesc != NULL) {
> >> +        Status = EFI_UNSUPPORTED;
> >> +        goto Out_Free;
> >> +      }
> >> +
> >> +      //
> >> +      // As per UDF 2.60 specification:
> >> +      //
> >> +      // For the purpose of interchange, Partition Maps shall be limited to
> >> +      // Partition Map type 1, except type 2 maps.
> >> +      //
> >> +      // NOTE: Type 1 Partitions are the only supported in this
> implementation.
> >> +      //
> >> +      LogicalVolDesc = AllocateZeroPool (sizeof (*LogicalVolDesc));
> >> +      if (LogicalVolDesc == NULL) {
> >> +        Status = EFI_OUT_OF_RESOURCES;
> >> +        goto Out_Free;
> >> +      }
> >> +
> >> +      //
> >> +      // Save Logical Volume Descriptor
> >> +      //
> >> +      CopyMem (LogicalVolDesc, Buffer, sizeof (*LogicalVolDesc));
> >> +    } else if (IS_PD (Buffer)) {
> >> +      //
> >> +      // Ensure only one PD (Partition Descriptor) is handled
> >> +      //
> >> +      if (PartitionDesc != NULL) {
> >> +        Status = EFI_UNSUPPORTED;
> >> +        goto Out_Free;
> >> +      }
> >> +
> >> +      //
> >> +      // Found a Partition Descriptor.
> >> +      //
> >> +      // As per UDF 2.60 specification:
> >> +      //
> >> +      // A Partition Descriptor Access Type of read-only, rewritable,
> >> +      // overwritable, write-once and pseudo-overwritable shall be
> >> +      // supported. There shall be exactly one prevailing Partition
> >> +      // Descriptor recorded per volume, with one exception. For Volume
> >> +      // Sets that consist of single volume, the volume may contain 2 non-
> >> +      // overlapping Partitions with 2 prevailing Partition Descriptors only
> >> +      // if one has an Access Type of read-only and the other has an
> >> +      // Access Type of rewritable, overwritable, or write-once. The
> >> +      // Logical Volume for this volume would consist of the contents of
> >> +      // both partitions.
> >> +      //
> >> +      PartitionDesc = AllocateZeroPool (sizeof (*PartitionDesc));
> >> +      if (PartitionDesc == NULL) {
> >> +        Status = EFI_OUT_OF_RESOURCES;
> >> +        goto Out_Free;
> >> +      }
> >> +
> >> +      //
> >> +      // Save Partition Descriptor
> >> +      //
> >> +      CopyMem (PartitionDesc, Buffer, sizeof (*PartitionDesc));
> >> +    }
> >> +    //
> >> +    // Go to next disk block
> >> +    //
> >> +    StartingLsn++;
> >> +  }
> >> +
> >> +  Status = EFI_VOLUME_CORRUPTED;
> >> +
> >> +  //
> >> +  // Check if LVD and PD were found
> >> +  //
> >> +  if (LogicalVolDesc != NULL && PartitionDesc != NULL) {
> >> +    //
> >> +    // Get partition number from Partition map in LVD descriptor
> >> +    //
> >> +    Status = GetPartitionNumber (LogicalVolDesc, &PartitionNum);
> >> +    if (EFI_ERROR (Status)) {
> >> +      goto Out_Free;
> >> +    }
> >> +
> >> +    //
> >> +    // Make sure we're handling expected Partition Descriptor
> >> +    //
> >> +    if (PartitionDesc->PartitionNumber != PartitionNum) {
> >> +      Status = EFI_VOLUME_CORRUPTED;
> >> +      goto Out_Free;
> >> +    }
> >> +
> >> +    //
> >> +    // Cover the main VDS area so UdfDxe driver will also be able to
> >> + get LVD
> >> and
> >> +    // PD descriptors out from the file system.
> >> +    //
> >> +    *MainVdsStartLsn = GuardMainVdsStartLsn;
> >> +    *LogicalVolEndLsn = *MainVdsStartLsn +
> >> + (UINT64)ExtentAd->ExtentLength;
> >> +
> >> +    //
> >> +    // Cover UDF partition area
> >> +    //
> >> +    *LogicalVolEndLsn +=
> >> +      ((UINT64)PartitionDesc->PartitionStartingLocation -
> >> +       *LogicalVolEndLsn) + PartitionDesc->PartitionLength - 1;
> >> +    //
> >> +    // Ensure to not attempt reading past end of device
> >> +    //
> >> +    if (*LogicalVolEndLsn > LastBlock) {
> >> +      Status = EFI_VOLUME_CORRUPTED;
> >> +    } else {
> >> +      Status = EFI_SUCCESS;
> >> +    }
> >> +  }
> >> +
> >> +Out_Free:
> >> +  //
> >> +  // Free block read buffer
> >> +  //
> >> +  FreePool (Buffer);
> >> +  //
> >> +  // Free Logical Volume Descriptor
> >> +  //
> >> +  if (LogicalVolDesc != NULL) {
> >> +    FreePool (LogicalVolDesc);
> >> +  }
> >> +  //
> >> +  // Free Partition Descriptor
> >> +  //
> >> +  if (PartitionDesc != NULL) {
> >> +    FreePool (PartitionDesc);
> >> +  }
> >> +
> >> +  return Status;
> >> +}
> >> +
> >> +EFI_STATUS
> >> +FindUdfLogicalVolume (
> >> +  IN EFI_BLOCK_IO_PROTOCOL  *BlockIo,
> >> +  IN EFI_DISK_IO_PROTOCOL   *DiskIo,
> >> +  OUT EFI_LBA               *StartingLBA,
> >> +  OUT EFI_LBA               *EndingLBA
> >> +  )
> >> +{
> >> +  EFI_STATUS Status;
> >> +  UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  AnchorPoint;
> >> +
> >> +  //
> >> +  // Find UDF volume identifiers
> >> +  //
> >> +  Status = FindUdfVolumeIdentifiers (BlockIo, DiskIo);  if
> >> + (EFI_ERROR
> >> + (Status)) {
> >> +    return Status;
> >> +  }
> >> +
> >> +  //
> >> +  // Find Anchor Volume Descriptor Pointer  //
> >>     Status = FindAnchorVolumeDescriptorPointer (BlockIo, DiskIo,
> >> &AnchorPoint);
> >>     if (EFI_ERROR (Status)) {
> >> -    return EFI_UNSUPPORTED;
> >> +    return Status;
> >>     }
> >>
> >> -  return EFI_SUCCESS;
> >> +  //
> >> +  // Find Logical Volume location
> >> +  //
> >> +  Status = FindLogicalVolumeLocation (
> >> +    BlockIo,
> >> +    DiskIo,
> >> +    &AnchorPoint,
> >> +    (UINT64 *)StartingLBA,
> >> +    (UINT64 *)EndingLBA
> >> +    );
> >> +
> >> +  return Status;
> >>   }
> >>
> >>   /**
> >> @@ -250,6 +523,8 @@ PartitionInstallUdfChildHandles (
> >>     EFI_GUID                     *VendorDefinedGuid;
> >>     EFI_GUID                     UdfDevPathGuid = EFI_UDF_DEVICE_PATH_GUID;
> >>     EFI_PARTITION_INFO_PROTOCOL  PartitionInfo;
> >> +  EFI_LBA                      StartingLBA;
> >> +  EFI_LBA                      EndingLBA;
> >>
> >>     Media = BlockIo->Media;
> >>
> >> @@ -291,9 +566,9 @@ PartitionInstallUdfChildHandles (
> >>     }
> >>
> >>     //
> >> -  // Check if block device supports an UDF file system
> >> +  // Find UDF logical volume on block device
> >>     //
> >> -  Status = SupportUdfFileSystem (BlockIo, DiskIo);
> >> +  Status = FindUdfLogicalVolume (BlockIo, DiskIo, &StartingLBA,
> >> + &EndingLBA);
> >>     if (EFI_ERROR (Status)) {
> >>       return EFI_NOT_FOUND;
> >>     }
> >> @@ -318,8 +593,8 @@ PartitionInstallUdfChildHandles (
> >>       DevicePath,
> >>       (EFI_DEVICE_PATH_PROTOCOL *)&gUdfDevicePath,
> >>       &PartitionInfo,
> >> -    0,
> >> -    Media->LastBlock,
> >> +    StartingLBA,
> >> +    EndingLBA,
> >>       Media->BlockSize
> >>       );
> >>     if (!EFI_ERROR (Status)) {
> >> --
> >> 2.11.0
> >
> >

Re: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition

[Laszlo Ersek]

Ray,

On 09/20/17 10:14, Ni, Ruiyu wrote:

> When you add the header comments, please run
> Python BaseTools/Source/Python/Ecc/Ecc.py -t MdeModulePkg/Universal/Disk/PartitionDxe -s
> To make sure there is no other coding style issue.

side question: do you mean "PatchCheck.py"?

I've never heard of "Ecc.py" before, what does it do? (The top comment
only says "This file is used to be the main entrance of ECC tool", and
the "BaseTools/UserManuals" directory doesn't seem to contain anything
related.)

Thanks,
Laslzo

Re: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition

[Ni, Ruiyu]

I am surprised that you don't know ECC tool. 😊
It's a much more complex tool than PatchCheck.py.
It's to report all coding style issues that doesn't follow EDKII C coding style.


Thanks/Ray

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Wednesday, September 20, 2017 5:59 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; Paulo Alcantara <pcacjr@zytor.com>;
> edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of
> UDF logical partition
> 
> Ray,
> 
> On 09/20/17 10:14, Ni, Ruiyu wrote:
> 
> > When you add the header comments, please run Python
> > BaseTools/Source/Python/Ecc/Ecc.py -t
> > MdeModulePkg/Universal/Disk/PartitionDxe -s To make sure there is no
> other coding style issue.
> 
> side question: do you mean "PatchCheck.py"?
> 
> I've never heard of "Ecc.py" before, what does it do? (The top comment only
> says "This file is used to be the main entrance of ECC tool", and the
> "BaseTools/UserManuals" directory doesn't seem to contain anything
> related.)
> 
> Thanks,
> Laslzo

Re: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition

[Laszlo Ersek]

On 09/20/17 12:11, Ni, Ruiyu wrote:
> I am surprised that you don't know ECC tool. 😊
> It's a much more complex tool than PatchCheck.py.
> It's to report all coding style issues that doesn't follow EDKII C coding style.

Well, before my previous email, I tried to execute the command line you named, but it didn't work:

$ source edksetup.sh

$ Ecc -t MdeModulePkg/Universal/Disk/PartitionDxe -s
Traceback (most recent call last):
  File "BaseTools/BinWrappers/PosixLike/../../Source/Python/Ecc/Ecc.py", line 24, in <module>
    from Check import Check
  File "BaseTools/Source/Python/Ecc/Check.py", line 20, in <module>
    import c
  File "BaseTools/Source/Python/Ecc/c.py", line 18, in <module>
    import CodeFragmentCollector
  File "BaseTools/Source/Python/Ecc/CodeFragmentCollector.py", line 23, in <module>
    import antlr3
ImportError: No module named antlr3

Now I'v checked both Fedora and RHEL7 packages for an "antlr3" python module, but it doesn't exist. The "antlr3" package itself exists in both distros, but the changelogs say,

> * Thu Feb 23 2012 Miloš Jakubíček <xjakub@fi.muni.cz>
> - 3.4-5 - Disable python runtime (incompatible with current antlr version)

The following RHBZ looks relevant: <https://bugzilla.redhat.com/show_bug.cgi?id=1313024>

Either way, I don't think I can use the ECC tool.

Thanks,
Laszlo


>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Wednesday, September 20, 2017 5:59 PM
>> To: Ni, Ruiyu <ruiyu.ni@intel.com>; Paulo Alcantara <pcacjr@zytor.com>;
>> edk2-devel@lists.01.org
>> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
>> Subject: Re: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of
>> UDF logical partition
>>
>> Ray,
>>
>> On 09/20/17 10:14, Ni, Ruiyu wrote:
>>
>>> When you add the header comments, please run Python
>>> BaseTools/Source/Python/Ecc/Ecc.py -t
>>> MdeModulePkg/Universal/Disk/PartitionDxe -s To make sure there is no
>> other coding style issue.
>>
>> side question: do you mean "PatchCheck.py"?
>>
>> I've never heard of "Ecc.py" before, what does it do? (The top comment only
>> says "This file is used to be the main entrance of ECC tool", and the
>> "BaseTools/UserManuals" directory doesn't seem to contain anything
>> related.)
>>
>> Thanks,
>> Laslzo

Re: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition

[Gao, Liming]

Laszlo:
  ECC depends on antlr V3.0.1. It can be downloaded from http://www.antlr3.org/download/Python/
  After download it, use python setup.py install to install it. I try this way on Windows OS, it can work. 

Thanks
Liming
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
> Sent: Wednesday, September 20, 2017 7:10 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; Paulo Alcantara <pcacjr@zytor.com>; edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition
> 
> On 09/20/17 12:11, Ni, Ruiyu wrote:
> > I am surprised that you don't know ECC tool. 😊
> > It's a much more complex tool than PatchCheck.py.
> > It's to report all coding style issues that doesn't follow EDKII C coding style.
> 
> Well, before my previous email, I tried to execute the command line you named, but it didn't work:
> 
> $ source edksetup.sh
> 
> $ Ecc -t MdeModulePkg/Universal/Disk/PartitionDxe -s
> Traceback (most recent call last):
>   File "BaseTools/BinWrappers/PosixLike/../../Source/Python/Ecc/Ecc.py", line 24, in <module>
>     from Check import Check
>   File "BaseTools/Source/Python/Ecc/Check.py", line 20, in <module>
>     import c
>   File "BaseTools/Source/Python/Ecc/c.py", line 18, in <module>
>     import CodeFragmentCollector
>   File "BaseTools/Source/Python/Ecc/CodeFragmentCollector.py", line 23, in <module>
>     import antlr3
> ImportError: No module named antlr3
> 
> Now I'v checked both Fedora and RHEL7 packages for an "antlr3" python module, but it doesn't exist. The "antlr3" package itself exists
> in both distros, but the changelogs say,
> 
> > * Thu Feb 23 2012 Miloš Jakubíček <xjakub@fi.muni.cz>
> > - 3.4-5 - Disable python runtime (incompatible with current antlr version)
> 
> The following RHBZ looks relevant: <https://bugzilla.redhat.com/show_bug.cgi?id=1313024>
> 
> Either way, I don't think I can use the ECC tool.
> 
> Thanks,
> Laszlo
> 
> 
> >> -----Original Message-----
> >> From: Laszlo Ersek [mailto:lersek@redhat.com]
> >> Sent: Wednesday, September 20, 2017 5:59 PM
> >> To: Ni, Ruiyu <ruiyu.ni@intel.com>; Paulo Alcantara <pcacjr@zytor.com>;
> >> edk2-devel@lists.01.org
> >> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> >> Subject: Re: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of
> >> UDF logical partition
> >>
> >> Ray,
> >>
> >> On 09/20/17 10:14, Ni, Ruiyu wrote:
> >>
> >>> When you add the header comments, please run Python
> >>> BaseTools/Source/Python/Ecc/Ecc.py -t
> >>> MdeModulePkg/Universal/Disk/PartitionDxe -s To make sure there is no
> >> other coding style issue.
> >>
> >> side question: do you mean "PatchCheck.py"?
> >>
> >> I've never heard of "Ecc.py" before, what does it do? (The top comment only
> >> says "This file is used to be the main entrance of ECC tool", and the
> >> "BaseTools/UserManuals" directory doesn't seem to contain anything
> >> related.)
> >>
> >> Thanks,
> >> Laslzo
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

[PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to support PartitionDxe changes

[Paulo Alcantara]

This patch reworks the driver to support Partition driver changes.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Paulo Alcantara <pcacjr@zytor.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
---
 MdeModulePkg/Universal/Disk/UdfDxe/File.c                 |  13 +-
 MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 515 ++++++++------------
 MdeModulePkg/Universal/Disk/UdfDxe/Udf.c                  |   7 -
 MdeModulePkg/Universal/Disk/UdfDxe/Udf.h                  |  88 +---
 4 files changed, 204 insertions(+), 419 deletions(-)

diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
index 01361141bb..f2c62967e8 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
@@ -100,6 +100,7 @@ UdfOpenVolume (
     &PrivFsData->Volume,
     &PrivFsData->Root
     );
+  ASSERT_EFI_ERROR (Status);
   if (EFI_ERROR (Status)) {
     goto Error_Find_Root_Dir;
   }
@@ -131,7 +132,6 @@ Error_Alloc_Priv_File_Data:
   CleanupFileInformation (&PrivFsData->Root);
 
 Error_Find_Root_Dir:
-  CleanupVolumeInformation (&PrivFsData->Volume);
 
 Error_Read_Udf_Volume:
 Error_Invalid_Params:
@@ -528,7 +528,6 @@ UdfClose (
   EFI_TPL                     OldTpl;
   EFI_STATUS                  Status;
   PRIVATE_UDF_FILE_DATA       *PrivFileData;
-  PRIVATE_UDF_SIMPLE_FS_DATA  *PrivFsData;
 
   OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
 
@@ -541,8 +540,6 @@ UdfClose (
 
   PrivFileData = PRIVATE_UDF_FILE_DATA_FROM_THIS (This);
 
-  PrivFsData = PRIVATE_UDF_SIMPLE_FS_DATA_FROM_THIS (PrivFileData->SimpleFs);
-
   if (!PrivFileData->IsRootDirectory) {
     CleanupFileInformation (&PrivFileData->File);
 
@@ -551,10 +548,6 @@ UdfClose (
     }
   }
 
-  if (--PrivFsData->OpenFiles == 0) {
-    CleanupVolumeInformation (&PrivFsData->Volume);
-  }
-
   FreePool ((VOID *)PrivFileData);
 
 Exit:
@@ -787,7 +780,7 @@ UdfGetInfo (
   } else if (CompareGuid (InformationType, &gEfiFileSystemInfoGuid)) {
     String = VolumeLabel;
 
-    FileSetDesc = PrivFsData->Volume.FileSetDescs[0];
+    FileSetDesc = &PrivFsData->Volume.FileSetDesc;
 
     OstaCompressed = &FileSetDesc->LogicalVolumeIdentifier[0];
 
@@ -846,7 +839,7 @@ UdfGetInfo (
     FileSystemInfo->Size        = FileSystemInfoLength;
     FileSystemInfo->ReadOnly    = TRUE;
     FileSystemInfo->BlockSize   =
-      LV_BLOCK_SIZE (&PrivFsData->Volume, UDF_DEFAULT_LV_NUM);
+      PrivFsData->Volume.LogicalVolDesc.LogicalBlockSize;
     FileSystemInfo->VolumeSize  = VolumeSize;
     FileSystemInfo->FreeSpace   = FreeSpaceSize;
 
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index 4609580b30..63b643e60a 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -60,154 +60,111 @@ FindAnchorVolumeDescriptorPointer (
 
 EFI_STATUS
 StartMainVolumeDescriptorSequence (
-  IN   EFI_BLOCK_IO_PROTOCOL                 *BlockIo,
-  IN   EFI_DISK_IO_PROTOCOL                  *DiskIo,
-  IN   UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  *AnchorPoint,
-  OUT  UDF_VOLUME_INFO                       *Volume
+  IN   EFI_BLOCK_IO_PROTOCOL    *BlockIo,
+  IN   EFI_DISK_IO_PROTOCOL     *DiskIo,
+  OUT  UDF_VOLUME_INFO          *Volume
   )
 {
-  EFI_STATUS                     Status;
-  UINT32                         BlockSize;
-  UDF_EXTENT_AD                  *ExtentAd;
-  UINT64                         StartingLsn;
-  UINT64                         EndingLsn;
-  VOID                           *Buffer;
-  UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc;
-  UDF_PARTITION_DESCRIPTOR       *PartitionDesc;
-  UINTN                          Index;
-  UINT32                         LogicalBlockSize;
+  EFI_STATUS  Status;
+  UINT32      BlockSize;
+  UINT64      BlockOffset;
+  VOID        *Buffer;
+  UINT32      LogicalBlockSize;
+
+  BlockSize = BlockIo->Media->BlockSize;
 
   //
-  // We've already found an ADVP on the volume. It contains the extent
-  // (MainVolumeDescriptorSequenceExtent) where the Main Volume Descriptor
-  // Sequence starts. Therefore, we'll look for Logical Volume Descriptors and
-  // Partitions Descriptors and save them in memory, accordingly.
-  //
-  // Note also that each descriptor will be aligned on a block size (BlockSize)
-  // boundary, so we need to read one block at a time.
+  // Allocate buffer for reading disk blocks
   //
-  BlockSize    = BlockIo->Media->BlockSize;
-  ExtentAd     = &AnchorPoint->MainVolumeDescriptorSequenceExtent;
-  StartingLsn  = (UINT64)ExtentAd->ExtentLocation;
-  EndingLsn    = StartingLsn + DivU64x32 (
-                                     (UINT64)ExtentAd->ExtentLength,
-                                     BlockSize
-                                     );
-
-  Volume->LogicalVolDescs =
-    (UDF_LOGICAL_VOLUME_DESCRIPTOR **)AllocateZeroPool (ExtentAd->ExtentLength);
-  if (Volume->LogicalVolDescs == NULL) {
-    return EFI_OUT_OF_RESOURCES;
-  }
-
-  Volume->PartitionDescs =
-    (UDF_PARTITION_DESCRIPTOR **)AllocateZeroPool (ExtentAd->ExtentLength);
-  if (Volume->PartitionDescs == NULL) {
-    Status = EFI_OUT_OF_RESOURCES;
-    goto Error_Alloc_Pds;
-  }
-
-  Buffer = AllocateZeroPool (BlockSize);
+  Buffer = AllocateZeroPool ((UINTN)BlockSize);
   if (Buffer == NULL) {
-    Status = EFI_OUT_OF_RESOURCES;
-    goto Error_Alloc_Buf;
+    return EFI_OUT_OF_RESOURCES;
   }
 
-  Volume->LogicalVolDescsNo  = 0;
-  Volume->PartitionDescsNo   = 0;
-
-  while (StartingLsn <= EndingLsn) {
+  //
+  // As per UDF 2.60 specification:
+  //
+  // ---
+  // For the purpose of interchange, Partition Maps shall be limited to
+  // Partition Map type 1, except type 2 maps.
+  // ---
+  // A Partition Descriptor Access Type of read-only, rewritable,
+  // overwritable, write-once and pseudo-overwritable shall be
+  // supported. There shall be exactly one prevailing Partition
+  // Descriptor recorded per volume, with one exception. For Volume
+  // Sets that consist of single volume, the volume may contain 2 non-
+  // overlapping Partitions with 2 prevailing Partition Descriptors only
+  // if one has an Access Type of read-only and the other has an
+  // Access Type of rewritable, overwritable, or write-once. The
+  // Logical Volume for this volume would consist of the contents of
+  // both partitions.
+  // ---
+  //
+  // The logical partition created by Partition driver is relative to the main
+  // VDS extent location, so we start the Main Volume Descriptor Sequence at
+  // LBA 0.
+  //
+  // We don't need to make sure we got both LVD and PD -- that is, if we are
+  // here, that means Partition driver was able to find them both previously.
+  //
+  for (BlockOffset = 0;
+       BlockOffset < MultU64x32 (BlockIo->Media->LastBlock,
+                                 BlockIo->Media->BlockSize);
+       BlockOffset += BlockSize) {
+    // Read disk block
+    //
     Status = DiskIo->ReadDisk (
       DiskIo,
       BlockIo->Media->MediaId,
-      MultU64x32 (StartingLsn, BlockSize),
+      BlockOffset,
       BlockSize,
       Buffer
       );
     if (EFI_ERROR (Status)) {
-      goto Error_Read_Disk_Blk;
+      goto Out_Free;
     }
 
+    //
+    // Check if read block is a Terminating Descriptor
+    //
     if (IS_TD (Buffer)) {
       //
-      // Found a Terminating Descriptor. Stop the sequence then.
+      // Terminate Main Volume Descriptor Sequence
       //
       break;
     }
 
     if (IS_LVD (Buffer)) {
       //
-      // Found a Logical Volume Descriptor.
+      // Save Logical Volume Descriptor
       //
-      LogicalVolDesc =
-        (UDF_LOGICAL_VOLUME_DESCRIPTOR *)
-        AllocateZeroPool (sizeof (UDF_LOGICAL_VOLUME_DESCRIPTOR));
-      if (LogicalVolDesc == NULL) {
-        Status = EFI_OUT_OF_RESOURCES;
-        goto Error_Alloc_Lvd;
-      }
-
-      CopyMem ((VOID *)LogicalVolDesc, Buffer,
-               sizeof (UDF_LOGICAL_VOLUME_DESCRIPTOR));
-      Volume->LogicalVolDescs[Volume->LogicalVolDescsNo++] = LogicalVolDesc;
+      CopyMem (&Volume->LogicalVolDesc, Buffer, sizeof (Volume->LogicalVolDesc));
     } else if (IS_PD (Buffer)) {
       //
-      // Found a Partition Descriptor.
+      // Save Partition Descriptor
       //
-      PartitionDesc =
-        (UDF_PARTITION_DESCRIPTOR *)
-        AllocateZeroPool (sizeof (UDF_PARTITION_DESCRIPTOR));
-      if (PartitionDesc == NULL) {
-        Status = EFI_OUT_OF_RESOURCES;
-        goto Error_Alloc_Pd;
-      }
-
-      CopyMem ((VOID *)PartitionDesc, Buffer,
-               sizeof (UDF_PARTITION_DESCRIPTOR));
-      Volume->PartitionDescs[Volume->PartitionDescsNo++] = PartitionDesc;
+      CopyMem (&Volume->PartitionDesc, Buffer, sizeof (Volume->PartitionDesc));
     }
-
-    StartingLsn++;
   }
 
   //
-  // When an UDF volume (revision 2.00 or higher) contains a File Entry rather
-  // than an Extended File Entry (which is not recommended as per spec), we need
-  // to make sure the size of a FE will be _at least_ 2048
-  // (UDF_LOGICAL_SECTOR_SIZE) bytes long to keep backward compatibility.
+  // Determine FE (File Entry) size
   //
-  LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
+  LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
   if (LogicalBlockSize >= UDF_LOGICAL_SECTOR_SIZE) {
-    Volume->FileEntrySize = LogicalBlockSize;
+    Volume->FileEntrySize = (UINTN)LogicalBlockSize;
   } else {
     Volume->FileEntrySize = UDF_LOGICAL_SECTOR_SIZE;
   }
 
-  FreePool (Buffer);
-
-  return EFI_SUCCESS;
-
-Error_Alloc_Pd:
-Error_Alloc_Lvd:
-  for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
-    FreePool ((VOID *)Volume->PartitionDescs[Index]);
-  }
-
-  for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
-    FreePool ((VOID *)Volume->LogicalVolDescs[Index]);
-  }
+  Status = EFI_SUCCESS;
 
-Error_Read_Disk_Blk:
+Out_Free:
+  //
+  // Free block read buffer
+  //
   FreePool (Buffer);
 
-Error_Alloc_Buf:
-  FreePool ((VOID *)Volume->PartitionDescs);
-  Volume->PartitionDescs = NULL;
-
-Error_Alloc_Pds:
-  FreePool ((VOID *)Volume->LogicalVolDescs);
-  Volume->LogicalVolDescs = NULL;
-
   return Status;
 }
 
@@ -223,11 +180,9 @@ GetPdFromLongAd (
   )
 {
   UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc;
-  UINTN                          Index;
-  UDF_PARTITION_DESCRIPTOR       *PartitionDesc;
   UINT16                         PartitionNum;
 
-  LogicalVolDesc = Volume->LogicalVolDescs[UDF_DEFAULT_LV_NUM];
+  LogicalVolDesc = &Volume->LogicalVolDesc;
 
   switch (LV_UDF_REVISION (LogicalVolDesc)) {
   case 0x0102:
@@ -252,19 +207,21 @@ GetPdFromLongAd (
     PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc->PartitionMaps[4]);
     break;
   case 0x0260:
-    //
-    // Fall through.
-    //
-  default:
     PartitionNum = LongAd->ExtentLocation.PartitionReferenceNumber;
     break;
+  default:
+    //
+    // Unhandled UDF revision
+    //
+    return NULL;
   }
 
-  for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
-    PartitionDesc = Volume->PartitionDescs[Index];
-    if (PartitionDesc->PartitionNumber == PartitionNum) {
-      return PartitionDesc;
-    }
+  //
+  // Check if partition number matches Partition Descriptor found in Main Volume
+  // Descriptor Sequence.
+  //
+  if (Volume->PartitionDesc.PartitionNumber == PartitionNum) {
+    return &Volume->PartitionDesc;
   }
 
   return NULL;
@@ -284,8 +241,9 @@ GetLongAdLsn (
   PartitionDesc = GetPdFromLongAd (Volume, LongAd);
   ASSERT (PartitionDesc != NULL);
 
-  return (UINT64)PartitionDesc->PartitionStartingLocation +
-                 LongAd->ExtentLocation.LogicalBlockNumber;
+  return (UINT64)PartitionDesc->PartitionStartingLocation -
+    Volume->MainVdsStartLocation +
+    LongAd->ExtentLocation.LogicalBlockNumber;
 }
 
 //
@@ -293,12 +251,13 @@ GetLongAdLsn (
 //
 UINT64
 GetShortAdLsn (
+  IN UDF_VOLUME_INFO                  *Volume,
   IN UDF_PARTITION_DESCRIPTOR         *PartitionDesc,
   IN UDF_SHORT_ALLOCATION_DESCRIPTOR  *ShortAd
   )
 {
-  return (UINT64)PartitionDesc->PartitionStartingLocation +
-    ShortAd->ExtentPosition;
+  return (UINT64)PartitionDesc->PartitionStartingLocation -
+    Volume->MainVdsStartLocation + ShortAd->ExtentPosition;
 }
 
 //
@@ -311,36 +270,39 @@ EFI_STATUS
 FindFileSetDescriptor (
   IN   EFI_BLOCK_IO_PROTOCOL    *BlockIo,
   IN   EFI_DISK_IO_PROTOCOL     *DiskIo,
-  IN   UDF_VOLUME_INFO          *Volume,
-  IN   UINTN                    LogicalVolDescNum,
-  OUT  UDF_FILE_SET_DESCRIPTOR  *FileSetDesc
+  IN   UDF_VOLUME_INFO          *Volume
   )
 {
   EFI_STATUS                     Status;
   UINT64                         Lsn;
   UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc;
 
-  LogicalVolDesc = Volume->LogicalVolDescs[LogicalVolDescNum];
+  LogicalVolDesc = &Volume->LogicalVolDesc;
   Lsn = GetLongAdLsn (Volume, &LogicalVolDesc->LogicalVolumeContentsUse);
 
   //
-  // Read extent (Long Ad).
+  // As per UDF 2.60 specification:
+  //
+  // There shall be exactly one File Set Descriptor recorded per Logical
+  // Volume.
+  //
+  // Read disk block
   //
   Status = DiskIo->ReadDisk (
     DiskIo,
     BlockIo->Media->MediaId,
     MultU64x32 (Lsn, LogicalVolDesc->LogicalBlockSize),
-    sizeof (UDF_FILE_SET_DESCRIPTOR),
-    (VOID *)FileSetDesc
+    sizeof (Volume->FileSetDesc),
+    &Volume->FileSetDesc
     );
   if (EFI_ERROR (Status)) {
     return Status;
   }
 
   //
-  // Check if the read extent contains a valid FSD's tag identifier.
+  // Check if read block is a File Set Descriptor
   //
-  if (!IS_FSD (FileSetDesc)) {
+  if (!IS_FSD (&Volume->FileSetDesc)) {
     return EFI_VOLUME_CORRUPTED;
   }
 
@@ -348,71 +310,6 @@ FindFileSetDescriptor (
 }
 
 //
-// Get all File Set Descriptors for each Logical Volume Descriptor.
-//
-EFI_STATUS
-GetFileSetDescriptors (
-  IN      EFI_BLOCK_IO_PROTOCOL  *BlockIo,
-  IN      EFI_DISK_IO_PROTOCOL   *DiskIo,
-  IN OUT  UDF_VOLUME_INFO        *Volume
-  )
-{
-  EFI_STATUS               Status;
-  UINTN                    Index;
-  UDF_FILE_SET_DESCRIPTOR  *FileSetDesc;
-  UINTN                    Count;
-
-  Volume->FileSetDescs =
-    (UDF_FILE_SET_DESCRIPTOR **)AllocateZeroPool (
-      Volume->LogicalVolDescsNo * sizeof (UDF_FILE_SET_DESCRIPTOR));
-  if (Volume->FileSetDescs == NULL) {
-    return EFI_OUT_OF_RESOURCES;
-  }
-
-  for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
-    FileSetDesc = AllocateZeroPool (sizeof (UDF_FILE_SET_DESCRIPTOR));
-    if (FileSetDesc == NULL) {
-      Status = EFI_OUT_OF_RESOURCES;
-      goto Error_Alloc_Fsd;
-    }
-
-    //
-    // Find a FSD for this LVD.
-    //
-    Status = FindFileSetDescriptor (
-      BlockIo,
-      DiskIo,
-      Volume,
-      Index,
-      FileSetDesc
-      );
-    if (EFI_ERROR (Status)) {
-      goto Error_Find_Fsd;
-    }
-
-    //
-    // Got one. Save it.
-    //
-    Volume->FileSetDescs[Index] = FileSetDesc;
-  }
-
-  Volume->FileSetDescsNo = Volume->LogicalVolDescsNo;
-  return EFI_SUCCESS;
-
-Error_Find_Fsd:
-  Count = Index + 1;
-  for (Index = 0; Index < Count; Index++) {
-    FreePool ((VOID *)Volume->FileSetDescs[Index]);
-  }
-
-  FreePool ((VOID *)Volume->FileSetDescs);
-  Volume->FileSetDescs = NULL;
-
-Error_Alloc_Fsd:
-  return Status;
-}
-
-//
 // Read Volume and File Structure on an UDF file system.
 //
 EFI_STATUS
@@ -424,9 +321,10 @@ ReadVolumeFileStructure (
 {
   EFI_STATUS                            Status;
   UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  AnchorPoint;
+  UDF_EXTENT_AD                         *ExtentAd;
 
   //
-  // Find an AVDP.
+  // Find Anchor Volume Descriptor Pointer
   //
   Status = FindAnchorVolumeDescriptorPointer (
     BlockIo,
@@ -438,12 +336,18 @@ ReadVolumeFileStructure (
   }
 
   //
-  // AVDP has been found. Start MVDS.
+  // Save Main VDS start block number
+  //
+  ExtentAd = &AnchorPoint.MainVolumeDescriptorSequenceExtent;
+
+  Volume->MainVdsStartLocation = (UINT64)ExtentAd->ExtentLocation;
+
+  //
+  // Start Main Volume Descriptor Sequence.
   //
   Status = StartMainVolumeDescriptorSequence (
     BlockIo,
     DiskIo,
-    &AnchorPoint,
     Volume
     );
   if (EFI_ERROR (Status)) {
@@ -699,6 +603,7 @@ GetAllocationDescriptorLsn (
     return GetLongAdLsn (Volume, (UDF_LONG_ALLOCATION_DESCRIPTOR *)Ad);
   } else if (RecordingFlags == SHORT_ADS_SEQUENCE) {
     return GetShortAdLsn (
+      Volume,
       GetPdFromLongAd (Volume, ParentIcb),
       (UDF_SHORT_ALLOCATION_DESCRIPTOR *)Ad
       );
@@ -740,7 +645,7 @@ GetAedAdsOffset (
     return EFI_OUT_OF_RESOURCES;
   }
 
-  LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
+  LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
 
   //
   // Read extent.
@@ -890,7 +795,7 @@ ReadFile (
   UINT32                  ExtentLength;
   UDF_FE_RECORDING_FLAGS  RecordingFlags;
 
-  LogicalBlockSize  = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
+  LogicalBlockSize  = Volume->LogicalVolDesc.LogicalBlockSize;
   DoFreeAed         = FALSE;
 
   //
@@ -1355,6 +1260,9 @@ ReadUdfVolumeInformation (
 {
   EFI_STATUS Status;
 
+  //
+  // Read all necessary UDF volume information and keep it private to the driver
+  //
   Status = ReadVolumeFileStructure (
     BlockIo,
     DiskIo,
@@ -1364,13 +1272,12 @@ ReadUdfVolumeInformation (
     return Status;
   }
 
-  Status = GetFileSetDescriptors (
-    BlockIo,
-    DiskIo,
-    Volume
-    );
+  //
+  // Find File Set Descriptor
+  //
+  Status = FindFileSetDescriptor (BlockIo, DiskIo, Volume);
   if (EFI_ERROR (Status)) {
-    CleanupVolumeInformation (Volume);
+    return Status;
   }
 
   return Status;
@@ -1407,7 +1314,7 @@ FindRootDirectory (
     BlockIo,
     DiskIo,
     Volume,
-    &Volume->FileSetDescs[0]->RootDirectoryIcb,
+    &Volume->FileSetDesc.RootDirectoryIcb,
     &File->FileEntry
     );
   if (EFI_ERROR (Status)) {
@@ -1424,7 +1331,7 @@ FindRootDirectory (
     L"\\",
     NULL,
     &Parent,
-    &Volume->FileSetDescs[0]->RootDirectoryIcb,
+    &Volume->FileSetDesc.RootDirectoryIcb,
     File
     );
   if (EFI_ERROR (Status)) {
@@ -1465,7 +1372,7 @@ FindFileEntry (
   UINT32      LogicalBlockSize;
 
   Lsn               = GetLongAdLsn (Volume, Icb);
-  LogicalBlockSize  = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
+  LogicalBlockSize  = Volume->LogicalVolDesc.LogicalBlockSize;
 
   *FileEntry = AllocateZeroPool (Volume->FileEntrySize);
   if (*FileEntry == NULL) {
@@ -1959,43 +1866,6 @@ Error_Find_File:
 }
 
 /**
-  Clean up in-memory UDF volume information.
-
-  @param[in] Volume Volume information pointer.
-
-**/
-VOID
-CleanupVolumeInformation (
-  IN UDF_VOLUME_INFO *Volume
-  )
-{
-  UINTN Index;
-
-  if (Volume->LogicalVolDescs != NULL) {
-    for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
-      FreePool ((VOID *)Volume->LogicalVolDescs[Index]);
-    }
-    FreePool ((VOID *)Volume->LogicalVolDescs);
-  }
-
-  if (Volume->PartitionDescs != NULL) {
-    for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
-      FreePool ((VOID *)Volume->PartitionDescs[Index]);
-    }
-    FreePool ((VOID *)Volume->PartitionDescs);
-  }
-
-  if (Volume->FileSetDescs != NULL) {
-    for (Index = 0; Index < Volume->FileSetDescsNo; Index++) {
-      FreePool ((VOID *)Volume->FileSetDescs[Index]);
-    }
-    FreePool ((VOID *)Volume->FileSetDescs);
-  }
-
-  ZeroMem ((VOID *)Volume, sizeof (UDF_VOLUME_INFO));
-}
-
-/**
   Clean up in-memory UDF file information.
 
   @param[in] File File information pointer.
@@ -2249,91 +2119,100 @@ GetVolumeSize (
   OUT  UINT64                 *FreeSpaceSize
   )
 {
-  UDF_EXTENT_AD                 ExtentAd;
-  UINT32                        LogicalBlockSize;
-  UINT64                        Lsn;
-  EFI_STATUS                    Status;
-  UDF_LOGICAL_VOLUME_INTEGRITY  *LogicalVolInt;
-  UINTN                         Index;
-  UINTN                         Length;
-  UINT32                        LsnsNo;
-
-  *VolumeSize     = 0;
-  *FreeSpaceSize  = 0;
-
-  for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
-    CopyMem ((VOID *)&ExtentAd,
-             (VOID *)&Volume->LogicalVolDescs[Index]->IntegritySequenceExtent,
-             sizeof (UDF_EXTENT_AD));
-    if (ExtentAd.ExtentLength == 0) {
-      continue;
-    }
+  EFI_STATUS                     Status;
+  UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc;
+  UDF_EXTENT_AD                  *ExtentAd;
+  UINT64                         Lsn;
+  UINT32                         LogicalBlockSize;
+  UDF_LOGICAL_VOLUME_INTEGRITY   *LogicalVolInt;
+  UINTN                          Index;
+  UINTN                          Length;
+  UINT32                         LsnsNo;
 
-    LogicalBlockSize = LV_BLOCK_SIZE (Volume, Index);
+  LogicalVolDesc = &Volume->LogicalVolDesc;
 
-  Read_Next_Sequence:
-    LogicalVolInt = (UDF_LOGICAL_VOLUME_INTEGRITY *)
-      AllocatePool (ExtentAd.ExtentLength);
-    if (LogicalVolInt == NULL) {
-      return EFI_OUT_OF_RESOURCES;
-    }
+  ExtentAd = &LogicalVolDesc->IntegritySequenceExtent;
 
-    Lsn = (UINT64)ExtentAd.ExtentLocation;
+  if (ExtentAd->ExtentLength == 0) {
+    return EFI_VOLUME_CORRUPTED;
+  }
 
-    Status = DiskIo->ReadDisk (
-      DiskIo,
-      BlockIo->Media->MediaId,
-      MultU64x32 (Lsn, LogicalBlockSize),
-      ExtentAd.ExtentLength,
-      (VOID *)LogicalVolInt
-      );
-    if (EFI_ERROR (Status)) {
-      FreePool ((VOID *)LogicalVolInt);
-      return Status;
-    }
+  LogicalVolInt = AllocatePool (ExtentAd->ExtentLength);
+  if (LogicalVolInt == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
 
-    if (!IS_LVID (LogicalVolInt)) {
-      FreePool ((VOID *)LogicalVolInt);
-      return EFI_VOLUME_CORRUPTED;
-    }
+  //
+  // Get location of Logical Volume Integrity Descriptor
+  //
+  Lsn = (UINT64)ExtentAd->ExtentLocation - Volume->MainVdsStartLocation;
 
-    Length = LogicalVolInt->NumberOfPartitions;
-    for (Index = 0; Index < Length; Index += sizeof (UINT32)) {
-      LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
-      if (LsnsNo == 0xFFFFFFFFUL) {
-        //
-        // Size not specified.
-        //
-        continue;
-      }
+  LogicalBlockSize = LogicalVolDesc->LogicalBlockSize;
 
-      *FreeSpaceSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
-    }
+  //
+  // Read disk block
+  //
+  Status = DiskIo->ReadDisk (
+    DiskIo,
+    BlockIo->Media->MediaId,
+    MultU64x32 (Lsn, LogicalBlockSize),
+    ExtentAd->ExtentLength,
+    LogicalVolInt
+    );
+  if (EFI_ERROR (Status)) {
+    goto Out_Free;
+  }
 
-    Length = (LogicalVolInt->NumberOfPartitions * sizeof (UINT32)) << 1;
-    for (; Index < Length; Index += sizeof (UINT32)) {
-      LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
-      if (LsnsNo == 0xFFFFFFFFUL) {
-        //
-        // Size not specified.
-        //
-        continue;
-      }
+  //
+  // Check if read block is a Logical Volume Integrity Descriptor
+  //
+  if (!IS_LVID (LogicalVolInt)) {
+    Status = EFI_VOLUME_CORRUPTED;
+    goto Out_Free;
+  }
 
-      *VolumeSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
-    }
+  *VolumeSize = 0;
+  *FreeSpaceSize = 0;
 
-    CopyMem ((VOID *)&ExtentAd,(VOID *)&LogicalVolInt->NextIntegrityExtent,
-             sizeof (UDF_EXTENT_AD));
-    if (ExtentAd.ExtentLength > 0) {
-      FreePool ((VOID *)LogicalVolInt);
-      goto Read_Next_Sequence;
+  Length = LogicalVolInt->NumberOfPartitions;
+  for (Index = 0; Index < Length; Index += sizeof (UINT32)) {
+    LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
+    //
+    // Check if size is not specified
+    //
+    if (LsnsNo == 0xFFFFFFFFUL) {
+      continue;
     }
+    //
+    // Accumulate free space size
+    //
+    *FreeSpaceSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
+  }
 
-    FreePool ((VOID *)LogicalVolInt);
+  Length = LogicalVolInt->NumberOfPartitions * sizeof (UINT32) * 2;
+  for (; Index < Length; Index += sizeof (UINT32)) {
+    LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
+    //
+    // Check if size is not specified
+    //
+    if (LsnsNo == 0xFFFFFFFFUL) {
+      continue;
+    }
+    //
+    // Accumulate used volume space
+    //
+    *VolumeSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
   }
 
-  return EFI_SUCCESS;
+  Status = EFI_SUCCESS;
+
+Out_Free:
+  //
+  // Free Logical Volume Integrity Descriptor
+  //
+  FreePool (LogicalVolInt);
+
+  return Status;
 }
 
 /**
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
index 49dc7077b7..d4163b89ca 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
@@ -276,13 +276,6 @@ UdfDriverBindingStop (
       NULL
       );
 
-    //
-    // Check if there's any open file. If so, clean them up.
-    //
-    if (PrivFsData->OpenFiles > 0) {
-      CleanupVolumeInformation (&PrivFsData->Volume);
-    }
-
     FreePool ((VOID *)PrivFsData);
   }
 
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
index 240d420ff5..c5f83914d8 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
@@ -49,16 +49,8 @@
     { 0x89, 0x56, 0x73, 0xCD, 0xA3, 0x26, 0xCD, 0x0A }  \
   }
 
-#define UDF_DEFAULT_LV_NUM 0
-
 #define IS_PVD(_Pointer) \
   ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 1))
-#define IS_PD(_Pointer) \
-  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5))
-#define IS_LVD(_Pointer) \
-  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6))
-#define IS_TD(_Pointer) \
-  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
 #define IS_FSD(_Pointer) \
   ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 256))
 #define IS_FE(_Pointer) \
@@ -152,14 +144,8 @@ typedef enum {
 #define IS_VALID_COMPRESSION_ID(_CompId) \
   ((BOOLEAN)((_CompId) == 8 || (_CompId) == 16))
 
-#define LV_BLOCK_SIZE(_Vol, _LvNum) \
-  (_Vol)->LogicalVolDescs[(_LvNum)]->LogicalBlockSize
-
 #define UDF_STANDARD_IDENTIFIER_LENGTH   5
 
-#define LV_UDF_REVISION(_Lv) \
-  *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix
-
 #pragma pack(1)
 
 typedef struct {
@@ -186,17 +172,6 @@ typedef struct {
 #pragma pack(1)
 
 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 {
   UINT16          TypeAndTimezone;
   INT16           Year;
   UINT8           Month;
@@ -210,17 +185,6 @@ typedef struct {
 } UDF_TIMESTAMP;
 
 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;
   UINT32                             PrevAllocationExtentDescriptor;
   UINT32                             LengthOfAllocationDescriptors;
@@ -235,37 +199,6 @@ typedef struct {
 } UDF_VOLUME_DESCRIPTOR;
 
 typedef struct {
-  UDF_DESCRIPTOR_TAG         DescriptorTag;
-  UINT32                     VolumeDescriptorSequenceNumber;
-  UINT16                     PartitionFlags;
-  UINT16                     PartitionNumber;
-  UDF_ENTITY_ID              PartitionContents;
-  UINT8                      PartitionContentsUse[128];
-  UINT32                     AccessType;
-  UINT32                     PartitionStartingLocation;
-  UINT32                     PartitionLength;
-  UDF_ENTITY_ID              ImplementationIdentifier;
-  UINT8                      ImplementationUse[128];
-  UINT8                      Reserved[156];
-} UDF_PARTITION_DESCRIPTOR;
-
-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;
-
-typedef struct {
   UDF_DESCRIPTOR_TAG             DescriptorTag;
   UDF_TIMESTAMP                  RecordingDateTime;
   UINT32                         IntegrityType;
@@ -389,12 +322,10 @@ typedef struct {
 // UDF filesystem driver's private data
 //
 typedef struct {
-  UDF_LOGICAL_VOLUME_DESCRIPTOR  **LogicalVolDescs;
-  UINTN                          LogicalVolDescsNo;
-  UDF_PARTITION_DESCRIPTOR       **PartitionDescs;
-  UINTN                          PartitionDescsNo;
-  UDF_FILE_SET_DESCRIPTOR        **FileSetDescs;
-  UINTN                          FileSetDescsNo;
+  UINT64                         MainVdsStartLocation;
+  UDF_LOGICAL_VOLUME_DESCRIPTOR  LogicalVolDesc;
+  UDF_PARTITION_DESCRIPTOR       PartitionDesc;
+  UDF_FILE_SET_DESCRIPTOR        FileSetDesc;
   UINTN                          FileEntrySize;
 } UDF_VOLUME_INFO;
 
@@ -883,17 +814,6 @@ ResolveSymlink (
   );
 
 /**
-  Clean up in-memory UDF volume information.
-
-  @param[in] Volume Volume information pointer.
-
-**/
-VOID
-CleanupVolumeInformation (
-  IN UDF_VOLUME_INFO *Volume
-  );
-
-/**
   Clean up in-memory UDF file information.
 
   @param[in] File File information pointer.
-- 
2.11.0

Re: [PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to support PartitionDxe changes

[Ni, Ruiyu]

Paulo,
With the change in partition driver, I suppose UdfDxe driver
only needs to take care of area covered by the partition descriptor.
But why StartMainVolumeDescriptorSequence() still reads
LVD, TD, and PD?

I thought the UdfDxe driver's logic can be simplified a lot.
There should be no duplicated logic in Partition driver and udf driver.

Can you explain more?

Thanks/Ray

> -----Original Message-----
> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
> Sent: Sunday, September 17, 2017 9:13 PM
> To: edk2-devel@lists.01.org
> Cc: Paulo Alcantara <pcacjr@zytor.com>; Dong, Eric <eric.dong@intel.com>;
> Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Laszlo
> Ersek <lersek@redhat.com>
> Subject: [PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to support
> PartitionDxe changes
> 
> This patch reworks the driver to support Partition driver changes.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Paulo Alcantara <pcacjr@zytor.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
> ---
>  MdeModulePkg/Universal/Disk/UdfDxe/File.c                 |  13 +-
>  MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 515
> ++++++++------------
>  MdeModulePkg/Universal/Disk/UdfDxe/Udf.c                  |   7 -
>  MdeModulePkg/Universal/Disk/UdfDxe/Udf.h                  |  88 +---
>  4 files changed, 204 insertions(+), 419 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> index 01361141bb..f2c62967e8 100644
> --- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> @@ -100,6 +100,7 @@ UdfOpenVolume (
>      &PrivFsData->Volume,
>      &PrivFsData->Root
>      );
> +  ASSERT_EFI_ERROR (Status);
>    if (EFI_ERROR (Status)) {
>      goto Error_Find_Root_Dir;
>    }
> @@ -131,7 +132,6 @@ Error_Alloc_Priv_File_Data:
>    CleanupFileInformation (&PrivFsData->Root);
> 
>  Error_Find_Root_Dir:
> -  CleanupVolumeInformation (&PrivFsData->Volume);
> 
>  Error_Read_Udf_Volume:
>  Error_Invalid_Params:
> @@ -528,7 +528,6 @@ UdfClose (
>    EFI_TPL                     OldTpl;
>    EFI_STATUS                  Status;
>    PRIVATE_UDF_FILE_DATA       *PrivFileData;
> -  PRIVATE_UDF_SIMPLE_FS_DATA  *PrivFsData;
> 
>    OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> 
> @@ -541,8 +540,6 @@ UdfClose (
> 
>    PrivFileData = PRIVATE_UDF_FILE_DATA_FROM_THIS (This);
> 
> -  PrivFsData = PRIVATE_UDF_SIMPLE_FS_DATA_FROM_THIS (PrivFileData-
> >SimpleFs);
> -
>    if (!PrivFileData->IsRootDirectory) {
>      CleanupFileInformation (&PrivFileData->File);
> 
> @@ -551,10 +548,6 @@ UdfClose (
>      }
>    }
> 
> -  if (--PrivFsData->OpenFiles == 0) {
> -    CleanupVolumeInformation (&PrivFsData->Volume);
> -  }
> -
>    FreePool ((VOID *)PrivFileData);
> 
>  Exit:
> @@ -787,7 +780,7 @@ UdfGetInfo (
>    } else if (CompareGuid (InformationType, &gEfiFileSystemInfoGuid)) {
>      String = VolumeLabel;
> 
> -    FileSetDesc = PrivFsData->Volume.FileSetDescs[0];
> +    FileSetDesc = &PrivFsData->Volume.FileSetDesc;
> 
>      OstaCompressed = &FileSetDesc->LogicalVolumeIdentifier[0];
> 
> @@ -846,7 +839,7 @@ UdfGetInfo (
>      FileSystemInfo->Size        = FileSystemInfoLength;
>      FileSystemInfo->ReadOnly    = TRUE;
>      FileSystemInfo->BlockSize   =
> -      LV_BLOCK_SIZE (&PrivFsData->Volume, UDF_DEFAULT_LV_NUM);
> +      PrivFsData->Volume.LogicalVolDesc.LogicalBlockSize;
>      FileSystemInfo->VolumeSize  = VolumeSize;
>      FileSystemInfo->FreeSpace   = FreeSpaceSize;
> 
> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> index 4609580b30..63b643e60a 100644
> --- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> @@ -60,154 +60,111 @@ FindAnchorVolumeDescriptorPointer (
> 
>  EFI_STATUS
>  StartMainVolumeDescriptorSequence (
> -  IN   EFI_BLOCK_IO_PROTOCOL                 *BlockIo,
> -  IN   EFI_DISK_IO_PROTOCOL                  *DiskIo,
> -  IN   UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  *AnchorPoint,
> -  OUT  UDF_VOLUME_INFO                       *Volume
> +  IN   EFI_BLOCK_IO_PROTOCOL    *BlockIo,
> +  IN   EFI_DISK_IO_PROTOCOL     *DiskIo,
> +  OUT  UDF_VOLUME_INFO          *Volume
>    )
>  {
> -  EFI_STATUS                     Status;
> -  UINT32                         BlockSize;
> -  UDF_EXTENT_AD                  *ExtentAd;
> -  UINT64                         StartingLsn;
> -  UINT64                         EndingLsn;
> -  VOID                           *Buffer;
> -  UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc;
> -  UDF_PARTITION_DESCRIPTOR       *PartitionDesc;
> -  UINTN                          Index;
> -  UINT32                         LogicalBlockSize;
> +  EFI_STATUS  Status;
> +  UINT32      BlockSize;
> +  UINT64      BlockOffset;
> +  VOID        *Buffer;
> +  UINT32      LogicalBlockSize;
> +
> +  BlockSize = BlockIo->Media->BlockSize;
> 
>    //
> -  // We've already found an ADVP on the volume. It contains the extent
> -  // (MainVolumeDescriptorSequenceExtent) where the Main Volume
> Descriptor
> -  // Sequence starts. Therefore, we'll look for Logical Volume Descriptors and
> -  // Partitions Descriptors and save them in memory, accordingly.
> -  //
> -  // Note also that each descriptor will be aligned on a block size (BlockSize)
> -  // boundary, so we need to read one block at a time.
> +  // Allocate buffer for reading disk blocks
>    //
> -  BlockSize    = BlockIo->Media->BlockSize;
> -  ExtentAd     = &AnchorPoint->MainVolumeDescriptorSequenceExtent;
> -  StartingLsn  = (UINT64)ExtentAd->ExtentLocation;
> -  EndingLsn    = StartingLsn + DivU64x32 (
> -                                     (UINT64)ExtentAd->ExtentLength,
> -                                     BlockSize
> -                                     );
> -
> -  Volume->LogicalVolDescs =
> -    (UDF_LOGICAL_VOLUME_DESCRIPTOR **)AllocateZeroPool (ExtentAd-
> >ExtentLength);
> -  if (Volume->LogicalVolDescs == NULL) {
> -    return EFI_OUT_OF_RESOURCES;
> -  }
> -
> -  Volume->PartitionDescs =
> -    (UDF_PARTITION_DESCRIPTOR **)AllocateZeroPool (ExtentAd-
> >ExtentLength);
> -  if (Volume->PartitionDescs == NULL) {
> -    Status = EFI_OUT_OF_RESOURCES;
> -    goto Error_Alloc_Pds;
> -  }
> -
> -  Buffer = AllocateZeroPool (BlockSize);
> +  Buffer = AllocateZeroPool ((UINTN)BlockSize);
>    if (Buffer == NULL) {
> -    Status = EFI_OUT_OF_RESOURCES;
> -    goto Error_Alloc_Buf;
> +    return EFI_OUT_OF_RESOURCES;
>    }
> 
> -  Volume->LogicalVolDescsNo  = 0;
> -  Volume->PartitionDescsNo   = 0;
> -
> -  while (StartingLsn <= EndingLsn) {
> +  //
> +  // As per UDF 2.60 specification:
> +  //
> +  // ---
> +  // For the purpose of interchange, Partition Maps shall be limited to
> + // Partition Map type 1, except type 2 maps.
> +  // ---
> +  // A Partition Descriptor Access Type of read-only, rewritable,  //
> + overwritable, write-once and pseudo-overwritable shall be  //
> + supported. There shall be exactly one prevailing Partition  //
> + Descriptor recorded per volume, with one exception. For Volume  //
> + Sets that consist of single volume, the volume may contain 2 non-  //
> + overlapping Partitions with 2 prevailing Partition Descriptors only
> + // if one has an Access Type of read-only and the other has an  //
> + Access Type of rewritable, overwritable, or write-once. The  //
> + Logical Volume for this volume would consist of the contents of  //
> + both partitions.
> +  // ---
> +  //
> +  // The logical partition created by Partition driver is relative to
> + the main  // VDS extent location, so we start the Main Volume
> + Descriptor Sequence at  // LBA 0.
> +  //
> +  // We don't need to make sure we got both LVD and PD -- that is, if
> + we are  // here, that means Partition driver was able to find them both
> previously.
> +  //
> +  for (BlockOffset = 0;
> +       BlockOffset < MultU64x32 (BlockIo->Media->LastBlock,
> +                                 BlockIo->Media->BlockSize);
> +       BlockOffset += BlockSize) {
> +    // Read disk block
> +    //
>      Status = DiskIo->ReadDisk (
>        DiskIo,
>        BlockIo->Media->MediaId,
> -      MultU64x32 (StartingLsn, BlockSize),
> +      BlockOffset,
>        BlockSize,
>        Buffer
>        );
>      if (EFI_ERROR (Status)) {
> -      goto Error_Read_Disk_Blk;
> +      goto Out_Free;
>      }
> 
> +    //
> +    // Check if read block is a Terminating Descriptor
> +    //
>      if (IS_TD (Buffer)) {
>        //
> -      // Found a Terminating Descriptor. Stop the sequence then.
> +      // Terminate Main Volume Descriptor Sequence
>        //
>        break;
>      }
> 
>      if (IS_LVD (Buffer)) {
>        //
> -      // Found a Logical Volume Descriptor.
> +      // Save Logical Volume Descriptor
>        //
> -      LogicalVolDesc =
> -        (UDF_LOGICAL_VOLUME_DESCRIPTOR *)
> -        AllocateZeroPool (sizeof (UDF_LOGICAL_VOLUME_DESCRIPTOR));
> -      if (LogicalVolDesc == NULL) {
> -        Status = EFI_OUT_OF_RESOURCES;
> -        goto Error_Alloc_Lvd;
> -      }
> -
> -      CopyMem ((VOID *)LogicalVolDesc, Buffer,
> -               sizeof (UDF_LOGICAL_VOLUME_DESCRIPTOR));
> -      Volume->LogicalVolDescs[Volume->LogicalVolDescsNo++] =
> LogicalVolDesc;
> +      CopyMem (&Volume->LogicalVolDesc, Buffer, sizeof
> + (Volume->LogicalVolDesc));
>      } else if (IS_PD (Buffer)) {
>        //
> -      // Found a Partition Descriptor.
> +      // Save Partition Descriptor
>        //
> -      PartitionDesc =
> -        (UDF_PARTITION_DESCRIPTOR *)
> -        AllocateZeroPool (sizeof (UDF_PARTITION_DESCRIPTOR));
> -      if (PartitionDesc == NULL) {
> -        Status = EFI_OUT_OF_RESOURCES;
> -        goto Error_Alloc_Pd;
> -      }
> -
> -      CopyMem ((VOID *)PartitionDesc, Buffer,
> -               sizeof (UDF_PARTITION_DESCRIPTOR));
> -      Volume->PartitionDescs[Volume->PartitionDescsNo++] = PartitionDesc;
> +      CopyMem (&Volume->PartitionDesc, Buffer, sizeof
> + (Volume->PartitionDesc));
>      }
> -
> -    StartingLsn++;
>    }
> 
>    //
> -  // When an UDF volume (revision 2.00 or higher) contains a File Entry rather
> -  // than an Extended File Entry (which is not recommended as per spec), we
> need
> -  // to make sure the size of a FE will be _at least_ 2048
> -  // (UDF_LOGICAL_SECTOR_SIZE) bytes long to keep backward compatibility.
> +  // Determine FE (File Entry) size
>    //
> -  LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
> +  LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
>    if (LogicalBlockSize >= UDF_LOGICAL_SECTOR_SIZE) {
> -    Volume->FileEntrySize = LogicalBlockSize;
> +    Volume->FileEntrySize = (UINTN)LogicalBlockSize;
>    } else {
>      Volume->FileEntrySize = UDF_LOGICAL_SECTOR_SIZE;
>    }
> 
> -  FreePool (Buffer);
> -
> -  return EFI_SUCCESS;
> -
> -Error_Alloc_Pd:
> -Error_Alloc_Lvd:
> -  for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
> -    FreePool ((VOID *)Volume->PartitionDescs[Index]);
> -  }
> -
> -  for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
> -    FreePool ((VOID *)Volume->LogicalVolDescs[Index]);
> -  }
> +  Status = EFI_SUCCESS;
> 
> -Error_Read_Disk_Blk:
> +Out_Free:
> +  //
> +  // Free block read buffer
> +  //
>    FreePool (Buffer);
> 
> -Error_Alloc_Buf:
> -  FreePool ((VOID *)Volume->PartitionDescs);
> -  Volume->PartitionDescs = NULL;
> -
> -Error_Alloc_Pds:
> -  FreePool ((VOID *)Volume->LogicalVolDescs);
> -  Volume->LogicalVolDescs = NULL;
> -
>    return Status;
>  }
> 
> @@ -223,11 +180,9 @@ GetPdFromLongAd (
>    )
>  {
>    UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc;
> -  UINTN                          Index;
> -  UDF_PARTITION_DESCRIPTOR       *PartitionDesc;
>    UINT16                         PartitionNum;
> 
> -  LogicalVolDesc = Volume->LogicalVolDescs[UDF_DEFAULT_LV_NUM];
> +  LogicalVolDesc = &Volume->LogicalVolDesc;
> 
>    switch (LV_UDF_REVISION (LogicalVolDesc)) {
>    case 0x0102:
> @@ -252,19 +207,21 @@ GetPdFromLongAd (
>      PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc->PartitionMaps[4]);
>      break;
>    case 0x0260:
> -    //
> -    // Fall through.
> -    //
> -  default:
>      PartitionNum = LongAd->ExtentLocation.PartitionReferenceNumber;
>      break;
> +  default:
> +    //
> +    // Unhandled UDF revision
> +    //
> +    return NULL;
>    }
> 
> -  for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
> -    PartitionDesc = Volume->PartitionDescs[Index];
> -    if (PartitionDesc->PartitionNumber == PartitionNum) {
> -      return PartitionDesc;
> -    }
> +  //
> +  // Check if partition number matches Partition Descriptor found in
> + Main Volume  // Descriptor Sequence.
> +  //
> +  if (Volume->PartitionDesc.PartitionNumber == PartitionNum) {
> +    return &Volume->PartitionDesc;
>    }
> 
>    return NULL;
> @@ -284,8 +241,9 @@ GetLongAdLsn (
>    PartitionDesc = GetPdFromLongAd (Volume, LongAd);
>    ASSERT (PartitionDesc != NULL);
> 
> -  return (UINT64)PartitionDesc->PartitionStartingLocation +
> -                 LongAd->ExtentLocation.LogicalBlockNumber;
> +  return (UINT64)PartitionDesc->PartitionStartingLocation -
> +    Volume->MainVdsStartLocation +
> +    LongAd->ExtentLocation.LogicalBlockNumber;
>  }
> 
>  //
> @@ -293,12 +251,13 @@ GetLongAdLsn (
>  //
>  UINT64
>  GetShortAdLsn (
> +  IN UDF_VOLUME_INFO                  *Volume,
>    IN UDF_PARTITION_DESCRIPTOR         *PartitionDesc,
>    IN UDF_SHORT_ALLOCATION_DESCRIPTOR  *ShortAd
>    )
>  {
> -  return (UINT64)PartitionDesc->PartitionStartingLocation +
> -    ShortAd->ExtentPosition;
> +  return (UINT64)PartitionDesc->PartitionStartingLocation -
> +    Volume->MainVdsStartLocation + ShortAd->ExtentPosition;
>  }
> 
>  //
> @@ -311,36 +270,39 @@ EFI_STATUS
>  FindFileSetDescriptor (
>    IN   EFI_BLOCK_IO_PROTOCOL    *BlockIo,
>    IN   EFI_DISK_IO_PROTOCOL     *DiskIo,
> -  IN   UDF_VOLUME_INFO          *Volume,
> -  IN   UINTN                    LogicalVolDescNum,
> -  OUT  UDF_FILE_SET_DESCRIPTOR  *FileSetDesc
> +  IN   UDF_VOLUME_INFO          *Volume
>    )
>  {
>    EFI_STATUS                     Status;
>    UINT64                         Lsn;
>    UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc;
> 
> -  LogicalVolDesc = Volume->LogicalVolDescs[LogicalVolDescNum];
> +  LogicalVolDesc = &Volume->LogicalVolDesc;
>    Lsn = GetLongAdLsn (Volume, &LogicalVolDesc-
> >LogicalVolumeContentsUse);
> 
>    //
> -  // Read extent (Long Ad).
> +  // As per UDF 2.60 specification:
> +  //
> +  // There shall be exactly one File Set Descriptor recorded per
> + Logical  // Volume.
> +  //
> +  // Read disk block
>    //
>    Status = DiskIo->ReadDisk (
>      DiskIo,
>      BlockIo->Media->MediaId,
>      MultU64x32 (Lsn, LogicalVolDesc->LogicalBlockSize),
> -    sizeof (UDF_FILE_SET_DESCRIPTOR),
> -    (VOID *)FileSetDesc
> +    sizeof (Volume->FileSetDesc),
> +    &Volume->FileSetDesc
>      );
>    if (EFI_ERROR (Status)) {
>      return Status;
>    }
> 
>    //
> -  // Check if the read extent contains a valid FSD's tag identifier.
> +  // Check if read block is a File Set Descriptor
>    //
> -  if (!IS_FSD (FileSetDesc)) {
> +  if (!IS_FSD (&Volume->FileSetDesc)) {
>      return EFI_VOLUME_CORRUPTED;
>    }
> 
> @@ -348,71 +310,6 @@ FindFileSetDescriptor (  }
> 
>  //
> -// Get all File Set Descriptors for each Logical Volume Descriptor.
> -//
> -EFI_STATUS
> -GetFileSetDescriptors (
> -  IN      EFI_BLOCK_IO_PROTOCOL  *BlockIo,
> -  IN      EFI_DISK_IO_PROTOCOL   *DiskIo,
> -  IN OUT  UDF_VOLUME_INFO        *Volume
> -  )
> -{
> -  EFI_STATUS               Status;
> -  UINTN                    Index;
> -  UDF_FILE_SET_DESCRIPTOR  *FileSetDesc;
> -  UINTN                    Count;
> -
> -  Volume->FileSetDescs =
> -    (UDF_FILE_SET_DESCRIPTOR **)AllocateZeroPool (
> -      Volume->LogicalVolDescsNo * sizeof (UDF_FILE_SET_DESCRIPTOR));
> -  if (Volume->FileSetDescs == NULL) {
> -    return EFI_OUT_OF_RESOURCES;
> -  }
> -
> -  for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
> -    FileSetDesc = AllocateZeroPool (sizeof (UDF_FILE_SET_DESCRIPTOR));
> -    if (FileSetDesc == NULL) {
> -      Status = EFI_OUT_OF_RESOURCES;
> -      goto Error_Alloc_Fsd;
> -    }
> -
> -    //
> -    // Find a FSD for this LVD.
> -    //
> -    Status = FindFileSetDescriptor (
> -      BlockIo,
> -      DiskIo,
> -      Volume,
> -      Index,
> -      FileSetDesc
> -      );
> -    if (EFI_ERROR (Status)) {
> -      goto Error_Find_Fsd;
> -    }
> -
> -    //
> -    // Got one. Save it.
> -    //
> -    Volume->FileSetDescs[Index] = FileSetDesc;
> -  }
> -
> -  Volume->FileSetDescsNo = Volume->LogicalVolDescsNo;
> -  return EFI_SUCCESS;
> -
> -Error_Find_Fsd:
> -  Count = Index + 1;
> -  for (Index = 0; Index < Count; Index++) {
> -    FreePool ((VOID *)Volume->FileSetDescs[Index]);
> -  }
> -
> -  FreePool ((VOID *)Volume->FileSetDescs);
> -  Volume->FileSetDescs = NULL;
> -
> -Error_Alloc_Fsd:
> -  return Status;
> -}
> -
> -//
>  // Read Volume and File Structure on an UDF file system.
>  //
>  EFI_STATUS
> @@ -424,9 +321,10 @@ ReadVolumeFileStructure (  {
>    EFI_STATUS                            Status;
>    UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  AnchorPoint;
> +  UDF_EXTENT_AD                         *ExtentAd;
> 
>    //
> -  // Find an AVDP.
> +  // Find Anchor Volume Descriptor Pointer
>    //
>    Status = FindAnchorVolumeDescriptorPointer (
>      BlockIo,
> @@ -438,12 +336,18 @@ ReadVolumeFileStructure (
>    }
> 
>    //
> -  // AVDP has been found. Start MVDS.
> +  // Save Main VDS start block number
> +  //
> +  ExtentAd = &AnchorPoint.MainVolumeDescriptorSequenceExtent;
> +
> +  Volume->MainVdsStartLocation = (UINT64)ExtentAd->ExtentLocation;
> +
> +  //
> +  // Start Main Volume Descriptor Sequence.
>    //
>    Status = StartMainVolumeDescriptorSequence (
>      BlockIo,
>      DiskIo,
> -    &AnchorPoint,
>      Volume
>      );
>    if (EFI_ERROR (Status)) {
> @@ -699,6 +603,7 @@ GetAllocationDescriptorLsn (
>      return GetLongAdLsn (Volume, (UDF_LONG_ALLOCATION_DESCRIPTOR
> *)Ad);
>    } else if (RecordingFlags == SHORT_ADS_SEQUENCE) {
>      return GetShortAdLsn (
> +      Volume,
>        GetPdFromLongAd (Volume, ParentIcb),
>        (UDF_SHORT_ALLOCATION_DESCRIPTOR *)Ad
>        );
> @@ -740,7 +645,7 @@ GetAedAdsOffset (
>      return EFI_OUT_OF_RESOURCES;
>    }
> 
> -  LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
> +  LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
> 
>    //
>    // Read extent.
> @@ -890,7 +795,7 @@ ReadFile (
>    UINT32                  ExtentLength;
>    UDF_FE_RECORDING_FLAGS  RecordingFlags;
> 
> -  LogicalBlockSize  = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
> +  LogicalBlockSize  = Volume->LogicalVolDesc.LogicalBlockSize;
>    DoFreeAed         = FALSE;
> 
>    //
> @@ -1355,6 +1260,9 @@ ReadUdfVolumeInformation (  {
>    EFI_STATUS Status;
> 
> +  //
> +  // Read all necessary UDF volume information and keep it private to
> + the driver  //
>    Status = ReadVolumeFileStructure (
>      BlockIo,
>      DiskIo,
> @@ -1364,13 +1272,12 @@ ReadUdfVolumeInformation (
>      return Status;
>    }
> 
> -  Status = GetFileSetDescriptors (
> -    BlockIo,
> -    DiskIo,
> -    Volume
> -    );
> +  //
> +  // Find File Set Descriptor
> +  //
> +  Status = FindFileSetDescriptor (BlockIo, DiskIo, Volume);
>    if (EFI_ERROR (Status)) {
> -    CleanupVolumeInformation (Volume);
> +    return Status;
>    }
> 
>    return Status;
> @@ -1407,7 +1314,7 @@ FindRootDirectory (
>      BlockIo,
>      DiskIo,
>      Volume,
> -    &Volume->FileSetDescs[0]->RootDirectoryIcb,
> +    &Volume->FileSetDesc.RootDirectoryIcb,
>      &File->FileEntry
>      );
>    if (EFI_ERROR (Status)) {
> @@ -1424,7 +1331,7 @@ FindRootDirectory (
>      L"\\",
>      NULL,
>      &Parent,
> -    &Volume->FileSetDescs[0]->RootDirectoryIcb,
> +    &Volume->FileSetDesc.RootDirectoryIcb,
>      File
>      );
>    if (EFI_ERROR (Status)) {
> @@ -1465,7 +1372,7 @@ FindFileEntry (
>    UINT32      LogicalBlockSize;
> 
>    Lsn               = GetLongAdLsn (Volume, Icb);
> -  LogicalBlockSize  = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
> +  LogicalBlockSize  = Volume->LogicalVolDesc.LogicalBlockSize;
> 
>    *FileEntry = AllocateZeroPool (Volume->FileEntrySize);
>    if (*FileEntry == NULL) {
> @@ -1959,43 +1866,6 @@ Error_Find_File:
>  }
> 
>  /**
> -  Clean up in-memory UDF volume information.
> -
> -  @param[in] Volume Volume information pointer.
> -
> -**/
> -VOID
> -CleanupVolumeInformation (
> -  IN UDF_VOLUME_INFO *Volume
> -  )
> -{
> -  UINTN Index;
> -
> -  if (Volume->LogicalVolDescs != NULL) {
> -    for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
> -      FreePool ((VOID *)Volume->LogicalVolDescs[Index]);
> -    }
> -    FreePool ((VOID *)Volume->LogicalVolDescs);
> -  }
> -
> -  if (Volume->PartitionDescs != NULL) {
> -    for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
> -      FreePool ((VOID *)Volume->PartitionDescs[Index]);
> -    }
> -    FreePool ((VOID *)Volume->PartitionDescs);
> -  }
> -
> -  if (Volume->FileSetDescs != NULL) {
> -    for (Index = 0; Index < Volume->FileSetDescsNo; Index++) {
> -      FreePool ((VOID *)Volume->FileSetDescs[Index]);
> -    }
> -    FreePool ((VOID *)Volume->FileSetDescs);
> -  }
> -
> -  ZeroMem ((VOID *)Volume, sizeof (UDF_VOLUME_INFO)); -}
> -
> -/**
>    Clean up in-memory UDF file information.
> 
>    @param[in] File File information pointer.
> @@ -2249,91 +2119,100 @@ GetVolumeSize (
>    OUT  UINT64                 *FreeSpaceSize
>    )
>  {
> -  UDF_EXTENT_AD                 ExtentAd;
> -  UINT32                        LogicalBlockSize;
> -  UINT64                        Lsn;
> -  EFI_STATUS                    Status;
> -  UDF_LOGICAL_VOLUME_INTEGRITY  *LogicalVolInt;
> -  UINTN                         Index;
> -  UINTN                         Length;
> -  UINT32                        LsnsNo;
> -
> -  *VolumeSize     = 0;
> -  *FreeSpaceSize  = 0;
> -
> -  for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
> -    CopyMem ((VOID *)&ExtentAd,
> -             (VOID *)&Volume->LogicalVolDescs[Index]-
> >IntegritySequenceExtent,
> -             sizeof (UDF_EXTENT_AD));
> -    if (ExtentAd.ExtentLength == 0) {
> -      continue;
> -    }
> +  EFI_STATUS                     Status;
> +  UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc;
> +  UDF_EXTENT_AD                  *ExtentAd;
> +  UINT64                         Lsn;
> +  UINT32                         LogicalBlockSize;
> +  UDF_LOGICAL_VOLUME_INTEGRITY   *LogicalVolInt;
> +  UINTN                          Index;
> +  UINTN                          Length;
> +  UINT32                         LsnsNo;
> 
> -    LogicalBlockSize = LV_BLOCK_SIZE (Volume, Index);
> +  LogicalVolDesc = &Volume->LogicalVolDesc;
> 
> -  Read_Next_Sequence:
> -    LogicalVolInt = (UDF_LOGICAL_VOLUME_INTEGRITY *)
> -      AllocatePool (ExtentAd.ExtentLength);
> -    if (LogicalVolInt == NULL) {
> -      return EFI_OUT_OF_RESOURCES;
> -    }
> +  ExtentAd = &LogicalVolDesc->IntegritySequenceExtent;
> 
> -    Lsn = (UINT64)ExtentAd.ExtentLocation;
> +  if (ExtentAd->ExtentLength == 0) {
> +    return EFI_VOLUME_CORRUPTED;
> +  }
> 
> -    Status = DiskIo->ReadDisk (
> -      DiskIo,
> -      BlockIo->Media->MediaId,
> -      MultU64x32 (Lsn, LogicalBlockSize),
> -      ExtentAd.ExtentLength,
> -      (VOID *)LogicalVolInt
> -      );
> -    if (EFI_ERROR (Status)) {
> -      FreePool ((VOID *)LogicalVolInt);
> -      return Status;
> -    }
> +  LogicalVolInt = AllocatePool (ExtentAd->ExtentLength);  if
> + (LogicalVolInt == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> 
> -    if (!IS_LVID (LogicalVolInt)) {
> -      FreePool ((VOID *)LogicalVolInt);
> -      return EFI_VOLUME_CORRUPTED;
> -    }
> +  //
> +  // Get location of Logical Volume Integrity Descriptor  //  Lsn =
> + (UINT64)ExtentAd->ExtentLocation - Volume->MainVdsStartLocation;
> 
> -    Length = LogicalVolInt->NumberOfPartitions;
> -    for (Index = 0; Index < Length; Index += sizeof (UINT32)) {
> -      LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
> -      if (LsnsNo == 0xFFFFFFFFUL) {
> -        //
> -        // Size not specified.
> -        //
> -        continue;
> -      }
> +  LogicalBlockSize = LogicalVolDesc->LogicalBlockSize;
> 
> -      *FreeSpaceSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
> -    }
> +  //
> +  // Read disk block
> +  //
> +  Status = DiskIo->ReadDisk (
> +    DiskIo,
> +    BlockIo->Media->MediaId,
> +    MultU64x32 (Lsn, LogicalBlockSize),
> +    ExtentAd->ExtentLength,
> +    LogicalVolInt
> +    );
> +  if (EFI_ERROR (Status)) {
> +    goto Out_Free;
> +  }
> 
> -    Length = (LogicalVolInt->NumberOfPartitions * sizeof (UINT32)) << 1;
> -    for (; Index < Length; Index += sizeof (UINT32)) {
> -      LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
> -      if (LsnsNo == 0xFFFFFFFFUL) {
> -        //
> -        // Size not specified.
> -        //
> -        continue;
> -      }
> +  //
> +  // Check if read block is a Logical Volume Integrity Descriptor  //
> + if (!IS_LVID (LogicalVolInt)) {
> +    Status = EFI_VOLUME_CORRUPTED;
> +    goto Out_Free;
> +  }
> 
> -      *VolumeSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
> -    }
> +  *VolumeSize = 0;
> +  *FreeSpaceSize = 0;
> 
> -    CopyMem ((VOID *)&ExtentAd,(VOID *)&LogicalVolInt-
> >NextIntegrityExtent,
> -             sizeof (UDF_EXTENT_AD));
> -    if (ExtentAd.ExtentLength > 0) {
> -      FreePool ((VOID *)LogicalVolInt);
> -      goto Read_Next_Sequence;
> +  Length = LogicalVolInt->NumberOfPartitions;
> +  for (Index = 0; Index < Length; Index += sizeof (UINT32)) {
> +    LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
> +    //
> +    // Check if size is not specified
> +    //
> +    if (LsnsNo == 0xFFFFFFFFUL) {
> +      continue;
>      }
> +    //
> +    // Accumulate free space size
> +    //
> +    *FreeSpaceSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);  }
> 
> -    FreePool ((VOID *)LogicalVolInt);
> +  Length = LogicalVolInt->NumberOfPartitions * sizeof (UINT32) * 2;
> + for (; Index < Length; Index += sizeof (UINT32)) {
> +    LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
> +    //
> +    // Check if size is not specified
> +    //
> +    if (LsnsNo == 0xFFFFFFFFUL) {
> +      continue;
> +    }
> +    //
> +    // Accumulate used volume space
> +    //
> +    *VolumeSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
>    }
> 
> -  return EFI_SUCCESS;
> +  Status = EFI_SUCCESS;
> +
> +Out_Free:
> +  //
> +  // Free Logical Volume Integrity Descriptor
> +  //
> +  FreePool (LogicalVolInt);
> +
> +  return Status;
>  }
> 
>  /**
> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
> b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
> index 49dc7077b7..d4163b89ca 100644
> --- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
> @@ -276,13 +276,6 @@ UdfDriverBindingStop (
>        NULL
>        );
> 
> -    //
> -    // Check if there's any open file. If so, clean them up.
> -    //
> -    if (PrivFsData->OpenFiles > 0) {
> -      CleanupVolumeInformation (&PrivFsData->Volume);
> -    }
> -
>      FreePool ((VOID *)PrivFsData);
>    }
> 
> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
> b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
> index 240d420ff5..c5f83914d8 100644
> --- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
> @@ -49,16 +49,8 @@
>      { 0x89, 0x56, 0x73, 0xCD, 0xA3, 0x26, 0xCD, 0x0A }  \
>    }
> 
> -#define UDF_DEFAULT_LV_NUM 0
> -
>  #define IS_PVD(_Pointer) \
>    ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 1)) -#define IS_PD(_Pointer) \
> -  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5)) -#define IS_LVD(_Pointer) \
> -  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6)) -#define IS_TD(_Pointer) \
> -  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))  #define IS_FSD(_Pointer) \
>    ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 256))  #define IS_FE(_Pointer) \
> @@ -152,14 +144,8 @@ typedef enum {  #define
> IS_VALID_COMPRESSION_ID(_CompId) \
>    ((BOOLEAN)((_CompId) == 8 || (_CompId) == 16))
> 
> -#define LV_BLOCK_SIZE(_Vol, _LvNum) \
> -  (_Vol)->LogicalVolDescs[(_LvNum)]->LogicalBlockSize
> -
>  #define UDF_STANDARD_IDENTIFIER_LENGTH   5
> 
> -#define LV_UDF_REVISION(_Lv) \
> -  *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix
> -
>  #pragma pack(1)
> 
>  typedef struct {
> @@ -186,17 +172,6 @@ typedef struct {
>  #pragma pack(1)
> 
>  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 {
>    UINT16          TypeAndTimezone;
>    INT16           Year;
>    UINT8           Month;
> @@ -210,17 +185,6 @@ typedef struct {
>  } UDF_TIMESTAMP;
> 
>  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;
>    UINT32                             PrevAllocationExtentDescriptor;
>    UINT32                             LengthOfAllocationDescriptors;
> @@ -235,37 +199,6 @@ typedef struct {
>  } UDF_VOLUME_DESCRIPTOR;
> 
>  typedef struct {
> -  UDF_DESCRIPTOR_TAG         DescriptorTag;
> -  UINT32                     VolumeDescriptorSequenceNumber;
> -  UINT16                     PartitionFlags;
> -  UINT16                     PartitionNumber;
> -  UDF_ENTITY_ID              PartitionContents;
> -  UINT8                      PartitionContentsUse[128];
> -  UINT32                     AccessType;
> -  UINT32                     PartitionStartingLocation;
> -  UINT32                     PartitionLength;
> -  UDF_ENTITY_ID              ImplementationIdentifier;
> -  UINT8                      ImplementationUse[128];
> -  UINT8                      Reserved[156];
> -} UDF_PARTITION_DESCRIPTOR;
> -
> -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;
> -
> -typedef struct {
>    UDF_DESCRIPTOR_TAG             DescriptorTag;
>    UDF_TIMESTAMP                  RecordingDateTime;
>    UINT32                         IntegrityType;
> @@ -389,12 +322,10 @@ typedef struct {
>  // UDF filesystem driver's private data  //  typedef struct {
> -  UDF_LOGICAL_VOLUME_DESCRIPTOR  **LogicalVolDescs;
> -  UINTN                          LogicalVolDescsNo;
> -  UDF_PARTITION_DESCRIPTOR       **PartitionDescs;
> -  UINTN                          PartitionDescsNo;
> -  UDF_FILE_SET_DESCRIPTOR        **FileSetDescs;
> -  UINTN                          FileSetDescsNo;
> +  UINT64                         MainVdsStartLocation;
> +  UDF_LOGICAL_VOLUME_DESCRIPTOR  LogicalVolDesc;
> +  UDF_PARTITION_DESCRIPTOR       PartitionDesc;
> +  UDF_FILE_SET_DESCRIPTOR        FileSetDesc;
>    UINTN                          FileEntrySize;
>  } UDF_VOLUME_INFO;
> 
> @@ -883,17 +814,6 @@ ResolveSymlink (
>    );
> 
>  /**
> -  Clean up in-memory UDF volume information.
> -
> -  @param[in] Volume Volume information pointer.
> -
> -**/
> -VOID
> -CleanupVolumeInformation (
> -  IN UDF_VOLUME_INFO *Volume
> -  );
> -
> -/**
>    Clean up in-memory UDF file information.
> 
>    @param[in] File File information pointer.
> --
> 2.11.0

Re: [PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to support PartitionDxe changes

[Paulo Alcantara]

On 9/17/2017 10:00 PM, Ni, Ruiyu wrote:
> Paulo,
> With the change in partition driver, I suppose UdfDxe driver
> only needs to take care of area covered by the partition descriptor.
> But why StartMainVolumeDescriptorSequence() still reads
> LVD, TD, and PD?

We still need the LVD again for a few reasons:

1. GetVolumeSize() needs to read LVID (Logical Volume Integrity 
Descriptor), which is located in LVD, in order to calculate free and 
used volume space.
2. To find FSD (File Set Descriptor), where the root directory is located.
3. To know which Partition Descriptor to use based on partition number 
from LVD->PartitionMaps field. I know UDF 2.60 spec says that there 
should be only one Partition Descriptor that covers the entire space of 
a single LVD, but it's good to make sure we're reading it correctly.

TD descriptor is used to know when stopping the Volume Descriptor 
Sequence and then we don't need to read all ExtentAd->ExtentLength 
blocks unnecessarily.

Thanks,
Paulo

> 
> I thought the UdfDxe driver's logic can be simplified a lot.
> There should be no duplicated logic in Partition driver and udf driver.
> 
> Can you explain more?
> 
> Thanks/Ray
> 
>> -----Original Message-----
>> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
>> Sent: Sunday, September 17, 2017 9:13 PM
>> To: edk2-devel@lists.01.org
>> Cc: Paulo Alcantara <pcacjr@zytor.com>; Dong, Eric <eric.dong@intel.com>;
>> Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Laszlo
>> Ersek <lersek@redhat.com>
>> Subject: [PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to support
>> PartitionDxe changes
>>
>> This patch reworks the driver to support Partition driver changes.
>>
>> Cc: Eric Dong <eric.dong@intel.com>
>> Cc: Paulo Alcantara <pcacjr@zytor.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Cc: Star Zeng <star.zeng@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
>> ---
>>   MdeModulePkg/Universal/Disk/UdfDxe/File.c                 |  13 +-
>>   MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 515
>> ++++++++------------
>>   MdeModulePkg/Universal/Disk/UdfDxe/Udf.c                  |   7 -
>>   MdeModulePkg/Universal/Disk/UdfDxe/Udf.h                  |  88 +---
>>   4 files changed, 204 insertions(+), 419 deletions(-)
>>
>> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>> b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>> index 01361141bb..f2c62967e8 100644
>> --- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>> @@ -100,6 +100,7 @@ UdfOpenVolume (
>>       &PrivFsData->Volume,
>>       &PrivFsData->Root
>>       );
>> +  ASSERT_EFI_ERROR (Status);
>>     if (EFI_ERROR (Status)) {
>>       goto Error_Find_Root_Dir;
>>     }
>> @@ -131,7 +132,6 @@ Error_Alloc_Priv_File_Data:
>>     CleanupFileInformation (&PrivFsData->Root);
>>
>>   Error_Find_Root_Dir:
>> -  CleanupVolumeInformation (&PrivFsData->Volume);
>>
>>   Error_Read_Udf_Volume:
>>   Error_Invalid_Params:
>> @@ -528,7 +528,6 @@ UdfClose (
>>     EFI_TPL                     OldTpl;
>>     EFI_STATUS                  Status;
>>     PRIVATE_UDF_FILE_DATA       *PrivFileData;
>> -  PRIVATE_UDF_SIMPLE_FS_DATA  *PrivFsData;
>>
>>     OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
>>
>> @@ -541,8 +540,6 @@ UdfClose (
>>
>>     PrivFileData = PRIVATE_UDF_FILE_DATA_FROM_THIS (This);
>>
>> -  PrivFsData = PRIVATE_UDF_SIMPLE_FS_DATA_FROM_THIS (PrivFileData-
>>> SimpleFs);
>> -
>>     if (!PrivFileData->IsRootDirectory) {
>>       CleanupFileInformation (&PrivFileData->File);
>>
>> @@ -551,10 +548,6 @@ UdfClose (
>>       }
>>     }
>>
>> -  if (--PrivFsData->OpenFiles == 0) {
>> -    CleanupVolumeInformation (&PrivFsData->Volume);
>> -  }
>> -
>>     FreePool ((VOID *)PrivFileData);
>>
>>   Exit:
>> @@ -787,7 +780,7 @@ UdfGetInfo (
>>     } else if (CompareGuid (InformationType, &gEfiFileSystemInfoGuid)) {
>>       String = VolumeLabel;
>>
>> -    FileSetDesc = PrivFsData->Volume.FileSetDescs[0];
>> +    FileSetDesc = &PrivFsData->Volume.FileSetDesc;
>>
>>       OstaCompressed = &FileSetDesc->LogicalVolumeIdentifier[0];
>>
>> @@ -846,7 +839,7 @@ UdfGetInfo (
>>       FileSystemInfo->Size        = FileSystemInfoLength;
>>       FileSystemInfo->ReadOnly    = TRUE;
>>       FileSystemInfo->BlockSize   =
>> -      LV_BLOCK_SIZE (&PrivFsData->Volume, UDF_DEFAULT_LV_NUM);
>> +      PrivFsData->Volume.LogicalVolDesc.LogicalBlockSize;
>>       FileSystemInfo->VolumeSize  = VolumeSize;
>>       FileSystemInfo->FreeSpace   = FreeSpaceSize;
>>
>> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
>> b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
>> index 4609580b30..63b643e60a 100644
>> --- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
>> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
>> @@ -60,154 +60,111 @@ FindAnchorVolumeDescriptorPointer (
>>
>>   EFI_STATUS
>>   StartMainVolumeDescriptorSequence (
>> -  IN   EFI_BLOCK_IO_PROTOCOL                 *BlockIo,
>> -  IN   EFI_DISK_IO_PROTOCOL                  *DiskIo,
>> -  IN   UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  *AnchorPoint,
>> -  OUT  UDF_VOLUME_INFO                       *Volume
>> +  IN   EFI_BLOCK_IO_PROTOCOL    *BlockIo,
>> +  IN   EFI_DISK_IO_PROTOCOL     *DiskIo,
>> +  OUT  UDF_VOLUME_INFO          *Volume
>>     )
>>   {
>> -  EFI_STATUS                     Status;
>> -  UINT32                         BlockSize;
>> -  UDF_EXTENT_AD                  *ExtentAd;
>> -  UINT64                         StartingLsn;
>> -  UINT64                         EndingLsn;
>> -  VOID                           *Buffer;
>> -  UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc;
>> -  UDF_PARTITION_DESCRIPTOR       *PartitionDesc;
>> -  UINTN                          Index;
>> -  UINT32                         LogicalBlockSize;
>> +  EFI_STATUS  Status;
>> +  UINT32      BlockSize;
>> +  UINT64      BlockOffset;
>> +  VOID        *Buffer;
>> +  UINT32      LogicalBlockSize;
>> +
>> +  BlockSize = BlockIo->Media->BlockSize;
>>
>>     //
>> -  // We've already found an ADVP on the volume. It contains the extent
>> -  // (MainVolumeDescriptorSequenceExtent) where the Main Volume
>> Descriptor
>> -  // Sequence starts. Therefore, we'll look for Logical Volume Descriptors and
>> -  // Partitions Descriptors and save them in memory, accordingly.
>> -  //
>> -  // Note also that each descriptor will be aligned on a block size (BlockSize)
>> -  // boundary, so we need to read one block at a time.
>> +  // Allocate buffer for reading disk blocks
>>     //
>> -  BlockSize    = BlockIo->Media->BlockSize;
>> -  ExtentAd     = &AnchorPoint->MainVolumeDescriptorSequenceExtent;
>> -  StartingLsn  = (UINT64)ExtentAd->ExtentLocation;
>> -  EndingLsn    = StartingLsn + DivU64x32 (
>> -                                     (UINT64)ExtentAd->ExtentLength,
>> -                                     BlockSize
>> -                                     );
>> -
>> -  Volume->LogicalVolDescs =
>> -    (UDF_LOGICAL_VOLUME_DESCRIPTOR **)AllocateZeroPool (ExtentAd-
>>> ExtentLength);
>> -  if (Volume->LogicalVolDescs == NULL) {
>> -    return EFI_OUT_OF_RESOURCES;
>> -  }
>> -
>> -  Volume->PartitionDescs =
>> -    (UDF_PARTITION_DESCRIPTOR **)AllocateZeroPool (ExtentAd-
>>> ExtentLength);
>> -  if (Volume->PartitionDescs == NULL) {
>> -    Status = EFI_OUT_OF_RESOURCES;
>> -    goto Error_Alloc_Pds;
>> -  }
>> -
>> -  Buffer = AllocateZeroPool (BlockSize);
>> +  Buffer = AllocateZeroPool ((UINTN)BlockSize);
>>     if (Buffer == NULL) {
>> -    Status = EFI_OUT_OF_RESOURCES;
>> -    goto Error_Alloc_Buf;
>> +    return EFI_OUT_OF_RESOURCES;
>>     }
>>
>> -  Volume->LogicalVolDescsNo  = 0;
>> -  Volume->PartitionDescsNo   = 0;
>> -
>> -  while (StartingLsn <= EndingLsn) {
>> +  //
>> +  // As per UDF 2.60 specification:
>> +  //
>> +  // ---
>> +  // For the purpose of interchange, Partition Maps shall be limited to
>> + // Partition Map type 1, except type 2 maps.
>> +  // ---
>> +  // A Partition Descriptor Access Type of read-only, rewritable,  //
>> + overwritable, write-once and pseudo-overwritable shall be  //
>> + supported. There shall be exactly one prevailing Partition  //
>> + Descriptor recorded per volume, with one exception. For Volume  //
>> + Sets that consist of single volume, the volume may contain 2 non-  //
>> + overlapping Partitions with 2 prevailing Partition Descriptors only
>> + // if one has an Access Type of read-only and the other has an  //
>> + Access Type of rewritable, overwritable, or write-once. The  //
>> + Logical Volume for this volume would consist of the contents of  //
>> + both partitions.
>> +  // ---
>> +  //
>> +  // The logical partition created by Partition driver is relative to
>> + the main  // VDS extent location, so we start the Main Volume
>> + Descriptor Sequence at  // LBA 0.
>> +  //
>> +  // We don't need to make sure we got both LVD and PD -- that is, if
>> + we are  // here, that means Partition driver was able to find them both
>> previously.
>> +  //
>> +  for (BlockOffset = 0;
>> +       BlockOffset < MultU64x32 (BlockIo->Media->LastBlock,
>> +                                 BlockIo->Media->BlockSize);
>> +       BlockOffset += BlockSize) {
>> +    // Read disk block
>> +    //
>>       Status = DiskIo->ReadDisk (
>>         DiskIo,
>>         BlockIo->Media->MediaId,
>> -      MultU64x32 (StartingLsn, BlockSize),
>> +      BlockOffset,
>>         BlockSize,
>>         Buffer
>>         );
>>       if (EFI_ERROR (Status)) {
>> -      goto Error_Read_Disk_Blk;
>> +      goto Out_Free;
>>       }
>>
>> +    //
>> +    // Check if read block is a Terminating Descriptor
>> +    //
>>       if (IS_TD (Buffer)) {
>>         //
>> -      // Found a Terminating Descriptor. Stop the sequence then.
>> +      // Terminate Main Volume Descriptor Sequence
>>         //
>>         break;
>>       }
>>
>>       if (IS_LVD (Buffer)) {
>>         //
>> -      // Found a Logical Volume Descriptor.
>> +      // Save Logical Volume Descriptor
>>         //
>> -      LogicalVolDesc =
>> -        (UDF_LOGICAL_VOLUME_DESCRIPTOR *)
>> -        AllocateZeroPool (sizeof (UDF_LOGICAL_VOLUME_DESCRIPTOR));
>> -      if (LogicalVolDesc == NULL) {
>> -        Status = EFI_OUT_OF_RESOURCES;
>> -        goto Error_Alloc_Lvd;
>> -      }
>> -
>> -      CopyMem ((VOID *)LogicalVolDesc, Buffer,
>> -               sizeof (UDF_LOGICAL_VOLUME_DESCRIPTOR));
>> -      Volume->LogicalVolDescs[Volume->LogicalVolDescsNo++] =
>> LogicalVolDesc;
>> +      CopyMem (&Volume->LogicalVolDesc, Buffer, sizeof
>> + (Volume->LogicalVolDesc));
>>       } else if (IS_PD (Buffer)) {
>>         //
>> -      // Found a Partition Descriptor.
>> +      // Save Partition Descriptor
>>         //
>> -      PartitionDesc =
>> -        (UDF_PARTITION_DESCRIPTOR *)
>> -        AllocateZeroPool (sizeof (UDF_PARTITION_DESCRIPTOR));
>> -      if (PartitionDesc == NULL) {
>> -        Status = EFI_OUT_OF_RESOURCES;
>> -        goto Error_Alloc_Pd;
>> -      }
>> -
>> -      CopyMem ((VOID *)PartitionDesc, Buffer,
>> -               sizeof (UDF_PARTITION_DESCRIPTOR));
>> -      Volume->PartitionDescs[Volume->PartitionDescsNo++] = PartitionDesc;
>> +      CopyMem (&Volume->PartitionDesc, Buffer, sizeof
>> + (Volume->PartitionDesc));
>>       }
>> -
>> -    StartingLsn++;
>>     }
>>
>>     //
>> -  // When an UDF volume (revision 2.00 or higher) contains a File Entry rather
>> -  // than an Extended File Entry (which is not recommended as per spec), we
>> need
>> -  // to make sure the size of a FE will be _at least_ 2048
>> -  // (UDF_LOGICAL_SECTOR_SIZE) bytes long to keep backward compatibility.
>> +  // Determine FE (File Entry) size
>>     //
>> -  LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
>> +  LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
>>     if (LogicalBlockSize >= UDF_LOGICAL_SECTOR_SIZE) {
>> -    Volume->FileEntrySize = LogicalBlockSize;
>> +    Volume->FileEntrySize = (UINTN)LogicalBlockSize;
>>     } else {
>>       Volume->FileEntrySize = UDF_LOGICAL_SECTOR_SIZE;
>>     }
>>
>> -  FreePool (Buffer);
>> -
>> -  return EFI_SUCCESS;
>> -
>> -Error_Alloc_Pd:
>> -Error_Alloc_Lvd:
>> -  for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
>> -    FreePool ((VOID *)Volume->PartitionDescs[Index]);
>> -  }
>> -
>> -  for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
>> -    FreePool ((VOID *)Volume->LogicalVolDescs[Index]);
>> -  }
>> +  Status = EFI_SUCCESS;
>>
>> -Error_Read_Disk_Blk:
>> +Out_Free:
>> +  //
>> +  // Free block read buffer
>> +  //
>>     FreePool (Buffer);
>>
>> -Error_Alloc_Buf:
>> -  FreePool ((VOID *)Volume->PartitionDescs);
>> -  Volume->PartitionDescs = NULL;
>> -
>> -Error_Alloc_Pds:
>> -  FreePool ((VOID *)Volume->LogicalVolDescs);
>> -  Volume->LogicalVolDescs = NULL;
>> -
>>     return Status;
>>   }
>>
>> @@ -223,11 +180,9 @@ GetPdFromLongAd (
>>     )
>>   {
>>     UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc;
>> -  UINTN                          Index;
>> -  UDF_PARTITION_DESCRIPTOR       *PartitionDesc;
>>     UINT16                         PartitionNum;
>>
>> -  LogicalVolDesc = Volume->LogicalVolDescs[UDF_DEFAULT_LV_NUM];
>> +  LogicalVolDesc = &Volume->LogicalVolDesc;
>>
>>     switch (LV_UDF_REVISION (LogicalVolDesc)) {
>>     case 0x0102:
>> @@ -252,19 +207,21 @@ GetPdFromLongAd (
>>       PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc->PartitionMaps[4]);
>>       break;
>>     case 0x0260:
>> -    //
>> -    // Fall through.
>> -    //
>> -  default:
>>       PartitionNum = LongAd->ExtentLocation.PartitionReferenceNumber;
>>       break;
>> +  default:
>> +    //
>> +    // Unhandled UDF revision
>> +    //
>> +    return NULL;
>>     }
>>
>> -  for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
>> -    PartitionDesc = Volume->PartitionDescs[Index];
>> -    if (PartitionDesc->PartitionNumber == PartitionNum) {
>> -      return PartitionDesc;
>> -    }
>> +  //
>> +  // Check if partition number matches Partition Descriptor found in
>> + Main Volume  // Descriptor Sequence.
>> +  //
>> +  if (Volume->PartitionDesc.PartitionNumber == PartitionNum) {
>> +    return &Volume->PartitionDesc;
>>     }
>>
>>     return NULL;
>> @@ -284,8 +241,9 @@ GetLongAdLsn (
>>     PartitionDesc = GetPdFromLongAd (Volume, LongAd);
>>     ASSERT (PartitionDesc != NULL);
>>
>> -  return (UINT64)PartitionDesc->PartitionStartingLocation +
>> -                 LongAd->ExtentLocation.LogicalBlockNumber;
>> +  return (UINT64)PartitionDesc->PartitionStartingLocation -
>> +    Volume->MainVdsStartLocation +
>> +    LongAd->ExtentLocation.LogicalBlockNumber;
>>   }
>>
>>   //
>> @@ -293,12 +251,13 @@ GetLongAdLsn (
>>   //
>>   UINT64
>>   GetShortAdLsn (
>> +  IN UDF_VOLUME_INFO                  *Volume,
>>     IN UDF_PARTITION_DESCRIPTOR         *PartitionDesc,
>>     IN UDF_SHORT_ALLOCATION_DESCRIPTOR  *ShortAd
>>     )
>>   {
>> -  return (UINT64)PartitionDesc->PartitionStartingLocation +
>> -    ShortAd->ExtentPosition;
>> +  return (UINT64)PartitionDesc->PartitionStartingLocation -
>> +    Volume->MainVdsStartLocation + ShortAd->ExtentPosition;
>>   }
>>
>>   //
>> @@ -311,36 +270,39 @@ EFI_STATUS
>>   FindFileSetDescriptor (
>>     IN   EFI_BLOCK_IO_PROTOCOL    *BlockIo,
>>     IN   EFI_DISK_IO_PROTOCOL     *DiskIo,
>> -  IN   UDF_VOLUME_INFO          *Volume,
>> -  IN   UINTN                    LogicalVolDescNum,
>> -  OUT  UDF_FILE_SET_DESCRIPTOR  *FileSetDesc
>> +  IN   UDF_VOLUME_INFO          *Volume
>>     )
>>   {
>>     EFI_STATUS                     Status;
>>     UINT64                         Lsn;
>>     UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc;
>>
>> -  LogicalVolDesc = Volume->LogicalVolDescs[LogicalVolDescNum];
>> +  LogicalVolDesc = &Volume->LogicalVolDesc;
>>     Lsn = GetLongAdLsn (Volume, &LogicalVolDesc-
>>> LogicalVolumeContentsUse);
>>
>>     //
>> -  // Read extent (Long Ad).
>> +  // As per UDF 2.60 specification:
>> +  //
>> +  // There shall be exactly one File Set Descriptor recorded per
>> + Logical  // Volume.
>> +  //
>> +  // Read disk block
>>     //
>>     Status = DiskIo->ReadDisk (
>>       DiskIo,
>>       BlockIo->Media->MediaId,
>>       MultU64x32 (Lsn, LogicalVolDesc->LogicalBlockSize),
>> -    sizeof (UDF_FILE_SET_DESCRIPTOR),
>> -    (VOID *)FileSetDesc
>> +    sizeof (Volume->FileSetDesc),
>> +    &Volume->FileSetDesc
>>       );
>>     if (EFI_ERROR (Status)) {
>>       return Status;
>>     }
>>
>>     //
>> -  // Check if the read extent contains a valid FSD's tag identifier.
>> +  // Check if read block is a File Set Descriptor
>>     //
>> -  if (!IS_FSD (FileSetDesc)) {
>> +  if (!IS_FSD (&Volume->FileSetDesc)) {
>>       return EFI_VOLUME_CORRUPTED;
>>     }
>>
>> @@ -348,71 +310,6 @@ FindFileSetDescriptor (  }
>>
>>   //
>> -// Get all File Set Descriptors for each Logical Volume Descriptor.
>> -//
>> -EFI_STATUS
>> -GetFileSetDescriptors (
>> -  IN      EFI_BLOCK_IO_PROTOCOL  *BlockIo,
>> -  IN      EFI_DISK_IO_PROTOCOL   *DiskIo,
>> -  IN OUT  UDF_VOLUME_INFO        *Volume
>> -  )
>> -{
>> -  EFI_STATUS               Status;
>> -  UINTN                    Index;
>> -  UDF_FILE_SET_DESCRIPTOR  *FileSetDesc;
>> -  UINTN                    Count;
>> -
>> -  Volume->FileSetDescs =
>> -    (UDF_FILE_SET_DESCRIPTOR **)AllocateZeroPool (
>> -      Volume->LogicalVolDescsNo * sizeof (UDF_FILE_SET_DESCRIPTOR));
>> -  if (Volume->FileSetDescs == NULL) {
>> -    return EFI_OUT_OF_RESOURCES;
>> -  }
>> -
>> -  for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
>> -    FileSetDesc = AllocateZeroPool (sizeof (UDF_FILE_SET_DESCRIPTOR));
>> -    if (FileSetDesc == NULL) {
>> -      Status = EFI_OUT_OF_RESOURCES;
>> -      goto Error_Alloc_Fsd;
>> -    }
>> -
>> -    //
>> -    // Find a FSD for this LVD.
>> -    //
>> -    Status = FindFileSetDescriptor (
>> -      BlockIo,
>> -      DiskIo,
>> -      Volume,
>> -      Index,
>> -      FileSetDesc
>> -      );
>> -    if (EFI_ERROR (Status)) {
>> -      goto Error_Find_Fsd;
>> -    }
>> -
>> -    //
>> -    // Got one. Save it.
>> -    //
>> -    Volume->FileSetDescs[Index] = FileSetDesc;
>> -  }
>> -
>> -  Volume->FileSetDescsNo = Volume->LogicalVolDescsNo;
>> -  return EFI_SUCCESS;
>> -
>> -Error_Find_Fsd:
>> -  Count = Index + 1;
>> -  for (Index = 0; Index < Count; Index++) {
>> -    FreePool ((VOID *)Volume->FileSetDescs[Index]);
>> -  }
>> -
>> -  FreePool ((VOID *)Volume->FileSetDescs);
>> -  Volume->FileSetDescs = NULL;
>> -
>> -Error_Alloc_Fsd:
>> -  return Status;
>> -}
>> -
>> -//
>>   // Read Volume and File Structure on an UDF file system.
>>   //
>>   EFI_STATUS
>> @@ -424,9 +321,10 @@ ReadVolumeFileStructure (  {
>>     EFI_STATUS                            Status;
>>     UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  AnchorPoint;
>> +  UDF_EXTENT_AD                         *ExtentAd;
>>
>>     //
>> -  // Find an AVDP.
>> +  // Find Anchor Volume Descriptor Pointer
>>     //
>>     Status = FindAnchorVolumeDescriptorPointer (
>>       BlockIo,
>> @@ -438,12 +336,18 @@ ReadVolumeFileStructure (
>>     }
>>
>>     //
>> -  // AVDP has been found. Start MVDS.
>> +  // Save Main VDS start block number
>> +  //
>> +  ExtentAd = &AnchorPoint.MainVolumeDescriptorSequenceExtent;
>> +
>> +  Volume->MainVdsStartLocation = (UINT64)ExtentAd->ExtentLocation;
>> +
>> +  //
>> +  // Start Main Volume Descriptor Sequence.
>>     //
>>     Status = StartMainVolumeDescriptorSequence (
>>       BlockIo,
>>       DiskIo,
>> -    &AnchorPoint,
>>       Volume
>>       );
>>     if (EFI_ERROR (Status)) {
>> @@ -699,6 +603,7 @@ GetAllocationDescriptorLsn (
>>       return GetLongAdLsn (Volume, (UDF_LONG_ALLOCATION_DESCRIPTOR
>> *)Ad);
>>     } else if (RecordingFlags == SHORT_ADS_SEQUENCE) {
>>       return GetShortAdLsn (
>> +      Volume,
>>         GetPdFromLongAd (Volume, ParentIcb),
>>         (UDF_SHORT_ALLOCATION_DESCRIPTOR *)Ad
>>         );
>> @@ -740,7 +645,7 @@ GetAedAdsOffset (
>>       return EFI_OUT_OF_RESOURCES;
>>     }
>>
>> -  LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
>> +  LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
>>
>>     //
>>     // Read extent.
>> @@ -890,7 +795,7 @@ ReadFile (
>>     UINT32                  ExtentLength;
>>     UDF_FE_RECORDING_FLAGS  RecordingFlags;
>>
>> -  LogicalBlockSize  = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
>> +  LogicalBlockSize  = Volume->LogicalVolDesc.LogicalBlockSize;
>>     DoFreeAed         = FALSE;
>>
>>     //
>> @@ -1355,6 +1260,9 @@ ReadUdfVolumeInformation (  {
>>     EFI_STATUS Status;
>>
>> +  //
>> +  // Read all necessary UDF volume information and keep it private to
>> + the driver  //
>>     Status = ReadVolumeFileStructure (
>>       BlockIo,
>>       DiskIo,
>> @@ -1364,13 +1272,12 @@ ReadUdfVolumeInformation (
>>       return Status;
>>     }
>>
>> -  Status = GetFileSetDescriptors (
>> -    BlockIo,
>> -    DiskIo,
>> -    Volume
>> -    );
>> +  //
>> +  // Find File Set Descriptor
>> +  //
>> +  Status = FindFileSetDescriptor (BlockIo, DiskIo, Volume);
>>     if (EFI_ERROR (Status)) {
>> -    CleanupVolumeInformation (Volume);
>> +    return Status;
>>     }
>>
>>     return Status;
>> @@ -1407,7 +1314,7 @@ FindRootDirectory (
>>       BlockIo,
>>       DiskIo,
>>       Volume,
>> -    &Volume->FileSetDescs[0]->RootDirectoryIcb,
>> +    &Volume->FileSetDesc.RootDirectoryIcb,
>>       &File->FileEntry
>>       );
>>     if (EFI_ERROR (Status)) {
>> @@ -1424,7 +1331,7 @@ FindRootDirectory (
>>       L"\\",
>>       NULL,
>>       &Parent,
>> -    &Volume->FileSetDescs[0]->RootDirectoryIcb,
>> +    &Volume->FileSetDesc.RootDirectoryIcb,
>>       File
>>       );
>>     if (EFI_ERROR (Status)) {
>> @@ -1465,7 +1372,7 @@ FindFileEntry (
>>     UINT32      LogicalBlockSize;
>>
>>     Lsn               = GetLongAdLsn (Volume, Icb);
>> -  LogicalBlockSize  = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
>> +  LogicalBlockSize  = Volume->LogicalVolDesc.LogicalBlockSize;
>>
>>     *FileEntry = AllocateZeroPool (Volume->FileEntrySize);
>>     if (*FileEntry == NULL) {
>> @@ -1959,43 +1866,6 @@ Error_Find_File:
>>   }
>>
>>   /**
>> -  Clean up in-memory UDF volume information.
>> -
>> -  @param[in] Volume Volume information pointer.
>> -
>> -**/
>> -VOID
>> -CleanupVolumeInformation (
>> -  IN UDF_VOLUME_INFO *Volume
>> -  )
>> -{
>> -  UINTN Index;
>> -
>> -  if (Volume->LogicalVolDescs != NULL) {
>> -    for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
>> -      FreePool ((VOID *)Volume->LogicalVolDescs[Index]);
>> -    }
>> -    FreePool ((VOID *)Volume->LogicalVolDescs);
>> -  }
>> -
>> -  if (Volume->PartitionDescs != NULL) {
>> -    for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
>> -      FreePool ((VOID *)Volume->PartitionDescs[Index]);
>> -    }
>> -    FreePool ((VOID *)Volume->PartitionDescs);
>> -  }
>> -
>> -  if (Volume->FileSetDescs != NULL) {
>> -    for (Index = 0; Index < Volume->FileSetDescsNo; Index++) {
>> -      FreePool ((VOID *)Volume->FileSetDescs[Index]);
>> -    }
>> -    FreePool ((VOID *)Volume->FileSetDescs);
>> -  }
>> -
>> -  ZeroMem ((VOID *)Volume, sizeof (UDF_VOLUME_INFO)); -}
>> -
>> -/**
>>     Clean up in-memory UDF file information.
>>
>>     @param[in] File File information pointer.
>> @@ -2249,91 +2119,100 @@ GetVolumeSize (
>>     OUT  UINT64                 *FreeSpaceSize
>>     )
>>   {
>> -  UDF_EXTENT_AD                 ExtentAd;
>> -  UINT32                        LogicalBlockSize;
>> -  UINT64                        Lsn;
>> -  EFI_STATUS                    Status;
>> -  UDF_LOGICAL_VOLUME_INTEGRITY  *LogicalVolInt;
>> -  UINTN                         Index;
>> -  UINTN                         Length;
>> -  UINT32                        LsnsNo;
>> -
>> -  *VolumeSize     = 0;
>> -  *FreeSpaceSize  = 0;
>> -
>> -  for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
>> -    CopyMem ((VOID *)&ExtentAd,
>> -             (VOID *)&Volume->LogicalVolDescs[Index]-
>>> IntegritySequenceExtent,
>> -             sizeof (UDF_EXTENT_AD));
>> -    if (ExtentAd.ExtentLength == 0) {
>> -      continue;
>> -    }
>> +  EFI_STATUS                     Status;
>> +  UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc;
>> +  UDF_EXTENT_AD                  *ExtentAd;
>> +  UINT64                         Lsn;
>> +  UINT32                         LogicalBlockSize;
>> +  UDF_LOGICAL_VOLUME_INTEGRITY   *LogicalVolInt;
>> +  UINTN                          Index;
>> +  UINTN                          Length;
>> +  UINT32                         LsnsNo;
>>
>> -    LogicalBlockSize = LV_BLOCK_SIZE (Volume, Index);
>> +  LogicalVolDesc = &Volume->LogicalVolDesc;
>>
>> -  Read_Next_Sequence:
>> -    LogicalVolInt = (UDF_LOGICAL_VOLUME_INTEGRITY *)
>> -      AllocatePool (ExtentAd.ExtentLength);
>> -    if (LogicalVolInt == NULL) {
>> -      return EFI_OUT_OF_RESOURCES;
>> -    }
>> +  ExtentAd = &LogicalVolDesc->IntegritySequenceExtent;
>>
>> -    Lsn = (UINT64)ExtentAd.ExtentLocation;
>> +  if (ExtentAd->ExtentLength == 0) {
>> +    return EFI_VOLUME_CORRUPTED;
>> +  }
>>
>> -    Status = DiskIo->ReadDisk (
>> -      DiskIo,
>> -      BlockIo->Media->MediaId,
>> -      MultU64x32 (Lsn, LogicalBlockSize),
>> -      ExtentAd.ExtentLength,
>> -      (VOID *)LogicalVolInt
>> -      );
>> -    if (EFI_ERROR (Status)) {
>> -      FreePool ((VOID *)LogicalVolInt);
>> -      return Status;
>> -    }
>> +  LogicalVolInt = AllocatePool (ExtentAd->ExtentLength);  if
>> + (LogicalVolInt == NULL) {
>> +    return EFI_OUT_OF_RESOURCES;
>> +  }
>>
>> -    if (!IS_LVID (LogicalVolInt)) {
>> -      FreePool ((VOID *)LogicalVolInt);
>> -      return EFI_VOLUME_CORRUPTED;
>> -    }
>> +  //
>> +  // Get location of Logical Volume Integrity Descriptor  //  Lsn =
>> + (UINT64)ExtentAd->ExtentLocation - Volume->MainVdsStartLocation;
>>
>> -    Length = LogicalVolInt->NumberOfPartitions;
>> -    for (Index = 0; Index < Length; Index += sizeof (UINT32)) {
>> -      LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
>> -      if (LsnsNo == 0xFFFFFFFFUL) {
>> -        //
>> -        // Size not specified.
>> -        //
>> -        continue;
>> -      }
>> +  LogicalBlockSize = LogicalVolDesc->LogicalBlockSize;
>>
>> -      *FreeSpaceSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
>> -    }
>> +  //
>> +  // Read disk block
>> +  //
>> +  Status = DiskIo->ReadDisk (
>> +    DiskIo,
>> +    BlockIo->Media->MediaId,
>> +    MultU64x32 (Lsn, LogicalBlockSize),
>> +    ExtentAd->ExtentLength,
>> +    LogicalVolInt
>> +    );
>> +  if (EFI_ERROR (Status)) {
>> +    goto Out_Free;
>> +  }
>>
>> -    Length = (LogicalVolInt->NumberOfPartitions * sizeof (UINT32)) << 1;
>> -    for (; Index < Length; Index += sizeof (UINT32)) {
>> -      LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
>> -      if (LsnsNo == 0xFFFFFFFFUL) {
>> -        //
>> -        // Size not specified.
>> -        //
>> -        continue;
>> -      }
>> +  //
>> +  // Check if read block is a Logical Volume Integrity Descriptor  //
>> + if (!IS_LVID (LogicalVolInt)) {
>> +    Status = EFI_VOLUME_CORRUPTED;
>> +    goto Out_Free;
>> +  }
>>
>> -      *VolumeSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
>> -    }
>> +  *VolumeSize = 0;
>> +  *FreeSpaceSize = 0;
>>
>> -    CopyMem ((VOID *)&ExtentAd,(VOID *)&LogicalVolInt-
>>> NextIntegrityExtent,
>> -             sizeof (UDF_EXTENT_AD));
>> -    if (ExtentAd.ExtentLength > 0) {
>> -      FreePool ((VOID *)LogicalVolInt);
>> -      goto Read_Next_Sequence;
>> +  Length = LogicalVolInt->NumberOfPartitions;
>> +  for (Index = 0; Index < Length; Index += sizeof (UINT32)) {
>> +    LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
>> +    //
>> +    // Check if size is not specified
>> +    //
>> +    if (LsnsNo == 0xFFFFFFFFUL) {
>> +      continue;
>>       }
>> +    //
>> +    // Accumulate free space size
>> +    //
>> +    *FreeSpaceSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);  }
>>
>> -    FreePool ((VOID *)LogicalVolInt);
>> +  Length = LogicalVolInt->NumberOfPartitions * sizeof (UINT32) * 2;
>> + for (; Index < Length; Index += sizeof (UINT32)) {
>> +    LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
>> +    //
>> +    // Check if size is not specified
>> +    //
>> +    if (LsnsNo == 0xFFFFFFFFUL) {
>> +      continue;
>> +    }
>> +    //
>> +    // Accumulate used volume space
>> +    //
>> +    *VolumeSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
>>     }
>>
>> -  return EFI_SUCCESS;
>> +  Status = EFI_SUCCESS;
>> +
>> +Out_Free:
>> +  //
>> +  // Free Logical Volume Integrity Descriptor
>> +  //
>> +  FreePool (LogicalVolInt);
>> +
>> +  return Status;
>>   }
>>
>>   /**
>> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
>> b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
>> index 49dc7077b7..d4163b89ca 100644
>> --- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
>> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
>> @@ -276,13 +276,6 @@ UdfDriverBindingStop (
>>         NULL
>>         );
>>
>> -    //
>> -    // Check if there's any open file. If so, clean them up.
>> -    //
>> -    if (PrivFsData->OpenFiles > 0) {
>> -      CleanupVolumeInformation (&PrivFsData->Volume);
>> -    }
>> -
>>       FreePool ((VOID *)PrivFsData);
>>     }
>>
>> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
>> b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
>> index 240d420ff5..c5f83914d8 100644
>> --- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
>> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
>> @@ -49,16 +49,8 @@
>>       { 0x89, 0x56, 0x73, 0xCD, 0xA3, 0x26, 0xCD, 0x0A }  \
>>     }
>>
>> -#define UDF_DEFAULT_LV_NUM 0
>> -
>>   #define IS_PVD(_Pointer) \
>>     ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 1)) -#define IS_PD(_Pointer) \
>> -  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5)) -#define IS_LVD(_Pointer) \
>> -  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6)) -#define IS_TD(_Pointer) \
>> -  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))  #define IS_FSD(_Pointer) \
>>     ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 256))  #define IS_FE(_Pointer) \
>> @@ -152,14 +144,8 @@ typedef enum {  #define
>> IS_VALID_COMPRESSION_ID(_CompId) \
>>     ((BOOLEAN)((_CompId) == 8 || (_CompId) == 16))
>>
>> -#define LV_BLOCK_SIZE(_Vol, _LvNum) \
>> -  (_Vol)->LogicalVolDescs[(_LvNum)]->LogicalBlockSize
>> -
>>   #define UDF_STANDARD_IDENTIFIER_LENGTH   5
>>
>> -#define LV_UDF_REVISION(_Lv) \
>> -  *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix
>> -
>>   #pragma pack(1)
>>
>>   typedef struct {
>> @@ -186,17 +172,6 @@ typedef struct {
>>   #pragma pack(1)
>>
>>   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 {
>>     UINT16          TypeAndTimezone;
>>     INT16           Year;
>>     UINT8           Month;
>> @@ -210,17 +185,6 @@ typedef struct {
>>   } UDF_TIMESTAMP;
>>
>>   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;
>>     UINT32                             PrevAllocationExtentDescriptor;
>>     UINT32                             LengthOfAllocationDescriptors;
>> @@ -235,37 +199,6 @@ typedef struct {
>>   } UDF_VOLUME_DESCRIPTOR;
>>
>>   typedef struct {
>> -  UDF_DESCRIPTOR_TAG         DescriptorTag;
>> -  UINT32                     VolumeDescriptorSequenceNumber;
>> -  UINT16                     PartitionFlags;
>> -  UINT16                     PartitionNumber;
>> -  UDF_ENTITY_ID              PartitionContents;
>> -  UINT8                      PartitionContentsUse[128];
>> -  UINT32                     AccessType;
>> -  UINT32                     PartitionStartingLocation;
>> -  UINT32                     PartitionLength;
>> -  UDF_ENTITY_ID              ImplementationIdentifier;
>> -  UINT8                      ImplementationUse[128];
>> -  UINT8                      Reserved[156];
>> -} UDF_PARTITION_DESCRIPTOR;
>> -
>> -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;
>> -
>> -typedef struct {
>>     UDF_DESCRIPTOR_TAG             DescriptorTag;
>>     UDF_TIMESTAMP                  RecordingDateTime;
>>     UINT32                         IntegrityType;
>> @@ -389,12 +322,10 @@ typedef struct {
>>   // UDF filesystem driver's private data  //  typedef struct {
>> -  UDF_LOGICAL_VOLUME_DESCRIPTOR  **LogicalVolDescs;
>> -  UINTN                          LogicalVolDescsNo;
>> -  UDF_PARTITION_DESCRIPTOR       **PartitionDescs;
>> -  UINTN                          PartitionDescsNo;
>> -  UDF_FILE_SET_DESCRIPTOR        **FileSetDescs;
>> -  UINTN                          FileSetDescsNo;
>> +  UINT64                         MainVdsStartLocation;
>> +  UDF_LOGICAL_VOLUME_DESCRIPTOR  LogicalVolDesc;
>> +  UDF_PARTITION_DESCRIPTOR       PartitionDesc;
>> +  UDF_FILE_SET_DESCRIPTOR        FileSetDesc;
>>     UINTN                          FileEntrySize;
>>   } UDF_VOLUME_INFO;
>>
>> @@ -883,17 +814,6 @@ ResolveSymlink (
>>     );
>>
>>   /**
>> -  Clean up in-memory UDF volume information.
>> -
>> -  @param[in] Volume Volume information pointer.
>> -
>> -**/
>> -VOID
>> -CleanupVolumeInformation (
>> -  IN UDF_VOLUME_INFO *Volume
>> -  );
>> -
>> -/**
>>     Clean up in-memory UDF file information.
>>
>>     @param[in] File File information pointer.
>> --
>> 2.11.0
> 
> 

Re: [PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to support PartitionDxe changes

[Ni, Ruiyu]

OK I understood the reason now.

I tried to understand the relationship between AVDP, LVD, and PD, but failed.
The code below in Partition driver seems to say the LVD and PD are close to each other.
Is it always true? If it is, I agree with the implementation.

    //
    // Cover the main VDS area so UdfDxe driver will also be able to get LVD and
    // PD descriptors out from the file system.
    //
    *MainVdsStartLsn = GuardMainVdsStartLsn;
    *LogicalVolEndLsn = *MainVdsStartLsn + (UINT64)ExtentAd->ExtentLength;

    //
    // Cover UDF partition area
    //
    *LogicalVolEndLsn +=
      ((UINT64)PartitionDesc->PartitionStartingLocation -
       *LogicalVolEndLsn) + PartitionDesc->PartitionLength - 1;
Thanks/Ray

> -----Original Message-----
> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
> Sent: Tuesday, September 19, 2017 12:51 AM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>;
> Laszlo Ersek <lersek@redhat.com>
> Subject: Re: [PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to
> support PartitionDxe changes
> 
> 
> 
> On 9/17/2017 10:00 PM, Ni, Ruiyu wrote:
> > Paulo,
> > With the change in partition driver, I suppose UdfDxe driver only
> > needs to take care of area covered by the partition descriptor.
> > But why StartMainVolumeDescriptorSequence() still reads LVD, TD, and
> > PD?
> 
> We still need the LVD again for a few reasons:
> 
> 1. GetVolumeSize() needs to read LVID (Logical Volume Integrity Descriptor),
> which is located in LVD, in order to calculate free and used volume space.
> 2. To find FSD (File Set Descriptor), where the root directory is located.
> 3. To know which Partition Descriptor to use based on partition number from
> LVD->PartitionMaps field. I know UDF 2.60 spec says that there should be
> only one Partition Descriptor that covers the entire space of a single LVD, but
> it's good to make sure we're reading it correctly.
> 
> TD descriptor is used to know when stopping the Volume Descriptor
> Sequence and then we don't need to read all ExtentAd->ExtentLength blocks
> unnecessarily.
> 
> Thanks,
> Paulo
> 
> >
> > I thought the UdfDxe driver's logic can be simplified a lot.
> > There should be no duplicated logic in Partition driver and udf driver.
> >
> > Can you explain more?
> >
> > Thanks/Ray
> >
> >> -----Original Message-----
> >> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
> >> Sent: Sunday, September 17, 2017 9:13 PM
> >> To: edk2-devel@lists.01.org
> >> Cc: Paulo Alcantara <pcacjr@zytor.com>; Dong, Eric
> >> <eric.dong@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star
> >> <star.zeng@intel.com>; Laszlo Ersek <lersek@redhat.com>
> >> Subject: [PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to
> support
> >> PartitionDxe changes
> >>
> >> This patch reworks the driver to support Partition driver changes.
> >>
> >> Cc: Eric Dong <eric.dong@intel.com>
> >> Cc: Paulo Alcantara <pcacjr@zytor.com>
> >> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> >> Cc: Star Zeng <star.zeng@intel.com>
> >> Cc: Laszlo Ersek <lersek@redhat.com>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
> >> ---
> >>   MdeModulePkg/Universal/Disk/UdfDxe/File.c                 |  13 +-
> >>   MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 515
> >> ++++++++------------
> >>   MdeModulePkg/Universal/Disk/UdfDxe/Udf.c                  |   7 -
> >>   MdeModulePkg/Universal/Disk/UdfDxe/Udf.h                  |  88 +---
> >>   4 files changed, 204 insertions(+), 419 deletions(-)
> >>
> >> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> >> b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> >> index 01361141bb..f2c62967e8 100644
> >> --- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> >> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> >> @@ -100,6 +100,7 @@ UdfOpenVolume (
> >>       &PrivFsData->Volume,
> >>       &PrivFsData->Root
> >>       );
> >> +  ASSERT_EFI_ERROR (Status);
> >>     if (EFI_ERROR (Status)) {
> >>       goto Error_Find_Root_Dir;
> >>     }
> >> @@ -131,7 +132,6 @@ Error_Alloc_Priv_File_Data:
> >>     CleanupFileInformation (&PrivFsData->Root);
> >>
> >>   Error_Find_Root_Dir:
> >> -  CleanupVolumeInformation (&PrivFsData->Volume);
> >>
> >>   Error_Read_Udf_Volume:
> >>   Error_Invalid_Params:
> >> @@ -528,7 +528,6 @@ UdfClose (
> >>     EFI_TPL                     OldTpl;
> >>     EFI_STATUS                  Status;
> >>     PRIVATE_UDF_FILE_DATA       *PrivFileData;
> >> -  PRIVATE_UDF_SIMPLE_FS_DATA  *PrivFsData;
> >>
> >>     OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> >>
> >> @@ -541,8 +540,6 @@ UdfClose (
> >>
> >>     PrivFileData = PRIVATE_UDF_FILE_DATA_FROM_THIS (This);
> >>
> >> -  PrivFsData = PRIVATE_UDF_SIMPLE_FS_DATA_FROM_THIS
> (PrivFileData-
> >>> SimpleFs);
> >> -
> >>     if (!PrivFileData->IsRootDirectory) {
> >>       CleanupFileInformation (&PrivFileData->File);
> >>
> >> @@ -551,10 +548,6 @@ UdfClose (
> >>       }
> >>     }
> >>
> >> -  if (--PrivFsData->OpenFiles == 0) {
> >> -    CleanupVolumeInformation (&PrivFsData->Volume);
> >> -  }
> >> -
> >>     FreePool ((VOID *)PrivFileData);
> >>
> >>   Exit:
> >> @@ -787,7 +780,7 @@ UdfGetInfo (
> >>     } else if (CompareGuid (InformationType, &gEfiFileSystemInfoGuid)) {
> >>       String = VolumeLabel;
> >>
> >> -    FileSetDesc = PrivFsData->Volume.FileSetDescs[0];
> >> +    FileSetDesc = &PrivFsData->Volume.FileSetDesc;
> >>
> >>       OstaCompressed = &FileSetDesc->LogicalVolumeIdentifier[0];
> >>
> >> @@ -846,7 +839,7 @@ UdfGetInfo (
> >>       FileSystemInfo->Size        = FileSystemInfoLength;
> >>       FileSystemInfo->ReadOnly    = TRUE;
> >>       FileSystemInfo->BlockSize   =
> >> -      LV_BLOCK_SIZE (&PrivFsData->Volume, UDF_DEFAULT_LV_NUM);
> >> +      PrivFsData->Volume.LogicalVolDesc.LogicalBlockSize;
> >>       FileSystemInfo->VolumeSize  = VolumeSize;
> >>       FileSystemInfo->FreeSpace   = FreeSpaceSize;
> >>
> >> diff --git
> >> a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> >> b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> >> index 4609580b30..63b643e60a 100644
> >> --- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> >> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> >> @@ -60,154 +60,111 @@ FindAnchorVolumeDescriptorPointer (
> >>
> >>   EFI_STATUS
> >>   StartMainVolumeDescriptorSequence (
> >> -  IN   EFI_BLOCK_IO_PROTOCOL                 *BlockIo,
> >> -  IN   EFI_DISK_IO_PROTOCOL                  *DiskIo,
> >> -  IN   UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  *AnchorPoint,
> >> -  OUT  UDF_VOLUME_INFO                       *Volume
> >> +  IN   EFI_BLOCK_IO_PROTOCOL    *BlockIo,
> >> +  IN   EFI_DISK_IO_PROTOCOL     *DiskIo,
> >> +  OUT  UDF_VOLUME_INFO          *Volume
> >>     )
> >>   {
> >> -  EFI_STATUS                     Status;
> >> -  UINT32                         BlockSize;
> >> -  UDF_EXTENT_AD                  *ExtentAd;
> >> -  UINT64                         StartingLsn;
> >> -  UINT64                         EndingLsn;
> >> -  VOID                           *Buffer;
> >> -  UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc;
> >> -  UDF_PARTITION_DESCRIPTOR       *PartitionDesc;
> >> -  UINTN                          Index;
> >> -  UINT32                         LogicalBlockSize;
> >> +  EFI_STATUS  Status;
> >> +  UINT32      BlockSize;
> >> +  UINT64      BlockOffset;
> >> +  VOID        *Buffer;
> >> +  UINT32      LogicalBlockSize;
> >> +
> >> +  BlockSize = BlockIo->Media->BlockSize;
> >>
> >>     //
> >> -  // We've already found an ADVP on the volume. It contains the
> >> extent
> >> -  // (MainVolumeDescriptorSequenceExtent) where the Main Volume
> >> Descriptor
> >> -  // Sequence starts. Therefore, we'll look for Logical Volume
> >> Descriptors and
> >> -  // Partitions Descriptors and save them in memory, accordingly.
> >> -  //
> >> -  // Note also that each descriptor will be aligned on a block size
> >> (BlockSize)
> >> -  // boundary, so we need to read one block at a time.
> >> +  // Allocate buffer for reading disk blocks
> >>     //
> >> -  BlockSize    = BlockIo->Media->BlockSize;
> >> -  ExtentAd     = &AnchorPoint->MainVolumeDescriptorSequenceExtent;
> >> -  StartingLsn  = (UINT64)ExtentAd->ExtentLocation;
> >> -  EndingLsn    = StartingLsn + DivU64x32 (
> >> -                                     (UINT64)ExtentAd->ExtentLength,
> >> -                                     BlockSize
> >> -                                     );
> >> -
> >> -  Volume->LogicalVolDescs =
> >> -    (UDF_LOGICAL_VOLUME_DESCRIPTOR **)AllocateZeroPool
> (ExtentAd-
> >>> ExtentLength);
> >> -  if (Volume->LogicalVolDescs == NULL) {
> >> -    return EFI_OUT_OF_RESOURCES;
> >> -  }
> >> -
> >> -  Volume->PartitionDescs =
> >> -    (UDF_PARTITION_DESCRIPTOR **)AllocateZeroPool (ExtentAd-
> >>> ExtentLength);
> >> -  if (Volume->PartitionDescs == NULL) {
> >> -    Status = EFI_OUT_OF_RESOURCES;
> >> -    goto Error_Alloc_Pds;
> >> -  }
> >> -
> >> -  Buffer = AllocateZeroPool (BlockSize);
> >> +  Buffer = AllocateZeroPool ((UINTN)BlockSize);
> >>     if (Buffer == NULL) {
> >> -    Status = EFI_OUT_OF_RESOURCES;
> >> -    goto Error_Alloc_Buf;
> >> +    return EFI_OUT_OF_RESOURCES;
> >>     }
> >>
> >> -  Volume->LogicalVolDescsNo  = 0;
> >> -  Volume->PartitionDescsNo   = 0;
> >> -
> >> -  while (StartingLsn <= EndingLsn) {
> >> +  //
> >> +  // As per UDF 2.60 specification:
> >> +  //
> >> +  // ---
> >> +  // For the purpose of interchange, Partition Maps shall be limited
> >> + to // Partition Map type 1, except type 2 maps.
> >> +  // ---
> >> +  // A Partition Descriptor Access Type of read-only, rewritable,
> >> + // overwritable, write-once and pseudo-overwritable shall be  //
> >> + supported. There shall be exactly one prevailing Partition  //
> >> + Descriptor recorded per volume, with one exception. For Volume  //
> >> + Sets that consist of single volume, the volume may contain 2 non-
> >> + // overlapping Partitions with 2 prevailing Partition Descriptors
> >> + only // if one has an Access Type of read-only and the other has an
> >> + // Access Type of rewritable, overwritable, or write-once. The  //
> >> + Logical Volume for this volume would consist of the contents of  //
> >> + both partitions.
> >> +  // ---
> >> +  //
> >> +  // The logical partition created by Partition driver is relative
> >> + to the main  // VDS extent location, so we start the Main Volume
> >> + Descriptor Sequence at  // LBA 0.
> >> +  //
> >> +  // We don't need to make sure we got both LVD and PD -- that is,
> >> + if we are  // here, that means Partition driver was able to find
> >> + them both
> >> previously.
> >> +  //
> >> +  for (BlockOffset = 0;
> >> +       BlockOffset < MultU64x32 (BlockIo->Media->LastBlock,
> >> +                                 BlockIo->Media->BlockSize);
> >> +       BlockOffset += BlockSize) {
> >> +    // Read disk block
> >> +    //
> >>       Status = DiskIo->ReadDisk (
> >>         DiskIo,
> >>         BlockIo->Media->MediaId,
> >> -      MultU64x32 (StartingLsn, BlockSize),
> >> +      BlockOffset,
> >>         BlockSize,
> >>         Buffer
> >>         );
> >>       if (EFI_ERROR (Status)) {
> >> -      goto Error_Read_Disk_Blk;
> >> +      goto Out_Free;
> >>       }
> >>
> >> +    //
> >> +    // Check if read block is a Terminating Descriptor
> >> +    //
> >>       if (IS_TD (Buffer)) {
> >>         //
> >> -      // Found a Terminating Descriptor. Stop the sequence then.
> >> +      // Terminate Main Volume Descriptor Sequence
> >>         //
> >>         break;
> >>       }
> >>
> >>       if (IS_LVD (Buffer)) {
> >>         //
> >> -      // Found a Logical Volume Descriptor.
> >> +      // Save Logical Volume Descriptor
> >>         //
> >> -      LogicalVolDesc =
> >> -        (UDF_LOGICAL_VOLUME_DESCRIPTOR *)
> >> -        AllocateZeroPool (sizeof (UDF_LOGICAL_VOLUME_DESCRIPTOR));
> >> -      if (LogicalVolDesc == NULL) {
> >> -        Status = EFI_OUT_OF_RESOURCES;
> >> -        goto Error_Alloc_Lvd;
> >> -      }
> >> -
> >> -      CopyMem ((VOID *)LogicalVolDesc, Buffer,
> >> -               sizeof (UDF_LOGICAL_VOLUME_DESCRIPTOR));
> >> -      Volume->LogicalVolDescs[Volume->LogicalVolDescsNo++] =
> >> LogicalVolDesc;
> >> +      CopyMem (&Volume->LogicalVolDesc, Buffer, sizeof
> >> + (Volume->LogicalVolDesc));
> >>       } else if (IS_PD (Buffer)) {
> >>         //
> >> -      // Found a Partition Descriptor.
> >> +      // Save Partition Descriptor
> >>         //
> >> -      PartitionDesc =
> >> -        (UDF_PARTITION_DESCRIPTOR *)
> >> -        AllocateZeroPool (sizeof (UDF_PARTITION_DESCRIPTOR));
> >> -      if (PartitionDesc == NULL) {
> >> -        Status = EFI_OUT_OF_RESOURCES;
> >> -        goto Error_Alloc_Pd;
> >> -      }
> >> -
> >> -      CopyMem ((VOID *)PartitionDesc, Buffer,
> >> -               sizeof (UDF_PARTITION_DESCRIPTOR));
> >> -      Volume->PartitionDescs[Volume->PartitionDescsNo++] =
> PartitionDesc;
> >> +      CopyMem (&Volume->PartitionDesc, Buffer, sizeof
> >> + (Volume->PartitionDesc));
> >>       }
> >> -
> >> -    StartingLsn++;
> >>     }
> >>
> >>     //
> >> -  // When an UDF volume (revision 2.00 or higher) contains a File
> >> Entry rather
> >> -  // than an Extended File Entry (which is not recommended as per
> >> spec), we need
> >> -  // to make sure the size of a FE will be _at least_ 2048
> >> -  // (UDF_LOGICAL_SECTOR_SIZE) bytes long to keep backward
> compatibility.
> >> +  // Determine FE (File Entry) size
> >>     //
> >> -  LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
> >> +  LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
> >>     if (LogicalBlockSize >= UDF_LOGICAL_SECTOR_SIZE) {
> >> -    Volume->FileEntrySize = LogicalBlockSize;
> >> +    Volume->FileEntrySize = (UINTN)LogicalBlockSize;
> >>     } else {
> >>       Volume->FileEntrySize = UDF_LOGICAL_SECTOR_SIZE;
> >>     }
> >>
> >> -  FreePool (Buffer);
> >> -
> >> -  return EFI_SUCCESS;
> >> -
> >> -Error_Alloc_Pd:
> >> -Error_Alloc_Lvd:
> >> -  for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
> >> -    FreePool ((VOID *)Volume->PartitionDescs[Index]);
> >> -  }
> >> -
> >> -  for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
> >> -    FreePool ((VOID *)Volume->LogicalVolDescs[Index]);
> >> -  }
> >> +  Status = EFI_SUCCESS;
> >>
> >> -Error_Read_Disk_Blk:
> >> +Out_Free:
> >> +  //
> >> +  // Free block read buffer
> >> +  //
> >>     FreePool (Buffer);
> >>
> >> -Error_Alloc_Buf:
> >> -  FreePool ((VOID *)Volume->PartitionDescs);
> >> -  Volume->PartitionDescs = NULL;
> >> -
> >> -Error_Alloc_Pds:
> >> -  FreePool ((VOID *)Volume->LogicalVolDescs);
> >> -  Volume->LogicalVolDescs = NULL;
> >> -
> >>     return Status;
> >>   }
> >>
> >> @@ -223,11 +180,9 @@ GetPdFromLongAd (
> >>     )
> >>   {
> >>     UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc;
> >> -  UINTN                          Index;
> >> -  UDF_PARTITION_DESCRIPTOR       *PartitionDesc;
> >>     UINT16                         PartitionNum;
> >>
> >> -  LogicalVolDesc = Volume->LogicalVolDescs[UDF_DEFAULT_LV_NUM];
> >> +  LogicalVolDesc = &Volume->LogicalVolDesc;
> >>
> >>     switch (LV_UDF_REVISION (LogicalVolDesc)) {
> >>     case 0x0102:
> >> @@ -252,19 +207,21 @@ GetPdFromLongAd (
> >>       PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc-
> >PartitionMaps[4]);
> >>       break;
> >>     case 0x0260:
> >> -    //
> >> -    // Fall through.
> >> -    //
> >> -  default:
> >>       PartitionNum = LongAd->ExtentLocation.PartitionReferenceNumber;
> >>       break;
> >> +  default:
> >> +    //
> >> +    // Unhandled UDF revision
> >> +    //
> >> +    return NULL;
> >>     }
> >>
> >> -  for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
> >> -    PartitionDesc = Volume->PartitionDescs[Index];
> >> -    if (PartitionDesc->PartitionNumber == PartitionNum) {
> >> -      return PartitionDesc;
> >> -    }
> >> +  //
> >> +  // Check if partition number matches Partition Descriptor found in
> >> + Main Volume  // Descriptor Sequence.
> >> +  //
> >> +  if (Volume->PartitionDesc.PartitionNumber == PartitionNum) {
> >> +    return &Volume->PartitionDesc;
> >>     }
> >>
> >>     return NULL;
> >> @@ -284,8 +241,9 @@ GetLongAdLsn (
> >>     PartitionDesc = GetPdFromLongAd (Volume, LongAd);
> >>     ASSERT (PartitionDesc != NULL);
> >>
> >> -  return (UINT64)PartitionDesc->PartitionStartingLocation +
> >> -                 LongAd->ExtentLocation.LogicalBlockNumber;
> >> +  return (UINT64)PartitionDesc->PartitionStartingLocation -
> >> +    Volume->MainVdsStartLocation +
> >> +    LongAd->ExtentLocation.LogicalBlockNumber;
> >>   }
> >>
> >>   //
> >> @@ -293,12 +251,13 @@ GetLongAdLsn (
> >>   //
> >>   UINT64
> >>   GetShortAdLsn (
> >> +  IN UDF_VOLUME_INFO                  *Volume,
> >>     IN UDF_PARTITION_DESCRIPTOR         *PartitionDesc,
> >>     IN UDF_SHORT_ALLOCATION_DESCRIPTOR  *ShortAd
> >>     )
> >>   {
> >> -  return (UINT64)PartitionDesc->PartitionStartingLocation +
> >> -    ShortAd->ExtentPosition;
> >> +  return (UINT64)PartitionDesc->PartitionStartingLocation -
> >> +    Volume->MainVdsStartLocation + ShortAd->ExtentPosition;
> >>   }
> >>
> >>   //
> >> @@ -311,36 +270,39 @@ EFI_STATUS
> >>   FindFileSetDescriptor (
> >>     IN   EFI_BLOCK_IO_PROTOCOL    *BlockIo,
> >>     IN   EFI_DISK_IO_PROTOCOL     *DiskIo,
> >> -  IN   UDF_VOLUME_INFO          *Volume,
> >> -  IN   UINTN                    LogicalVolDescNum,
> >> -  OUT  UDF_FILE_SET_DESCRIPTOR  *FileSetDesc
> >> +  IN   UDF_VOLUME_INFO          *Volume
> >>     )
> >>   {
> >>     EFI_STATUS                     Status;
> >>     UINT64                         Lsn;
> >>     UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc;
> >>
> >> -  LogicalVolDesc = Volume->LogicalVolDescs[LogicalVolDescNum];
> >> +  LogicalVolDesc = &Volume->LogicalVolDesc;
> >>     Lsn = GetLongAdLsn (Volume, &LogicalVolDesc-
> >>> LogicalVolumeContentsUse);
> >>
> >>     //
> >> -  // Read extent (Long Ad).
> >> +  // As per UDF 2.60 specification:
> >> +  //
> >> +  // There shall be exactly one File Set Descriptor recorded per
> >> + Logical  // Volume.
> >> +  //
> >> +  // Read disk block
> >>     //
> >>     Status = DiskIo->ReadDisk (
> >>       DiskIo,
> >>       BlockIo->Media->MediaId,
> >>       MultU64x32 (Lsn, LogicalVolDesc->LogicalBlockSize),
> >> -    sizeof (UDF_FILE_SET_DESCRIPTOR),
> >> -    (VOID *)FileSetDesc
> >> +    sizeof (Volume->FileSetDesc),
> >> +    &Volume->FileSetDesc
> >>       );
> >>     if (EFI_ERROR (Status)) {
> >>       return Status;
> >>     }
> >>
> >>     //
> >> -  // Check if the read extent contains a valid FSD's tag identifier.
> >> +  // Check if read block is a File Set Descriptor
> >>     //
> >> -  if (!IS_FSD (FileSetDesc)) {
> >> +  if (!IS_FSD (&Volume->FileSetDesc)) {
> >>       return EFI_VOLUME_CORRUPTED;
> >>     }
> >>
> >> @@ -348,71 +310,6 @@ FindFileSetDescriptor (  }
> >>
> >>   //
> >> -// Get all File Set Descriptors for each Logical Volume Descriptor.
> >> -//
> >> -EFI_STATUS
> >> -GetFileSetDescriptors (
> >> -  IN      EFI_BLOCK_IO_PROTOCOL  *BlockIo,
> >> -  IN      EFI_DISK_IO_PROTOCOL   *DiskIo,
> >> -  IN OUT  UDF_VOLUME_INFO        *Volume
> >> -  )
> >> -{
> >> -  EFI_STATUS               Status;
> >> -  UINTN                    Index;
> >> -  UDF_FILE_SET_DESCRIPTOR  *FileSetDesc;
> >> -  UINTN                    Count;
> >> -
> >> -  Volume->FileSetDescs =
> >> -    (UDF_FILE_SET_DESCRIPTOR **)AllocateZeroPool (
> >> -      Volume->LogicalVolDescsNo * sizeof (UDF_FILE_SET_DESCRIPTOR));
> >> -  if (Volume->FileSetDescs == NULL) {
> >> -    return EFI_OUT_OF_RESOURCES;
> >> -  }
> >> -
> >> -  for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
> >> -    FileSetDesc = AllocateZeroPool (sizeof (UDF_FILE_SET_DESCRIPTOR));
> >> -    if (FileSetDesc == NULL) {
> >> -      Status = EFI_OUT_OF_RESOURCES;
> >> -      goto Error_Alloc_Fsd;
> >> -    }
> >> -
> >> -    //
> >> -    // Find a FSD for this LVD.
> >> -    //
> >> -    Status = FindFileSetDescriptor (
> >> -      BlockIo,
> >> -      DiskIo,
> >> -      Volume,
> >> -      Index,
> >> -      FileSetDesc
> >> -      );
> >> -    if (EFI_ERROR (Status)) {
> >> -      goto Error_Find_Fsd;
> >> -    }
> >> -
> >> -    //
> >> -    // Got one. Save it.
> >> -    //
> >> -    Volume->FileSetDescs[Index] = FileSetDesc;
> >> -  }
> >> -
> >> -  Volume->FileSetDescsNo = Volume->LogicalVolDescsNo;
> >> -  return EFI_SUCCESS;
> >> -
> >> -Error_Find_Fsd:
> >> -  Count = Index + 1;
> >> -  for (Index = 0; Index < Count; Index++) {
> >> -    FreePool ((VOID *)Volume->FileSetDescs[Index]);
> >> -  }
> >> -
> >> -  FreePool ((VOID *)Volume->FileSetDescs);
> >> -  Volume->FileSetDescs = NULL;
> >> -
> >> -Error_Alloc_Fsd:
> >> -  return Status;
> >> -}
> >> -
> >> -//
> >>   // Read Volume and File Structure on an UDF file system.
> >>   //
> >>   EFI_STATUS
> >> @@ -424,9 +321,10 @@ ReadVolumeFileStructure (  {
> >>     EFI_STATUS                            Status;
> >>     UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  AnchorPoint;
> >> +  UDF_EXTENT_AD                         *ExtentAd;
> >>
> >>     //
> >> -  // Find an AVDP.
> >> +  // Find Anchor Volume Descriptor Pointer
> >>     //
> >>     Status = FindAnchorVolumeDescriptorPointer (
> >>       BlockIo,
> >> @@ -438,12 +336,18 @@ ReadVolumeFileStructure (
> >>     }
> >>
> >>     //
> >> -  // AVDP has been found. Start MVDS.
> >> +  // Save Main VDS start block number  //  ExtentAd =
> >> + &AnchorPoint.MainVolumeDescriptorSequenceExtent;
> >> +
> >> +  Volume->MainVdsStartLocation = (UINT64)ExtentAd->ExtentLocation;
> >> +
> >> +  //
> >> +  // Start Main Volume Descriptor Sequence.
> >>     //
> >>     Status = StartMainVolumeDescriptorSequence (
> >>       BlockIo,
> >>       DiskIo,
> >> -    &AnchorPoint,
> >>       Volume
> >>       );
> >>     if (EFI_ERROR (Status)) {
> >> @@ -699,6 +603,7 @@ GetAllocationDescriptorLsn (
> >>       return GetLongAdLsn (Volume,
> (UDF_LONG_ALLOCATION_DESCRIPTOR
> >> *)Ad);
> >>     } else if (RecordingFlags == SHORT_ADS_SEQUENCE) {
> >>       return GetShortAdLsn (
> >> +      Volume,
> >>         GetPdFromLongAd (Volume, ParentIcb),
> >>         (UDF_SHORT_ALLOCATION_DESCRIPTOR *)Ad
> >>         );
> >> @@ -740,7 +645,7 @@ GetAedAdsOffset (
> >>       return EFI_OUT_OF_RESOURCES;
> >>     }
> >>
> >> -  LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
> >> +  LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
> >>
> >>     //
> >>     // Read extent.
> >> @@ -890,7 +795,7 @@ ReadFile (
> >>     UINT32                  ExtentLength;
> >>     UDF_FE_RECORDING_FLAGS  RecordingFlags;
> >>
> >> -  LogicalBlockSize  = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
> >> +  LogicalBlockSize  = Volume->LogicalVolDesc.LogicalBlockSize;
> >>     DoFreeAed         = FALSE;
> >>
> >>     //
> >> @@ -1355,6 +1260,9 @@ ReadUdfVolumeInformation (  {
> >>     EFI_STATUS Status;
> >>
> >> +  //
> >> +  // Read all necessary UDF volume information and keep it private
> >> + to the driver  //
> >>     Status = ReadVolumeFileStructure (
> >>       BlockIo,
> >>       DiskIo,
> >> @@ -1364,13 +1272,12 @@ ReadUdfVolumeInformation (
> >>       return Status;
> >>     }
> >>
> >> -  Status = GetFileSetDescriptors (
> >> -    BlockIo,
> >> -    DiskIo,
> >> -    Volume
> >> -    );
> >> +  //
> >> +  // Find File Set Descriptor
> >> +  //
> >> +  Status = FindFileSetDescriptor (BlockIo, DiskIo, Volume);
> >>     if (EFI_ERROR (Status)) {
> >> -    CleanupVolumeInformation (Volume);
> >> +    return Status;
> >>     }
> >>
> >>     return Status;
> >> @@ -1407,7 +1314,7 @@ FindRootDirectory (
> >>       BlockIo,
> >>       DiskIo,
> >>       Volume,
> >> -    &Volume->FileSetDescs[0]->RootDirectoryIcb,
> >> +    &Volume->FileSetDesc.RootDirectoryIcb,
> >>       &File->FileEntry
> >>       );
> >>     if (EFI_ERROR (Status)) {
> >> @@ -1424,7 +1331,7 @@ FindRootDirectory (
> >>       L"\\",
> >>       NULL,
> >>       &Parent,
> >> -    &Volume->FileSetDescs[0]->RootDirectoryIcb,
> >> +    &Volume->FileSetDesc.RootDirectoryIcb,
> >>       File
> >>       );
> >>     if (EFI_ERROR (Status)) {
> >> @@ -1465,7 +1372,7 @@ FindFileEntry (
> >>     UINT32      LogicalBlockSize;
> >>
> >>     Lsn               = GetLongAdLsn (Volume, Icb);
> >> -  LogicalBlockSize  = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
> >> +  LogicalBlockSize  = Volume->LogicalVolDesc.LogicalBlockSize;
> >>
> >>     *FileEntry = AllocateZeroPool (Volume->FileEntrySize);
> >>     if (*FileEntry == NULL) {
> >> @@ -1959,43 +1866,6 @@ Error_Find_File:
> >>   }
> >>
> >>   /**
> >> -  Clean up in-memory UDF volume information.
> >> -
> >> -  @param[in] Volume Volume information pointer.
> >> -
> >> -**/
> >> -VOID
> >> -CleanupVolumeInformation (
> >> -  IN UDF_VOLUME_INFO *Volume
> >> -  )
> >> -{
> >> -  UINTN Index;
> >> -
> >> -  if (Volume->LogicalVolDescs != NULL) {
> >> -    for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
> >> -      FreePool ((VOID *)Volume->LogicalVolDescs[Index]);
> >> -    }
> >> -    FreePool ((VOID *)Volume->LogicalVolDescs);
> >> -  }
> >> -
> >> -  if (Volume->PartitionDescs != NULL) {
> >> -    for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
> >> -      FreePool ((VOID *)Volume->PartitionDescs[Index]);
> >> -    }
> >> -    FreePool ((VOID *)Volume->PartitionDescs);
> >> -  }
> >> -
> >> -  if (Volume->FileSetDescs != NULL) {
> >> -    for (Index = 0; Index < Volume->FileSetDescsNo; Index++) {
> >> -      FreePool ((VOID *)Volume->FileSetDescs[Index]);
> >> -    }
> >> -    FreePool ((VOID *)Volume->FileSetDescs);
> >> -  }
> >> -
> >> -  ZeroMem ((VOID *)Volume, sizeof (UDF_VOLUME_INFO)); -}
> >> -
> >> -/**
> >>     Clean up in-memory UDF file information.
> >>
> >>     @param[in] File File information pointer.
> >> @@ -2249,91 +2119,100 @@ GetVolumeSize (
> >>     OUT  UINT64                 *FreeSpaceSize
> >>     )
> >>   {
> >> -  UDF_EXTENT_AD                 ExtentAd;
> >> -  UINT32                        LogicalBlockSize;
> >> -  UINT64                        Lsn;
> >> -  EFI_STATUS                    Status;
> >> -  UDF_LOGICAL_VOLUME_INTEGRITY  *LogicalVolInt;
> >> -  UINTN                         Index;
> >> -  UINTN                         Length;
> >> -  UINT32                        LsnsNo;
> >> -
> >> -  *VolumeSize     = 0;
> >> -  *FreeSpaceSize  = 0;
> >> -
> >> -  for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
> >> -    CopyMem ((VOID *)&ExtentAd,
> >> -             (VOID *)&Volume->LogicalVolDescs[Index]-
> >>> IntegritySequenceExtent,
> >> -             sizeof (UDF_EXTENT_AD));
> >> -    if (ExtentAd.ExtentLength == 0) {
> >> -      continue;
> >> -    }
> >> +  EFI_STATUS                     Status;
> >> +  UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc;
> >> +  UDF_EXTENT_AD                  *ExtentAd;
> >> +  UINT64                         Lsn;
> >> +  UINT32                         LogicalBlockSize;
> >> +  UDF_LOGICAL_VOLUME_INTEGRITY   *LogicalVolInt;
> >> +  UINTN                          Index;
> >> +  UINTN                          Length;
> >> +  UINT32                         LsnsNo;
> >>
> >> -    LogicalBlockSize = LV_BLOCK_SIZE (Volume, Index);
> >> +  LogicalVolDesc = &Volume->LogicalVolDesc;
> >>
> >> -  Read_Next_Sequence:
> >> -    LogicalVolInt = (UDF_LOGICAL_VOLUME_INTEGRITY *)
> >> -      AllocatePool (ExtentAd.ExtentLength);
> >> -    if (LogicalVolInt == NULL) {
> >> -      return EFI_OUT_OF_RESOURCES;
> >> -    }
> >> +  ExtentAd = &LogicalVolDesc->IntegritySequenceExtent;
> >>
> >> -    Lsn = (UINT64)ExtentAd.ExtentLocation;
> >> +  if (ExtentAd->ExtentLength == 0) {
> >> +    return EFI_VOLUME_CORRUPTED;
> >> +  }
> >>
> >> -    Status = DiskIo->ReadDisk (
> >> -      DiskIo,
> >> -      BlockIo->Media->MediaId,
> >> -      MultU64x32 (Lsn, LogicalBlockSize),
> >> -      ExtentAd.ExtentLength,
> >> -      (VOID *)LogicalVolInt
> >> -      );
> >> -    if (EFI_ERROR (Status)) {
> >> -      FreePool ((VOID *)LogicalVolInt);
> >> -      return Status;
> >> -    }
> >> +  LogicalVolInt = AllocatePool (ExtentAd->ExtentLength);  if
> >> + (LogicalVolInt == NULL) {
> >> +    return EFI_OUT_OF_RESOURCES;
> >> +  }
> >>
> >> -    if (!IS_LVID (LogicalVolInt)) {
> >> -      FreePool ((VOID *)LogicalVolInt);
> >> -      return EFI_VOLUME_CORRUPTED;
> >> -    }
> >> +  //
> >> +  // Get location of Logical Volume Integrity Descriptor  //  Lsn =
> >> + (UINT64)ExtentAd->ExtentLocation - Volume->MainVdsStartLocation;
> >>
> >> -    Length = LogicalVolInt->NumberOfPartitions;
> >> -    for (Index = 0; Index < Length; Index += sizeof (UINT32)) {
> >> -      LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
> >> -      if (LsnsNo == 0xFFFFFFFFUL) {
> >> -        //
> >> -        // Size not specified.
> >> -        //
> >> -        continue;
> >> -      }
> >> +  LogicalBlockSize = LogicalVolDesc->LogicalBlockSize;
> >>
> >> -      *FreeSpaceSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
> >> -    }
> >> +  //
> >> +  // Read disk block
> >> +  //
> >> +  Status = DiskIo->ReadDisk (
> >> +    DiskIo,
> >> +    BlockIo->Media->MediaId,
> >> +    MultU64x32 (Lsn, LogicalBlockSize),
> >> +    ExtentAd->ExtentLength,
> >> +    LogicalVolInt
> >> +    );
> >> +  if (EFI_ERROR (Status)) {
> >> +    goto Out_Free;
> >> +  }
> >>
> >> -    Length = (LogicalVolInt->NumberOfPartitions * sizeof (UINT32)) << 1;
> >> -    for (; Index < Length; Index += sizeof (UINT32)) {
> >> -      LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
> >> -      if (LsnsNo == 0xFFFFFFFFUL) {
> >> -        //
> >> -        // Size not specified.
> >> -        //
> >> -        continue;
> >> -      }
> >> +  //
> >> +  // Check if read block is a Logical Volume Integrity Descriptor
> >> + // if (!IS_LVID (LogicalVolInt)) {
> >> +    Status = EFI_VOLUME_CORRUPTED;
> >> +    goto Out_Free;
> >> +  }
> >>
> >> -      *VolumeSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
> >> -    }
> >> +  *VolumeSize = 0;
> >> +  *FreeSpaceSize = 0;
> >>
> >> -    CopyMem ((VOID *)&ExtentAd,(VOID *)&LogicalVolInt-
> >>> NextIntegrityExtent,
> >> -             sizeof (UDF_EXTENT_AD));
> >> -    if (ExtentAd.ExtentLength > 0) {
> >> -      FreePool ((VOID *)LogicalVolInt);
> >> -      goto Read_Next_Sequence;
> >> +  Length = LogicalVolInt->NumberOfPartitions;
> >> +  for (Index = 0; Index < Length; Index += sizeof (UINT32)) {
> >> +    LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
> >> +    //
> >> +    // Check if size is not specified
> >> +    //
> >> +    if (LsnsNo == 0xFFFFFFFFUL) {
> >> +      continue;
> >>       }
> >> +    //
> >> +    // Accumulate free space size
> >> +    //
> >> +    *FreeSpaceSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
> >> + }
> >>
> >> -    FreePool ((VOID *)LogicalVolInt);
> >> +  Length = LogicalVolInt->NumberOfPartitions * sizeof (UINT32) * 2;
> >> + for (; Index < Length; Index += sizeof (UINT32)) {
> >> +    LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
> >> +    //
> >> +    // Check if size is not specified
> >> +    //
> >> +    if (LsnsNo == 0xFFFFFFFFUL) {
> >> +      continue;
> >> +    }
> >> +    //
> >> +    // Accumulate used volume space
> >> +    //
> >> +    *VolumeSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
> >>     }
> >>
> >> -  return EFI_SUCCESS;
> >> +  Status = EFI_SUCCESS;
> >> +
> >> +Out_Free:
> >> +  //
> >> +  // Free Logical Volume Integrity Descriptor
> >> +  //
> >> +  FreePool (LogicalVolInt);
> >> +
> >> +  return Status;
> >>   }
> >>
> >>   /**
> >> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
> >> b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
> >> index 49dc7077b7..d4163b89ca 100644
> >> --- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
> >> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
> >> @@ -276,13 +276,6 @@ UdfDriverBindingStop (
> >>         NULL
> >>         );
> >>
> >> -    //
> >> -    // Check if there's any open file. If so, clean them up.
> >> -    //
> >> -    if (PrivFsData->OpenFiles > 0) {
> >> -      CleanupVolumeInformation (&PrivFsData->Volume);
> >> -    }
> >> -
> >>       FreePool ((VOID *)PrivFsData);
> >>     }
> >>
> >> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
> >> b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
> >> index 240d420ff5..c5f83914d8 100644
> >> --- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
> >> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
> >> @@ -49,16 +49,8 @@
> >>       { 0x89, 0x56, 0x73, 0xCD, 0xA3, 0x26, 0xCD, 0x0A }  \
> >>     }
> >>
> >> -#define UDF_DEFAULT_LV_NUM 0
> >> -
> >>   #define IS_PVD(_Pointer) \
> >>     ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 1)) -#define IS_PD(_Pointer)
> >> \
> >> -  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5)) -#define IS_LVD(_Pointer)
> >> \
> >> -  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6)) -#define IS_TD(_Pointer)
> >> \
> >> -  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))  #define IS_FSD(_Pointer)
> \
> >>     ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 256))  #define
> >> IS_FE(_Pointer) \ @@ -152,14 +144,8 @@ typedef enum {  #define
> >> IS_VALID_COMPRESSION_ID(_CompId) \
> >>     ((BOOLEAN)((_CompId) == 8 || (_CompId) == 16))
> >>
> >> -#define LV_BLOCK_SIZE(_Vol, _LvNum) \
> >> -  (_Vol)->LogicalVolDescs[(_LvNum)]->LogicalBlockSize
> >> -
> >>   #define UDF_STANDARD_IDENTIFIER_LENGTH   5
> >>
> >> -#define LV_UDF_REVISION(_Lv) \
> >> -  *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix
> >> -
> >>   #pragma pack(1)
> >>
> >>   typedef struct {
> >> @@ -186,17 +172,6 @@ typedef struct {
> >>   #pragma pack(1)
> >>
> >>   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 {
> >>     UINT16          TypeAndTimezone;
> >>     INT16           Year;
> >>     UINT8           Month;
> >> @@ -210,17 +185,6 @@ typedef struct {
> >>   } UDF_TIMESTAMP;
> >>
> >>   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;
> >>     UINT32                             PrevAllocationExtentDescriptor;
> >>     UINT32                             LengthOfAllocationDescriptors;
> >> @@ -235,37 +199,6 @@ typedef struct {
> >>   } UDF_VOLUME_DESCRIPTOR;
> >>
> >>   typedef struct {
> >> -  UDF_DESCRIPTOR_TAG         DescriptorTag;
> >> -  UINT32                     VolumeDescriptorSequenceNumber;
> >> -  UINT16                     PartitionFlags;
> >> -  UINT16                     PartitionNumber;
> >> -  UDF_ENTITY_ID              PartitionContents;
> >> -  UINT8                      PartitionContentsUse[128];
> >> -  UINT32                     AccessType;
> >> -  UINT32                     PartitionStartingLocation;
> >> -  UINT32                     PartitionLength;
> >> -  UDF_ENTITY_ID              ImplementationIdentifier;
> >> -  UINT8                      ImplementationUse[128];
> >> -  UINT8                      Reserved[156];
> >> -} UDF_PARTITION_DESCRIPTOR;
> >> -
> >> -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;
> >> -
> >> -typedef struct {
> >>     UDF_DESCRIPTOR_TAG             DescriptorTag;
> >>     UDF_TIMESTAMP                  RecordingDateTime;
> >>     UINT32                         IntegrityType;
> >> @@ -389,12 +322,10 @@ typedef struct {
> >>   // UDF filesystem driver's private data  //  typedef struct {
> >> -  UDF_LOGICAL_VOLUME_DESCRIPTOR  **LogicalVolDescs;
> >> -  UINTN                          LogicalVolDescsNo;
> >> -  UDF_PARTITION_DESCRIPTOR       **PartitionDescs;
> >> -  UINTN                          PartitionDescsNo;
> >> -  UDF_FILE_SET_DESCRIPTOR        **FileSetDescs;
> >> -  UINTN                          FileSetDescsNo;
> >> +  UINT64                         MainVdsStartLocation;
> >> +  UDF_LOGICAL_VOLUME_DESCRIPTOR  LogicalVolDesc;
> >> +  UDF_PARTITION_DESCRIPTOR       PartitionDesc;
> >> +  UDF_FILE_SET_DESCRIPTOR        FileSetDesc;
> >>     UINTN                          FileEntrySize;
> >>   } UDF_VOLUME_INFO;
> >>
> >> @@ -883,17 +814,6 @@ ResolveSymlink (
> >>     );
> >>
> >>   /**
> >> -  Clean up in-memory UDF volume information.
> >> -
> >> -  @param[in] Volume Volume information pointer.
> >> -
> >> -**/
> >> -VOID
> >> -CleanupVolumeInformation (
> >> -  IN UDF_VOLUME_INFO *Volume
> >> -  );
> >> -
> >> -/**
> >>     Clean up in-memory UDF file information.
> >>
> >>     @param[in] File File information pointer.
> >> --
> >> 2.11.0
> >
> >

Re: [PATCH v2 0/3] UDF partition driver fix

[Ni, Ruiyu]

Paulo,
Could you please paste a "map -r" output on a CDROM which
contains Eltorito volume?
I want to confirm that the result is expected.

Thanks/Ray

> -----Original Message-----
> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
> Sent: Sunday, September 17, 2017 9:13 PM
> To: edk2-devel@lists.01.org
> Cc: Paulo Alcantara <pcacjr@zytor.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Laszlo
> Ersek <lersek@redhat.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH v2 0/3] UDF partition driver fix
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=707
> 
> Hi,
> 
> This patchset fixes a bug in Partition driver that UDF partitions occupied the
> entire disk space instead of using LVD space only.
> 
> BTW, I've only tested it under OVMF and built it with GCC only. That would
> be appreciable if someone could build with other toolchains and see if this
> doesn't break.
> 
> I used a Windows 10 ISO image with UdfDxe disabled and enabled. The `map
> -r` output seemed OK. No breakage when booting an OS off an ElTorito
> partition from an UDF bridge disk.
> 
> v1->v2:
>  - Followed Laszlo's suggestions to submit a proper patchset. Thanks!
>  - As I'm still waiting for Ruiyu and Star to test this fix, I took
>    advantage of it and did some code cleanups :-)
> 
> Repo:   https://github.com/pcacjr/edk2.git
> Branch: udf-partition-fix-v2
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Reported-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
> ---
> 
> Paulo Alcantara (3):
>   MdePkg: Add UDF volume structure definitions
>   MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition
>   MdeModulePkg/UdfDxe: Rework driver to support PartitionDxe changes
> 
>  MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c            | 323
> +++++++++++-
>  MdeModulePkg/Universal/Disk/UdfDxe/File.c                 |  13 +-
>  MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 515
> ++++++++------------
>  MdeModulePkg/Universal/Disk/UdfDxe/Udf.c                  |   7 -
>  MdeModulePkg/Universal/Disk/UdfDxe/Udf.h                  |  88 +---
>  MdePkg/Include/IndustryStandard/Udf.h                     |  63 +++
>  6 files changed, 566 insertions(+), 443 deletions(-)
> 
> --
> 2.11.0

Re: [PATCH v2 0/3] UDF partition driver fix

[Paulo Alcantara]

Hi Ruiyu,

On 9/18/2017 1:52 AM, Ni, Ruiyu wrote:
> Paulo,
> Could you please paste a "map -r" output on a CDROM which
> contains Eltorito volume?
> I want to confirm that the result is expected.

With UdfDxe driver disabled in OVMF:

 >Mapping table
 >      FS0: Alias(s):CD0c65535a1:;BLK2:
 >          PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0)/CDROM(0x1)
 >     BLK0: Alias(s):
 >          PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0)
 >     BLK1: Alias(s):
 >          PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0)/CDROM(0x0)
 >     BLK3: Alias(s):
 > 
PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0)/VenMedia(C5BD4D42-1A76-4996-8956-73CDA326CD0A)

With UdfDxe driver enabled in OVMF:

 > Mapping table
 >      FS0: Alias(s):CD0c65535a1:;BLK2:
 >          PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0)/CDROM(0x1)
 >      FS1: Alias(s):F0c65535a:;BLK3:
 > 
PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0)/VenMedia(C5BD4D42-1A76-4996-8956-73CDA326CD0A)
 >     BLK0: Alias(s):
 >          PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0)
 >     BLK1: Alias(s):
 >          PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0)/CDROM(0x0)

Thanks!
Paulo

> 
> Thanks/Ray
> 
>> -----Original Message-----
>> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
>> Sent: Sunday, September 17, 2017 9:13 PM
>> To: edk2-devel@lists.01.org
>> Cc: Paulo Alcantara <pcacjr@zytor.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Laszlo
>> Ersek <lersek@redhat.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star
>> <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
>> Subject: [PATCH v2 0/3] UDF partition driver fix
>>
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=707
>>
>> Hi,
>>
>> This patchset fixes a bug in Partition driver that UDF partitions occupied the
>> entire disk space instead of using LVD space only.
>>
>> BTW, I've only tested it under OVMF and built it with GCC only. That would
>> be appreciable if someone could build with other toolchains and see if this
>> doesn't break.
>>
>> I used a Windows 10 ISO image with UdfDxe disabled and enabled. The `map
>> -r` output seemed OK. No breakage when booting an OS off an ElTorito
>> partition from an UDF bridge disk.
>>
>> v1->v2:
>>   - Followed Laszlo's suggestions to submit a proper patchset. Thanks!
>>   - As I'm still waiting for Ruiyu and Star to test this fix, I took
>>     advantage of it and did some code cleanups :-)
>>
>> Repo:   https://github.com/pcacjr/edk2.git
>> Branch: udf-partition-fix-v2
>>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Cc: Star Zeng <star.zeng@intel.com>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Reported-by: Ruiyu Ni <ruiyu.ni@intel.com>
>> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
>> ---
>>
>> Paulo Alcantara (3):
>>    MdePkg: Add UDF volume structure definitions
>>    MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition
>>    MdeModulePkg/UdfDxe: Rework driver to support PartitionDxe changes
>>
>>   MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c            | 323
>> +++++++++++-
>>   MdeModulePkg/Universal/Disk/UdfDxe/File.c                 |  13 +-
>>   MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 515
>> ++++++++------------
>>   MdeModulePkg/Universal/Disk/UdfDxe/Udf.c                  |   7 -
>>   MdeModulePkg/Universal/Disk/UdfDxe/Udf.h                  |  88 +---
>>   MdePkg/Include/IndustryStandard/Udf.h                     |  63 +++
>>   6 files changed, 566 insertions(+), 443 deletions(-)
>>
>> --
>> 2.11.0
> 
>