public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Gao, Zhichao" <zhichao.gao@intel.com>
To: "Ni, Ray" <ray.ni@intel.com>,
	"devel@edk2.groups.io" <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
Date: Wed, 12 Aug 2020 00:44:39 +0000	[thread overview]
Message-ID: <CH2PR11MB442495F92EB92EAE39F166F3F6420@CH2PR11MB4424.namprd11.prod.outlook.com> (raw)
In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5C6690F1@SHSMSX104.ccr.corp.intel.com>

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
> > >
> >
> 


  reply	other threads:[~2020-08-12  0:45 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-11  6:42 [PATCH 0/3] MdeModulePkg/PartitionDxe: Make the parition driver match the spec Gao, Zhichao
2020-08-11  6:43 ` [PATCH 1/3] MdeModulePkg/PartitionDxe: Put the UDF check ahead of MBR Gao, Zhichao
2020-08-11  8:04   ` Ni, Ray
2020-08-11  8:33     ` Gao, Zhichao
2020-08-11 13:48       ` Ni, Ray
2020-08-12  0:44         ` Gao, Zhichao [this message]
2020-08-12  1:57           ` [edk2-devel] " Ni, Ray
2020-08-11  6:43 ` [PATCH 2/3] MdeModulePkg/PartitionDxe: Remove the check for special MBR Gao, Zhichao
2020-08-11  8:05   ` Ni, Ray
2020-08-11  8:28     ` [edk2-devel] " Gao, Zhichao
2020-08-11 13:49       ` Ni, Ray
2020-08-11  6:43 ` [PATCH 3/3] MdeModulePkg/PartitionDxe: Fix the incorrect LBA size in child hander Gao, Zhichao
2020-08-11  8:08   ` Ni, Ray
2020-08-11  7:54 ` [PATCH 0/3] MdeModulePkg/PartitionDxe: Make the parition driver match the spec Gary Lin

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=CH2PR11MB442495F92EB92EAE39F166F3F6420@CH2PR11MB4424.namprd11.prod.outlook.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