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.255.1619801647278302306 for ; Fri, 30 Apr 2021 09:54:07 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=dKjMH/0w; 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=1619801646; 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=y+IEb/3Hh0RbDKv+jQWydoh8CBoDabFkgVOoSNUCxVA=; b=dKjMH/0w1t8olASq27yrmgFoCfL3jTtgPjXCSlZhrxlE5McRGe3L3C/lUs2nS2FXhAf9k3 FdHsx6qrbvIpwoJMGSLiUiONQ6zrahtE91Y1KdD3+lZ1Nm8sod8TyH52DOQp0BRoqPgrHV YJ0V2Sg4D6d/vuULCCyrjUn/3cRwD/s= 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-175-6TwKIWmgN2exbAk36_Pv6Q-1; Fri, 30 Apr 2021 12:53:59 -0400 X-MC-Unique: 6TwKIWmgN2exbAk36_Pv6Q-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id B5D3F800C7A; Fri, 30 Apr 2021 16:53:57 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-248.ams2.redhat.com [10.36.112.248]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4095219C66; Fri, 30 Apr 2021 16:53:55 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v3 2/5] 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: <2fdde57707b52ae39c49341c9d97053aaff56e4a.1619716333.git.thomas.lendacky@amd.com> From: "Laszlo Ersek" Message-ID: <49b61fa3-f78e-1bf9-89c2-9e20b067503c@redhat.com> Date: Fri, 30 Apr 2021 18:53:54 +0200 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 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/29/21 19:19, Lendacky, Thomas wrote: > On 4/29/21 12:12 PM, Tom Lendacky wrote: >> 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 > > Sorry, Laszlo, I forgot to include your Acked-by: on this patch. np Acked-by: Laszlo Ersek Thanks Laszlo > > Tom > >> --- >> 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 b716541ad170..41b0c8cc5312 100644 >> --- a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c >> +++ b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c >> @@ -680,6 +680,7 @@ MmioExit ( >> UINTN Bytes; >> UINT64 *Register; >> UINT8 OpCode, SignByte; >> + UINTN Address; >> >> Bytes = 0; >> >> @@ -729,6 +730,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) >> // >> @@ -811,6 +863,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) >> // >> @@ -888,6 +998,7 @@ MmioExit ( >> break; >> >> default: >> + DEBUG ((DEBUG_ERROR, "Invalid MMIO opcode (%x)\n", OpCode)); >> Status = GP_EXCEPTION; >> ASSERT (FALSE); >> } >> > > > > >