On 08/09/17 17:45, Andrew Fish wrote:
>
>> On Aug 9, 2017, at 2:44 AM, Laszlo Ersek <ler...@redhat.com> 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 <jordan.l.jus...@intel.com>
>>> Cc: Laszlo Ersek <ler...@redhat.com>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Paulo Alcantara <pca...@zytor.com>
>>> ---
>>> 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.
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
>> 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
