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

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

On 1/30/24 18:15, Tom Lendacky wrote:
> 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(-)

Thanks -- I'd like Min or Jiewen to approve this, then I'm glad to merge it.

Laszlo

> 
> 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 = {



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