[edk2,2/2] ArmVirtPkg: enable EBC interpreter for AArch64 QEMU

Message ID 1471441502-21873-3-git-send-email-leif.lindholm@linaro.org
State New
Headers show

Commit Message

Leif Lindholm Aug. 17, 2016, 1:45 p.m.
Since we now have EBC support for AArch64, enable it by default
on the QEMU platform.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

---
 ArmVirtPkg/ArmVirtQemu.dsc           | 1 +
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 5 +++++
 2 files changed, 6 insertions(+)

-- 
2.1.4

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

Comments

Ard Biesheuvel Aug. 17, 2016, 1:47 p.m. | #1
On 17 August 2016 at 15:45, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> Since we now have EBC support for AArch64, enable it by default

> on the QEMU platform.

>

> Contributed-under: TianoCore Contribution Agreement 1.0

> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---

>  ArmVirtPkg/ArmVirtQemu.dsc           | 1 +

>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 5 +++++

>  2 files changed, 6 insertions(+)

>

> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc

> index 9f88786..fb851e7 100644

> --- a/ArmVirtPkg/ArmVirtQemu.dsc

> +++ b/ArmVirtPkg/ArmVirtQemu.dsc

> @@ -401,3 +401,4 @@ [Components.AARCH64]

>      <LibraryClasses>

>        NULL|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf

>    }

> +  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf


Please add it to ArmVirtPkg/ArmVirtQemuKernel.dsc as well:
ArmVirtQemuFvMain.fdf.inc is shared between them.

With that,

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>


> diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc

> index ad7037f..0a5b899 100644

> --- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc

> +++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc

> @@ -146,6 +146,11 @@ [FV.FvMain]

>    #

>    INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf

>    INF OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf

> +

> +  #

> +  # EBC support

> +  #

> +  INF MdeModulePkg/Universal/EbcDxe/EbcDxe.inf

>  !endif

>

>    #

> --

> 2.1.4

>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Laszlo Ersek Aug. 17, 2016, 1:55 p.m. | #2
On 08/17/16 15:47, Ard Biesheuvel wrote:
> On 17 August 2016 at 15:45, Leif Lindholm <leif.lindholm@linaro.org> wrote:

>> Since we now have EBC support for AArch64, enable it by default

>> on the QEMU platform.

>>

>> Contributed-under: TianoCore Contribution Agreement 1.0

>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

>> ---

>>  ArmVirtPkg/ArmVirtQemu.dsc           | 1 +

>>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 5 +++++

>>  2 files changed, 6 insertions(+)

>>

>> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc

>> index 9f88786..fb851e7 100644

>> --- a/ArmVirtPkg/ArmVirtQemu.dsc

>> +++ b/ArmVirtPkg/ArmVirtQemu.dsc

>> @@ -401,3 +401,4 @@ [Components.AARCH64]

>>      <LibraryClasses>

>>        NULL|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf

>>    }

>> +  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf

> 

> Please add it to ArmVirtPkg/ArmVirtQemuKernel.dsc as well:

> ArmVirtQemuFvMain.fdf.inc is shared between them.

> 

> With that,

> 

> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>


Any reason for not adding it to the Xen platform as well?

What kind of modules do we expect to come in EBC? Option ROMs perhaps?
In that case, Xen may indeed be irrelevant here (I have no clue about
adding oproms to emulated or assigned devices under Xen, so I can't make
an argument either way.) If we expect general stuff (for example, even
UEFI_APPLICATION modules) to arrive in EBC, then I think Xen would be a
justified addition.

Thanks
Laszlo

>> diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc

>> index ad7037f..0a5b899 100644

>> --- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc

>> +++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc

>> @@ -146,6 +146,11 @@ [FV.FvMain]

>>    #

>>    INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf

>>    INF OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf

>> +

>> +  #

>> +  # EBC support

>> +  #

>> +  INF MdeModulePkg/Universal/EbcDxe/EbcDxe.inf

>>  !endif

