Re: [PATCH 2/2] ArmVirtPkg: Add Ramdisk support to ArmVirtPkg platforms

[Laszlo Ersek <lersek@redhat.com>]

On 08/18/16 07:17, Vikas C Sajjan wrote:
> Adds the RAMDisk support to ArmVirtPkg platforms.
> This patch actually ports OvmfPkg commit 259d87146b07 to
> ArmVirtPkg.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Vikas C Sajjan <vikas.cha.sajjan@hpe.com>
> ---
>  ArmVirtPkg/ArmVirt.dsc.inc           | 7 +++++++
>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 3 +++
>  ArmVirtPkg/ArmVirtRules.fdf.inc      | 2 ++
>  ArmVirtPkg/ArmVirtXen.fdf            | 3 +++
>  4 files changed, 15 insertions(+)
> 
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index 9e215c2..66068f7 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -253,6 +253,10 @@
>    gEfiMdePkgTokenSpaceGuid.PcdComponentName2Disable|TRUE
>    gEfiMdePkgTokenSpaceGuid.PcdDriverDiagnostics2Disable|TRUE
>  
> +[PcdsFeatureFlag.AARCH64]
> +  # Activate AcpiSdtProtocol
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
> +
>    #
>    # Control what commands are supported from the UI
>    # Turn these on and off to add features or save size

Similar problem to the one with the previous patch: this change
restricts PcdEmbeddedMacBoot etc to AARCH64. They used to be under
[PcdsFeatureFlag.common]'s scope.

> @@ -382,6 +386,9 @@
>    MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
>    MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
>  
> +  # Ramdisk support
> +  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
> +

Looks good, this is under [Components.common].

>  [Components.AARCH64]
>    #
>    # ACPI Support
> diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> index ad7037f..14d06bb 100644
> --- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> +++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> @@ -177,3 +177,6 @@ READ_LOCK_STATUS   = TRUE
>    FILE FREEFORM = PCD(gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdLogoFile) {
>      SECTION RAW = MdeModulePkg/Logo/Logo.bmp
>    }
> +
> +  # Ramdisk support
> +  INF MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf

The rest of the file uses a commenting style like

  #
  # Ramdisk support
  #

Not very important, but preferable.

> diff --git a/ArmVirtPkg/ArmVirtRules.fdf.inc b/ArmVirtPkg/ArmVirtRules.fdf.inc
> index 8952c67..5ff3004 100644
> --- a/ArmVirtPkg/ArmVirtRules.fdf.inc
> +++ b/ArmVirtPkg/ArmVirtRules.fdf.inc
> @@ -85,6 +85,8 @@
>      DXE_DEPEX    DXE_DEPEX              Optional $(INF_OUTPUT)/$(MODULE_NAME).depex
>      PE32         PE32                   $(INF_OUTPUT)/$(MODULE_NAME).efi
>      UI           STRING="$(MODULE_NAME)" Optional
> +    RAW          ACPI  Optional               |.acpi
> +    RAW          ASL   Optional               |.aml
>    }
>  
>  [Rule.Common.DXE_RUNTIME_DRIVER]

Looks okay (modifies [Rule.Common.DXE_DRIVER])

> diff --git a/ArmVirtPkg/ArmVirtXen.fdf b/ArmVirtPkg/ArmVirtXen.fdf
> index 0a97bd8..36671a8 100644
> --- a/ArmVirtPkg/ArmVirtXen.fdf
> +++ b/ArmVirtPkg/ArmVirtXen.fdf
> @@ -188,6 +188,9 @@ READ_LOCK_STATUS   = TRUE
>    INF ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.inf
>  !endif
>  
> + # Ramdisk support
> + INF MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
> +
>  [FV.FVMAIN_COMPACT]
>  FvAlignment        = 16
>  ERASE_POLARITY     = 1
> 

This hunk looks good too.

Thanks
Laszlo