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 E298A21E9781E for ; Fri, 15 Sep 2017 08:48:23 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C0F2F267CA; Fri, 15 Sep 2017 15:51:23 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com C0F2F267CA Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.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 00C767B5B0; Fri, 15 Sep 2017 15:51:21 +0000 (UTC) To: "Ni, Ruiyu" , Paulo Alcantara , "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> From: Laszlo Ersek Message-ID: <508e1df7-fe00-126b-d583-c81db8514e10@redhat.com> Date: Fri, 15 Sep 2017 17:51:20 +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: <734D49CCEBEEF84792F5B80ED585239D5BA42713@SHSMSX151.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Fri, 15 Sep 2017 15:51:23 +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 15:48:24 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 from Mantis#1835 (I'm at liberty to quote it because I wrote it): > 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. Thank you, Laszlo