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 DBF8F2095DE56 for ; Thu, 10 Aug 2017 02:26:20 -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 83AD970024; Thu, 10 Aug 2017 09:28:39 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 83AD970024 Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-96.phx2.redhat.com [10.3.116.96]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2D64A60BEB; Thu, 10 Aug 2017 09:28:37 +0000 (UTC) To: Andrew Fish Cc: Jordan Justen , edk2-devel@lists.01.org, Ard Biesheuvel References: <20170808193143.18128-1-pcacjr@zytor.com> <20170808193143.18128-5-pcacjr@zytor.com> <3801BBFD-12B7-485E-8740-F333A4823E9B@apple.com> <61EDB625-E675-4F52-B3D7-3FC71E41EFFC@apple.com> From: Laszlo Ersek Message-ID: <38504bc5-6eee-36d2-4c9e-d02fefbc73f9@redhat.com> Date: Thu, 10 Aug 2017 11:28:37 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <61EDB625-E675-4F52-B3D7-3FC71E41EFFC@apple.com> 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.27]); Thu, 10 Aug 2017 09:28:39 +0000 (UTC) Subject: Re: [PATCH 4/4] OvmfPkg: Introduce UDF_ENABLE build flag 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: Thu, 10 Aug 2017 09:26:21 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit On 08/09/17 19:51, Andrew Fish wrote: > >> On Aug 9, 2017, at 10:33 AM, Laszlo Ersek wrote: >> >> On 08/09/17 17:45, Andrew Fish wrote: >>> >>>> On Aug 9, 2017, at 2:44 AM, Laszlo Ersek wrote: >>>> >>>> CC Ard and Andrew >>>> >>>> On 08/08/17 21:31, Paulo Alcantara wrote: >>>>> By defining this build flag, OVMF will support booting from UDF file >>>>> systems. >>>>> >>>>> Cc: Jordan Justen >>>>> Cc: Laszlo Ersek >>>>> Contributed-under: TianoCore Contribution Agreement 1.1 >>>>> Signed-off-by: Paulo Alcantara >>>>> --- >>>>> OvmfPkg/OvmfPkgIa32.dsc | 7 +++++++ >>>>> OvmfPkg/OvmfPkgIa32.fdf | 3 +++ >>>>> OvmfPkg/OvmfPkgIa32X64.dsc | 7 +++++++ >>>>> OvmfPkg/OvmfPkgIa32X64.fdf | 3 +++ >>>>> OvmfPkg/OvmfPkgX64.dsc | 7 +++++++ >>>>> OvmfPkg/OvmfPkgX64.fdf | 3 +++ >>>>> 6 files changed, 30 insertions(+) >>>> >>>> Ray already mentioned that PcdUdfFileSystemSupport is not needed. I >>>> agree. Similarly, I think UDF_ENABLE is also not needed, the new driver >>>> should be added to the DSC and FDF files right after "Fat.inf" (like you >>>> are doing it now, just unconditionally). >>>> >>>> Furthermore, can you please do the same in the ArmVirtPkg DSC and FDF >>>> files? (Just grep the tree for "Fat.inf".) EmulatorPkg and Nt32Pkg are >>>> further emulation platforms that might want to include this. >>>> >>>> My reason for suggesting the unconditional inclusion is the following >>>> sentence from the UEFI 2.7 spec: >>>> >>>> 13 Protocols — Media Access >>>> 13.3 File System Format >>>> 13.3.2 Partition Discovery >>>> 13.3.2.1 ISO-9660 and El Torito >>>> >>>> [...] DVD-ROM images formatted as required by the UDF 2.0 >>>> specification (OSTA Universal Disk Format Specification, Revision 2.0) >>>> can be booted by EFI. [...] >>>> >>>> It does not say "may be bootable", it says "can be booted". >>>> >>>> It would be interesting to see the Mantis ticket (if any) that got this >>>> language into the spec, without the edk2 reference implementation >>>> providing a UDF driver. >>>> - Using the Mantis simple search function, "UDF" brings up nothing. >>>> - From some googling, this sentence appears to go back to EFI 1.10 at >>>> the least. >>>> >>>> Andrew, do you remember the history of the quoted sentence? >>>> >>> >>> UDF defines a "UDF Bridge" disk that is "El Torito" compatible. So UDF punted on boot ability by allowing compatibility with CD-ROMs. >>> >>> EFI supports booting from an ISO-9660 file system that conforms to the “El Torito” Bootable CD-ROM Format Specification on a DVD- ROM. A DVD-ROM that contains an ISO-9660 file system is defined as a “UDF Bridge” disk. Booting from CD-ROM and DVD-ROM is accomplished using the same methods. >>> >>> I'm fine with adding a UDF file system driver, but it is not required from a UEFI Spec conformance point of view. >> >> But the one sentence that I quoted above (from the spec) gives rise to >> this exact impression: >> >> [...] DVD-ROM images formatted as required by the UDF 2.0 >> specification (OSTA Universal Disk Format Specification, Revision 2.0) >> can be booted by EFI. [...] >> >> Yes, it goes on to talk about "UDF Bridge", but this sentence per se >> seems to require booting UDF optical media. The next sentence ("EFI >> supports booting from an ISO-9660 file system...") does not start with >> "Namely,". >> >> So is this a spec bug? >> >> I'd like to clarify this, because the intent of the above phrase >> determines whether: >> - edk2, as-is, conforms or does not conform to that passage of the spec, >> - optical media that is formatted with UDF (and no ISO9660/ElTorito >> compat) qualifies as EFI-compatible (that edk2 currently cannot boot). >> >> Could you recommend improved wording for the spec? I'd be happy to file >> a Mantis item on your behalf. >> > > Sure feel free to file a mantis. I think the missing chunk of info is that the only definition of booting in the UDF spec is the "UDF Bridge" format to be compatible with El-Torito. Thus the statement is factual. I think the UDF spec basically states to be bootable it must be a UEF Bridge disk and thus be compatible with El-Torito. > > Maybe turning things around a bit helps. > > The UDF 2.0 specification (..) requires a bootable DVD-ROM be formatted as a "UDF Bridge" disk to be bootable... > > Anyway feel free to start a thread on the spec mailing list. I've filed https://mantis.uefi.org/mantis/view.php?id=1835 Thank you for your help! Laszlo >>> PS "El Torito" was the restaurant that Curtis and Stan wrote out the proposal on a napkin. Luckily device paths are not called "House of Teriyaki", or even worse the nickname "rats and rice". >>> >>>> Paulo, I'll check if I can test your driver with some 3rd party media >>>> (i.e., a DVD image that I don't prepare myself). >>>> >>>> Thank you! >>>> Laszlo >>>> >>>>> >>>>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc >>>>> index 5a14325f73..c71c332efd 100644 >>>>> --- a/OvmfPkg/OvmfPkgIa32.dsc >>>>> +++ b/OvmfPkg/OvmfPkgIa32.dsc >>>>> @@ -39,6 +39,7 @@ >>>>> DEFINE HTTP_BOOT_ENABLE = FALSE >>>>> DEFINE SMM_REQUIRE = FALSE >>>>> DEFINE TLS_ENABLE = FALSE >>>>> + DEFINE UDF_ENABLE = FALSE >>>>> >>>>> # >>>>> # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to >>>>> @@ -409,6 +410,9 @@ >>>>> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE >>>>> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE >>>>> !endif >>>>> +!if $(UDF_ENABLE) == TRUE >>>>> + gEfiMdeModulePkgTokenSpaceGuid.PcdUdfFileSystemSupport|TRUE >>>>> +!endif >>>>> >>>>> [PcdsFixedAtBuild] >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 >>>>> @@ -685,6 +689,9 @@ >>>>> MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf >>>>> MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf >>>>> FatPkg/EnhancedFatDxe/Fat.inf >>>>> +!if $(UDF_ENABLE) == TRUE >>>>> + MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf >>>>> +!endif >>>>> MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf >>>>> MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf >>>>> OvmfPkg/SataControllerDxe/SataControllerDxe.inf >>>>> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf >>>>> index 5e5ade2a1f..2da1fcbe1f 100644 >>>>> --- a/OvmfPkg/OvmfPkgIa32.fdf >>>>> +++ b/OvmfPkg/OvmfPkgIa32.fdf >>>>> @@ -282,6 +282,9 @@ INF MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf >>>>> INF MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf >>>>> >>>>> INF FatPkg/EnhancedFatDxe/Fat.inf >>>>> +!if $(UDF_ENABLE) == TRUE >>>>> +INF MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf >>>>> +!endif >>>>> >>>>> !ifndef $(USE_OLD_SHELL) >>>>> INF ShellPkg/Application/Shell/Shell.inf >>>>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc >>>>> index 2f17a70db8..d0785cca13 100644 >>>>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc >>>>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc >>>>> @@ -39,6 +39,7 @@ >>>>> DEFINE HTTP_BOOT_ENABLE = FALSE >>>>> DEFINE SMM_REQUIRE = FALSE >>>>> DEFINE TLS_ENABLE = FALSE >>>>> + DEFINE UDF_ENABLE = FALSE >>>>> >>>>> # >>>>> # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to >>>>> @@ -414,6 +415,9 @@ >>>>> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE >>>>> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE >>>>> !endif >>>>> +!if $(UDF_ENABLE) == TRUE >>>>> + gEfiMdeModulePkgTokenSpaceGuid.PcdUdfFileSystemSupport|TRUE >>>>> +!endif >>>>> >>>>> [PcdsFixedAtBuild] >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 >>>>> @@ -694,6 +698,9 @@ >>>>> MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf >>>>> MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf >>>>> FatPkg/EnhancedFatDxe/Fat.inf >>>>> +!if $(UDF_ENABLE) == TRUE >>>>> + MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf >>>>> +!endif >>>>> MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf >>>>> MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf >>>>> OvmfPkg/SataControllerDxe/SataControllerDxe.inf >>>>> diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf >>>>> index aa0d8c69f3..0fdd359051 100644 >>>>> --- a/OvmfPkg/OvmfPkgIa32X64.fdf >>>>> +++ b/OvmfPkg/OvmfPkgIa32X64.fdf >>>>> @@ -283,6 +283,9 @@ INF MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf >>>>> INF MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf >>>>> >>>>> INF FatPkg/EnhancedFatDxe/Fat.inf >>>>> +!if $(UDF_ENABLE) == TRUE >>>>> +INF MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf >>>>> +!endif >>>>> >>>>> !ifndef $(USE_OLD_SHELL) >>>>> INF ShellPkg/Application/Shell/Shell.inf >>>>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc >>>>> index c0bd5d0ea6..a25d8b1e99 100644 >>>>> --- a/OvmfPkg/OvmfPkgX64.dsc >>>>> +++ b/OvmfPkg/OvmfPkgX64.dsc >>>>> @@ -39,6 +39,7 @@ >>>>> DEFINE HTTP_BOOT_ENABLE = FALSE >>>>> DEFINE SMM_REQUIRE = FALSE >>>>> DEFINE TLS_ENABLE = FALSE >>>>> + DEFINE UDF_ENABLE = FALSE >>>>> >>>>> # >>>>> # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to >>>>> @@ -414,6 +415,9 @@ >>>>> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE >>>>> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE >>>>> !endif >>>>> +!if $(UDF_ENABLE) == TRUE >>>>> + gEfiMdeModulePkgTokenSpaceGuid.PcdUdfFileSystemSupport|TRUE >>>>> +!endif >>>>> >>>>> [PcdsFixedAtBuild] >>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 >>>>> @@ -692,6 +696,9 @@ >>>>> MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf >>>>> MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf >>>>> FatPkg/EnhancedFatDxe/Fat.inf >>>>> +!if $(UDF_ENABLE) == TRUE >>>>> + MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf >>>>> +!endif >>>>> MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf >>>>> MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf >>>>> OvmfPkg/SataControllerDxe/SataControllerDxe.inf >>>>> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf >>>>> index 98a0cf17da..8ae591c1f5 100644 >>>>> --- a/OvmfPkg/OvmfPkgX64.fdf >>>>> +++ b/OvmfPkg/OvmfPkgX64.fdf >>>>> @@ -283,6 +283,9 @@ INF MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf >>>>> INF MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf >>>>> >>>>> INF FatPkg/EnhancedFatDxe/Fat.inf >>>>> +!if $(UDF_ENABLE) == TRUE >>>>> +INF MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf >>>>> +!endif >>>>> >>>>> !ifndef $(USE_OLD_SHELL) >>>>> INF ShellPkg/Application/Shell/Shell.inf >>>>> >>>> >>>> _______________________________________________ >>>> edk2-devel mailing list >>>> edk2-devel@lists.01.org >>>> https://lists.01.org/mailman/listinfo/edk2-devel > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel >