diff mbox

[edk2,1/4] ArmVirtPkg: align ArmVirtQemuKernel.fdf with ArmVirtQemu.fdf

Message ID 1468314987-19885-2-git-send-email-ard.biesheuvel@linaro.org
State New
Headers show

Commit Message

Ard Biesheuvel July 12, 2016, 9:16 a.m. UTC 
The platform ArmVirtQemuKernel is intended as an alternative for
ArmVirtQemu that only deviates in the way it is invoked by QEMU, either
from flash address 0x0 (the default ARM reset vector) or via the Linux
kernel boot protocol. So clean up a couple of discrepancies that crept
in over time, i.e., missing VirtioRngDxe and HighMemDxe, and the
conditional inclusion of the ACPI related drivers.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 ArmVirtPkg/ArmVirtQemuKernel.fdf | 4 ++++
 1 file changed, 4 insertions(+)

-- 
1.9.1

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

Comments

Laszlo Ersek July 12, 2016, 9:33 a.m. UTC | #1 
On 07/12/16 11:16, Ard Biesheuvel wrote:
> The platform ArmVirtQemuKernel is intended as an alternative for

> ArmVirtQemu that only deviates in the way it is invoked by QEMU, either

> from flash address 0x0 (the default ARM reset vector) or via the Linux

> kernel boot protocol. So clean up a couple of discrepancies that crept

> in over time, i.e., missing VirtioRngDxe and HighMemDxe, and the

> conditional inclusion of the ACPI related drivers.

> 

> Contributed-under: TianoCore Contribution Agreement 1.0

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

> ---

>  ArmVirtPkg/ArmVirtQemuKernel.fdf | 4 ++++

>  1 file changed, 4 insertions(+)

> 

> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.fdf b/ArmVirtPkg/ArmVirtQemuKernel.fdf

> index 1229e6bd43cc..dcea9771a288 100644

> --- a/ArmVirtPkg/ArmVirtQemuKernel.fdf

> +++ b/ArmVirtPkg/ArmVirtQemuKernel.fdf

> @@ -128,6 +128,7 @@ [FV.FvMain]

>    INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf

>    INF ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf

>    INF ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf

> +  INF ArmVirtPkg/HighMemDxe/HighMemDxe.inf

>  

>    #

>    # PI DXE Drivers producing Architectural Protocols (EFI Services)

> @@ -175,6 +176,7 @@ [FV.FvMain]

>    INF OvmfPkg/VirtioBlkDxe/VirtioBlk.inf

>    INF OvmfPkg/VirtioNetDxe/VirtioNet.inf

>    INF OvmfPkg/VirtioScsiDxe/VirtioScsi.inf

> +  INF OvmfPkg/VirtioRngDxe/VirtioRng.inf

>  

>    #

>    # UEFI application (Shell Embedded Boot Loader)

> @@ -218,11 +220,13 @@ [FV.FvMain]

>    INF MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf

>    INF OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf

>  

> +!if $(ARCH) == AARCH64

>    #

>    # ACPI Support

>    #

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

>    INF OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf

> +!endif

>  

>    #

>    # PCI support

> 


The first two hunks look good, but the last one doesn't seem complete.
If you check commit 8e2efec6b206a, the "ArmVirtPkg/ArmVirtQemu.fdf"
change was accompanied by a matching change in
"ArmVirtPkg/ArmVirtQemu.dsc". That seems to be missing from
"ArmVirtPkg/ArmVirtQemuKernel.dsc" after this patch. It will not cause
build errors, but AcpiTableDxe and QemuFwCfgAcpiPlatformDxe will be
built in vain, when building ArmVirtQemuKernel for ARM.

I think we might want to split this patch in two -- first, add the two
simple driver lines, then replicate 8e2efec6b206a separately (and fully)
to ArmVirtQemuKernel. What do you think?

Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel July 12, 2016, 9:34 a.m. UTC | #2 
On 12 July 2016 at 11:33, Laszlo Ersek <lersek@redhat.com> wrote:
> On 07/12/16 11:16, Ard Biesheuvel wrote:

>> The platform ArmVirtQemuKernel is intended as an alternative for

