On Thu, 2019-08-15 at 15:43 -0300, Eduardo Habkost wrote: > 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); > >
Yes that does the trick. My analysis was a bit off the mark. This makes more sense than what I was doing. Thanks! Bruce