Laszlo,

On 15/09/2017 12:51, Laszlo Ersek wrote:
On 09/15/17 09:38, Ni, Ruiyu wrote:
Paulo,
Supporting multiple LVDs can be considered as a long-term task, or can be added
per request.
But I think a urgent fix in PartitionDxe driver is to:
1. create child BLK only for the portion covered by the LVD
2. Do not create child BLK for LVD that's actually an Eltorito LVD

I submitted a Bugzilla to record this:
https://bugzilla.tianocore.org/show_bug.cgi?id=707

Ray,

I've just assigned this bug to myself, for investigation.

I think I understand the problem, and the algorithm that's needed for
solving the problem is not complex:

- iterate over the logical volume descriptors
- if the current one is ElTorito, continue iterating
- if the current one is not ElTorito, stop, and then install a child for
   only the portion of the parent BlkIo (using *inclusive* start and stop
   block numbers) that are covered by the current logical volume
   descriptor
- if no LVD is found that is *not* ElTorito, then install no children,
   and report failure
- future extension: produce children for all non-ElTorito LVDs.

The problem is that I have absolutely no knowledge of UDF specifics, and
it would take me quite a bit of time to learn enough to actually
implement the details for the above algorithm. I don't think this
breakage should be allowed to stay in the tree until either I learn
enough to fix it, or Paulo finds enough time to write the code (he
doesn't have to start learning from zero, but apparently can't find the
time to fix the breakage).

If I understand correctly, this is exactly the kind of regression that
is *not* localized, it messes up the system. Booting from UDF *Bridge*
disks is *already* required by the UEFI spec, and it used to work before
the UDF-related changes. I will not quote section "13.3.2.1 ISO-9660 and
El Torito" of the UEFI-2.7 spec here, because it is confusing; instead I
will quote the improved / proposed wording froam Mantis#1835 (I'm at
liberty to quote it because I wrote it):

I can still boot my Windows 10 ISO image which is an UDF bridge disk image with the UDF changes. With or without UdfDxe driver. The problem is creating UDF child handles that occupy the entire medium instead of a single LVD, hence you can observe the wrong new partition handles & device paths from Ray's mapping out.


  > A DVD-ROM image formatted as required by the UDF 2.0 specification
(OSTA Universal Disk Format Specification, Revision 2.0) shall be
booted by UEFI if:

- the DVD-ROM image conforms to the "UDF Bridge" format defined in the
UDF 2.0 specification, and

- the DVD-ROM image contains exactly one ISO-9660 file system, and

- the ISO-9660 file system conforms to the "El Torito" Bootable CD-ROM
Format Specification.

Booting from a DVD-ROM satisfying these requirements is accomplished
using the same methods as booting from a CD-ROM: the ISO-9660 file
system shall be booted.

Again, such disks / disk images could *already* be booted on edk2,
before changing PartitionDxe and adding UdfDxe. By introducing the
PartitionDxe changes, previous functionality is disturbed.

Namely, according to your description at the start of this thread,
PartitionInstallUdfChildHandles() creates handles (with BlockIo
protocols and device paths) for such volumes -- ElTorito ones -- that
are already *owned* by other components in the firmware. In other words,
PartitionInstallUdfChildHandles() infringes on the ownership &
responsibilities of other components, when there is a UDF *Bridge*
volume in the system, which was handled correctly before.
>

This is the textbook definition of regression. My suggestion is that we
disable PartitionInstallUdfChildHandles() entirely, at least
temporarily. This should give Paulo enough time to fix the bug *well*. I
strongly suggest that we don't try to rush the fix; it could only lead
to even more problems.

I'm going to submit a patch series that introduces a feature PCD and
short-circuits PartitionInstallUdfChildHandles() if the PCD is clear.

Since I can't fix it in a pleasant time, as well as it's not a simple fix, that might help us a lot.

Thanks!
Paulo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to