public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ni, Ruiyu" <ruiyu.ni@intel.com>
To: Paulo Alcantara <pcacjr@zytor.com>,
	"edk2-devel@lists.01.org" <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
Date: Mon, 18 Sep 2017 01:00:29 +0000	[thread overview]
Message-ID: <734D49CCEBEEF84792F5B80ED585239D5BA44272@SHSMSX151.ccr.corp.intel.com> (raw)
In-Reply-To: <600e6e752b285bbe960ad22d8ae969cbbd84b85b.1505653040.git.pcacjr@zytor.com>

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



  reply	other threads:[~2017-09-18  0:57 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-17 13:13 [PATCH v2 0/3] UDF partition driver fix Paulo Alcantara
2017-09-17 13:13 ` [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions Paulo Alcantara
2017-09-18  0:28   ` Ni, Ruiyu
2017-09-18  0:42     ` Ni, Ruiyu
2017-09-18 13:52       ` Paulo Alcantara
2017-09-18 13:50     ` Paulo Alcantara
2017-09-17 13:13 ` [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition Paulo Alcantara
2017-09-18  0:54   ` Ni, Ruiyu
2017-09-18 16:38     ` Paulo Alcantara
2017-09-20  8:14       ` Ni, Ruiyu
2017-09-20  9:59         ` Laszlo Ersek
2017-09-20 10:11           ` Ni, Ruiyu
2017-09-20 11:09             ` Laszlo Ersek
2017-09-20 12:36               ` Gao, Liming
2017-09-17 13:13 ` [PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to support PartitionDxe changes Paulo Alcantara
2017-09-18  1:00   ` Ni, Ruiyu [this message]
2017-09-18 16:51     ` Paulo Alcantara
2017-09-20  7:52       ` Ni, Ruiyu
2017-09-18  4:52 ` [PATCH v2 0/3] UDF partition driver fix Ni, Ruiyu
2017-09-18 17:49   ` Paulo Alcantara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=734D49CCEBEEF84792F5B80ED585239D5BA44272@SHSMSX151.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox