Re: [PATCH v9 14/46] OvmfPkg/VmgExitLib: Support string IO for IOIO_PROT NAE events

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

On 06/05/20 15:27, Tom Lendacky wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
> 
> Add support to the #VC exception handler to handle string IO. This
> requires expanding the IO instruction parsing to recognize string based
> IO instructions as well as preparing an un-encrypted buffer to be used
> to transfer (either to or from the guest) the string contents for the IO
> operation. The SW_EXITINFO2 and SW_SCRATCH fields of the GHCB are set
> appropriately for the operation. Multiple VMGEXIT invocations may be
> needed to complete the string IO operation.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 90 +++++++++++++++++---
>  1 file changed, 76 insertions(+), 14 deletions(-)
> 
> diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
> index 7c2bb12df10a..b74b13045cfd 100644
> --- a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
> +++ b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
> @@ -397,6 +397,26 @@ IoioExitInfo (
>    ExitInfo = 0;
>  
>    switch (*(InstructionData->OpCodes)) {
> +  //
> +  // INS opcodes
> +  //
> +  case 0x6C:
> +  case 0x6D:
> +    ExitInfo |= IOIO_TYPE_INS;
> +    ExitInfo |= IOIO_SEG_ES;
> +    ExitInfo |= ((Regs->Rdx & 0xffff) << 16);
> +    break;
> +
> +  //
> +  // OUTS opcodes
> +  //
> +  case 0x6E:
> +  case 0x6F:
> +    ExitInfo |= IOIO_TYPE_OUTS;
> +    ExitInfo |= IOIO_SEG_DS;
> +    ExitInfo |= ((Regs->Rdx & 0xffff) << 16);
> +    break;
> +
>    //
>    // IN immediate opcodes
>    //
> @@ -445,6 +465,8 @@ IoioExitInfo (
>    //
>    // Single-byte opcodes
>    //
> +  case 0x6C:
> +  case 0x6E:
>    case 0xE4:
>    case 0xE6:
>    case 0xEC:
> @@ -506,30 +528,70 @@ IoioExit (
>    IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
>    )
>  {
> -  UINT64  ExitInfo1, Status;
> +  UINT64   ExitInfo1, ExitInfo2, Status;
> +  BOOLEAN  IsString;
>  
>    ExitInfo1 = IoioExitInfo (Regs, InstructionData);
>    if (ExitInfo1 == 0) {
>      return UnsupportedExit (Ghcb, Regs, InstructionData);
>    }
>  
> -  if ((ExitInfo1 & IOIO_TYPE_IN) != 0) {
> -    Ghcb->SaveArea.Rax = 0;
> +  IsString = ((ExitInfo1 & IOIO_TYPE_STR) != 0) ? TRUE : FALSE;
> +  if (IsString) {
> +    UINTN  IoBytes, VmgExitBytes;
> +    UINTN  GhcbCount, OpCount;
> +
> +    Status = 0;
> +
> +    IoBytes = IOIO_DATA_BYTES(ExitInfo1);

(1) missing space before the opening paren, but we're at v9. So my ACK
stands.

Thanks
Laszlo

> +    GhcbCount = sizeof (Ghcb->SharedBuffer) / IoBytes;
> +
> +    OpCount = ((ExitInfo1 & IOIO_REP) != 0) ? Regs->Rcx : 1;
> +    while (OpCount) {
> +      ExitInfo2 = MIN (OpCount, GhcbCount);
> +      VmgExitBytes = ExitInfo2 * IoBytes;
> +
> +      if ((ExitInfo1 & IOIO_TYPE_IN) == 0) {
> +        CopyMem (Ghcb->SharedBuffer, (VOID *) Regs->Rsi, VmgExitBytes);
> +        Regs->Rsi += VmgExitBytes;
> +      }
> +
> +      Ghcb->SaveArea.SwScratch = (UINT64) Ghcb->SharedBuffer;
> +      Status = VmgExit (Ghcb, SVM_EXIT_IOIO_PROT, ExitInfo1, ExitInfo2);
> +      if (Status != 0) {
> +        return Status;
> +      }
> +
> +      if ((ExitInfo1 & IOIO_TYPE_IN) != 0) {
> +        CopyMem ((VOID *) Regs->Rdi, Ghcb->SharedBuffer, VmgExitBytes);
> +        Regs->Rdi += VmgExitBytes;
> +      }
> +
> +      if ((ExitInfo1 & IOIO_REP) != 0) {
> +        Regs->Rcx -= ExitInfo2;
> +      }
> +
> +      OpCount -= ExitInfo2;
> +    }
>    } else {
> -    CopyMem (&Ghcb->SaveArea.Rax, &Regs->Rax, IOIO_DATA_BYTES (ExitInfo1));
> -  }
> -  GhcbSetRegValid (Ghcb, GhcbRax);
> +    if ((ExitInfo1 & IOIO_TYPE_IN) != 0) {
> +      Ghcb->SaveArea.Rax = 0;
> +    } else {
> +      CopyMem (&Ghcb->SaveArea.Rax, &Regs->Rax, IOIO_DATA_BYTES (ExitInfo1));
> +    }
> +    GhcbSetRegValid (Ghcb, GhcbRax);
>  
> -  Status = VmgExit (Ghcb, SVM_EXIT_IOIO_PROT, ExitInfo1, 0);
> -  if (Status != 0) {
> -    return Status;
> -  }
> +    Status = VmgExit (Ghcb, SVM_EXIT_IOIO_PROT, ExitInfo1, 0);
> +    if (Status != 0) {
> +      return Status;
> +    }
>  
> -  if ((ExitInfo1 & IOIO_TYPE_IN) != 0) {
> -    if (!GhcbIsRegValid (Ghcb, GhcbRax)) {
> -      return UnsupportedExit (Ghcb, Regs, InstructionData);
> +    if ((ExitInfo1 & IOIO_TYPE_IN) != 0) {
> +      if (!GhcbIsRegValid (Ghcb, GhcbRax)) {
> +        return UnsupportedExit (Ghcb, Regs, InstructionData);
> +      }
> +      CopyMem (&Regs->Rax, &Ghcb->SaveArea.Rax, IOIO_DATA_BYTES (ExitInfo1));
>      }
> -    CopyMem (&Regs->Rax, &Ghcb->SaveArea.Rax, IOIO_DATA_BYTES (ExitInfo1));
>    }
>  
>    return 0;
>