On 05/19/20 23:50, Lendacky, Thomas wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
>
> Under SEV-ES, a CPUID intercept generates a #VC exception. VMGEXIT must be
> used to allow the hypervisor to handle this intercept.
>
> Add support to construct the required GHCB values to support a CPUID NAE
> event. Additionally, CPUID 0x0000_000d requires XCR0 to be supplied in
> the GHCB, so add support to issue the XGETBV instruction.
>
> Cc: Jordan Justen <jordan.l.jus...@intel.com>
> Cc: Laszlo Ersek <ler...@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheu...@arm.com>
> Signed-off-by: Tom Lendacky <thomas.lenda...@amd.com>
> ---
> .../Library/VmgExitLib/X64/VmgExitVcHandler.c | 58 +++++++++++++++++++
> 1 file changed, 58 insertions(+)
>
> diff --git a/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c
> b/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c
> index 906b32e93d53..2f62795edf61 100644
> --- a/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c
> +++ b/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c
> @@ -12,6 +12,8 @@
> #include <Library/VmgExitLib.h>
> #include <Register/Amd/Msr.h>
>
> +#define CR4_OSXSAVE (1 << 18)
> +
(1) Please drop this macro, and:
> //
> // Instruction execution mode definition
> //
> @@ -637,6 +639,58 @@ IoioExit (
> return 0;
> }
>
> +/**
> + Handle a CPUID event.
> +
> + Use the VMGEXIT instruction to handle a CPUID event.
> +
> + @param[in, out] Ghcb Pointer to the Guest-Hypervisor
> Communication
> + Block
> + @param[in, out] Regs x64 processor context
> + @param[in] InstructionData Instruction parsing context
> +
> + @retval 0 Event handled successfully
> + @retval Others New exception value to propagate
> +
> +**/
> +STATIC
> +UINT64
> +CpuidExit (
> + IN OUT GHCB *Ghcb,
> + IN OUT EFI_SYSTEM_CONTEXT_X64 *Regs,
> + IN SEV_ES_INSTRUCTION_DATA *InstructionData
> + )
> +{
> + UINT64 Status;
> +
> + Ghcb->SaveArea.Rax = Regs->Rax;
> + GhcbSetRegValid (Ghcb, GhcbRax);
> + Ghcb->SaveArea.Rcx = Regs->Rcx;
> + GhcbSetRegValid (Ghcb, GhcbRcx);
> + if (Regs->Rax == 0x0000000d) {
(2a) Can we use CPUID_EXTENDED_STATE here, from <Register/Intel/Cpuid.h>?
(2b) If so, I'd suggest updating the commit message too: replace "CPUID
0x0000_000d" with "CPUID 0x0000_000d (CPUID_EXTENDED_STATE)".
> + Ghcb->SaveArea.XCr0 = (AsmReadCr4 () & CR4_OSXSAVE) ? AsmXGetBv (0) : 1;
(3) Here, please use the IA32_CR4 type from <Library/BaseLib.h>:
IA32_CR4 Cr4;
Cr4.UintN = AsmReadCr4 ();
Ghcb->SaveArea.XCr0 = (Cr4.Bits.OSXSAVE == 1) ? AsmXGetBv (0) : 1;
Some of the style requests I made under earlier patches in this series
apply here, so I won't spell them out again.
With the style updated:
Acked-by: Laszlo Ersek <ler...@redhat.com>
Thanks
Laszlo
> + GhcbSetRegValid (Ghcb, GhcbXCr0);
> + }
> +
> + Status = VmgExit (Ghcb, SVM_EXIT_CPUID, 0, 0);
> + if (Status) {
> + return Status;
> + }
> +
> + if (!GhcbIsRegValid (Ghcb, GhcbRax) ||
> + !GhcbIsRegValid (Ghcb, GhcbRbx) ||
> + !GhcbIsRegValid (Ghcb, GhcbRcx) ||
> + !GhcbIsRegValid (Ghcb, GhcbRdx)) {
> + return UnsupportedExit (Ghcb, Regs, InstructionData);
> + }
> + Regs->Rax = Ghcb->SaveArea.Rax;
> + Regs->Rbx = Ghcb->SaveArea.Rbx;
> + Regs->Rcx = Ghcb->SaveArea.Rcx;
> + Regs->Rdx = Ghcb->SaveArea.Rdx;
> +
> + return 0;
> +}
> +
> /**
> Handle a #VC exception.
>
> @@ -681,6 +735,10 @@ VmgExitHandleVc (
>
> ExitCode = Regs->ExceptionData;
> switch (ExitCode) {
> + case SVM_EXIT_CPUID:
> + NaeExit = CpuidExit;
> + break;
> +
> case SVM_EXIT_IOIO_PROT:
> NaeExit = IoioExit;
> break;
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#60110): https://edk2.groups.io/g/devel/message/60110
Mute This Topic: https://groups.io/mt/74336569/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
