Re: [PATCH 1/1] ArmVirtPkg: fix ASSERT in ArmVirtGicArchLib with virtualization=on

["Laszlo Ersek" <lersek@redhat.com>]

On 03/11/20 16:32, Leif Lindholm wrote:
> ArmVirtGicArchLib was originally implemented before virtualization
> emulation was implemented in QEMU, and the GICv2 model implemented only
> the physical copy of control registers.
> 
> Enabling virtualization emulation to QEMU adds also the virtual copy,
> doubling the RegSize returned by FindCompatibleNodeReg () in
> ArmVirtGicArchLibConstructor (). This triggered an ASSERT when running
> QEMU with -M virt,virtualization=on. Address this by testing for both
> possible valid values of RegSize.
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2588
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Leif Lindholm <leif@nuviainc.com>
> ---
>  ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c b/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
> index af6b3af60edf..5448865ad8e8 100644
> --- a/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
> +++ b/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
> @@ -110,7 +110,12 @@ ArmVirtGicArchLibConstructor (
>      break;
>  
>    case 2:
> -    ASSERT (RegSize == 32);
> +    //
> +    // When the GICv2 is emulated with virtualization=on, it adds a virtual
> +    // set of control registers. This means the register property can be
> +    // either 32 or 64 bytes in size.
> +    //
> +    ASSERT ((RegSize == 32) || (RegSize == 64));
>  
>      DistBase = SwapBytes64 (Reg[0]);
>      CpuBase  = SwapBytes64 (Reg[2]);
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>