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

2017-08-10 Thread Laszlo Ersek
On 08/09/17 19:51, Andrew Fish wrote:
> 
>> On Aug 9, 2017, at 10:33 AM, Laszlo Ersek  wrote:
>>
>> On 08/09/17 17:45, Andrew Fish wrote:
>>>
 On Aug 9, 2017, at 2:44 AM, Laszlo Ersek  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 
> 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?

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

I've filed

  https://mantis.uefi.org/mantis/view.php?id=1835

Thank you for your help!
Laszlo



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

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

2017-08-09 Thread Andrew Fish

> On Aug 9, 2017, at 10:33 AM, Laszlo Ersek  wrote:
> 
> On 08/09/17 17:45, Andrew Fish wrote:
>> 
>>> On Aug 9, 2017, at 2:44 AM, Laszlo Ersek  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 
 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?
>>> 
>> 
>> 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
 +++ 

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

2017-08-09 Thread Laszlo Ersek
On 08/09/17 17:45, Andrew Fish wrote:
> 
>> On Aug 9, 2017, at 2:44 AM, Laszlo Ersek  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 
>>> 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?
>>
> 
> 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
>>>   

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

2017-08-09 Thread Andrew Fish

> On Aug 9, 2017, at 2:44 AM, Laszlo Ersek  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 
>> 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?
> 

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. 

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

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

2017-08-09 Thread Paulo Alcantara

Hi,

On 8/9/2017 6:44 AM, Laszlo Ersek 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 
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).


OK - I'll send a v2 as soon as I can with its removal.



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.


Sure. That'd be great if someone could help me testing on those 
platforms. I'll try to set up an environment and then test it on them.



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


Thanks!

Paulo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


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

2017-08-09 Thread Laszlo Ersek
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
>