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


Gao, Zhichao
 

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@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Ray Ni <ray.ni@...>
Cc: Gary Lin <glin@...>
Cc: Andrew Fish <afish@...>
Signed-off-by: Zhichao Gao <zhichao.gao@...>
---
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


Ni, Ray
 

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@...>
Sent: Tuesday, August 11, 2020 2:43 PM
To: devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>; Ni, Ray <ray.ni@...>; Gary Lin
<glin@...>; Andrew Fish <afish@...>
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@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Ray Ni <ray.ni@...>
Cc: Gary Lin <glin@...>
Cc: Andrew Fish <afish@...>
Signed-off-by: Zhichao Gao <zhichao.gao@...>
---
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


Gao, Zhichao
 

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@...>
Sent: Tuesday, August 11, 2020 4:04 PM
To: Gao, Zhichao <zhichao.gao@...>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>;
Gary Lin <glin@...>; Andrew Fish <afish@...>
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@...>
Sent: Tuesday, August 11, 2020 2:43 PM
To: devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Wu, Hao A
<hao.a.wu@...>; Ni, Ray <ray.ni@...>; Gary Lin
<glin@...>; Andrew Fish <afish@...>
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@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Ray Ni <ray.ni@...>
Cc: Gary Lin <glin@...>
Cc: Andrew Fish <afish@...>
Signed-off-by: Zhichao Gao <zhichao.gao@...>
---
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


Ni, Ray
 

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@...>
Sent: Tuesday, August 11, 2020 4:34 PM
To: Ni, Ray <ray.ni@...>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>;
Gary Lin <glin@...>; Andrew Fish <afish@...>
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@...>
Sent: Tuesday, August 11, 2020 4:04 PM
To: Gao, Zhichao <zhichao.gao@...>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>;
Gary Lin <glin@...>; Andrew Fish <afish@...>
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@...>
Sent: Tuesday, August 11, 2020 2:43 PM
To: devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Wu, Hao A
<hao.a.wu@...>; Ni, Ray <ray.ni@...>; Gary Lin
<glin@...>; Andrew Fish <afish@...>
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@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Ray Ni <ray.ni@...>
Cc: Gary Lin <glin@...>
Cc: Andrew Fish <afish@...>
Signed-off-by: Zhichao Gao <zhichao.gao@...>
---
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


Gao, Zhichao
 

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@...>
Sent: Tuesday, August 11, 2020 9:49 PM
To: Gao, Zhichao <zhichao.gao@...>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>;
Gary Lin <glin@...>; Andrew Fish <afish@...>
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@...>
Sent: Tuesday, August 11, 2020 4:34 PM
To: Ni, Ray <ray.ni@...>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Wu, Hao A
<hao.a.wu@...>; Gary Lin <glin@...>; Andrew Fish
<afish@...>
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@...>
Sent: Tuesday, August 11, 2020 4:04 PM
To: Gao, Zhichao <zhichao.gao@...>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Wu, Hao A
<hao.a.wu@...>; Gary Lin <glin@...>; Andrew Fish
<afish@...>
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@...>
Sent: Tuesday, August 11, 2020 2:43 PM
To: devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Wu, Hao A
<hao.a.wu@...>; Ni, Ray <ray.ni@...>; Gary Lin
<glin@...>; Andrew Fish <afish@...>
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@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Ray Ni <ray.ni@...>
Cc: Gary Lin <glin@...>
Cc: Andrew Fish <afish@...>
Signed-off-by: Zhichao Gao <zhichao.gao@...>
---
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


Ni, Ray
 

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@...>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>; Gary Lin <glin@...>; Andrew Fish
<afish@...>
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@...>
Sent: Tuesday, August 11, 2020 9:49 PM
To: Gao, Zhichao <zhichao.gao@...>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>;
Gary Lin <glin@...>; Andrew Fish <afish@...>
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@...>
Sent: Tuesday, August 11, 2020 4:34 PM
To: Ni, Ray <ray.ni@...>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Wu, Hao A
<hao.a.wu@...>; Gary Lin <glin@...>; Andrew Fish
<afish@...>
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@...>
Sent: Tuesday, August 11, 2020 4:04 PM
To: Gao, Zhichao <zhichao.gao@...>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Wu, Hao A
<hao.a.wu@...>; Gary Lin <glin@...>; Andrew Fish
<afish@...>
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@...>
Sent: Tuesday, August 11, 2020 2:43 PM
To: devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Wu, Hao A
<hao.a.wu@...>; Ni, Ray <ray.ni@...>; Gary Lin
<glin@...>; Andrew Fish <afish@...>
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@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Ray Ni <ray.ni@...>
Cc: Gary Lin <glin@...>
Cc: Andrew Fish <afish@...>
Signed-off-by: Zhichao Gao <zhichao.gao@...>
---
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