>>

>>    #

>> --

>> 2.1.4

>>


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Aug. 17, 2016, 2:04 p.m. | #3
On 17 August 2016 at 15:55, Laszlo Ersek <lersek@redhat.com> wrote:
> On 08/17/16 15:47, Ard Biesheuvel wrote:

>> On 17 August 2016 at 15:45, Leif Lindholm <leif.lindholm@linaro.org> wrote:

>>> Since we now have EBC support for AArch64, enable it by default

>>> on the QEMU platform.

>>>

>>> Contributed-under: TianoCore Contribution Agreement 1.0

>>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

>>> ---

>>>  ArmVirtPkg/ArmVirtQemu.dsc           | 1 +

>>>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 5 +++++

>>>  2 files changed, 6 insertions(+)

>>>

>>> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc

>>> index 9f88786..fb851e7 100644

>>> --- a/ArmVirtPkg/ArmVirtQemu.dsc

>>> +++ b/ArmVirtPkg/ArmVirtQemu.dsc

>>> @@ -401,3 +401,4 @@ [Components.AARCH64]

>>>      <LibraryClasses>

>>>        NULL|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf

>>>    }

>>> +  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf

>>

>> Please add it to ArmVirtPkg/ArmVirtQemuKernel.dsc as well:

>> ArmVirtQemuFvMain.fdf.inc is shared between them.

>>

>> With that,

>>

>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>

> Any reason for not adding it to the Xen platform as well?

>

> What kind of modules do we expect to come in EBC? Option ROMs perhaps?


Yes, that is the primary purpose, although any DXE/UEFI driver could
be distributed as an EBC module, I suppose (and we have an EBC version
of fat.efi in FatBinPkg)

> In that case, Xen may indeed be irrelevant here (I have no clue about

> adding oproms to emulated or assigned devices under Xen, so I can't make

> an argument either way.) If we expect general stuff (for example, even

> UEFI_APPLICATION modules) to arrive in EBC, then I think Xen would be a

> justified addition.

>


I remember reading somewhere that EBC was strictly for drivers, not
applications, but I cannot find a reference anymore in the spec.

I wouldn't mind adding it to Xen, but I don't see the point tbh

Thanks,
Ard.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif Lindholm Aug. 17, 2016, 2:04 p.m. | #4
On Wed, Aug 17, 2016 at 03:55:01PM +0200, Laszlo Ersek wrote:
> On 08/17/16 15:47, Ard Biesheuvel wrote:

> > On 17 August 2016 at 15:45, Leif Lindholm <leif.lindholm@linaro.org> wrote:

> >> Since we now have EBC support for AArch64, enable it by default

> >> on the QEMU platform.

> >>

> >> Contributed-under: TianoCore Contribution Agreement 1.0

> >> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

> >> ---

> >>  ArmVirtPkg/ArmVirtQemu.dsc           | 1 +

> >>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 5 +++++

> >>  2 files changed, 6 insertions(+)

> >>

> >> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc

> >> index 9f88786..fb851e7 100644

> >> --- a/ArmVirtPkg/ArmVirtQemu.dsc

> >> +++ b/ArmVirtPkg/ArmVirtQemu.dsc

> >> @@ -401,3 +401,4 @@ [Components.AARCH64]

> >>      <LibraryClasses>

> >>        NULL|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf

> >>    }

> >> +  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf

> > 

> > Please add it to ArmVirtPkg/ArmVirtQemuKernel.dsc as well:

> > ArmVirtQemuFvMain.fdf.inc is shared between them.

> > 

> > With that,

> > 

> > Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> 

> Any reason for not adding it to the Xen platform as well?


Mainly that I'm cluesless as to how to test it.
More than happy for this one to be thrown away and replaced by
something more useful :)

> What kind of modules do we expect to come in EBC? Option ROMs perhaps?

> In that case, Xen may indeed be irrelevant here (I have no clue about

> adding oproms to emulated or assigned devices under Xen, so I can't make

> an argument either way.) If we expect general stuff (for example, even

> UEFI_APPLICATION modules) to arrive in EBC, then I think Xen would be a

> justified addition.


Current test cases include HelloWorld and apparently the FAT driver.
Unless we start playing with PCI pass-through, I would expect EBC on
the virt platforms would be used mostly for applications/protocols.

Regards,

Leif

> Thanks

> Laszlo

> 

> >> diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc

> >> index ad7037f..0a5b899 100644

> >> --- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc

> >> +++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc

> >> @@ -146,6 +146,11 @@ [FV.FvMain]

> >>    #

> >>    INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf

> >>    INF OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf

> >> +

> >> +  #

> >> +  # EBC support

> >> +  #

> >> +  INF MdeModulePkg/Universal/EbcDxe/EbcDxe.inf

> >>  !endif

> >>

> >>    #

> >> --

> >> 2.1.4

> >>

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Laszlo Ersek Aug. 19, 2016, 1:42 a.m. | #5
On 08/17/16 16:04, Ard Biesheuvel wrote:
> On 17 August 2016 at 15:55, Laszlo Ersek <lersek@redhat.com> wrote:

>> On 08/17/16 15:47, Ard Biesheuvel wrote:

>>> On 17 August 2016 at 15:45, Leif Lindholm <leif.lindholm@linaro.org> wrote:

>>>> Since we now have EBC support for AArch64, enable it by default

>>>> on the QEMU platform.

>>>>

>>>> Contributed-under: TianoCore Contribution Agreement 1.0

>>>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

>>>> ---

>>>>  ArmVirtPkg/ArmVirtQemu.dsc           | 1 +

>>>>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 5 +++++

>>>>  2 files changed, 6 insertions(+)

>>>>

>>>> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc

>>>> index 9f88786..fb851e7 100644

>>>> --- a/ArmVirtPkg/ArmVirtQemu.dsc

>>>> +++ b/ArmVirtPkg/ArmVirtQemu.dsc

>>>> @@ -401,3 +401,4 @@ [Components.AARCH64]

>>>>      <LibraryClasses>

>>>>        NULL|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf

>>>>    }

>>>> +  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf

>>>

>>> Please add it to ArmVirtPkg/ArmVirtQemuKernel.dsc as well:

>>> ArmVirtQemuFvMain.fdf.inc is shared between them.

>>>

>>> With that,

>>>

>>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>>

>> Any reason for not adding it to the Xen platform as well?

>>

>> What kind of modules do we expect to come in EBC? Option ROMs perhaps?

> 

> Yes, that is the primary purpose, although any DXE/UEFI driver could

> be distributed as an EBC module, I suppose (and we have an EBC version

> of fat.efi in FatBinPkg)

> 

>> In that case, Xen may indeed be irrelevant here (I have no clue about

>> adding oproms to emulated or assigned devices under Xen, so I can't make

>> an argument either way.) If we expect general stuff (for example, even

>> UEFI_APPLICATION modules) to arrive in EBC, then I think Xen would be a

>> justified addition.

>>

> 

> I remember reading somewhere that EBC was strictly for drivers, not

> applications, but I cannot find a reference anymore in the spec.

> 

> I wouldn't mind adding it to Xen, but I don't see the point tbh


I don't insist, I'd just like to avoid the Qemu <-> Xen platform
divergence getting out of hand :) At some point we'll compare those
files in the future, and ask ourselves why EBC (and the rest :)) are
only in Qemu.

Anyway, this should not block the patch; I just figured I'd raise it.

Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patch

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 9f88786..fb851e7 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -401,3 +401,4 @@  [Components.AARCH64]
     <LibraryClasses>
       NULL|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
   }
+  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
index ad7037f..0a5b899 100644
--- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
+++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
@@ -146,6 +146,11 @@  [FV.FvMain]
   #
   INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
   INF OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
+
+  #
+  # EBC support
+  #
+  INF MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
 !endif
 
   #