Does it have functional impact? Thanks, Star -----Original Message----- From: Paulo Alcantara [mailto:pca...@zytor.com] Sent: Thursday, September 21, 2017 9:29 PM To: Wu, Hao A <hao.a...@intel.com>; edk2-devel@lists.01.org Cc: Ni, Ruiyu <ruiyu...@intel.com>; Laszlo Ersek <ler...@redhat.com>; Dong, Eric <eric.d...@intel.com>; Zeng, Star <star.z...@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...@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-boun...@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.d...@intel.com> >> Cc: Ruiyu Ni <ruiyu...@intel.com> >> Cc: Star Zeng <star.z...@intel.com> >> Cc: Laszlo Ersek <ler...@redhat.com> >> Reported-by: Ruiyu Ni <ruiyu...@intel.com> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Paulo Alcantara <pca...@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. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel