From: Paulo Alcantara <pcacjr@gmail.com>
To: "Zeng, Star" <star.zeng@intel.com>,
"Wu, Hao A" <hao.a.wu@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Ni, Ruiyu" <ruiyu.ni@intel.com>,
Laszlo Ersek <lersek@redhat.com>,
"Dong, Eric" <eric.dong@intel.com>
Subject: Re: [PATCH v3 2/2] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition
Date: Thu, 21 Sep 2017 11:16:34 -0300 [thread overview]
Message-ID: <CAO1t2O2KqTQjH6Zeknxw0--LGk5HOemb0kRptHzLbnZdwBLTgQ@mail.gmail.com> (raw)
In-Reply-To: <7CD2E165-74AE-4E76-A686-BA8CE17720DE@zytor.com>
On Thu, Sep 21, 2017 at 10:52 AM, Paulo Alcantara <pcacjr@zytor.com> wrote:
>
>
> On September 21, 2017 10:47:07 AM GMT-03:00, "Zeng, Star" <star.zeng@intel.com> wrote:
>>Does it have functional impact?
>
> Nope. When Partition driver walks the partition detect routine table, it executes each routine and then checks for EFI_NO_MEDIA and EFI_MEDIA_CHANGED only. So, the child handle is installed and the EFI_NOT_FOUND is a no-op in that case.
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:pcacjr@zytor.com]
>>Sent: Thursday, September 21, 2017 9:29 PM
>>To: Wu, Hao A <hao.a.wu@intel.com>; edk2-devel@lists.01.org
>>Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>;
>>Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
>>Subject: RE: [edk2] [PATCH v3 2/2] MdeModulePkg/PartitionDxe: Fix
>>creation of UDF logical partition
>>
>>
>>
>>On September 21, 2017 9:44:10 AM GMT-03:00, "Wu, Hao A"
>><hao.a.wu@intel.com> wrote:
>>>One small comment, within function PartitionInstallUdfChildHandles():
>>>
>>> ...
>>> //
>>> // Install partition child handle for UDF file system
>>> //
>>> Status = PartitionInstallChildHandle (
>>> ...
>>> );
>>>if (!EFI_ERROR (Status)) { <----- Is this a typo? "if (EFI_ERROR
>>>(Status)) {"
>>> Status = EFI_NOT_FOUND;
>>> }
>>
>>Yes, it is. Good catch! Could you please fix that for me by removing
>>the if condition? Otherwise I can send a v4 later with that.
>>
>>Thanks!
>>Paulo
>>
>>>
>>>
>>>Best Regards,
>>>Hao Wu
>>>
>>>
>>>> -----Original Message-----
>>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf
>>>Of Paulo
>>>> Alcantara
>>>> Sent: Thursday, September 21, 2017 2:16 AM
>>>> To: edk2-devel@lists.01.org
>>>> Cc: Ni, Ruiyu; Laszlo Ersek; Dong, Eric; Zeng, Star
>>>> Subject: [edk2] [PATCH v3 2/2] MdeModulePkg/PartitionDxe: Fix
>>>creation of
>>>> UDF logical partition
>>>>
>>>> Do not reserve entire block device size for an UDF file system -
>>>> instead, reserve the appropriate space (UDF logical volume space)
>>for
>>>> it.
>>>>
>>>> Additionally, only create a logical partition for UDF logical
>>volumes
>>>> that are currently supported by EDK2 UDF file system implementation.
>>>For
>>>> instance, an UDF volume with a single LVD and a single Physical
>>(Type
>>>1)
>>>> Partition will be supported.
>>>>
>>>> Cc: Eric Dong <eric.dong@intel.com>
>>>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>>>> Cc: Star Zeng <star.zeng@intel.com>
>>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>>> Reported-by: Ruiyu Ni <ruiyu.ni@intel.com>
>>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>>> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
>>>> ---
>>>> MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 363
>>>++++++++++--
>>>> MdeModulePkg/Universal/Disk/UdfDxe/File.c | 16 +-
>>>> MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 627
>>>> ++++++++------------
>>>> MdeModulePkg/Universal/Disk/UdfDxe/Udf.c | 7 -
>>>> MdeModulePkg/Universal/Disk/UdfDxe/Udf.h | 158
>>>++---
>>>> 5 files changed, 606 insertions(+), 565 deletions(-)
>>>>
>>>> diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
>>>> b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
>>>> index 609f56cef6..572ba7a81a 100644
>>>> --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
>>>> +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
>>>> @@ -64,11 +64,12 @@ FindAnchorVolumeDescriptorPointer (
>>>> OUT UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER *AnchorPoint
>>>> )
>>>> {
>>>> - EFI_STATUS Status;
>>>> - UINT32 BlockSize;
>>>> - EFI_LBA EndLBA;
>>>> - EFI_LBA DescriptorLBAs[4];
>>>> - UINTN Index;
>>>> + EFI_STATUS Status;
>>>> + UINT32 BlockSize;
>>>> + EFI_LBA EndLBA;
>>>> + EFI_LBA DescriptorLBAs[4];
>>>> + UINTN Index;
>>>> + UDF_DESCRIPTOR_TAG *DescriptorTag;
>>>>
>>>> BlockSize = BlockIo->Media->BlockSize;
>>>> EndLBA = BlockIo->Media->LastBlock; @@ -88,10 +89,13 @@
>>>> FindAnchorVolumeDescriptorPointer (
>>>> if (EFI_ERROR (Status)) {
>>>> return Status;
>>>> }
>>>> +
>>>> + DescriptorTag = &AnchorPoint->DescriptorTag;
>>>> +
>>>> //
>>>> // Check if read LBA has a valid AVDP descriptor.
>>>> //
>>>> - if (IS_AVDP (AnchorPoint)) {
>>>> + if (DescriptorTag->TagIdentifier ==
>>>UdfAnchorVolumeDescriptorPointer) {
>>>> return EFI_SUCCESS;
>>>> }
>>>> }
>>>> @@ -102,23 +106,18 @@ FindAnchorVolumeDescriptorPointer ( }
>>>>
>>>> /**
>>>> - Check if block device supports a valid UDF file system as
>>>specified by OSTA
>>>> - Universal Disk Format Specification 2.60.
>>>> + Find UDF volume identifiers in a Volume Recognition Sequence.
>>>>
>>>> - @param[in] BlockIo BlockIo interface.
>>>> - @param[in] DiskIo DiskIo interface.
>>>> + @param[in] BlockIo BlockIo interface.
>>>> + @param[in] DiskIo DiskIo interface.
>>>>
>>>> - @retval EFI_SUCCESS UDF file system found.
>>>> - @retval EFI_UNSUPPORTED UDF file system not found.
>>>> - @retval EFI_NO_MEDIA The device has no media.
>>>> - @retval EFI_DEVICE_ERROR The device reported an error.
>>>> - @retval EFI_VOLUME_CORRUPTED The file system structures are
>>>corrupted.
>>>> - @retval EFI_OUT_OF_RESOURCES The scan was not successful due to
>>>lack of
>>>> - resources.
>>>> + @retval EFI_SUCCESS UDF volume identifiers were
>>found.
>>>> + @retval EFI_NOT_FOUND UDF volume identifiers were not
>>>found.
>>>> + @retval other Failed to perform disk I/O.
>>>>
>>>> **/
>>>> EFI_STATUS
>>>> -SupportUdfFileSystem (
>>>> +FindUdfVolumeIdentifiers (
>>>> IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
>>>> IN EFI_DISK_IO_PROTOCOL *DiskIo
>>>> )
>>>> @@ -128,7 +127,6 @@ SupportUdfFileSystem (
>>>> UINT64 EndDiskOffset;
>>>> CDROM_VOLUME_DESCRIPTOR VolDescriptor;
>>>> CDROM_VOLUME_DESCRIPTOR TerminatingVolDescriptor;
>>>> - UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER AnchorPoint;
>>>>
>>>> ZeroMem ((VOID *)&TerminatingVolDescriptor, sizeof
>>>> (CDROM_VOLUME_DESCRIPTOR));
>>>>
>>>> @@ -167,7 +165,7 @@ SupportUdfFileSystem (
>>>> (CompareMem ((VOID *)&VolDescriptor,
>>>> (VOID *)&TerminatingVolDescriptor,
>>>> sizeof (CDROM_VOLUME_DESCRIPTOR)) == 0)) {
>>>> - return EFI_UNSUPPORTED;
>>>> + return EFI_NOT_FOUND;
>>>> }
>>>> }
>>>>
>>>> @@ -176,7 +174,7 @@ SupportUdfFileSystem (
>>>> //
>>>> Offset += UDF_LOGICAL_SECTOR_SIZE;
>>>> if (Offset >= EndDiskOffset) {
>>>> - return EFI_UNSUPPORTED;
>>>> + return EFI_NOT_FOUND;
>>>> }
>>>>
>>>> Status = DiskIo->ReadDisk (
>>>> @@ -196,7 +194,7 @@ SupportUdfFileSystem (
>>>> (CompareMem ((VOID *)VolDescriptor.Unknown.Id,
>>>> (VOID *)UDF_NSR3_IDENTIFIER,
>>>> sizeof (VolDescriptor.Unknown.Id)) != 0)) {
>>>> - return EFI_UNSUPPORTED;
>>>> + return EFI_NOT_FOUND;
>>>> }
>>>>
>>>> //
>>>> @@ -204,7 +202,7 @@ SupportUdfFileSystem (
>>>> //
>>>> Offset += UDF_LOGICAL_SECTOR_SIZE;
>>>> if (Offset >= EndDiskOffset) {
>>>> - return EFI_UNSUPPORTED;
>>>> + return EFI_NOT_FOUND;
>>>> }
>>>>
>>>> Status = DiskIo->ReadDisk (
>>>> @@ -221,15 +219,291 @@ SupportUdfFileSystem (
>>>> if (CompareMem ((VOID *)VolDescriptor.Unknown.Id,
>>>> (VOID *)UDF_TEA_IDENTIFIER,
>>>> sizeof (VolDescriptor.Unknown.Id)) != 0) {
>>>> - return EFI_UNSUPPORTED;
>>>> + return EFI_NOT_FOUND;
>>>> + }
>>>> +
>>>> + return EFI_SUCCESS;
>>>> +}
>>>> +
>>>> +/**
>>>> + Check if Logical Volume Descriptor is supported by current EDK2
>>>UDF file
>>>> + system implementation.
>>>> +
>>>> + @param[in] LogicalVolDesc Logical Volume Descriptor pointer.
>>>> +
>>>> + @retval TRUE Logical Volume Descriptor is
>>>supported.
>>>> + @retval FALSE Logical Volume Descriptor is not
>>>supported.
>>>> +
>>>> +**/
>>>> +BOOLEAN
>>>> +IsLogicalVolumeDescriptorSupported (
>>>> + UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc
>>>> + )
>>>> +{
>>>> + //
>>>> + // Check for a valid UDF revision range
>>>> + //
>>>> + switch (UDF_LVD_REVISION (LogicalVolDesc)) {
>>>> + case 0x0102:
>>>> + case 0x0150:
>>>> + case 0x0200:
>>>> + case 0x0201:
>>>> + case 0x0250:
>>>> + case 0x0260:
>>>> + break;
>>>> + default:
>>>> + return FALSE;
>>>> + }
>>>> +
>>>> + //
>>>> + // Check for a single Partition Map // if
>>>> + (LogicalVolDesc->NumberOfPartitionMaps > 1) {
>>>> + return FALSE;
>>>> + }
>>>> + //
>>>> + // UDF 1.02 revision supports only Type 1 (Physical) partitions,
>>>but
>>>> + // let's check it any way.
>>>> + //
>>>> + // PartitionMap[0] -> type
>>>> + // PartitionMap[1] -> length (in bytes) // if
>>>> + (LogicalVolDesc->PartitionMaps[0] != 1 ||
>>>> + LogicalVolDesc->PartitionMaps[1] != 6) {
>>>> + return FALSE;
>>>> + }
>>>> +
>>>> + return TRUE;
>>>> +}
>>>> +
>>>> +/**
>>>> + Find UDF logical volume location and whether it is supported by
>>>current EDK2
>>>> + UDF file system implementation.
>>>> +
>>>> + @param[in] BlockIo BlockIo interface.
>>>> + @param[in] DiskIo DiskIo interface.
>>>> + @param[in] AnchorPoint Anchor volume descriptor pointer.
>>>> + @param[out] MainVdsStartBlock Main VDS starting block number.
>>>> + @param[out] MainVdsEndBlock Main VDS ending block number.
>>>> +
>>>> + @retval EFI_SUCCESS UDF logical volume was found.
>>>> + @retval EFI_VOLUME_CORRUPTED UDF file system structures are
>>>> corrupted.
>>>> + @retval EFI_UNSUPPORTED UDF logical volume is not
>>>supported.
>>>> + @retval other Failed to perform disk I/O.
>>>> +
>>>> +**/
>>>> +EFI_STATUS
>>>> +FindLogicalVolumeLocation (
>>>> + IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
>>>> + IN EFI_DISK_IO_PROTOCOL *DiskIo,
>>>> + IN UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER *AnchorPoint,
>>>> + OUT UINT64 *MainVdsStartBlock,
>>>> + OUT UINT64 *MainVdsEndBlock
>>>> + )
>>>> +{
>>>> + EFI_STATUS Status;
>>>> + UINT32 BlockSize;
>>>> + EFI_LBA LastBlock;
>>>> + UDF_EXTENT_AD *ExtentAd;
>>>> + UINT64 SeqBlocksNum;
>>>> + UINT64 SeqStartBlock;
>>>> + UINT64 GuardMainVdsStartBlock;
>>>> + VOID *Buffer;
>>>> + UINT64 SeqEndBlock;
>>>> + BOOLEAN StopSequence;
>>>> + UINTN LvdsCount;
>>>> + UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
>>>> + UDF_DESCRIPTOR_TAG *DescriptorTag;
>>>> +
>>>> + BlockSize = BlockIo->Media->BlockSize; LastBlock =
>>>> + BlockIo->Media->LastBlock; ExtentAd =
>>>> + &AnchorPoint->MainVolumeDescriptorSequenceExtent;
>>>> +
>>>> + //
>>>> + // UDF 2.60, 2.2.3.1 struct MainVolumeDescriptorSequenceExtent
>>>> + //
>>>> + // The Main Volume Descriptor Sequence Extent shall have a
>>minimum
>>>length
>>>> of
>>>> + // 16 logical sectors.
>>>> + //
>>>> + // Also make sure it does not exceed maximum number of blocks in
>>>the disk.
>>>> + //
>>>> + SeqBlocksNum = DivU64x32 ((UINT64)ExtentAd->ExtentLength,
>>>BlockSize);
>>>> + if (SeqBlocksNum < 16 || (EFI_LBA)SeqBlocksNum > LastBlock + 1) {
>>>> + return EFI_VOLUME_CORRUPTED;
>>>> + }
>>>> +
>>>> + //
>>>> + // Check for valid Volume Descriptor Sequence starting block
>>>number
>>>> + //
>>>> + SeqStartBlock = (UINT64)ExtentAd->ExtentLocation; if
>>>> + (SeqStartBlock > LastBlock ||
>>>> + SeqStartBlock + SeqBlocksNum - 1 > LastBlock) {
>>>> + return EFI_VOLUME_CORRUPTED;
>>>> }
>>>>
>>>> + GuardMainVdsStartBlock = SeqStartBlock;
>>>> +
>>>> + //
>>>> + // Allocate buffer for reading disk blocks // Buffer =
>>>> + AllocateZeroPool ((UINTN)BlockSize); if (Buffer == NULL) {
>>>> + return EFI_OUT_OF_RESOURCES;
>>>> + }
>>>> +
>>>> + SeqEndBlock = SeqStartBlock + SeqBlocksNum; StopSequence =
>>FALSE;
>>>> + LvdsCount = 0; Status = EFI_VOLUME_CORRUPTED; // // Start Main
>>>> + Volume Descriptor Sequence // for (; SeqStartBlock < SeqEndBlock
>>
>>>> + && !StopSequence;
>>>SeqStartBlock++) {
>>>> + //
>>>> + // Read disk block
>>>> + //
>>>> + Status = BlockIo->ReadBlocks (
>>>> + BlockIo,
>>>> + BlockIo->Media->MediaId,
>>>> + SeqStartBlock,
>>>> + BlockSize,
>>>> + Buffer
>>>> + );
>>>> + if (EFI_ERROR (Status)) {
>>>> + goto Out_Free;
>>>> + }
>>>> +
>>>> + DescriptorTag = Buffer;
>>>> +
>>>> + //
>>>> + // ECMA 167, 8.4.1 Contents of a Volume Descriptor Sequence
>>>> + //
>>>> + // - A Volume Descriptor Sequence shall contain one or more
>>>Primary
>>>> Volume
>>>> + // Descriptors.
>>>> + // - A Volume Descriptor Sequence shall contain zero or more
>>>> Implementation
>>>> + // Use Volume Descriptors.
>>>> + // - A Volume Descriptor Sequence shall contain zero or more
>>>Partition
>>>> + // Descriptors.
>>>> + // - A Volume Descriptor Sequence shall contain zero or more
>>>Logical
>>>> Volume
>>>> + // Descriptors.
>>>> + // - A Volume Descriptor Sequence shall contain zero or more
>>>Unallocated
>>>> + // Space Descriptors.
>>>> + //
>>>> + switch (UDF_TAG_ID (DescriptorTag)) {
>>>> + case UdfPrimaryVolumeDescriptor:
>>>> + case UdfImplemenationUseVolumeDescriptor:
>>>> + case UdfPartitionDescriptor:
>>>> + case UdfUnallocatedSpaceDescriptor:
>>>> + break;
>>>> +
>>>> + case UdfLogicalVolumeDescriptor:
>>>> + LogicalVolDesc = Buffer;
>>>> +
>>>> + //
>>>> + // Check for existence of a single LVD and whether it is
>>>supported by
>>>> + // current EDK2 UDF file system implementation.
>>>> + //
>>>> + if (++LvdsCount > 1 ||
>>>> + !IsLogicalVolumeDescriptorSupported (LogicalVolDesc)) {
>>>> + Status = EFI_UNSUPPORTED;
>>>> + StopSequence = TRUE;
>>>> + }
>>>> +
>>>> + break;
>>>> +
>>>> + case UdfTerminatingDescriptor:
>>>> + //
>>>> + // Stop the sequence when we find a Terminating Descriptor
>>>> + // (aka Unallocated Sector), se we don't have to walk all the
>>>unallocated
>>>> + // area unnecessarily.
>>>> + //
>>>> + StopSequence = TRUE;
>>>> + break;
>>>> +
>>>> + default:
>>>> + //
>>>> + // An invalid Volume Descriptor has been found in the
>>sequece.
>>>Volume is
>>>> + // corrupted.
>>>> + //
>>>> + Status = EFI_VOLUME_CORRUPTED;
>>>> + goto Out_Free;
>>>> + }
>>>> + }
>>>> +
>>>> + //
>>>> + // Check if LVD was found
>>>> + //
>>>> + if (!EFI_ERROR (Status) && LvdsCount == 1) {
>>>> + *MainVdsStartBlock = GuardMainVdsStartBlock;
>>>> + //
>>>> + // We do not need to read either LVD or PD descriptors to know
>>>the last
>>>> + // valid block in the found UDF file system. It's already
>>>LastBlock.
>>>> + //
>>>> + *MainVdsEndBlock = LastBlock;
>>>> +
>>>> + Status = EFI_SUCCESS;
>>>> + }
>>>> +
>>>> +Out_Free:
>>>> + //
>>>> + // Free block read buffer
>>>> + //
>>>> + FreePool (Buffer);
>>>> +
>>>> + return Status;
>>>> +}
>>>> +
>>>> +/**
>>>> + Find a supported UDF file system in block device.
>>>> +
>>>> + @param[in] BlockIo BlockIo interface.
>>>> + @param[in] DiskIo DiskIo interface.
>>>> + @param[out] StartingLBA UDF file system starting LBA.
>>>> + @param[out] EndingLBA UDF file system starting LBA.
>>>> +
>>>> + @retval EFI_SUCCESS UDF file system was found.
>>>> + @retval other UDF file system was not found.
>>>> +
>>>> +**/
>>>> +EFI_STATUS
>>>> +FindUdfFileSystem (
>>>> + IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
>>>> + IN EFI_DISK_IO_PROTOCOL *DiskIo,
>>>> + OUT EFI_LBA *StartingLBA,
>>>> + OUT EFI_LBA *EndingLBA
>>>> + )
>>>> +{
>>>> + EFI_STATUS Status;
>>>> + UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER AnchorPoint;
>>>> +
>>>> + //
>>>> + // Find UDF volume identifiers
>>>> + //
>>>> + Status = FindUdfVolumeIdentifiers (BlockIo, DiskIo); if
>>>> + (EFI_ERROR (Status)) {
>>>> + return Status;
>>>> + }
>>>> +
>>>> + //
>>>> + // Find Anchor Volume Descriptor Pointer //
>>>> Status = FindAnchorVolumeDescriptorPointer (BlockIo, DiskIo,
>>>&AnchorPoint);
>>>> if (EFI_ERROR (Status)) {
>>>> - return EFI_UNSUPPORTED;
>>>> + return Status;
>>>> }
>>>>
>>>> - return EFI_SUCCESS;
>>>> + //
>>>> + // Find Logical Volume location
>>>> + //
>>>> + Status = FindLogicalVolumeLocation (
>>>> + BlockIo,
>>>> + DiskIo,
>>>> + &AnchorPoint,
>>>> + (UINT64 *)StartingLBA,
>>>> + (UINT64 *)EndingLBA
>>>> + );
>>>> +
>>>> + return Status;
>>>> }
>>>>
>>>> /**
>>>> @@ -263,9 +537,9 @@ PartitionInstallUdfChildHandles (
>>>> UINT32 RemainderByMediaBlockSize;
>>>> EFI_STATUS Status;
>>>> EFI_BLOCK_IO_MEDIA *Media;
>>>> - EFI_DEVICE_PATH_PROTOCOL *DevicePathNode;
>>>> - EFI_GUID *VendorDefinedGuid;
>>>> EFI_PARTITION_INFO_PROTOCOL PartitionInfo;
>>>> + EFI_LBA StartingLBA;
>>>> + EFI_LBA EndingLBA;
>>>>
>>>> Media = BlockIo->Media;
>>>>
>>>> @@ -281,35 +555,10 @@ PartitionInstallUdfChildHandles (
>>>> return EFI_NOT_FOUND;
>>>> }
>>>>
>>>> - DevicePathNode = DevicePath;
>>>> - while (!IsDevicePathEnd (DevicePathNode)) {
>>>> - //
>>>> - // Do not allow checking for UDF file systems in CDROM "El
>>>Torito"
>>>> - // partitions, and skip duplicate installation of UDF file
>>>system child
>>>> - // nodes.
>>>> - //
>>>> - if (DevicePathType (DevicePathNode) == MEDIA_DEVICE_PATH) {
>>>> - if (DevicePathSubType (DevicePathNode) == MEDIA_CDROM_DP) {
>>>> - return EFI_NOT_FOUND;
>>>> - }
>>>> - if (DevicePathSubType (DevicePathNode) == MEDIA_VENDOR_DP) {
>>>> - VendorDefinedGuid = (EFI_GUID *)((UINTN)DevicePathNode +
>>>> - OFFSET_OF
>>>(VENDOR_DEVICE_PATH, Guid));
>>>> - if (CompareGuid (VendorDefinedGuid, &gUdfDevPathGuid)) {
>>>> - return EFI_NOT_FOUND;
>>>> - }
>>>> - }
>>>> - }
>>>> - //
>>>> - // Try next device path node
>>>> - //
>>>> - DevicePathNode = NextDevicePathNode (DevicePathNode);
>>>> - }
>>>> -
>>>> //
>>>> - // Check if block device supports an UDF file system
>>>> + // Search for an UDF file system on block device
>>>> //
>>>> - Status = SupportUdfFileSystem (BlockIo, DiskIo);
>>>> + Status = FindUdfFileSystem (BlockIo, DiskIo, &StartingLBA,
>>>&EndingLBA);
>>>> if (EFI_ERROR (Status)) {
>>>> return EFI_NOT_FOUND;
>>>> }
>>>> @@ -334,8 +583,8 @@ PartitionInstallUdfChildHandles (
>>>> DevicePath,
>>>> (EFI_DEVICE_PATH_PROTOCOL *)&gUdfDevicePath,
>>>> &PartitionInfo,
>>>> - 0,
>>>> - Media->LastBlock,
>>>> + StartingLBA,
>>>> + EndingLBA,
>>>> Media->BlockSize
>>>> );
>>>> if (!EFI_ERROR (Status)) {
>>>> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>>>> b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>>>> index 625f2c5637..6f07bf2066 100644
>>>> --- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>>>> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>>>> @@ -131,7 +131,6 @@ Error_Alloc_Priv_File_Data:
>>>> CleanupFileInformation (&PrivFsData->Root);
>>>>
>>>> Error_Find_Root_Dir:
>>>> - CleanupVolumeInformation (&PrivFsData->Volume);
>>>>
>>>> Error_Read_Udf_Volume:
>>>> Error_Invalid_Params:
>>>> @@ -429,7 +428,7 @@ UdfRead (
>>>> }
>>>> ASSERT (NewFileEntryData != NULL);
>>>>
>>>> - if (IS_FE_SYMLINK (NewFileEntryData)) {
>>>> + if (FE_ICB_FILE_TYPE (NewFileEntryData) == UdfFileEntrySymlink)
>>>{
>>>> Status = ResolveSymlink (
>>>> BlockIo,
>>>> DiskIo,
>>>> @@ -529,7 +528,6 @@ UdfClose (
>>>> EFI_TPL OldTpl;
>>>> EFI_STATUS Status;
>>>> PRIVATE_UDF_FILE_DATA *PrivFileData;
>>>> - PRIVATE_UDF_SIMPLE_FS_DATA *PrivFsData;
>>>>
>>>> OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
>>>>
>>>> @@ -542,8 +540,6 @@ UdfClose (
>>>>
>>>> PrivFileData = PRIVATE_UDF_FILE_DATA_FROM_THIS (This);
>>>>
>>>> - PrivFsData = PRIVATE_UDF_SIMPLE_FS_DATA_FROM_THIS (PrivFileData-
>>>> >SimpleFs);
>>>> -
>>>> if (!PrivFileData->IsRootDirectory) {
>>>> CleanupFileInformation (&PrivFileData->File);
>>>>
>>>> @@ -552,10 +548,6 @@ UdfClose (
>>>> }
>>>> }
>>>>
>>>> - if (--PrivFsData->OpenFiles == 0) {
>>>> - CleanupVolumeInformation (&PrivFsData->Volume);
>>>> - }
>>>> -
>>>> FreePool ((VOID *)PrivFileData);
>>>>
>>>> Exit:
>>>> @@ -652,7 +644,7 @@ UdfGetPosition (
>>>> // As per UEFI spec, if the file handle is a directory, then the
>>>current file
>>>> // position has no meaning and the operation is not supported.
>>>> //
>>>> - if (IS_FID_DIRECTORY_FILE
>>>(&PrivFileData->File.FileIdentifierDesc)) {
>>>> + if (IS_FID_DIRECTORY_FILE
>>(PrivFileData->File.FileIdentifierDesc))
>>>{
>>>> return EFI_UNSUPPORTED;
>>>> }
>>>>
>>>> @@ -788,7 +780,7 @@ UdfGetInfo (
>>>> } else if (CompareGuid (InformationType,
>>&gEfiFileSystemInfoGuid))
>>>{
>>>> String = VolumeLabel;
>>>>
>>>> - FileSetDesc = PrivFsData->Volume.FileSetDescs[0];
>>>> + FileSetDesc = &PrivFsData->Volume.FileSetDesc;
>>>>
>>>> OstaCompressed = &FileSetDesc->LogicalVolumeIdentifier[0];
>>>>
>>>> @@ -847,7 +839,7 @@ UdfGetInfo (
>>>> FileSystemInfo->Size = FileSystemInfoLength;
>>>> FileSystemInfo->ReadOnly = TRUE;
>>>> FileSystemInfo->BlockSize =
>>>> - LV_BLOCK_SIZE (&PrivFsData->Volume, UDF_DEFAULT_LV_NUM);
>>>> + PrivFsData->Volume.LogicalVolDesc.LogicalBlockSize;
>>>> FileSystemInfo->VolumeSize = VolumeSize;
>>>> FileSystemInfo->FreeSpace = FreeSpaceSize;
>>>>
>>>> diff --git
>>>a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
>>>> b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
>>>> index 5df267761f..62d817989f 100644
>>>> --- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
>>>> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
>>>> @@ -38,11 +38,12 @@ FindAnchorVolumeDescriptorPointer (
>>>> OUT UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER *AnchorPoint
>>>> )
>>>> {
>>>> - EFI_STATUS Status;
>>>> - UINT32 BlockSize;
>>>> - EFI_LBA EndLBA;
>>>> - EFI_LBA DescriptorLBAs[4];
>>>> - UINTN Index;
>>>> + EFI_STATUS Status;
>>>> + UINT32 BlockSize;
>>>> + EFI_LBA EndLBA;
>>>> + EFI_LBA DescriptorLBAs[4];
>>>> + UINTN Index;
>>>> + UDF_DESCRIPTOR_TAG *DescriptorTag;
>>>>
>>>> BlockSize = BlockIo->Media->BlockSize;
>>>> EndLBA = BlockIo->Media->LastBlock; @@ -62,10 +63,13 @@
>>>> FindAnchorVolumeDescriptorPointer (
>>>> if (EFI_ERROR (Status)) {
>>>> return Status;
>>>> }
>>>> +
>>>> + DescriptorTag = &AnchorPoint->DescriptorTag;
>>>> +
>>>> //
>>>> // Check if read LBA has a valid AVDP descriptor.
>>>> //
>>>> - if (IS_AVDP (AnchorPoint)) {
>>>> + if (DescriptorTag->TagIdentifier ==
>>>UdfAnchorVolumeDescriptorPointer) {
>>>> return EFI_SUCCESS;
>>>> }
>>>> }
>>>> @@ -99,148 +103,98 @@ StartMainVolumeDescriptorSequence (
>>>> OUT UDF_VOLUME_INFO *Volume
>>>> )
>>>> {
>>>> - EFI_STATUS Status;
>>>> - UINT32 BlockSize;
>>>> - UDF_EXTENT_AD *ExtentAd;
>>>> - UINT64 StartingLsn;
>>>> - UINT64 EndingLsn;
>>>> - VOID *Buffer;
>>>> - UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
>>>> - UDF_PARTITION_DESCRIPTOR *PartitionDesc;
>>>> - UINTN Index;
>>>> - UINT32 LogicalBlockSize;
>>>> + EFI_STATUS Status;
>>>> + UINT32 BlockSize;
>>>> + UDF_EXTENT_AD *ExtentAd;
>>>> + EFI_LBA SeqStartBlock;
>>>> + EFI_LBA SeqEndBlock;
>>>> + BOOLEAN StopSequence;
>>>> + VOID *Buffer;
>>>> + UDF_DESCRIPTOR_TAG *DescriptorTag;
>>>> + UINT32 LogicalBlockSize;
>>>> +
>>>> + BlockSize = BlockIo->Media->BlockSize; ExtentAd =
>>>> + &AnchorPoint->MainVolumeDescriptorSequenceExtent;
>>>>
>>>> //
>>>> - // We've already found an ADVP on the volume. It contains the
>>>extent
>>>> - // (MainVolumeDescriptorSequenceExtent) where the Main Volume
>>>> Descriptor
>>>> - // Sequence starts. Therefore, we'll look for Logical Volume
>>>Descriptors and
>>>> - // Partitions Descriptors and save them in memory, accordingly.
>>>> - //
>>>> - // Note also that each descriptor will be aligned on a block size
>>>(BlockSize)
>>>> - // boundary, so we need to read one block at a time.
>>>> + // Allocate buffer for reading disk blocks
>>>> //
>>>> - BlockSize = BlockIo->Media->BlockSize;
>>>> - ExtentAd = &AnchorPoint->MainVolumeDescriptorSequenceExtent;
>>>> - StartingLsn = (UINT64)ExtentAd->ExtentLocation;
>>>> - EndingLsn = StartingLsn + DivU64x32 (
>>>> -
>>(UINT64)ExtentAd->ExtentLength,
>>>> - BlockSize
>>>> - );
>>>> -
>>>> - Volume->LogicalVolDescs =
>>>> - (UDF_LOGICAL_VOLUME_DESCRIPTOR **)AllocateZeroPool (ExtentAd-
>>>> >ExtentLength);
>>>> - if (Volume->LogicalVolDescs == NULL) {
>>>> - return EFI_OUT_OF_RESOURCES;
>>>> - }
>>>> -
>>>> - Volume->PartitionDescs =
>>>> - (UDF_PARTITION_DESCRIPTOR **)AllocateZeroPool (ExtentAd-
>>>> >ExtentLength);
>>>> - if (Volume->PartitionDescs == NULL) {
>>>> - Status = EFI_OUT_OF_RESOURCES;
>>>> - goto Error_Alloc_Pds;
>>>> - }
>>>> -
>>>> - Buffer = AllocateZeroPool (BlockSize);
>>>> + Buffer = AllocateZeroPool ((UINTN)BlockSize);
>>>> if (Buffer == NULL) {
>>>> - Status = EFI_OUT_OF_RESOURCES;
>>>> - goto Error_Alloc_Buf;
>>>> + return EFI_OUT_OF_RESOURCES;
>>>> }
>>>>
>>>> - Volume->LogicalVolDescsNo = 0;
>>>> - Volume->PartitionDescsNo = 0;
>>>> -
>>>> - while (StartingLsn <= EndingLsn) {
>>>> - Status = DiskIo->ReadDisk (
>>>> - DiskIo,
>>>> + //
>>>> + // The logical partition created by Partition driver is relative
>>>to the main
>>>> + // VDS extent location, so we start the Main Volume Descriptor
>>>Sequence at
>>>> + // LBA 0.
>>>> + //
>>>> + // We don't need to check again if we have valid Volume
>>>Descriptors here
>>>> since
>>>> + // Partition driver already did.
>>>> + //
>>>> + SeqStartBlock = 0;
>>>> + SeqEndBlock = SeqStartBlock + DivU64x32
>>>((UINT64)ExtentAd->ExtentLength,
>>>> + BlockSize);
>>StopSequence
>>>> + = FALSE; for (; SeqStartBlock < SeqEndBlock && !StopSequence;
>>>SeqStartBlock++) {
>>>> + //
>>>> + // Read disk block
>>>> + //
>>>> + Status = BlockIo->ReadBlocks (
>>>> + BlockIo,
>>>> BlockIo->Media->MediaId,
>>>> - MultU64x32 (StartingLsn, BlockSize),
>>>> + SeqStartBlock,
>>>> BlockSize,
>>>> Buffer
>>>> );
>>>> if (EFI_ERROR (Status)) {
>>>> - goto Error_Read_Disk_Blk;
>>>> + goto Out_Free;
>>>> }
>>>>
>>>> - if (IS_TD (Buffer)) {
>>>> + DescriptorTag = Buffer;
>>>> +
>>>> + switch (UDF_TAG_ID (DescriptorTag)) {
>>>> + case UdfPartitionDescriptor:
>>>> //
>>>> - // Found a Terminating Descriptor. Stop the sequence then.
>>>> + // Save Partition Descriptor
>>>> //
>>>> + CopyMem (&Volume->PartitionDesc, Buffer, sizeof (Volume-
>>>> >PartitionDesc));
>>>> break;
>>>> - }
>>>>
>>>> - if (IS_LVD (Buffer)) {
>>>> + case UdfLogicalVolumeDescriptor:
>>>> //
>>>> - // Found a Logical Volume Descriptor.
>>>> + // Save Logical Volume Descriptor
>>>> //
>>>> - LogicalVolDesc =
>>>> - (UDF_LOGICAL_VOLUME_DESCRIPTOR *)
>>>> - AllocateZeroPool (sizeof (UDF_LOGICAL_VOLUME_DESCRIPTOR));
>>>> - if (LogicalVolDesc == NULL) {
>>>> - Status = EFI_OUT_OF_RESOURCES;
>>>> - goto Error_Alloc_Lvd;
>>>> - }
>>>> + CopyMem (&Volume->LogicalVolDesc, Buffer, sizeof (Volume-
>>>> >LogicalVolDesc));
>>>> + break;
>>>>
>>>> - CopyMem ((VOID *)LogicalVolDesc, Buffer,
>>>> - sizeof (UDF_LOGICAL_VOLUME_DESCRIPTOR));
>>>> - Volume->LogicalVolDescs[Volume->LogicalVolDescsNo++] =
>>>> LogicalVolDesc;
>>>> - } else if (IS_PD (Buffer)) {
>>>> - //
>>>> - // Found a Partition Descriptor.
>>>> - //
>>>> - PartitionDesc =
>>>> - (UDF_PARTITION_DESCRIPTOR *)
>>>> - AllocateZeroPool (sizeof (UDF_PARTITION_DESCRIPTOR));
>>>> - if (PartitionDesc == NULL) {
>>>> - Status = EFI_OUT_OF_RESOURCES;
>>>> - goto Error_Alloc_Pd;
>>>> - }
>>>> + case UdfTerminatingDescriptor:
>>>> + StopSequence = TRUE;
>>>> + break;
>>>>
>>>> - CopyMem ((VOID *)PartitionDesc, Buffer,
>>>> - sizeof (UDF_PARTITION_DESCRIPTOR));
>>>> - Volume->PartitionDescs[Volume->PartitionDescsNo++] =
>>>PartitionDesc;
>>>> + default:
>>>> + ;
>>>> }
>>>> -
>>>> - StartingLsn++;
>>>> }
>>>>
>>>> //
>>>> - // When an UDF volume (revision 2.00 or higher) contains a File
>>>Entry rather
>>>> - // than an Extended File Entry (which is not recommended as per
>>>spec), we
>>>> need
>>>> - // to make sure the size of a FE will be _at least_ 2048
>>>> - // (UDF_LOGICAL_SECTOR_SIZE) bytes long to keep backward
>>>compatibility.
>>>> + // Determine FE (File Entry) size
>>>> //
>>>> - LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
>>>> + LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
>>>> if (LogicalBlockSize >= UDF_LOGICAL_SECTOR_SIZE) {
>>>> - Volume->FileEntrySize = LogicalBlockSize;
>>>> + Volume->FileEntrySize = (UINTN)LogicalBlockSize;
>>>> } else {
>>>> Volume->FileEntrySize = UDF_LOGICAL_SECTOR_SIZE;
>>>> }
>>>>
>>>> - FreePool (Buffer);
>>>> + Status = EFI_SUCCESS;
>>>>
>>>> - return EFI_SUCCESS;
>>>> -
>>>> -Error_Alloc_Pd:
>>>> -Error_Alloc_Lvd:
>>>> - for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
>>>> - FreePool ((VOID *)Volume->PartitionDescs[Index]);
>>>> - }
>>>> -
>>>> - for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
>>>> - FreePool ((VOID *)Volume->LogicalVolDescs[Index]);
>>>> - }
>>>> -
>>>> -Error_Read_Disk_Blk:
>>>> +Out_Free:
>>>> + //
>>>> + // Free block read buffer
>>>> + //
>>>> FreePool (Buffer);
>>>>
>>>> -Error_Alloc_Buf:
>>>> - FreePool ((VOID *)Volume->PartitionDescs);
>>>> - Volume->PartitionDescs = NULL;
>>>> -
>>>> -Error_Alloc_Pds:
>>>> - FreePool ((VOID *)Volume->LogicalVolDescs);
>>>> - Volume->LogicalVolDescs = NULL;
>>>> -
>>>> return Status;
>>>> }
>>>>
>>>> @@ -262,48 +216,53 @@ GetPdFromLongAd (
>>>> )
>>>> {
>>>> UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
>>>> - UINTN Index;
>>>> - UDF_PARTITION_DESCRIPTOR *PartitionDesc;
>>>> UINT16 PartitionNum;
>>>>
>>>> - LogicalVolDesc = Volume->LogicalVolDescs[UDF_DEFAULT_LV_NUM];
>>>> + LogicalVolDesc = &Volume->LogicalVolDesc;
>>>>
>>>> - switch (LV_UDF_REVISION (LogicalVolDesc)) {
>>>> + switch (UDF_LVD_REVISION (LogicalVolDesc)) {
>>>> case 0x0102:
>>>> + case 0x0150:
>>>> + case 0x0200:
>>>> + case 0x0201:
>>>> + case 0x0250:
>>>> + case 0x0260:
>>>> //
>>>> - // As per UDF 1.02 specification:
>>>> + // UDF 1.02 specification:
>>>> //
>>>> // There shall be exactly one prevailing Logical Volume
>>>Descriptor recorded
>>>> // per Volume Set. The Partition Maps field shall contain only
>>>Type 1
>>>> // Partition Maps.
>>>> //
>>>> - PartitionNum = *(UINT16
>>>*)((UINTN)&LogicalVolDesc->PartitionMaps[4]);
>>>> - break;
>>>> - case 0x0150:
>>>> + // UDF 1.50 through 2.60 specs say:
>>>> //
>>>> - // Ensure Type 1 Partition map. Other types aren't supported in
>>>this
>>>> - // implementation.
>>>> + // For the purpose of interchange partition maps shall be
>>>limited to
>>>> + // Partition Map type 1, except type 2 maps as described in the
>>>document.
>>>> + //
>>>> + // NOTE: Only one Type 1 (Physical) Partition is supported. It
>>>has been
>>>> + // checked already in Partition driver for existence of a
>>single
>>>Type 1
>>>> + // Partition map, so we don't have to double check here.
>>>> + //
>>>> + // Partition reference number can also be retrieved from
>>>> + // LongAd->ExtentLocation.PartitionReferenceNumber, however the
>>>spec
>>>> says
>>>> + // it may be 0, so let's not rely on it.
>>>> //
>>>> - if (LogicalVolDesc->PartitionMaps[0] != 1 ||
>>>> - LogicalVolDesc->PartitionMaps[1] != 6) {
>>>> - return NULL;
>>>> - }
>>>> PartitionNum = *(UINT16
>>>*)((UINTN)&LogicalVolDesc->PartitionMaps[4]);
>>>> break;
>>>> - case 0x0260:
>>>> +
>>>> + default:
>>>> //
>>>> - // Fall through.
>>>> + // Unsupported UDF revision
>>>> //
>>>> - default:
>>>> - PartitionNum = LongAd->ExtentLocation.PartitionReferenceNumber;
>>>> - break;
>>>> + return NULL;
>>>> }
>>>>
>>>> -
>
> --
> Sent from my Android device with K-9 Mail. Please excuse my brevity.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
--
Paulo Alcantara, HP
Speaking for myself only.
next prev parent reply other threads:[~2017-09-21 14:13 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-20 18:16 [PATCH v3 0/2] UDF partition driver fix Paulo Alcantara
2017-09-20 18:16 ` [PATCH v3 1/2] MdePkg: Add UDF volume structure definitions Paulo Alcantara
2017-09-22 2:50 ` Ni, Ruiyu
2017-09-22 13:54 ` Paulo Alcantara
2017-09-20 18:16 ` [PATCH v3 2/2] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition Paulo Alcantara
2017-09-21 8:08 ` Wu, Hao A
2017-09-21 8:49 ` Zeng, Star
2017-09-21 13:22 ` Paulo Alcantara
2017-09-21 12:44 ` Wu, Hao A
2017-09-21 13:29 ` Paulo Alcantara
2017-09-21 13:47 ` Zeng, Star
2017-09-21 13:52 ` Paulo Alcantara
2017-09-21 14:16 ` Paulo Alcantara [this message]
2017-09-22 2:26 ` Zeng, Star
2017-09-22 2:59 ` Ni, Ruiyu
2017-09-20 19:25 ` [PATCH v3 0/2] UDF partition driver fix Laszlo Ersek
2017-09-20 19:38 ` 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=CAO1t2O2KqTQjH6Zeknxw0--LGk5HOemb0kRptHzLbnZdwBLTgQ@mail.gmail.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