On Wed, Feb 28, 2018 at 03:10:25PM -0600, Brijesh Singh wrote: > When SEV is enabled, CPUID 0x8000_001F should provide additional > information regarding the feature (such as which page table bit is used > to mark the pages as encrypted etc). > > The details for memory encryption CPUID is available in AMD APM > (https://support.amd.com/TechDocs/24594.pdf) Section E.4.17 > > Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx> > Cc: Richard Henderson <rth@xxxxxxxxxxx> > Cc: Eduardo Habkost <ehabkost@xxxxxxxxxx> > Signed-off-by: Brijesh Singh <brijesh.singh@xxxxxxx> > --- > target/i386/cpu.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index b5e431e769da..7a3cec59402b 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -26,6 +26,7 @@ > #include "sysemu/hvf.h" > #include "sysemu/cpus.h" > #include "kvm_i386.h" > +#include "sev_i386.h" > > #include "qemu/error-report.h" > #include "qemu/option.h" > @@ -3612,6 +3613,13 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > *ecx = 0; > *edx = 0; > break; > + case 0x8000001F: What I'm worried about here is ABI stability and migration safety guarantees. Let's see: > + *eax = sev_enabled() ? 0x2 : 0; sev_enabled() just checks if sev_state is set. sev_state is only set by sev_guest_init(). sev_guest_init() is only called if MachineState::memory_encryption is set. The value is a function of command-line options only. Good. > + *ebx = sev_get_cbit_position(); This is 0 if sev_state is NULL (see above). Good. If sev_state is set, it returns sev_state->cbitpos. s->cbitpos is set based on the "cbitpos" property of QSevGuestInfo only (it's only validated against host CPUID). The property has no host-dependent default and needs to be set explicitly. This means sev_get_cbit_position() is a function of command-line options only, too. Good. > + *ebx |= sev_get_reduced_phys_bits() << 6; Logic is very similar to sev_get_cbit_position(), and depends on the "reduced-phys-bits" property of QSevGuestInfo only. Good. > + *ecx = 0; > + *edx = 0; > + break; > default: > /* reserved values: zero */ > *eax = 0; > @@ -4041,6 +4049,11 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp) > if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_SVM) { > x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel, 0x8000000A); > } > + > + /* SEV requires CPUID[0x8000001F] */ > + if (sev_enabled()) { > + x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel, 0x8000001F); > + } Reviewed-by: Eduardo Habkost <ehabkost@xxxxxxxxxx> > } > > /* Set cpuid_*level* based on cpuid_min_*level, if not explicitly set */ > -- > 2.14.3 > > -- Eduardo
