Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 19 May 2021 18:47:46 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Reiji Watanabe <reijiw@google.com>
Cc:     Paolo Bonzini <pbonzini@redhat.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 02/43] KVM: VMX: Set EDX at INIT with CPUID.0x1,
 Family-Model-Stepping
Message-ID: <YKVdUtvSg7/I7Ses@google.com>
References: <20210424004645.3950558-1-seanjc@google.com>
 <20210424004645.3950558-3-seanjc@google.com>
 <CAAeT=FyNo1CGvnamc3_J9EEQUn6WcdkMp50-QgmLYYVCFA2fZA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAAeT=FyNo1CGvnamc3_J9EEQUn6WcdkMp50-QgmLYYVCFA2fZA@mail.gmail.com>
Precedence: bulk

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@google.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. :-)