Skip to content

Conversation

@ZhichaoGao
Copy link
Contributor

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2823
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2843

Refer to the 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

But the code implementation is:

  1. GPT
  2. MBR
  3. ISO 9660 (El Torito) (UDF)
  4. no partition found

Which would cause the ISO 9960 image with MBR info be treated as MBR
device. That would cause unexpect behavior. So fix it to follow the spec
description.

The fix of the PartitionInstallChildHandle would change the boot behavior
of Linux ISO image with MBR table. So add it after the order adjustment
to make no impact of the boot.

V2:

  1. Add description of the different behavior between grub boot from MBR path and
    from CD path
  2. change patch BaseTools/Source/C/GNUmakefile: Added a condition for handling invalid A... #2 to revert "MdeModulePkg/PartitionDxe: Skip the MBR that add for CD-ROM"

Cc: Jian J Wang [email protected]
Cc: Hao A Wu [email protected]
Cc: Ray Ni [email protected]
Cc: Gary Lin [email protected]
Cc: Andrew Fish [email protected]
Signed-off-by: Zhichao Gao [email protected]

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:
If the CD/DVD's MBR be handled correctly, it would be enumerated
as a bootable device with MBR path and FAT filesystem. Some Linux
Distributions boot from such path (FAT with MBR path for ISO) would
come into the grub console instead of the installation selection.
With this change, the CD/DVD would always be enumerated with CD path.
And it would always boot to the installation selection.

Cc: Jian J Wang <[email protected]>
Cc: Hao A Wu <[email protected]>
Cc: Ray Ni <[email protected]>
Cc: Gary Lin <[email protected]>
Cc: Andrew Fish <[email protected]>
Signed-off-by: Zhichao Gao <[email protected]>
Reviewed-by: Ray Ni <[email protected]>
Reviewed-by: Hao A Wu <[email protected]>
Tested-by: Gary Lin <[email protected]>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2823

Revert "MdeModulePkg/PartitionDxe: Skip the MBR that add for CD-ROM"

Follow the spec definition, the ISO 9660 (and UDF) would be
checked before the MBR. So it is not required to skip such
MBR talbe that contian the entire block device.

Cc: Jian J Wang <[email protected]>
Cc: Hao A Wu <[email protected]>
Cc: Ray Ni <[email protected]>
Cc: Gary Lin <[email protected]>
Cc: Andrew Fish <[email protected]>
Signed-off-by: Zhichao Gao <[email protected]>
Reviewed-by: Ray Ni <[email protected]>
Reviewed-by: Hao A Wu <[email protected]>
Tested-by: Gary Lin <[email protected]>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2843

PartitionInstallChildHandle's parameters Start and End is counted
by the BlockSize, but in the implementation it uses the parent
device's BlockSize to calculate the new Start, End and LastBlock.
It would cause the driver report incorrect block scope and the file
system would fail to be found with right block scope.
So correct it to the right value.

Cc: Jian J Wang <[email protected]>
Cc: Hao A Wu <[email protected]>
Cc: Ray Ni <[email protected]>
Cc: Gary Lin <[email protected]>
Cc: Andrew Fish <[email protected]>
Signed-off-by: Zhichao Gao <[email protected]>
Reviewed-by: Ray Ni <[email protected]>
Reviewed-by: Hao A Wu <[email protected]>
Tested-by: Gary Lin <[email protected]>
@mergify mergify bot closed this Aug 13, 2020
@mergify
Copy link

mergify bot commented Aug 13, 2020

All checks passed. Auto close personal build.

@hwu25 hwu25 added the push Auto push patch series in PR if all checks pass label Aug 13, 2020
@ZhichaoGao ZhichaoGao reopened this Aug 13, 2020
@mergify mergify bot merged commit e0eacd7 into tianocore:master Aug 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

push Auto push patch series in PR if all checks pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants