linux-kernel - Re: [PATCH 02/43] KVM: VMX: Set EDX at INIT with CPUID.0x1, Family-Model-Stepping

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YKVdUtvSg7/I7Ses@google.com>
Date:   Wed, 19 May 2021 18:47:46 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Reiji Watanabe <reijiw@...gle.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 02/43] KVM: VMX: Set EDX at INIT with CPUID.0x1,
 Family-Model-Stepping

On Tue, May 18, 2021, Reiji Watanabe wrote:
> > @@ -4504,7 +4505,11 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> >
> >         vmx->msr_ia32_umwait_control = 0;
> >
> > -       vmx->vcpu.arch.regs[VCPU_REGS_RDX] = get_rdx_init_val();
> > +       eax = 1;
> > +       if (!kvm_cpuid(vcpu, &eax, &dummy, &dummy, &dummy, true))
> > +               eax = get_rdx_init_val();
> > +       kvm_rdx_write(vcpu, eax);
> 
> Reviewed-by: Reiji Watanabe <reijiw@...gle.com>
> 
> For RESET, I assume that rdx should be set by userspace
> when userspace changes CPUID.0x1.EAX.

Ya, although the ideal solution is to add a proper RESET ioctl() so userspace can
configure the vCPU model and then pull RESET#.

> BTW, I would think having a default CPUID for CPUID.(EAX=0x1) would be better
> for consistency of a vCPU state for RESET.  I would think it doesn't matter
> practically anyway though.

Probably, but that would require defining default values for all of CPUID.0x0 and
CPUID.0x1, which is a can of worms I'd rather not open.  E.g. vendor info, basic
feature set, APIC ID, etc... would all need default values.  On the other hand,
the EDX value stuffing predates CPUID, so using 0x600 isn't provably wrong, just
a bit anachronistic. :-)