[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] current QEMU can't start pc-q35-2.12 SEV guest
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] current QEMU can't start pc-q35-2.12 SEV guest |
Date: |
Thu, 15 Aug 2019 15:43:51 -0300 |
On Thu, Aug 15, 2019 at 12:49:58AM +0000, Bruce Rogers wrote:
> Hi,
>
> I ran into a case where a guest on a SEV capable host, which was
> enabled to use SEV and using an older machine type was no longer able
> to run when the QEMU version had been updated.
>
> Specifically, when the guest was installed and running under a v2.12
> QEMU, set up for SEV (ok it was v2.11 with SEV support backported, but
> the details still apply), using a command line such as follows:
>
> qemu-system-x86_64 -cpu EPYC-IBRS \
> -machine pc-q35-2.12,accel=kvm,memory-encryption=sev0 \
> -object sev-guest,id=sev0,...
>
> The guest ran fine, using SEV memory enryption.
>
> Later the version of QEMU was updated to v3.1.0, and the same guest now
> hung at boot, when using the exact same command line. (Current QEMU
> still has the same problem.)
>
> Upon investigation, I find that the handling of xlevel in
> target/i386/cpu.c relies includes an explicit detection of SEV being
> enabled and sets the cpuid_min_xlevel in the CPUX86State structure to
> 0x8000001F as the required minimum for SEV support. This normally is
> used to set the xlevel the guest sees, allowing it to use SEV.
>
> The compat settings for the v2.12 machine type include an xlevel value
> associated with it (0x8000000A). Unfortunately the processing of the
> compat settings gets conflated with the logic of handling a user
> explicitly specifying an xlevel on the command line, which is treated
> as an "override" condition, overriding the other xlevel selections
> which would otherwise be done in the QEMU cpu code.
>
> So, in the scenario I describe above, the original, working case would
> provide an cpuid xlevel value of 0x8000001F to the guest (correct), and
> the failing case ends up providing the value 0x8000000A (incorrect).
>
> It seems to me that the handling of the compat settings and the
> explicit xlevel setting by the user should be processed separately, but
> I don't see how to do that easily.
>
> How should this problem be resolved?
>
> In my case, I've added to the code which is for checking a user
> provided xlevel value, the check again for sev_enabled(), and if that's
> the case, I still apply the cpuid_min_xlevel value. This works for the
> time being, but doesn't seem to be the right solution.
>
I believe this is my fault. On commit e00516475c27 ("i386:
Enable TOPOEXT feature on AMD EPYC CPU"), I had added
xlevel=0x8000000A compat entries, but they were supposed to be
min-xlevel=0x8000000A.
Does this patch solve the problem?
---
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 549c437050..11d5a3cd3a 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -154,8 +154,8 @@ const size_t pc_compat_3_0_len =
G_N_ELEMENTS(pc_compat_3_0);
GlobalProperty pc_compat_2_12[] = {
{ TYPE_X86_CPU, "legacy-cache", "on" },
{ TYPE_X86_CPU, "topoext", "off" },
- { "EPYC-" TYPE_X86_CPU, "xlevel", "0x8000000a" },
- { "EPYC-IBPB-" TYPE_X86_CPU, "xlevel", "0x8000000a" },
+ { "EPYC-" TYPE_X86_CPU, "min-xlevel", "0x8000000a" },
+ { "EPYC-IBPB-" TYPE_X86_CPU, "min-xlevel", "0x8000000a" },
};
const size_t pc_compat_2_12_len = G_N_ELEMENTS(pc_compat_2_12);
--
Eduardo