Re: [PATCH 2/9] OvmfPkg/VmgExitLib: Set the SW exit fields when performing VMGEXIT

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

On 10/10/20 18:07, Tom Lendacky wrote:
> From: Tom Lendacky <thomas.lendacky@amd.com>
> 
> All fields that are set in the GHCB should have their associated bit in
> the GHCB ValidBitmap field set. Add support to set the bits for the
> software exit information fields when performing a VMGEXIT (SwExitCode,
> SwExitInfo1, SwExitInfo2).
> 
> Fixes: 61bacc0fa16f ("OvmfPkg/VmgExitLib: Implement library support for VmgExitLib in OVMF")
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  OvmfPkg/Library/VmgExitLib/VmgExitLib.c | 30 ++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitLib.c b/OvmfPkg/Library/VmgExitLib/VmgExitLib.c
> index 53040cc6f649..6cf649c6101b 100644
> --- a/OvmfPkg/Library/VmgExitLib/VmgExitLib.c
> +++ b/OvmfPkg/Library/VmgExitLib/VmgExitLib.c
> @@ -78,6 +78,32 @@ VmgExitErrorCheck (
>    return Status;
>  }
>  
> +/**
> +  Marks a field at the specified offset as valid in the GHCB.
> +
> +  The ValidBitmap area represents the areas of the GHCB that have been marked
> +  valid. Set the area of the GHCB at the specified offset as valid.
> +
> +  @param[in, out] Ghcb    Pointer to the Guest-Hypervisor Communication Block
> +  @param[in] Offset       Offset in the GHCB to mark valid
> +
> +**/
> +STATIC
> +VOID
> +GhcbSetOffsetValid (
> +  IN OUT GHCB               *Ghcb,
> +  IN     GHCB_QWORD_OFFSET  Offset
> +  )
> +{
> +  UINT32  OffsetIndex;
> +  UINT32  OffsetBit;
> +
> +  OffsetIndex = Offset / 8;
> +  OffsetBit   = Offset & 0x07;

(1) I suggest improving the consistency of operators -- please either
use division and remainder ("Offset / 8" and "Offset % 8"), or bit shift
and masking ("Offset >> 3" and "Offset & 0x7")

With that:

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

Thanks
Laszlo

> +
> +  Ghcb->SaveArea.ValidBitmap[OffsetIndex] |= (1 << OffsetBit);
> +}
> +
>  /**
>    Perform VMGEXIT.
>  
> @@ -110,6 +136,10 @@ VmgExit (
>    Ghcb->SaveArea.SwExitInfo1 = ExitInfo1;
>    Ghcb->SaveArea.SwExitInfo2 = ExitInfo2;
>  
> +  GhcbSetOffsetValid (Ghcb, GhcbSwExitCode);
> +  GhcbSetOffsetValid (Ghcb, GhcbSwExitInfo1);
> +  GhcbSetOffsetValid (Ghcb, GhcbSwExitInfo2);
> +
>    //
>    // Guest memory is used for the guest-hypervisor communication, so fence
>    // the invocation of the VMGEXIT instruction to ensure GHCB accesses are
>