From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.groups.io with SMTP id smtpd.web10.857.1619629802878546183 for ; Wed, 28 Apr 2021 10:10:03 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=aNSbM93j; spf=pass (domain: redhat.com, ip: 216.205.24.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1619629802; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=uKTUVD63N625QnLrguDeFMaJVQ2VbgfJoKDpqJTDfpo=; b=aNSbM93jR32+BncjPwezMt0gpvwG+AX6OH2wLMA1rI/rh3u4I3/GflaRY/RZGqrjBOmZHn Ari/2ZPfDWBpmbDNrEqdtxt4jHFDnhBbhvYh+ageAYo4l8sNFabOJRfre/VvB/oluzl9K0 bh98gyDteC3wF993esiyDgXqk8+hedM= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-567-vWOW_0opMT6ctUoLK3TIkA-1; Wed, 28 Apr 2021 13:09:57 -0400 X-MC-Unique: vWOW_0opMT6ctUoLK3TIkA-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 026AE8042AD; Wed, 28 Apr 2021 17:09:55 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-188.ams2.redhat.com [10.36.114.188]) by smtp.corp.redhat.com (Postfix) with ESMTP id 76A9255760; Wed, 28 Apr 2021 17:09:52 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v2 2/4] OvmfPkg/VmgExitLib: Add support for new MMIO MOV opcodes To: devel@edk2.groups.io, thomas.lendacky@amd.com Cc: Joerg Roedel , Borislav Petkov , Ard Biesheuvel , Jordan Justen , Brijesh Singh , Erdem Aktas , James Bottomley , Jiewen Yao , Min Xu References: <3f4995357e4c980eac70e9b3be070a37b3a6fe66.1619540470.git.thomas.lendacky@amd.com> From: "Laszlo Ersek" Message-ID: Date: Wed, 28 Apr 2021 19:09:51 +0200 MIME-Version: 1.0 In-Reply-To: <3f4995357e4c980eac70e9b3be070a37b3a6fe66.1619540470.git.thomas.lendacky@amd.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 04/27/21 18:21, Lendacky, Thomas wrote: > From: Tom Lendacky > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3345 > > Enabling TPM support results in guest termination of an SEV-ES guest > because it uses MMIO opcodes that are not currently supported. > > Add support for the new MMIO opcodes (0xA0 - 0xA3), MOV instructions which > use a memory offset directly encoded in the instruction. Also, add a DEBUG > statement to identify an unsupported MMIO opcode being used. > > Fixes: c45f678a1ea2080344e125dc55b14e4b9f98483d > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Cc: Jordan Justen > Cc: Brijesh Singh > Cc: Erdem Aktas > Cc: James Bottomley > Cc: Jiewen Yao > Cc: Min Xu > Signed-off-by: Tom Lendacky > --- > OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 111 ++++++++++++++++++++ > 1 file changed, 111 insertions(+) > > diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c > index dd117f971134..4d001406d30f 100644 > --- a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c > +++ b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c > @@ -678,6 +678,7 @@ MmioExit ( > UINTN Bytes; > UINT64 *Register; > UINT8 OpCode, SignByte; > + UINTN Address; > > Bytes = 0; > > @@ -727,6 +728,57 @@ MmioExit ( > } > break; > > + // > + // MMIO write (MOV moffsetX, aX) > + // > + case 0xA2: > + Bytes = 1; > + // > + // fall through > + // > + case 0xA3: > + Bytes = ((Bytes != 0) ? Bytes : > + (InstructionData->DataSize == Size16Bits) ? 2 : > + (InstructionData->DataSize == Size32Bits) ? 4 : > + (InstructionData->DataSize == Size64Bits) ? 8 : > + 0); > + > + InstructionData->ImmediateSize = (UINTN) (1 << InstructionData->AddrSize); > + InstructionData->End += InstructionData->ImmediateSize; > + > + // > + // This code is X64 only, so a possible 8-byte copy to a UINTN is ok. > + // Use a STATIC_ASSERT to be certain the code is being built as X64. > + // > + STATIC_ASSERT ( > + sizeof (UINTN) == sizeof (UINT64), > + "sizeof (UINTN) != sizeof (UINT64), this file must be built as X64" > + ); > + > + Address = 0; > + CopyMem ( > + &Address, > + InstructionData->Immediate, > + InstructionData->ImmediateSize > + ); > + > + Status = ValidateMmioMemory (Ghcb, Address, Bytes); > + if (Status != 0) { > + return Status; > + } > + > + ExitInfo1 = Address; > + ExitInfo2 = Bytes; > + CopyMem (Ghcb->SharedBuffer, &Regs->Rax, Bytes); > + > + Ghcb->SaveArea.SwScratch = (UINT64) Ghcb->SharedBuffer; > + VmgSetOffsetValid (Ghcb, GhcbSwScratch); > + Status = VmgExit (Ghcb, SVM_EXIT_MMIO_WRITE, ExitInfo1, ExitInfo2); > + if (Status != 0) { > + return Status; > + } > + break; > + > // > // MMIO write (MOV reg/memX, immX) > // > @@ -809,6 +861,64 @@ MmioExit ( > CopyMem (Register, Ghcb->SharedBuffer, Bytes); > break; > > + // > + // MMIO read (MOV aX, moffsetX) > + // > + case 0xA0: > + Bytes = 1; > + // > + // fall through > + // > + case 0xA1: > + Bytes = ((Bytes != 0) ? Bytes : > + (InstructionData->DataSize == Size16Bits) ? 2 : > + (InstructionData->DataSize == Size32Bits) ? 4 : > + (InstructionData->DataSize == Size64Bits) ? 8 : > + 0); > + > + InstructionData->ImmediateSize = (UINTN) (1 << InstructionData->AddrSize); > + InstructionData->End += InstructionData->ImmediateSize; > + > + // > + // This code is X64 only, so a possible 8-byte copy to a UINTN is ok. > + // Use a STATIC_ASSERT to be certain the code is being built as X64. > + // > + STATIC_ASSERT ( > + sizeof (UINTN) == sizeof (UINT64), > + "sizeof (UINTN) != sizeof (UINT64), this file must be built as X64" > + ); > + > + Address = 0; > + CopyMem ( > + &Address, > + InstructionData->Immediate, > + InstructionData->ImmediateSize > + ); > + > + Status = ValidateMmioMemory (Ghcb, Address, Bytes); > + if (Status != 0) { > + return Status; > + } > + > + ExitInfo1 = Address; > + ExitInfo2 = Bytes; > + > + Ghcb->SaveArea.SwScratch = (UINT64) Ghcb->SharedBuffer; > + VmgSetOffsetValid (Ghcb, GhcbSwScratch); > + Status = VmgExit (Ghcb, SVM_EXIT_MMIO_READ, ExitInfo1, ExitInfo2); > + if (Status != 0) { > + return Status; > + } > + > + if (Bytes == 4) { > + // > + // Zero-extend for 32-bit operation > + // > + Regs->Rax = 0; > + } > + CopyMem (&Regs->Rax, Ghcb->SharedBuffer, Bytes); > + break; > + > // > // MMIO read w/ zero-extension ((MOVZX regX, reg/memX) > // > @@ -886,6 +996,7 @@ MmioExit ( > break; > > default: > + DEBUG ((DEBUG_ERROR, "Invalid MMIO opcode (%x)\n", OpCode)); > Status = GP_EXCEPTION; > ASSERT (FALSE); > } > Thanks for the updates! Acked-by: Laszlo Ersek Laszlo