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

[Paulo Alcantara <pcacjr@zytor.com>]

On September 21, 2017 9:44:10 AM GMT-03:00, "Wu, Hao A" <hao.a.wu@intel.com> wrote:
>One small comment, within function PartitionInstallUdfChildHandles():
>
>  ...
>  //
>  // Install partition child handle for UDF file system
>  //
>  Status = PartitionInstallChildHandle (
>    ...
>    );
>if (!EFI_ERROR (Status)) {  <----- Is this a typo? "if (EFI_ERROR
>(Status)) {"  
>    Status = EFI_NOT_FOUND;
>  }

Yes, it is. Good catch! Could you please fix that for me by removing the if condition? Otherwise I can send a v4 later with that.

Thanks!
Paulo

>  
>
>Best Regards,
>Hao Wu
>
>
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf
>Of Paulo
>> Alcantara
>> Sent: Thursday, September 21, 2017 2:16 AM
>> To: edk2-devel@lists.01.org
>> Cc: Ni, Ruiyu; Laszlo Ersek; Dong, Eric; Zeng, Star
>> Subject: [edk2] [PATCH v3 2/2] MdeModulePkg/PartitionDxe: Fix
>creation of
>> UDF logical partition
>> 
>> Do not reserve entire block device size for an UDF file system -
>> instead, reserve the appropriate space (UDF logical volume space) for
>> it.
>> 
>> Additionally, only create a logical partition for UDF logical volumes
>> that are currently supported by EDK2 UDF file system implementation.
>For
>> instance, an UDF volume with a single LVD and a single Physical (Type
>1)
>> Partition will be supported.
>> 
>> 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            | 363
>++++++++++--
>>  MdeModulePkg/Universal/Disk/UdfDxe/File.c                 |  16 +-
>>  MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 627
>> ++++++++------------
>>  MdeModulePkg/Universal/Disk/UdfDxe/Udf.c                  |   7 -
>>  MdeModulePkg/Universal/Disk/UdfDxe/Udf.h                  | 158
>++---
>>  5 files changed, 606 insertions(+), 565 deletions(-)
>> 
>> diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
>> b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
>> index 609f56cef6..572ba7a81a 100644
>> --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
>> +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
>> @@ -64,11 +64,12 @@ FindAnchorVolumeDescriptorPointer (
>>    OUT  UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  *AnchorPoint
>>    )
>>  {
>> -  EFI_STATUS  Status;
>> -  UINT32      BlockSize;
>> -  EFI_LBA     EndLBA;
>> -  EFI_LBA     DescriptorLBAs[4];
>> -  UINTN       Index;
>> +  EFI_STATUS          Status;
>> +  UINT32              BlockSize;
>> +  EFI_LBA             EndLBA;
>> +  EFI_LBA             DescriptorLBAs[4];
>> +  UINTN               Index;
>> +  UDF_DESCRIPTOR_TAG  *DescriptorTag;
>> 
>>    BlockSize = BlockIo->Media->BlockSize;
>>    EndLBA = BlockIo->Media->LastBlock;
>> @@ -88,10 +89,13 @@ FindAnchorVolumeDescriptorPointer (
>>      if (EFI_ERROR (Status)) {
>>        return Status;
>>      }
>> +
>> +    DescriptorTag = &AnchorPoint->DescriptorTag;
>> +
>>      //
>>      // Check if read LBA has a valid AVDP descriptor.
>>      //
>> -    if (IS_AVDP (AnchorPoint)) {
>> +    if (DescriptorTag->TagIdentifier ==
>UdfAnchorVolumeDescriptorPointer) {
>>        return EFI_SUCCESS;
>>      }
>>    }
>> @@ -102,23 +106,18 @@ FindAnchorVolumeDescriptorPointer (
>>  }
>> 
>>  /**
>> -  Check if block device supports a valid UDF file system as
>specified by OSTA
>> -  Universal Disk Format Specification 2.60.
>> +  Find UDF volume identifiers in a Volume Recognition Sequence.
>> 
>> -  @param[in]   BlockIo  BlockIo interface.
>> -  @param[in]   DiskIo   DiskIo interface.
>> +  @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.
>> +  @retval EFI_SUCCESS             UDF volume identifiers were found.
>> +  @retval EFI_NOT_FOUND           UDF volume identifiers were not
>found.
>> +  @retval other                   Failed to perform disk I/O.
>> 
>>  **/
>>  EFI_STATUS
>> -SupportUdfFileSystem (
>> +FindUdfVolumeIdentifiers (
>>    IN EFI_BLOCK_IO_PROTOCOL  *BlockIo,
>>    IN EFI_DISK_IO_PROTOCOL   *DiskIo
>>    )
>> @@ -128,7 +127,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));
>> 
>> @@ -167,7 +165,7 @@ SupportUdfFileSystem (
>>          (CompareMem ((VOID *)&VolDescriptor,
>>                       (VOID *)&TerminatingVolDescriptor,
>>                       sizeof (CDROM_VOLUME_DESCRIPTOR)) == 0)) {
>> -      return EFI_UNSUPPORTED;
>> +      return EFI_NOT_FOUND;
>>      }
>>    }
>> 
>> @@ -176,7 +174,7 @@ SupportUdfFileSystem (
>>    //
>>    Offset += UDF_LOGICAL_SECTOR_SIZE;
>>    if (Offset >= EndDiskOffset) {
>> -    return EFI_UNSUPPORTED;
>> +    return EFI_NOT_FOUND;
>>    }
>> 
>>    Status = DiskIo->ReadDisk (
>> @@ -196,7 +194,7 @@ SupportUdfFileSystem (
>>        (CompareMem ((VOID *)VolDescriptor.Unknown.Id,
>>                     (VOID *)UDF_NSR3_IDENTIFIER,
>>                     sizeof (VolDescriptor.Unknown.Id)) != 0)) {
>> -    return EFI_UNSUPPORTED;
>> +    return EFI_NOT_FOUND;
>>    }
>> 
>>    //
>> @@ -204,7 +202,7 @@ SupportUdfFileSystem (
>>    //
>>    Offset += UDF_LOGICAL_SECTOR_SIZE;
>>    if (Offset >= EndDiskOffset) {
>> -    return EFI_UNSUPPORTED;
>> +    return EFI_NOT_FOUND;
>>    }
>> 
>>    Status = DiskIo->ReadDisk (
>> @@ -221,15 +219,291 @@ SupportUdfFileSystem (
>>    if (CompareMem ((VOID *)VolDescriptor.Unknown.Id,
>>                    (VOID *)UDF_TEA_IDENTIFIER,
>>                    sizeof (VolDescriptor.Unknown.Id)) != 0) {
>> -    return EFI_UNSUPPORTED;
>> +    return EFI_NOT_FOUND;
>> +  }
>> +
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +/**
>> +  Check if Logical Volume Descriptor is supported by current EDK2
>UDF file
>> +  system implementation.
>> +
>> +  @param[in]  LogicalVolDesc  Logical Volume Descriptor pointer.
>> +
>> +  @retval TRUE                Logical Volume Descriptor is
>supported.
>> +  @retval FALSE               Logical Volume Descriptor is not
>supported.
>> +
>> +**/
>> +BOOLEAN
>> +IsLogicalVolumeDescriptorSupported (
>> +  UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc
>> +  )
>> +{
>> +  //
>> +  // Check for a valid UDF revision range
>> +  //
>> +  switch (UDF_LVD_REVISION (LogicalVolDesc)) {
>> +  case 0x0102:
>> +  case 0x0150:
>> +  case 0x0200:
>> +  case 0x0201:
>> +  case 0x0250:
>> +  case 0x0260:
>> +    break;
>> +  default:
>> +    return FALSE;
>> +  }
>> +
>> +  //
>> +  // Check for a single Partition Map
>> +  //
>> +  if (LogicalVolDesc->NumberOfPartitionMaps > 1) {
>> +    return FALSE;
>> +  }
>> +  //
>> +  // UDF 1.02 revision supports only Type 1 (Physical) partitions,
>but
>> +  // let's check it any way.
>> +  //
>> +  // PartitionMap[0] -> type
>> +  // PartitionMap[1] -> length (in bytes)
>> +  //
>> +  if (LogicalVolDesc->PartitionMaps[0] != 1 ||
>> +      LogicalVolDesc->PartitionMaps[1] != 6) {
>> +    return FALSE;
>> +  }
>> +
>> +  return TRUE;
>> +}
>> +
>> +/**
>> +  Find UDF logical volume location and whether it is supported by
>current EDK2
>> +  UDF file system implementation.
>> +
>> +  @param[in]  BlockIo             BlockIo interface.
>> +  @param[in]  DiskIo              DiskIo interface.
>> +  @param[in]  AnchorPoint         Anchor volume descriptor pointer.
>> +  @param[out] MainVdsStartBlock   Main VDS starting block number.
>> +  @param[out] MainVdsEndBlock     Main VDS ending block number.
>> +
>> +  @retval EFI_SUCCESS             UDF logical volume was found.
>> +  @retval EFI_VOLUME_CORRUPTED    UDF file system structures are
>> corrupted.
>> +  @retval EFI_UNSUPPORTED         UDF logical volume is not
>supported.
>> +  @retval other                   Failed to perform disk I/O.
>> +
>> +**/
>> +EFI_STATUS
>> +FindLogicalVolumeLocation (
>> +  IN   EFI_BLOCK_IO_PROTOCOL                 *BlockIo,
>> +  IN   EFI_DISK_IO_PROTOCOL                  *DiskIo,
>> +  IN   UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  *AnchorPoint,
>> +  OUT  UINT64                                *MainVdsStartBlock,
>> +  OUT  UINT64                                *MainVdsEndBlock
>> +  )
>> +{
>> +  EFI_STATUS                     Status;
>> +  UINT32                         BlockSize;
>> +  EFI_LBA                        LastBlock;
>> +  UDF_EXTENT_AD                  *ExtentAd;
>> +  UINT64                         SeqBlocksNum;
>> +  UINT64                         SeqStartBlock;
>> +  UINT64                         GuardMainVdsStartBlock;
>> +  VOID                           *Buffer;
>> +  UINT64                         SeqEndBlock;
>> +  BOOLEAN                        StopSequence;
>> +  UINTN                          LvdsCount;
>> +  UDF_LOGICAL_VOLUME_DESCRIPTOR  *LogicalVolDesc;
>> +  UDF_DESCRIPTOR_TAG             *DescriptorTag;
>> +
>> +  BlockSize = BlockIo->Media->BlockSize;
>> +  LastBlock = BlockIo->Media->LastBlock;
>> +  ExtentAd = &AnchorPoint->MainVolumeDescriptorSequenceExtent;
>> +
>> +  //
>> +  // UDF 2.60, 2.2.3.1 struct MainVolumeDescriptorSequenceExtent
>> +  //
>> +  // The Main Volume Descriptor Sequence Extent shall have a minimum
>length
>> of
>> +  // 16 logical sectors.
>> +  //
>> +  // Also make sure it does not exceed maximum number of blocks in
>the disk.
>> +  //
>> +  SeqBlocksNum = DivU64x32 ((UINT64)ExtentAd->ExtentLength,
>BlockSize);
>> +  if (SeqBlocksNum < 16 || (EFI_LBA)SeqBlocksNum > LastBlock + 1) {
>> +    return EFI_VOLUME_CORRUPTED;
>> +  }
>> +
>> +  //
>> +  // Check for valid Volume Descriptor Sequence starting block
>number
>> +  //
>> +  SeqStartBlock = (UINT64)ExtentAd->ExtentLocation;
>> +  if (SeqStartBlock > LastBlock ||
>> +      SeqStartBlock + SeqBlocksNum - 1 > LastBlock) {
>> +    return EFI_VOLUME_CORRUPTED;
>>    }
>> 
>> +  GuardMainVdsStartBlock = SeqStartBlock;
>> +
>> +  //
>> +  // Allocate buffer for reading disk blocks
>> +  //
>> +  Buffer = AllocateZeroPool ((UINTN)BlockSize);
>> +  if (Buffer == NULL) {
>> +    return EFI_OUT_OF_RESOURCES;
>> +  }
>> +
>> +  SeqEndBlock = SeqStartBlock + SeqBlocksNum;
>> +  StopSequence = FALSE;
>> +  LvdsCount = 0;
>> +  Status = EFI_VOLUME_CORRUPTED;
>> +  //
>> +  // Start Main Volume Descriptor Sequence
>> +  //
>> +  for (; SeqStartBlock < SeqEndBlock && !StopSequence;
>SeqStartBlock++) {
>> +    //
>> +    // Read disk block
>> +    //
>> +    Status = BlockIo->ReadBlocks (
>> +      BlockIo,
>> +      BlockIo->Media->MediaId,
>> +      SeqStartBlock,
>> +      BlockSize,
>> +      Buffer
>> +      );
>> +    if (EFI_ERROR (Status)) {
>> +      goto Out_Free;
>> +    }
>> +
>> +    DescriptorTag = Buffer;
>> +
>> +    //
>> +    // ECMA 167, 8.4.1 Contents of a Volume Descriptor Sequence
>> +    //
>> +    // - A Volume Descriptor Sequence shall contain one or more
>Primary
>> Volume
>> +    //   Descriptors.
>> +    // - A Volume Descriptor Sequence shall contain zero or more
>> Implementation
>> +    //   Use Volume Descriptors.
>> +    // - A Volume Descriptor Sequence shall contain zero or more
>Partition
>> +    //   Descriptors.
>> +    // - A Volume Descriptor Sequence shall contain zero or more
>Logical
>> Volume
>> +    //   Descriptors.
>> +    // - A Volume Descriptor Sequence shall contain zero or more
>Unallocated
>> +    //   Space Descriptors.
>> +    //
>> +    switch (UDF_TAG_ID (DescriptorTag)) {
>> +    case UdfPrimaryVolumeDescriptor:
>> +    case UdfImplemenationUseVolumeDescriptor:
>> +    case UdfPartitionDescriptor:
>> +    case UdfUnallocatedSpaceDescriptor:
>> +      break;
>> +
>> +    case UdfLogicalVolumeDescriptor:
>> +      LogicalVolDesc = Buffer;
>> +
>> +      //
>> +      // Check for existence of a single LVD and whether it is
>supported by
>> +      // current EDK2 UDF file system implementation.
>> +      //
>> +      if (++LvdsCount > 1 ||
>> +          !IsLogicalVolumeDescriptorSupported (LogicalVolDesc)) {
>> +        Status = EFI_UNSUPPORTED;
>> +        StopSequence = TRUE;
>> +      }
>> +
>> +      break;
>> +
>> +    case UdfTerminatingDescriptor:
>> +      //
>> +      // Stop the sequence when we find a Terminating Descriptor
>> +      // (aka Unallocated Sector), se we don't have to walk all the
>unallocated
>> +      // area unnecessarily.
>> +      //
>> +      StopSequence = TRUE;
>> +      break;
>> +
>> +    default:
>> +      //
>> +      // An invalid Volume Descriptor has been found in the sequece.
>Volume is
>> +      // corrupted.
>> +      //
>> +      Status = EFI_VOLUME_CORRUPTED;
>> +      goto Out_Free;
>> +    }
>> +  }
>> +
>> +  //
>> +  // Check if LVD was found
>> +  //
>> +  if (!EFI_ERROR (Status) && LvdsCount == 1) {
>> +    *MainVdsStartBlock = GuardMainVdsStartBlock;
>> +    //
>> +    // We do not need to read either LVD or PD descriptors to know
>the last
>> +    // valid block in the found UDF file system. It's already
>LastBlock.
>> +    //
>> +    *MainVdsEndBlock = LastBlock;
>> +
>> +    Status = EFI_SUCCESS;
>> +  }
>> +
>> +Out_Free:
>> +  //
>> +  // Free block read buffer
>> +  //
>> +  FreePool (Buffer);
>> +
>> +  return Status;
>> +}
>> +
>> +/**
>> +  Find a supported UDF file system in block device.
>> +
>> +  @param[in]  BlockIo             BlockIo interface.
>> +  @param[in]  DiskIo              DiskIo interface.
>> +  @param[out] StartingLBA         UDF file system starting LBA.
>> +  @param[out] EndingLBA           UDF file system starting LBA.
>> +
>> +  @retval EFI_SUCCESS             UDF file system was found.
>> +  @retval other                   UDF file system was not found.
>> +
>> +**/
>> +EFI_STATUS
>> +FindUdfFileSystem (
>> +  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;
>>  }
>> 
>>  /**
>> @@ -263,9 +537,9 @@ PartitionInstallUdfChildHandles (
>>    UINT32                       RemainderByMediaBlockSize;
>>    EFI_STATUS                   Status;
>>    EFI_BLOCK_IO_MEDIA           *Media;
>> -  EFI_DEVICE_PATH_PROTOCOL     *DevicePathNode;
>> -  EFI_GUID                     *VendorDefinedGuid;
>>    EFI_PARTITION_INFO_PROTOCOL  PartitionInfo;
>> +  EFI_LBA                      StartingLBA;
>> +  EFI_LBA                      EndingLBA;
>> 
>>    Media = BlockIo->Media;
>> 
>> @@ -281,35 +555,10 @@ PartitionInstallUdfChildHandles (
>>      return EFI_NOT_FOUND;
>>    }
>> 
>> -  DevicePathNode = DevicePath;
>> -  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, &gUdfDevPathGuid)) {
>> -          return EFI_NOT_FOUND;
>> -        }
>> -      }
>> -    }
>> -    //
>> -    // Try next device path node
>> -    //
>> -    DevicePathNode = NextDevicePathNode (DevicePathNode);
>> -  }
>> -
>>    //
>> -  // Check if block device supports an UDF file system
>> +  // Search for an UDF file system on block device
>>    //
>> -  Status = SupportUdfFileSystem (BlockIo, DiskIo);
>> +  Status = FindUdfFileSystem (BlockIo, DiskIo, &StartingLBA,
>&EndingLBA);
>>    if (EFI_ERROR (Status)) {
>>      return EFI_NOT_FOUND;
>>    }
>> @@ -334,8 +583,8 @@ PartitionInstallUdfChildHandles (
>>      DevicePath,
>>      (EFI_DEVICE_PATH_PROTOCOL *)&gUdfDevicePath,
>>      &PartitionInfo,
>> -    0,
>> -    Media->LastBlock,
>> +    StartingLBA,
>> +    EndingLBA,
>>      Media->BlockSize
>>      );
>>    if (!EFI_ERROR (Status)) {
>> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>> b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>> index 625f2c5637..6f07bf2066 100644
>> --- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>> @@ -131,7 +131,6 @@ Error_Alloc_Priv_File_Data:
>>    CleanupFileInformation (&PrivFsData->Root);
>> 
>>  Error_Find_Root_Dir:
>> -  CleanupVolumeInformation (&PrivFsData->Volume);
>> 
>>  Error_Read_Udf_Volume:
>>  Error_Invalid_Params:
>> @@ -429,7 +428,7 @@ UdfRead (
>>      }
>>      ASSERT (NewFileEntryData != NULL);
>> 
>> -    if (IS_FE_SYMLINK (NewFileEntryData)) {
>> +    if (FE_ICB_FILE_TYPE (NewFileEntryData) == UdfFileEntrySymlink)
>{
>>        Status = ResolveSymlink (
>>          BlockIo,
>>          DiskIo,
>> @@ -529,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);
>> 
>> @@ -542,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);
>> 
>> @@ -552,10 +548,6 @@ UdfClose (
>>      }
>>    }
>> 
>> -  if (--PrivFsData->OpenFiles == 0) {
>> -    CleanupVolumeInformation (&PrivFsData->Volume);
>> -  }
>> -
>>    FreePool ((VOID *)PrivFileData);
>> 
>>  Exit:
>> @@ -652,7 +644,7 @@ UdfGetPosition (
>>    // As per UEFI spec, if the file handle is a directory, then the
>current file
>>    // position has no meaning and the operation is not supported.
>>    //
>> -  if (IS_FID_DIRECTORY_FILE
>(&PrivFileData->File.FileIdentifierDesc)) {
>> +  if (IS_FID_DIRECTORY_FILE (PrivFileData->File.FileIdentifierDesc))
>{
>>      return  EFI_UNSUPPORTED;
>>    }
>> 
>> @@ -788,7 +780,7 @@ UdfGetInfo (
>>    } else if (CompareGuid (InformationType, &gEfiFileSystemInfoGuid))
>{
>>      String = VolumeLabel;
>> 
>> -    FileSetDesc = PrivFsData->Volume.FileSetDescs[0];
>> +    FileSetDesc = &PrivFsData->Volume.FileSetDesc;
>> 
>>      OstaCompressed = &FileSetDesc->LogicalVolumeIdentifier[0];
>> 
>> @@ -847,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 5df267761f..62d817989f 100644
>> --- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
>> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
>> @@ -38,11 +38,12 @@ FindAnchorVolumeDescriptorPointer (
>>    OUT  UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  *AnchorPoint
>>    )
>>  {
>> -  EFI_STATUS  Status;
>> -  UINT32      BlockSize;
>> -  EFI_LBA     EndLBA;
>> -  EFI_LBA     DescriptorLBAs[4];
>> -  UINTN       Index;
>> +  EFI_STATUS          Status;
>> +  UINT32              BlockSize;
>> +  EFI_LBA             EndLBA;
>> +  EFI_LBA             DescriptorLBAs[4];
>> +  UINTN               Index;
>> +  UDF_DESCRIPTOR_TAG  *DescriptorTag;
>> 
>>    BlockSize = BlockIo->Media->BlockSize;
>>    EndLBA = BlockIo->Media->LastBlock;
>> @@ -62,10 +63,13 @@ FindAnchorVolumeDescriptorPointer (
>>      if (EFI_ERROR (Status)) {
>>        return Status;
>>      }
>> +
>> +    DescriptorTag = &AnchorPoint->DescriptorTag;
>> +
>>      //
>>      // Check if read LBA has a valid AVDP descriptor.
>>      //
>> -    if (IS_AVDP (AnchorPoint)) {
>> +    if (DescriptorTag->TagIdentifier ==
>UdfAnchorVolumeDescriptorPointer) {
>>        return EFI_SUCCESS;
>>      }
>>    }
>> @@ -99,148 +103,98 @@ StartMainVolumeDescriptorSequence (
>>    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;
>> +  UDF_EXTENT_AD         *ExtentAd;
>> +  EFI_LBA               SeqStartBlock;
>> +  EFI_LBA               SeqEndBlock;
>> +  BOOLEAN               StopSequence;
>> +  VOID                  *Buffer;
>> +  UDF_DESCRIPTOR_TAG    *DescriptorTag;
>> +  UINT32                LogicalBlockSize;
>> +
>> +  BlockSize = BlockIo->Media->BlockSize;
>> +  ExtentAd = &AnchorPoint->MainVolumeDescriptorSequenceExtent;
>> 
>>    //
>> -  // 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) {
>> -    Status = DiskIo->ReadDisk (
>> -      DiskIo,
>> +  //
>> +  // 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 check again if we have valid Volume
>Descriptors here
>> since
>> +  // Partition driver already did.
>> +  //
>> +  SeqStartBlock = 0;
>> +  SeqEndBlock = SeqStartBlock + DivU64x32
>((UINT64)ExtentAd->ExtentLength,
>> +                                           BlockSize);
>> +  StopSequence = FALSE;
>> +  for (; SeqStartBlock < SeqEndBlock && !StopSequence;
>SeqStartBlock++) {
>> +    //
>> +    // Read disk block
>> +    //
>> +    Status = BlockIo->ReadBlocks (
>> +      BlockIo,
>>        BlockIo->Media->MediaId,
>> -      MultU64x32 (StartingLsn, BlockSize),
>> +      SeqStartBlock,
>>        BlockSize,
>>        Buffer
>>        );
>>      if (EFI_ERROR (Status)) {
>> -      goto Error_Read_Disk_Blk;
>> +      goto Out_Free;
>>      }
>> 
>> -    if (IS_TD (Buffer)) {
>> +    DescriptorTag = Buffer;
>> +
>> +    switch (UDF_TAG_ID (DescriptorTag)) {
>> +    case UdfPartitionDescriptor:
>>        //
>> -      // Found a Terminating Descriptor. Stop the sequence then.
>> +      // Save Partition Descriptor
>>        //
>> +      CopyMem (&Volume->PartitionDesc, Buffer, sizeof (Volume-
>> >PartitionDesc));
>>        break;
>> -    }
>> 
>> -    if (IS_LVD (Buffer)) {
>> +    case UdfLogicalVolumeDescriptor:
>>        //
>> -      // 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 (&Volume->LogicalVolDesc, Buffer, sizeof (Volume-
>> >LogicalVolDesc));
>> +      break;
>> 
>> -      CopyMem ((VOID *)LogicalVolDesc, Buffer,
>> -               sizeof (UDF_LOGICAL_VOLUME_DESCRIPTOR));
>> -      Volume->LogicalVolDescs[Volume->LogicalVolDescsNo++] =
>> LogicalVolDesc;
>> -    } else if (IS_PD (Buffer)) {
>> -      //
>> -      // Found a Partition Descriptor.
>> -      //
>> -      PartitionDesc =
>> -        (UDF_PARTITION_DESCRIPTOR *)
>> -        AllocateZeroPool (sizeof (UDF_PARTITION_DESCRIPTOR));
>> -      if (PartitionDesc == NULL) {
>> -        Status = EFI_OUT_OF_RESOURCES;
>> -        goto Error_Alloc_Pd;
>> -      }
>> +    case UdfTerminatingDescriptor:
>> +      StopSequence = TRUE;
>> +      break;
>> 
>> -      CopyMem ((VOID *)PartitionDesc, Buffer,
>> -               sizeof (UDF_PARTITION_DESCRIPTOR));
>> -      Volume->PartitionDescs[Volume->PartitionDescsNo++] =
>PartitionDesc;
>> +    default:
>> +      ;
>>      }
>> -
>> -    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);
>> +  Status = EFI_SUCCESS;
>> 
>> -  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]);
>> -  }
>> -
>> -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;
>>  }
>> 
>> @@ -262,48 +216,53 @@ 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)) {
>> +  switch (UDF_LVD_REVISION (LogicalVolDesc)) {
>>    case 0x0102:
>> +  case 0x0150:
>> +  case 0x0200:
>> +  case 0x0201:
>> +  case 0x0250:
>> +  case 0x0260:
>>      //
>> -    // As per UDF 1.02 specification:
>> +    // UDF 1.02 specification:
>>      //
>>      // There shall be exactly one prevailing Logical Volume
>Descriptor recorded
>>      // per Volume Set. The Partition Maps field shall contain only
>Type 1
>>      // Partition Maps.
>>      //
>> -    PartitionNum = *(UINT16
>*)((UINTN)&LogicalVolDesc->PartitionMaps[4]);
>> -    break;
>> -  case 0x0150:
>> +    // UDF 1.50 through 2.60 specs say:
>>      //
>> -    // Ensure Type 1 Partition map. Other types aren't supported in
>this
>> -    // implementation.
>> +    // For the purpose of interchange partition maps shall be
>limited to
>> +    // Partition Map type 1, except type 2 maps as described in the
>document.
>> +    //
>> +    // NOTE: Only one Type 1 (Physical) Partition is supported. It
>has been
>> +    // checked already in Partition driver for existence of a single
>Type 1
>> +    // Partition map, so we don't have to double check here.
>> +    //
>> +    // Partition reference number can also be retrieved from
>> +    // LongAd->ExtentLocation.PartitionReferenceNumber, however the
>spec
>> says
>> +    // it may be 0, so let's not rely on it.
>>      //
>> -    if (LogicalVolDesc->PartitionMaps[0] != 1 ||
>> -        LogicalVolDesc->PartitionMaps[1] != 6) {
>> -      return NULL;
>> -    }
>>      PartitionNum = *(UINT16
>*)((UINTN)&LogicalVolDesc->PartitionMaps[4]);
>>      break;
>> -  case 0x0260:
>> +
>> +  default:
>>      //
>> -    // Fall through.
>> +    // Unsupported UDF revision
>>      //
>> -  default:
>> -    PartitionNum = LongAd->ExtentLocation.PartitionReferenceNumber;
>> -    break;
>> +    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;
>> @@ -329,13 +288,15 @@ GetLongAdLsn (
>>    PartitionDesc = GetPdFromLongAd (Volume, LongAd);
>>    ASSERT (PartitionDesc != NULL);
>> 
>> -  return (UINT64)PartitionDesc->PartitionStartingLocation +
>> -                 LongAd->ExtentLocation.LogicalBlockNumber;
>> +  return (UINT64)PartitionDesc->PartitionStartingLocation -
>> +    Volume->MainVdsStartLocation +
>> +    LongAd->ExtentLocation.LogicalBlockNumber;
>>  }
>> 
>>  /**
>>    Return logical sector number of a given Short Allocation
>Descriptor.
>> 
>> +  @param[in]  Volume              Volume pointer.
>>    @param[in]  PartitionDesc       Partition Descriptor pointer.
>>    @param[in]  ShortAd             Short Allocation Descriptor
>pointer.
>> 
>> @@ -344,14 +305,13 @@ GetLongAdLsn (
>>  **/
>>  UINT64
>>  GetShortAdLsn (
>> +  IN UDF_VOLUME_INFO                  *Volume,
>>    IN UDF_PARTITION_DESCRIPTOR         *PartitionDesc,
>>    IN UDF_SHORT_ALLOCATION_DESCRIPTOR  *ShortAd
>>    )
>>  {
>> -  ASSERT (PartitionDesc != NULL);
>> -
>> -  return (UINT64)PartitionDesc->PartitionStartingLocation +
>> -    ShortAd->ExtentPosition;
>> +  return (UINT64)PartitionDesc->PartitionStartingLocation -
>> +    Volume->MainVdsStartLocation + ShortAd->ExtentPos

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.