Re: [edk2-devel] [PATCH] OvmfPkg/IoMmuDxe: Provide an implementation for SetAttribute

["Min Xu" <min.m.xu@intel.com>]

It works in TDX.

Tested-by: Min Xu <min.m.xu@intel.com>
Reviewed-by: Min Xu <min.m.xu@intel.com>

> -----Original Message-----
> From: Tom Lendacky <thomas.lendacky@amd.com>
> Sent: Wednesday, January 31, 2024 1:16 AM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Aktas, Erdem
> <erdemaktas@google.com>; Gerd Hoffmann <kraxel@redhat.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Laszlo Ersek <lersek@redhat.com>; Xu, Min
> M <min.m.xu@intel.com>; Michael Roth <michael.roth@amd.com>
> Subject: [PATCH] OvmfPkg/IoMmuDxe: Provide an implementation for
> SetAttribute
> 
> A recent change to the PciIoMap() function now propagates the return code
> from the IoMmu protocol SetAttribute() operation. The implementation of
> this operation in OvmfPkg/IoMmuDxe/CcIoMmu.c returns
> EFI_UNSUPPORTED, resulting in a failure to boot the guest.
> 
> Provide an implementation for SetAttribute() that validates the IoMmu access
> method being requested against the IoMmu mapping operation.
> 
> Suggested-by: Laszlo Ersek <lersek@redhat.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  OvmfPkg/IoMmuDxe/CcIoMmu.c | 55 +++++++++++++++++++-
>  1 file changed, 53 insertions(+), 2 deletions(-)
> 
> diff --git a/OvmfPkg/IoMmuDxe/CcIoMmu.c
> b/OvmfPkg/IoMmuDxe/CcIoMmu.c index b83a9690062b..795b945dacb0
> 100644
> --- a/OvmfPkg/IoMmuDxe/CcIoMmu.c
> +++ b/OvmfPkg/IoMmuDxe/CcIoMmu.c
> @@ -5,7 +5,7 @@
>    operations must be performed on unencrypted buffer hence we use a
> bounce
>    buffer to map the guest buffer into an unencrypted DMA buffer.
> 
> -  Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> +  Copyright (c) 2017 - 2024, AMD Inc. All rights reserved.<BR>
>    Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> 
>    SPDX-License-Identifier: BSD-2-Clause-Patent @@ -751,7 +751,58 @@
> IoMmuSetAttribute (
>    IN UINT64                IoMmuAccess
>    )
>  {
> -  return EFI_UNSUPPORTED;
> +  MAP_INFO    *MapInfo;
> +  EFI_STATUS  Status;
> +
> +  DEBUG ((DEBUG_VERBOSE, "%a: Mapping=0x%p Access=%lu\n", __func__,
> + Mapping, IoMmuAccess));
> +
> +  if (Mapping == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Status = EFI_SUCCESS;
> +
> +  //
> +  // An IoMmuAccess value of 0 is always accepted, validate any non-zero
> value.
> +  //
> +  if (IoMmuAccess != 0) {
> +    MapInfo = (MAP_INFO *)Mapping;
> +
> +    //
> +    // The mapping operation already implied the access mode. Validate that
> +    // the supplied access mode matches operation access mode.
> +    //
> +    switch (MapInfo->Operation) {
> +      case EdkiiIoMmuOperationBusMasterRead:
> +      case EdkiiIoMmuOperationBusMasterRead64:
> +        if (IoMmuAccess != EDKII_IOMMU_ACCESS_READ) {
> +          Status = EFI_INVALID_PARAMETER;
> +        }
> +
> +        break;
> +
> +      case EdkiiIoMmuOperationBusMasterWrite:
> +      case EdkiiIoMmuOperationBusMasterWrite64:
> +        if (IoMmuAccess != EDKII_IOMMU_ACCESS_WRITE) {
> +          Status = EFI_INVALID_PARAMETER;
> +        }
> +
> +        break;
> +
> +      case EdkiiIoMmuOperationBusMasterCommonBuffer:
> +      case EdkiiIoMmuOperationBusMasterCommonBuffer64:
> +        if (IoMmuAccess != (EDKII_IOMMU_ACCESS_READ |
> EDKII_IOMMU_ACCESS_WRITE)) {
> +          Status = EFI_INVALID_PARAMETER;
> +        }
> +
> +        break;
> +
> +      default:
> +        Status = EFI_UNSUPPORTED;
> +    }
> +  }
> +
> +  return Status;
>  }
> 
>  EDKII_IOMMU_PROTOCOL  mIoMmu = {
> --
> 2.42.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114847): https://edk2.groups.io/g/devel/message/114847
Mute This Topic: https://groups.io/mt/104058148/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-