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

[Paulo Alcantara <pcacjr@zytor.com>]

On September 21, 2017 10:47:07 AM GMT-03:00, "Zeng, Star" <star.zeng@intel.com> wrote:
>Does it have functional impact?

Nope. When Partition driver walks the partition detect routine table, it executes each routine and then checks for EFI_NO_MEDIA and EFI_MEDIA_CHANGED only. So, the child handle is installed and the EFI_NOT_FOUND is a no-op in that case.

Thanks!
Paulo

>
>Thanks,
>Star
>-----Original Message-----
>From: Paulo Alcantara [mailto:pcacjr@zytor.com] 
>Sent: Thursday, September 21, 2017 9:29 PM
>To: Wu, Hao A <hao.a.wu@intel.com>; edk2-devel@lists.01.org
>Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>;
>Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
>Subject: RE: [edk2] [PATCH v3 2/2] MdeModulePkg/PartitionDxe: Fix
>creation of UDF logical partition
>
>
>
>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;
>>>    }
>>> 
>>> -  

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