>> ArmVirtQemu that only deviates in the way it is invoked by QEMU, either

>> from flash address 0x0 (the default ARM reset vector) or via the Linux

>> kernel boot protocol. So clean up a couple of discrepancies that crept

>> in over time, i.e., missing VirtioRngDxe and HighMemDxe, and the

>> conditional inclusion of the ACPI related drivers.

>>

>> Contributed-under: TianoCore Contribution Agreement 1.0

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

>> ---

>>  ArmVirtPkg/ArmVirtQemuKernel.fdf | 4 ++++

>>  1 file changed, 4 insertions(+)

>>

>> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.fdf b/ArmVirtPkg/ArmVirtQemuKernel.fdf

>> index 1229e6bd43cc..dcea9771a288 100644

>> --- a/ArmVirtPkg/ArmVirtQemuKernel.fdf

>> +++ b/ArmVirtPkg/ArmVirtQemuKernel.fdf

>> @@ -128,6 +128,7 @@ [FV.FvMain]

>>    INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf

>>    INF ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf

>>    INF ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf

>> +  INF ArmVirtPkg/HighMemDxe/HighMemDxe.inf

>>

>>    #

>>    # PI DXE Drivers producing Architectural Protocols (EFI Services)

>> @@ -175,6 +176,7 @@ [FV.FvMain]

>>    INF OvmfPkg/VirtioBlkDxe/VirtioBlk.inf

>>    INF OvmfPkg/VirtioNetDxe/VirtioNet.inf

>>    INF OvmfPkg/VirtioScsiDxe/VirtioScsi.inf

>> +  INF OvmfPkg/VirtioRngDxe/VirtioRng.inf

>>

>>    #

>>    # UEFI application (Shell Embedded Boot Loader)

>> @@ -218,11 +220,13 @@ [FV.FvMain]

>>    INF MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf

>>    INF OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf

>>

>> +!if $(ARCH) == AARCH64

>>    #

>>    # ACPI Support

>>    #

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

>>    INF OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf

>> +!endif

>>

>>    #

>>    # PCI support

>>

>

> The first two hunks look good, but the last one doesn't seem complete.

> If you check commit 8e2efec6b206a, the "ArmVirtPkg/ArmVirtQemu.fdf"

> change was accompanied by a matching change in

> "ArmVirtPkg/ArmVirtQemu.dsc". That seems to be missing from

> "ArmVirtPkg/ArmVirtQemuKernel.dsc" after this patch. It will not cause

> build errors, but AcpiTableDxe and QemuFwCfgAcpiPlatformDxe will be

> built in vain, when building ArmVirtQemuKernel for ARM.

>


Indeed, I didn't think of that.

> I think we might want to split this patch in two -- first, add the two

> simple driver lines, then replicate 8e2efec6b206a separately (and fully)

> to ArmVirtQemuKernel. What do you think?

>


Yes, that works for me.

Thanks,
Ard.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox

Patch

diff --git a/ArmVirtPkg/ArmVirtQemuKernel.fdf b/ArmVirtPkg/ArmVirtQemuKernel.fdf
index 1229e6bd43cc..dcea9771a288 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.fdf
+++ b/ArmVirtPkg/ArmVirtQemuKernel.fdf
@@ -128,6 +128,7 @@  [FV.FvMain]
   INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
   INF ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf
   INF ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
+  INF ArmVirtPkg/HighMemDxe/HighMemDxe.inf
 
   #
   # PI DXE Drivers producing Architectural Protocols (EFI Services)
@@ -175,6 +176,7 @@  [FV.FvMain]
   INF OvmfPkg/VirtioBlkDxe/VirtioBlk.inf
   INF OvmfPkg/VirtioNetDxe/VirtioNet.inf
   INF OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
+  INF OvmfPkg/VirtioRngDxe/VirtioRng.inf
 
   #
   # UEFI application (Shell Embedded Boot Loader)
@@ -218,11 +220,13 @@  [FV.FvMain]
   INF MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
   INF OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
 
+!if $(ARCH) == AARCH64
   #
   # ACPI Support
   #
   INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
   INF OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
+!endif
 
   #
   # PCI support