On 06/24/20 10:32, Ni, Ray wrote: > Zhichao, > Will two BlockIo child devices cover the same range in the ISO after your > patch? > > I don't think that's a valid behavior.
Right; I'm surprised to find the Red Hat reference in both the commit message and the BZ. I can't say I understand the issue at hand, but are we sure this isn't a bug -- a standards conformance problem -- with the ISO in question? Zhichao, do you have a link to an ISO image that triggers the problem, and can you provide a hexdump of the part of the image that is either buggy, or triggers the problem in edk2? With a more specific issue description, I might be able to find someone at Red Hat who could comment with more insight than myself. Thanks! Laszlo > > Thanks, > Ray > >> -----Original Message----- >> From: Gao, Zhichao <zhichao....@intel.com> >> Sent: Wednesday, June 24, 2020 1:56 PM >> To: devel@edk2.groups.io >> Cc: Wu, Hao A <hao.a...@intel.com>; Ni, Ray <ray...@intel.com>; Wang, Jian J >> <jian.j.w...@intel.com>; Gao, Liming >> <liming....@intel.com> >> Subject: [PATCH] MdeModulePkg/PartitionDxe: Seperate the Udf handler >> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2823 >> >> Some linux iso like Redhat would contain both MBR info in the first >> 512 bytes and volume info at the beginning of 32KB offset. >> But the partition driver would only choose one of the GPT, MBR, and >> UDF(el torito compatible) to install the partition. That would lose >> one info for such linux ISO during one connect. And UDF(el torito >> compatible) is not conflicted with MBR/GPT. So partition driver should >> check UDF and MBR/GPT separately. >> >> Cc: Hao A Wu <hao.a...@intel.com> >> Cc: Ray Ni <ray...@intel.com> >> Cc: Jian J Wang <jian.j.w...@intel.com> >> Cc: Liming Gao <liming....@intel.com> >> Signed-off-by: Zhichao Gao <zhichao....@intel.com> >> --- >> .../Universal/Disk/PartitionDxe/Partition.c | 52 +++++++++++++------ >> 1 file changed, 37 insertions(+), 15 deletions(-) >> >> diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c >> b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c >> index d1c878ad2e..562490db4f 100644 >> --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c >> +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c >> @@ -5,7 +5,7 @@ >> MBR, and GPT partition schemes are supported. >> >> Copyright (c) 2018 Qualcomm Datacenter Technologies, Inc. >> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR> >> +Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.<BR> >> SPDX-License-Identifier: BSD-2-Clause-Patent >> >> **/ >> @@ -39,7 +39,6 @@ EFI_DRIVER_BINDING_PROTOCOL gPartitionDriverBinding = { >> PARTITION_DETECT_ROUTINE mPartitionDetectRoutineTable[] = { >> PartitionInstallGptChildHandles, >> PartitionInstallMbrChildHandles, >> - PartitionInstallUdfChildHandles, >> NULL >> }; >> >> @@ -189,6 +188,8 @@ PartitionDriverBindingStart ( >> { >> EFI_STATUS Status; >> EFI_STATUS OpenStatus; >> + EFI_STATUS UdfStatus; >> + EFI_STATUS GptMbrStatus; >> EFI_BLOCK_IO_PROTOCOL *BlockIo; >> EFI_BLOCK_IO2_PROTOCOL *BlockIo2; >> EFI_DISK_IO_PROTOCOL *DiskIo; >> @@ -300,26 +301,47 @@ PartitionDriverBindingStart ( >> if (BlockIo->Media->MediaPresent || >> (BlockIo->Media->RemovableMedia && >> !BlockIo->Media->LogicalPartition)) { >> // >> - // Try for GPT, then legacy MBR partition types, and then UDF and El >> Torito. >> - // If the media supports a given partition type install child handles to >> - // represent the partitions described by the media. >> + // Try for UDF (El TOrito compatible) and GPT/MBR partition types. >> + // If the media supports a given partition type, it would install child >> handles >> + // to represent the partitions described by the media. >> + // Notes: GPT is conflicted with MBR. It would only install one of them >> to describe >> + // the partition. UDF (or El Torito) can exist with MBR, so need to >> check it along with >> + // GPT/MBR. >> // >> + UdfStatus = PartitionInstallUdfChildHandles ( >> + This, >> + ControllerHandle, >> + DiskIo, >> + DiskIo2, >> + BlockIo, >> + BlockIo2, >> + ParentDevicePath >> + ); >> + >> Routine = &mPartitionDetectRoutineTable[0]; >> while (*Routine != NULL) { >> - Status = (*Routine) ( >> - This, >> - ControllerHandle, >> - DiskIo, >> - DiskIo2, >> - BlockIo, >> - BlockIo2, >> - ParentDevicePath >> - ); >> - if (!EFI_ERROR (Status) || Status == EFI_MEDIA_CHANGED || Status == >> EFI_NO_MEDIA) { >> + GptMbrStatus = (*Routine) ( >> + This, >> + ControllerHandle, >> + DiskIo, >> + DiskIo2, >> + BlockIo, >> + BlockIo2, >> + ParentDevicePath >> + ); >> + if (!EFI_ERROR (GptMbrStatus) || GptMbrStatus == EFI_MEDIA_CHANGED || >> GptMbrStatus == EFI_NO_MEDIA) { >> break; >> } >> Routine++; >> } >> + >> + if (!EFI_ERROR (UdfStatus) || !EFI_ERROR (GptMbrStatus)) { >> + Status = EFI_SUCCESS; >> + } else if (UdfStatus == EFI_MEDIA_CHANGED || GptMbrStatus == >> EFI_MEDIA_CHANGED) { >> + Status = EFI_MEDIA_CHANGED; >> + } else if (UdfStatus == EFI_NO_MEDIA || GptMbrStatus == EFI_NO_MEDIA) { >> + Status = EFI_NO_MEDIA; >> + } >> } >> // >> // In the case that the driver is already started (OpenStatus == >> EFI_ALREADY_STARTED), >> -- >> 2.21.0.windows.1 > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#61652): https://edk2.groups.io/g/devel/message/61652 Mute This Topic: https://groups.io/mt/75076631/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
