Re: [edk2-devel] [PATCH 1/3] MdeModulePkg/PartitionDxe: Put the UDF check ahead of MBR

["Ni, Ray" <ray.ni@intel.com>]

Thanks. Good enough!

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gao, Zhichao
> Sent: Wednesday, August 12, 2020 8:45 AM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gary Lin <glin@suse.com>; Andrew Fish
> <afish@apple.com>
> Subject: Re: [edk2-devel] [PATCH 1/3] MdeModulePkg/PartitionDxe: Put the UDF check ahead of MBR
> 
> OK. I would put the detail in next version patch. Let me put a sample here:
> 
> With patch #3 but without patch #1, the MBR table of ISO 9660 image can be handled correctly, i.e. it would be treat as
> MBR block device. We can find the bootable image thru MBR path FAT filesystem. When boot Linux Distribution, it comes
> into the grub console instead of the installation selection.
> 
> Thanks,
> Zhichao
> 
> > -----Original Message-----
> > From: Ni, Ray <ray.ni@intel.com>
> > Sent: Tuesday, August 11, 2020 9:49 PM
> > To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> > Gary Lin <glin@suse.com>; Andrew Fish <afish@apple.com>
> > Subject: RE: [PATCH 1/3] MdeModulePkg/PartitionDxe: Put the UDF check ahead
> > of MBR
> >
> > > > This would also solve the issue that ISO image with MBR would be
> > > > treat as MBR device instead of CD/DVD. That would make the behavior
> > > > of the image boot different.
> >
> > Can you please explain this in detail?
> > It's ok to not provide the "root" cause of why the image boot behavior is different.
> > Saying the specific issue can help people to understand the issue in future.
> >
> > > -----Original Message-----
> > > From: Gao, Zhichao <zhichao.gao@intel.com>
> > > Sent: Tuesday, August 11, 2020 4:34 PM
> > > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > > <hao.a.wu@intel.com>; Gary Lin <glin@suse.com>; Andrew Fish
> > > <afish@apple.com>
> > > Subject: RE: [PATCH 1/3] MdeModulePkg/PartitionDxe: Put the UDF check
> > > ahead of MBR
> > >
> > > Ray,
> > >
> > > The MBR info is correct. The order change is to avoid the MBR being
> > > checked before UDF/ISO 9660 check.
> > > That is why I make the patch #3 in the last of the patch set.
> > >
> > > Thanks,
> > > Zhichao
> > >
> > > > -----Original Message-----
> > > > From: Ni, Ray <ray.ni@intel.com>
> > > > Sent: Tuesday, August 11, 2020 4:04 PM
> > > > To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
> > > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > > > <hao.a.wu@intel.com>; Gary Lin <glin@suse.com>; Andrew Fish
> > > > <afish@apple.com>
> > > > Subject: RE: [PATCH 1/3] MdeModulePkg/PartitionDxe: Put the UDF
> > > > check
> > > ahead
> > > > of MBR
> > > >
> > > > Zhichao,
> > > > Can you also add notes in the commit message describing that for
> > > > some ISOs (better with more specific ISO info), the MBR information is not
> > correct?
> > > >
> > > > Thanks,
> > > > Ray
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Gao, Zhichao <zhichao.gao@intel.com>
> > > > > Sent: Tuesday, August 11, 2020 2:43 PM
> > > > > To: devel@edk2.groups.io
> > > > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > > > > <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Gary Lin
> > > > > <glin@suse.com>; Andrew Fish <afish@apple.com>
> > > > > Subject: [PATCH 1/3] MdeModulePkg/PartitionDxe: Put the UDF check
> > > > > ahead of MBR
> > > > >
> > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2823
> > > > >
> > > > > Refer to UEFI spec 2.8, Section 13.3.2, a block device should be
> > > > > scanned as below order:
> > > > > 1. GPT
> > > > > 2. ISO 9660 (El Torito) (UDF should aslo be here) 3. MBR 4. no
> > > > > partition found
> > > > > Note: UDF is using the same boot method as CD, so put it in the
> > > > > same priority with ISO 9660.
> > > > >
> > > > > This would also solve the issue that ISO image with MBR would be
> > > > > treat as MBR device instead of CD/DVD. That would make the
> > > > > behavior of the image boot different.
> > > > >
> > > > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > > > > Cc: Ray Ni <ray.ni@intel.com>
> > > > > Cc: Gary Lin <glin@suse.com>
> > > > > Cc: Andrew Fish <afish@apple.com>
> > > > > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > > > > ---
> > > > >  MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c | 10
> > > > > +++++++++-
> > > > >  1 file changed, 9 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> > > > > b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> > > > > index 6a43c3cafb..473e091320 100644
> > > > > --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> > > > > +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> > > > > @@ -35,11 +35,19 @@ EFI_DRIVER_BINDING_PROTOCOL
> > > > > gPartitionDriverBinding = {
> > > > >
> > > > >  //
> > > > >  // Prioritized function list to detect partition table.
> > > > > +// Refer to UEFI Spec 13.3.2 Partition Discovery, the block
> > > > > +device // should be scanned in below order:
> > > > > +// 1. GPT
> > > > > +// 2. ISO 9660 (El Torito) (or UDF) // 3. MBR // 4. no partiton
> > > > > +found // Note: UDF is using a same method as booting from CD-ROM,
> > > > > +so put it
> > > > along
> > > > > +//        with CD-ROM check.
> > > > >  //
> > > > >  PARTITION_DETECT_ROUTINE mPartitionDetectRoutineTable[] = {
> > > > >    PartitionInstallGptChildHandles,
> > > > > -  PartitionInstallMbrChildHandles,
> > > > >    PartitionInstallUdfChildHandles,
> > > > > +  PartitionInstallMbrChildHandles,
> > > > >    NULL
> > > > >  };
> > > > >
> > > > > --
> > > > > 2.21.0.windows.1
> > > >
> > >
> >
> 
> 
>