Re: [PATCH 4/4] OvmfPkg: Introduce UDF_ENABLE build flag

[Laszlo Ersek <lersek@redhat.com>]

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.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Paulo Alcantara <pcacjr@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?

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
>