Messages in this thread | | | Subject | Re: [PATCH v2 2/7] KVM: X86: Go on updating other CPUID leaves when leaf 1 is absent | From | Xiaoyao Li <> | Date | Fri, 3 Jul 2020 06:28:39 +0800 |
| |
On 7/3/2020 3:02 AM, Sean Christopherson wrote: > On Thu, Jul 02, 2020 at 11:54:03AM -0700, Sean Christopherson wrote: >> On Tue, Jun 23, 2020 at 07:58:11PM +0800, Xiaoyao Li wrote: >>> As handling of bits other leaf 1 added over time, kvm_update_cpuid() >>> should not return directly if leaf 1 is absent, but should go on >>> updateing other CPUID leaves. >>> >>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> >> >> This should probably be marked for stable. >> >>> --- >>> arch/x86/kvm/cpuid.c | 23 +++++++++++------------ >>> 1 file changed, 11 insertions(+), 12 deletions(-) >>> >>> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c >>> index 1d13bad42bf9..0164dac95ef5 100644 >>> --- a/arch/x86/kvm/cpuid.c >>> +++ b/arch/x86/kvm/cpuid.c >>> @@ -60,22 +60,21 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu) >>> struct kvm_lapic *apic = vcpu->arch.apic; >>> >>> best = kvm_find_cpuid_entry(vcpu, 1, 0); >>> - if (!best) >>> - return 0; >> >> Rather than wrap the existing code, what about throwing it in a separate >> helper? That generates an easier to read diff and also has the nice >> property of getting 'apic' out of the common code. > > Hrm, that'd be overkill once the apic code is moved in a few patches. > What if you keep the cpuid updates wrapped (as in this patch), but then > do > > if (best && apic) { > } > > for the apic path? That'll minimize churn for code that is disappearing, > e.g. will make future git archaeologists happy :-).
Sure. I'll do it in next version.
>>> - >>> - /* Update OSXSAVE bit */ >>> - if (boot_cpu_has(X86_FEATURE_XSAVE) && best->function == 0x1) >>> - cpuid_entry_change(best, X86_FEATURE_OSXSAVE, >>> + if (best) { >>> + /* Update OSXSAVE bit */ >>> + if (boot_cpu_has(X86_FEATURE_XSAVE)) >>> + cpuid_entry_change(best, X86_FEATURE_OSXSAVE, >>> kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE)); >>> >>> - cpuid_entry_change(best, X86_FEATURE_APIC, >>> + cpuid_entry_change(best, X86_FEATURE_APIC, >>> vcpu->arch.apic_base & MSR_IA32_APICBASE_ENABLE); >>> >>> - if (apic) { >>> - if (cpuid_entry_has(best, X86_FEATURE_TSC_DEADLINE_TIMER)) >>> - apic->lapic_timer.timer_mode_mask = 3 << 17; >>> - else >>> - apic->lapic_timer.timer_mode_mask = 1 << 17; >>> + if (apic) { >>> + if (cpuid_entry_has(best, X86_FEATURE_TSC_DEADLINE_TIMER)) >>> + apic->lapic_timer.timer_mode_mask = 3 << 17; >>> + else >>> + apic->lapic_timer.timer_mode_mask = 1 << 17; >>> + } >>> } >>> >>> best = kvm_find_cpuid_entry(vcpu, 7, 0); >>> -- >>> 2.18.2 >>>
| |