Laszlo, You are surprised because the issue is related to Redhat, or because the word "Redhat" should not be mentioned in the Bugzilla?
http://manpages.ubuntu.com/manpages/bionic/man8/mkudffs.8.html is a tool that creates UDF file system with a compatible MBR table in the top 32KB specially for Windows. Search " WHOLE DISK VS PARTITION" in the web page. I think maybe edk2 Mbr logic should (but doesn't) skip MBR whose partition starts from LBA #0 (includes MBR itself). > -----Original Message----- > From: Laszlo Ersek <ler...@redhat.com> > Sent: Wednesday, June 24, 2020 7:58 PM > To: devel@edk2.groups.io; Ni, Ray <ray...@intel.com>; Gao, Zhichao > <zhichao....@intel.com> > Cc: Wu, Hao A <hao.a...@intel.com>; Wang, Jian J <jian.j.w...@intel.com>; > Gao, Liming <liming....@intel.com> > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/PartitionDxe: Seperate the Udf > handler > > 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 (#61698): https://edk2.groups.io/g/devel/message/61698 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] -=-=-=-=-=-=-=-=-=-=-=-
