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 3C46A21E1DAD9 for ; Wed, 9 Aug 2017 02:42:41 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6725CB5682; Wed, 9 Aug 2017 09:44:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 6725CB5682 Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-102.phx2.redhat.com [10.3.116.102]) by smtp.corp.redhat.com (Postfix) with ESMTP id 057E96C213; Wed, 9 Aug 2017 09:44:56 +0000 (UTC) To: Paulo Alcantara Cc: edk2-devel@lists.01.org, Jordan Justen , Ard Biesheuvel , Andrew Fish References: <20170808193143.18128-1-pcacjr@zytor.com> <20170808193143.18128-5-pcacjr@zytor.com> From: Laszlo Ersek Message-ID: Date: Wed, 9 Aug 2017 11:44:56 +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: <20170808193143.18128-5-pcacjr@zytor.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Wed, 09 Aug 2017 09:44:58 +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: Wed, 09 Aug 2017 09:42:41 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit 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? 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 >