Re: [PATCH 10/12] OvmfPkg/IoMmuDxe: implement in-place decryption/encryption for Map/Unmap

[Brijesh Singh <brijesh.singh@amd.com>]

On 8/2/17 4:24 PM, Laszlo Ersek wrote:

[Snip]
> At the moment, we have the foll+    // The buffer at MapInfo->CryptedAddress comes from AllocateBuffer().
>      //
>      MapInfo->PlainTextAddress = MapInfo->CryptedAddress;
> -
>      //
> -    // Therefore no mapping is necessary.
> +    // Stash the crypted data.
>      //
> -    *DeviceAddress = MapInfo->PlainTextAddress;
> -    *Mapping       = NO_MAPPING;
> -    FreePool (MapInfo);
> -    return EFI_SUCCESS;
> +    CommonBufferHeader = (COMMON_BUFFER_HEADER *)(
> +                           (UINTN)MapInfo->CryptedAddress - EFI_PAGE_SIZE
> +                           );

One question, per spec, is it legal for client to call Map() at some
offset within allocated buffer ?

e.g something like this:

* AllocateBuffer (, 1, &Buffer);
* MapBuffer = Buffer + 10;
* Map (, BusMasterCommonBuffer, MappedBuffer, 10, ..) // Bascially Map
10 bytes from offset 10

If this is legal then we may need to build MapInfo during
AllocateBuffer() to locate the "StashBuffer". So far, I have not came
across this usecase but I wanted to check with you so that we don't fail
on corner cases. Good part if you have ASSERT() so we should be able to
catch them (if any).

> +    ASSERT (CommonBufferHeader->Signature == COMMON_BUFFER_SIG);
> +    CopyMem (
> +      CommonBufferHeader->StashBuffer,
> +      (VOID *)(UINTN)MapInfo->CryptedAddress,
> +      MapInfo->NumberOfBytes
> +      );
> +    //
> +    // Point "DecryptionSource" to the stash buffer so that we decrypt
> +    // it to the original location, after the switch statement.
> +    //
> +    DecryptionSource = CommonBufferHeader->StashBuffer;
> +    break;
>  
[Snip]