Re: Functionality issues in UDF support

["Ni, Ruiyu" <ruiyu.ni@intel.com>]

Laszlo,
Please do not add a PCD for this. Too many PCDs are no good to the project.

发自我的 iPhone

> 在 2017年9月16日，上午12:27，Paulo Alcantara <pcacjr@zytor.com> 写道：
> 
> 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