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.web09.5596.1603140688920286085 for ; Mon, 19 Oct 2020 13:51:29 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=O0y7l9rS; 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=1603140688; 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=e4lwiBDh8nWhXKO7aTu94BSKWamn5BwgLFM5U3Wiar4=; b=O0y7l9rSXcsHRKyJLgitVG8oNxiIHpXE0nU8j5CE4/QXIPtL+vc/vtnJsqBt0zZGsBpgb8 IjwouLt4wwg70NqZV7qDPcruBwPBmsWIbM2SDfhRYXkykBjjBRnXrgo9zrqdwDhZ27e6HY h4PxBgi8UKxmsuNfq/Ns/OaRVUBOwbE= 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-131--48nuln3OAWAKkexYi8eBQ-1; Mon, 19 Oct 2020 16:51:23 -0400 X-MC-Unique: -48nuln3OAWAKkexYi8eBQ-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A60E68049C8; Mon, 19 Oct 2020 20:51:22 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-137.ams2.redhat.com [10.36.113.137]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2DDFF10013C1; Mon, 19 Oct 2020 20:51:20 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v2 03/11] OvmfPkg/VmgExitLib: Implement new VmgExitLib interfaces To: devel@edk2.groups.io, thomas.lendacky@amd.com Cc: Brijesh Singh , Jordan Justen , Ard Biesheuvel References: <495003dd5667796b7e79ac30b8d72308e46bd91a.1602864557.git.thomas.lendacky@amd.com> From: "Laszlo Ersek" Message-ID: Date: Mon, 19 Oct 2020 22:51:20 +0200 MIME-Version: 1.0 In-Reply-To: <495003dd5667796b7e79ac30b8d72308e46bd91a.1602864557.git.thomas.lendacky@amd.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 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 10/16/20 18:09, Lendacky, Thomas wrote: > From: Tom Lendacky > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3008 > > The VmgExitLib library added two new interfaces, VmgSetOffsetValid() and > VmgIsOffsetValid(), that must now be implemented in the OvmfPkg version > of the library. > > Implement VmgSetOffsetValid() and VmgIsOffsetValid() and update existing > code, that is directly accessing ValidBitmap, to use the new interfaces. > > Cc: Jordan Justen > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Cc: Tom Lendacky > Cc: Brijesh Singh > Signed-off-by: Tom Lendacky > --- > OvmfPkg/Library/VmgExitLib/VmgExitLib.c | 54 +++++++++ > OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 118 +++++--------------- > 2 files changed, 85 insertions(+), 87 deletions(-) > > diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitLib.c b/OvmfPkg/Library/VmgExitLib/VmgExitLib.c > index 53040cc6f649..3072c2265df7 100644 > --- a/OvmfPkg/Library/VmgExitLib/VmgExitLib.c > +++ b/OvmfPkg/Library/VmgExitLib/VmgExitLib.c > @@ -157,3 +157,57 @@ VmgDone ( > { > } > > +/** > + Marks a field at the specified offset as valid in the GHCB. > + > + The ValidBitmap area represents the areas of the GHCB that have been marked > + valid. Set the bit in ValidBitmap for the input offset. > + > + @param[in, out] Ghcb Pointer to the Guest-Hypervisor Communication Block > + @param[in] Offset Qword offset in the GHCB to mark valid > + > +**/ > +VOID > +EFIAPI > +VmgSetOffsetValid ( > + IN OUT GHCB *Ghcb, > + IN GHCB_QWORD_OFFSET Offset > + ) > +{ > + UINT32 OffsetIndex; > + UINT32 OffsetBit; > + > + OffsetIndex = Offset / 8; > + OffsetBit = Offset % 8; > + > + Ghcb->SaveArea.ValidBitmap[OffsetIndex] |= (1 << OffsetBit); > +} > + > +/** > + Checks if a specified offset is valid in the GHCB. > + > + The ValidBitmap area represents the areas of the GHCB that have been marked > + valid. Return whether the bit in the ValidBitmap is set for the input offset. > + > + @param[in] Ghcb A pointer to the GHCB > + @param[in] Offset Qword offset in the GHCB to mark valid > + > + @retval TRUE Offset is marked vald in the GHCB s/vald/valid/ with that: Reviewed-by: Laszlo Ersek Thanks laszlo > + @retval FALSE Offset is not marked valid in the GHCB > + > +**/ > +BOOLEAN > +EFIAPI > +VmgIsOffsetValid ( > + IN GHCB *Ghcb, > + IN GHCB_QWORD_OFFSET Offset > + ) > +{ > + UINT32 OffsetIndex; > + UINT32 OffsetBit; > + > + OffsetIndex = Offset / 8; > + OffsetBit = Offset % 8; > + > + return ((Ghcb->SaveArea.ValidBitmap[OffsetIndex] & (1 << OffsetBit)) != 0); > +} > diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c > index c5484a3f478c..7d14341d592b 100644 > --- a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c > +++ b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c > @@ -135,62 +135,6 @@ typedef struct { > } SEV_ES_PER_CPU_DATA; > > > -/** > - Checks the GHCB to determine if the specified register has been marked valid. > - > - The ValidBitmap area represents the areas of the GHCB that have been marked > - valid. Return an indication of whether the area of the GHCB that holds the > - specified register has been marked valid. > - > - @param[in] Ghcb Pointer to the Guest-Hypervisor Communication Block > - @param[in] Reg Offset in the GHCB of the register to check > - > - @retval TRUE Register has been marked vald in the GHCB > - @retval FALSE Register has not been marked valid in the GHCB > - > -**/ > -STATIC > -BOOLEAN > -GhcbIsRegValid ( > - IN GHCB *Ghcb, > - IN GHCB_QWORD_OFFSET Reg > - ) > -{ > - UINT32 RegIndex; > - UINT32 RegBit; > - > - RegIndex = Reg / 8; > - RegBit = Reg & 0x07; > - > - return ((Ghcb->SaveArea.ValidBitmap[RegIndex] & (1 << RegBit)) != 0); > -} > - > -/** > - Marks a register as valid in the GHCB. > - > - The ValidBitmap area represents the areas of the GHCB that have been marked > - valid. Set the area of the GHCB that holds the specified register as valid. > - > - @param[in, out] Ghcb Pointer to the Guest-Hypervisor Communication Block > - @param[in] Reg Offset in the GHCB of the register to mark valid > - > -**/ > -STATIC > -VOID > -GhcbSetRegValid ( > - IN OUT GHCB *Ghcb, > - IN GHCB_QWORD_OFFSET Reg > - ) > -{ > - UINT32 RegIndex; > - UINT32 RegBit; > - > - RegIndex = Reg / 8; > - RegBit = Reg & 0x07; > - > - Ghcb->SaveArea.ValidBitmap[RegIndex] |= (1 << RegBit); > -} > - > /** > Return a pointer to the contents of the specified register. > > @@ -891,9 +835,9 @@ MwaitExit ( > DecodeModRm (Regs, InstructionData); > > Ghcb->SaveArea.Rax = Regs->Rax; > - GhcbSetRegValid (Ghcb, GhcbRax); > + VmgSetOffsetValid (Ghcb, GhcbRax); > Ghcb->SaveArea.Rcx = Regs->Rcx; > - GhcbSetRegValid (Ghcb, GhcbRcx); > + VmgSetOffsetValid (Ghcb, GhcbRcx); > > return VmgExit (Ghcb, SVM_EXIT_MWAIT, 0, 0); > } > @@ -923,11 +867,11 @@ MonitorExit ( > DecodeModRm (Regs, InstructionData); > > Ghcb->SaveArea.Rax = Regs->Rax; // Identity mapped, so VA = PA > - GhcbSetRegValid (Ghcb, GhcbRax); > + VmgSetOffsetValid (Ghcb, GhcbRax); > Ghcb->SaveArea.Rcx = Regs->Rcx; > - GhcbSetRegValid (Ghcb, GhcbRcx); > + VmgSetOffsetValid (Ghcb, GhcbRcx); > Ghcb->SaveArea.Rdx = Regs->Rdx; > - GhcbSetRegValid (Ghcb, GhcbRdx); > + VmgSetOffsetValid (Ghcb, GhcbRdx); > > return VmgExit (Ghcb, SVM_EXIT_MONITOR, 0, 0); > } > @@ -988,9 +932,9 @@ RdtscpExit ( > return Status; > } > > - if (!GhcbIsRegValid (Ghcb, GhcbRax) || > - !GhcbIsRegValid (Ghcb, GhcbRcx) || > - !GhcbIsRegValid (Ghcb, GhcbRdx)) { > + if (!VmgIsOffsetValid (Ghcb, GhcbRax) || > + !VmgIsOffsetValid (Ghcb, GhcbRcx) || > + !VmgIsOffsetValid (Ghcb, GhcbRdx)) { > return UnsupportedExit (Ghcb, Regs, InstructionData); > } > Regs->Rax = Ghcb->SaveArea.Rax; > @@ -1027,16 +971,16 @@ VmmCallExit ( > DecodeModRm (Regs, InstructionData); > > Ghcb->SaveArea.Rax = Regs->Rax; > - GhcbSetRegValid (Ghcb, GhcbRax); > + VmgSetOffsetValid (Ghcb, GhcbRax); > Ghcb->SaveArea.Cpl = (UINT8) (Regs->Cs & 0x3); > - GhcbSetRegValid (Ghcb, GhcbCpl); > + VmgSetOffsetValid (Ghcb, GhcbCpl); > > Status = VmgExit (Ghcb, SVM_EXIT_VMMCALL, 0, 0); > if (Status != 0) { > return Status; > } > > - if (!GhcbIsRegValid (Ghcb, GhcbRax)) { > + if (!VmgIsOffsetValid (Ghcb, GhcbRax)) { > return UnsupportedExit (Ghcb, Regs, InstructionData); > } > Regs->Rax = Ghcb->SaveArea.Rax; > @@ -1074,15 +1018,15 @@ MsrExit ( > case 0x30: // WRMSR > ExitInfo1 = 1; > Ghcb->SaveArea.Rax = Regs->Rax; > - GhcbSetRegValid (Ghcb, GhcbRax); > + VmgSetOffsetValid (Ghcb, GhcbRax); > Ghcb->SaveArea.Rdx = Regs->Rdx; > - GhcbSetRegValid (Ghcb, GhcbRdx); > + VmgSetOffsetValid (Ghcb, GhcbRdx); > // > // fall through > // > case 0x32: // RDMSR > Ghcb->SaveArea.Rcx = Regs->Rcx; > - GhcbSetRegValid (Ghcb, GhcbRcx); > + VmgSetOffsetValid (Ghcb, GhcbRcx); > break; > default: > return UnsupportedExit (Ghcb, Regs, InstructionData); > @@ -1094,8 +1038,8 @@ MsrExit ( > } > > if (ExitInfo1 == 0) { > - if (!GhcbIsRegValid (Ghcb, GhcbRax) || > - !GhcbIsRegValid (Ghcb, GhcbRdx)) { > + if (!VmgIsOffsetValid (Ghcb, GhcbRax) || > + !VmgIsOffsetValid (Ghcb, GhcbRdx)) { > return UnsupportedExit (Ghcb, Regs, InstructionData); > } > Regs->Rax = Ghcb->SaveArea.Rax; > @@ -1311,7 +1255,7 @@ IoioExit ( > } else { > CopyMem (&Ghcb->SaveArea.Rax, &Regs->Rax, IOIO_DATA_BYTES (ExitInfo1)); > } > - GhcbSetRegValid (Ghcb, GhcbRax); > + VmgSetOffsetValid (Ghcb, GhcbRax); > > Status = VmgExit (Ghcb, SVM_EXIT_IOIO_PROT, ExitInfo1, 0); > if (Status != 0) { > @@ -1319,7 +1263,7 @@ IoioExit ( > } > > if ((ExitInfo1 & IOIO_TYPE_IN) != 0) { > - if (!GhcbIsRegValid (Ghcb, GhcbRax)) { > + if (!VmgIsOffsetValid (Ghcb, GhcbRax)) { > return UnsupportedExit (Ghcb, Regs, InstructionData); > } > CopyMem (&Regs->Rax, &Ghcb->SaveArea.Rax, IOIO_DATA_BYTES (ExitInfo1)); > @@ -1379,15 +1323,15 @@ CpuidExit ( > UINT64 Status; > > Ghcb->SaveArea.Rax = Regs->Rax; > - GhcbSetRegValid (Ghcb, GhcbRax); > + VmgSetOffsetValid (Ghcb, GhcbRax); > Ghcb->SaveArea.Rcx = Regs->Rcx; > - GhcbSetRegValid (Ghcb, GhcbRcx); > + VmgSetOffsetValid (Ghcb, GhcbRcx); > if (Regs->Rax == CPUID_EXTENDED_STATE) { > IA32_CR4 Cr4; > > Cr4.UintN = AsmReadCr4 (); > Ghcb->SaveArea.XCr0 = (Cr4.Bits.OSXSAVE == 1) ? AsmXGetBv (0) : 1; > - GhcbSetRegValid (Ghcb, GhcbXCr0); > + VmgSetOffsetValid (Ghcb, GhcbXCr0); > } > > Status = VmgExit (Ghcb, SVM_EXIT_CPUID, 0, 0); > @@ -1395,10 +1339,10 @@ CpuidExit ( > return Status; > } > > - if (!GhcbIsRegValid (Ghcb, GhcbRax) || > - !GhcbIsRegValid (Ghcb, GhcbRbx) || > - !GhcbIsRegValid (Ghcb, GhcbRcx) || > - !GhcbIsRegValid (Ghcb, GhcbRdx)) { > + if (!VmgIsOffsetValid (Ghcb, GhcbRax) || > + !VmgIsOffsetValid (Ghcb, GhcbRbx) || > + !VmgIsOffsetValid (Ghcb, GhcbRcx) || > + !VmgIsOffsetValid (Ghcb, GhcbRdx)) { > return UnsupportedExit (Ghcb, Regs, InstructionData); > } > Regs->Rax = Ghcb->SaveArea.Rax; > @@ -1434,15 +1378,15 @@ RdpmcExit ( > UINT64 Status; > > Ghcb->SaveArea.Rcx = Regs->Rcx; > - GhcbSetRegValid (Ghcb, GhcbRcx); > + VmgSetOffsetValid (Ghcb, GhcbRcx); > > Status = VmgExit (Ghcb, SVM_EXIT_RDPMC, 0, 0); > if (Status != 0) { > return Status; > } > > - if (!GhcbIsRegValid (Ghcb, GhcbRax) || > - !GhcbIsRegValid (Ghcb, GhcbRdx)) { > + if (!VmgIsOffsetValid (Ghcb, GhcbRax) || > + !VmgIsOffsetValid (Ghcb, GhcbRdx)) { > return UnsupportedExit (Ghcb, Regs, InstructionData); > } > Regs->Rax = Ghcb->SaveArea.Rax; > @@ -1480,8 +1424,8 @@ RdtscExit ( > return Status; > } > > - if (!GhcbIsRegValid (Ghcb, GhcbRax) || > - !GhcbIsRegValid (Ghcb, GhcbRdx)) { > + if (!VmgIsOffsetValid (Ghcb, GhcbRax) || > + !VmgIsOffsetValid (Ghcb, GhcbRdx)) { > return UnsupportedExit (Ghcb, Regs, InstructionData); > } > Regs->Rax = Ghcb->SaveArea.Rax; > @@ -1531,7 +1475,7 @@ Dr7WriteExit ( > // Using a value of 0 for ExitInfo1 means RAX holds the value > // > Ghcb->SaveArea.Rax = *Register; > - GhcbSetRegValid (Ghcb, GhcbRax); > + VmgSetOffsetValid (Ghcb, GhcbRax); > > Status = VmgExit (Ghcb, SVM_EXIT_DR7_WRITE, 0, 0); > if (Status != 0) { >