From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id A5A9B21E9781D for ; Fri, 15 Sep 2017 09:42:15 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 825B48046F; Fri, 15 Sep 2017 16:45:15 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 825B48046F Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-125-162.rdu2.redhat.com [10.10.125.162]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6AC657E49C; Fri, 15 Sep 2017 16:45:13 +0000 (UTC) To: Paulo Alcantara , "Ni, Ruiyu" , "Yao, Jiewen" Cc: "Wu, Hao A" , "edk2-devel@lists.01.org" , "Zeng, Star" References: <734D49CCEBEEF84792F5B80ED585239D5BA4206B@SHSMSX151.ccr.corp.intel.com> <081B7D33-9F33-40CE-B569-62CC8C204B56@zytor.com> <734D49CCEBEEF84792F5B80ED585239D5BA420DF@SHSMSX151.ccr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D5BA4212E@SHSMSX151.ccr.corp.intel.com> <1C01A5C1-09DE-4747-BA65-4EB668D76094@zytor.com> <734D49CCEBEEF84792F5B80ED585239D5BA42270@SHSMSX151.ccr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D5BA42713@SHSMSX151.ccr.corp.intel.com> <508e1df7-fe00-126b-d583-c81db8514e10@redhat.com> From: Laszlo Ersek Message-ID: <2eaad62b-48ae-3db0-a04c-1f4f55ee12e8@redhat.com> Date: Fri, 15 Sep 2017 18:45:11 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Fri, 15 Sep 2017 16:45:15 +0000 (UTC) Subject: Re: Functionality issues in UDF support X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 15 Sep 2017 16:42:15 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit On 09/15/17 18:27, Paulo Alcantara wrote: > 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. I understand that -- the regression is not necessarily an OS boot regression, yet the fact that now the protocol database contains handles and protocols that should not be there is a regression. > >> >>   > 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. Thank you for your agreement / understanding -- I'd *very much* like general UDF support to mature to the point where it can be enabled by default. Thanks Laszlo