> On Aug 9, 2017, at 10:33 AM, Laszlo Ersek <ler...@redhat.com> wrote:
> 
> 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.
> 

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
>>> 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

Reply via email to