Re: [PATCH V2 3/4] OvmfPkg/IoMmuDxe: Add SEV support for reserved shared memory

["Lendacky, Thomas" <thomas.lendacky@amd.com>]

On 12/12/22 23:48, Min Xu wrote:
> From: Tom Lendacky <thomas.lendacky@amd.com>
> 
> Add support to use the reserved shared memory within the IoMmu library.
> This improves boot times for all SEV guests, with SEV-SNP benefiting the
> most as it avoids the page state change call to the hypervisor.

Thanks for including the patch, Min.

One difference between my original and yours noted below, feel free to 
ignore, though.

Otherwise, tested and validated.

> 
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Min Xu <min.m.xu@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>   OvmfPkg/IoMmuDxe/CcIoMmu.c     | 81 +++++++++++++++++-----------------
>   OvmfPkg/IoMmuDxe/IoMmuBuffer.c | 54 ++++++++++++++++++-----
>   2 files changed, 83 insertions(+), 52 deletions(-)
> 
> diff --git a/OvmfPkg/IoMmuDxe/CcIoMmu.c b/OvmfPkg/IoMmuDxe/CcIoMmu.c
> index 1479af469881..e5cbf037c50d 100644
> --- a/OvmfPkg/IoMmuDxe/CcIoMmu.c
> +++ b/OvmfPkg/IoMmuDxe/CcIoMmu.c
> @@ -223,30 +223,33 @@ IoMmuMap (
>         goto FreeMapInfo;
>     }
>   
> -  if (CC_GUEST_IS_SEV (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
> -    //
> -    // Clear the memory encryption mask on the plaintext buffer.
> -    //
> -    Status = MemEncryptSevClearPageEncMask (
> -               0,
> -               MapInfo->PlainTextAddress,
> -               MapInfo->NumberOfPages
> -               );
> -  } else if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
> +  if (MapInfo->ReservedMemBitmap == 0) {
>       //
>       // Set the memory shared bit.

My original patch deleted this line since it is different for each type of 
CC guest and commented about in each if block.

Thanks,
Tom

>       // If MapInfo->ReservedMemBitmap is 0, it means the bounce buffer is not allocated
>       // from the pre-allocated shared memory, so it must be converted to shared memory here.
>       //
> -    if (MapInfo->ReservedMemBitmap == 0) {
> +    if (CC_GUEST_IS_SEV (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
> +      //
> +      // Clear the memory encryption mask on the plaintext buffer.
> +      //
> +      Status = MemEncryptSevClearPageEncMask (
> +                 0,
> +                 MapInfo->PlainTextAddress,
> +                 MapInfo->NumberOfPages
> +                 );
> +    } else if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
> +      //
> +      // Set the memory shared bit.
> +      //
>         Status = MemEncryptTdxSetPageSharedBit (
>                    0,
>                    MapInfo->PlainTextAddress,
>                    MapInfo->NumberOfPages
>                    );
> +    } else {
> +      ASSERT (FALSE);
>       }
> -  } else {
> -    ASSERT (FALSE);
>     }
>   
>     ASSERT_EFI_ERROR (Status);