Paulo, Are you ok to the macro related comments I provided to patch V3.#1? If yes, could you please update the patch V3.#2 accordingly?
I noticed that you put some structures that seem to be UDF standard into the UdfDxe driver local header file. I am ok with your decision. For public interfaces, none is better than bad. I also checked a bit of your updated code, but didn't dig into very details. I am ok to the logic. Thanks/Ray > -----Original Message----- > From: Zeng, Star > Sent: Friday, September 22, 2017 10:27 AM > To: Paulo Alcantara <pca...@gmail.com>; 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 > > Ok, I will go to help push the patch series if Ray has no any comments. > > Thanks, > Star > -----Original Message----- > From: Paulo Alcantara [mailto:pca...@gmail.com] > Sent: Thursday, September 21, 2017 10:17 PM > To: Zeng, Star <star.z...@intel.com>; 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> > Subject: Re: [edk2] [PATCH v3 2/2] MdeModulePkg/PartitionDxe: Fix > creation of UDF logical partition > > On Thu, Sep 21, 2017 at 10:52 AM, Paulo Alcantara <pca...@zytor.com> > wrote: > > > > > > On September 21, 2017 10:47:07 AM GMT-03:00, "Zeng, Star" > <star.z...@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. > > BTW, I performed a quick test with UdfDxe enabled and disabled in OVMF > and the removal did not seem to break or change any behavior. > > Thanks! > Paulo > > > > > Thanks! > > Paulo > > > >> > >>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; > >>>> } > >>>> > >>>> - > > > > -- > > 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 > > > > -- > Paulo Alcantara, HP > Speaking for myself only. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
