On Tue, Dec 5, 2023 at 11:28 PM Michael Roth <michael.roth@xxxxxxx> wrote:
> @@ -3637,12 +3638,18 @@ static int kvm_get_sregs(X86CPU *cpu)
> env->gdt.limit = sregs.gdt.limit;
> env->gdt.base = sregs.gdt.base;
>
> + cr0_old = env->cr[0];
> env->cr[0] = sregs.cr0;
> env->cr[2] = sregs.cr2;
> env->cr[3] = sregs.cr3;
> env->cr[4] = sregs.cr4;
>
> env->efer = sregs.efer;
> + if (sev_es_enabled() && env->efer & MSR_EFER_LME) {
> + if (!(cr0_old & CR0_PG_MASK) && env->cr[0] & CR0_PG_MASK) {
> + env->efer |= MSR_EFER_LMA;
> + }
> + }
There is no need to check cr0_old or sev_es_enabled(); EFER.LMA is
simply EFER.LME && CR0.PG.
Alternatively, sev_es_enabled() could be an assertion, that is:
if ((env->efer & MSR_EFER_LME) && (env->cr[0] & CR0_PG_MASK) &&
!(env->efer & MSR_EFER_LMA)) {
/* Workaround for... */
assert(sev_es_enabled());
env->efer |= MSR_EFER_LMA;
}
What do you think?
Thanks,
Paolo
> /* changes to apic base and cr8/tpr are read back via kvm_arch_post_run */
> x86_update_hflags(env);
> @@ -3654,6 +3661,7 @@ static int kvm_get_sregs2(X86CPU *cpu)
> {
> CPUX86State *env = &cpu->env;
> struct kvm_sregs2 sregs;
> + target_ulong cr0_old;
> int i, ret;
>
> ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_SREGS2, &sregs);
> @@ -3676,12 +3684,18 @@ static int kvm_get_sregs2(X86CPU *cpu)
> env->gdt.limit = sregs.gdt.limit;
> env->gdt.base = sregs.gdt.base;
>
> + cr0_old = env->cr[0];
> env->cr[0] = sregs.cr0;
> env->cr[2] = sregs.cr2;
> env->cr[3] = sregs.cr3;
> env->cr[4] = sregs.cr4;
>
> env->efer = sregs.efer;
> + if (sev_es_enabled() && env->efer & MSR_EFER_LME) {
> + if (!(cr0_old & CR0_PG_MASK) && env->cr[0] & CR0_PG_MASK) {
> + env->efer |= MSR_EFER_LMA;
> + }
> + }
>
> env->pdptrs_valid = sregs.flags & KVM_SREGS2_FLAGS_PDPTRS_VALID;
>
> --
> 2.25.1
>
