> On Aug 9, 2017, at 10:33 AM, Laszlo Ersek <[email protected]> wrote: > > On 08/09/17 17:45, Andrew Fish wrote: >> >>> On Aug 9, 2017, at 2:44 AM, Laszlo Ersek <[email protected]> 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 <[email protected]> >>>> Cc: Laszlo Ersek <[email protected]> >>>> Contributed-under: TianoCore Contribution Agreement 1.1 >>>> Signed-off-by: Paulo Alcantara <[email protected]> >>>> --- >>>> 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. Thanks, Andrew Fish > Thanks, > Laszlo > >> >> Thanks, >> >> Andrew Fish >> >> 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 >>> [email protected] >>> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
