linux-kernel - Re: [PATCH v8 7/7] KVM: x86: virtualize cpuid faulting

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: <CALzav=dS2eQ3frTxVPqZd4qz05bq68FaJHM+e2-oiXwvOHd_zA@mail.gmail.com>
Date:   Fri, 4 Nov 2016 17:09:02 -0700
From:   David Matlack <dmatlack@...gle.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     Kyle Huey <me@...ehuey.com>,
        "Robert O'Callahan" <robert@...llahan.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Andy Lutomirski <luto@...nel.org>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, X86 ML <x86@...nel.org>,
        Radim Krčmář <rkrcmar@...hat.com>,
        Jeff Dike <jdike@...toit.com>,
        Richard Weinberger <richard@....at>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Shuah Khan <shuah@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Borislav Petkov <bp@...e.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Len Brown <len.brown@...el.com>,
        "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
        Dmitry Safonov <dsafonov@...tuozzo.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Eduardo Habkost <ehabkost@...hat.com>
Subject: Re: [PATCH v8 7/7] KVM: x86: virtualize cpuid faulting

On Fri, Nov 4, 2016 at 2:57 PM, Paolo Bonzini <pbonzini@...hat.com> wrote:
>
> On 04/11/2016 21:34, David Matlack wrote:
>> On Mon, Oct 31, 2016 at 6:37 PM, Kyle Huey <me@...ehuey.com> wrote:
>>> +       case MSR_PLATFORM_INFO:
>>> +               /* cpuid faulting is supported */
>>> +               msr_info->data = PLATINFO_CPUID_FAULT;
>>> +               break;
>>
>> This could break save/restore, if for example, a VM is migrated to a
>> version of KVM without MSR_PLATFORM_INFO support. I think the way to
>> handle this is to make MSR_PLATFORM_INFO writeable (but only from
>> userspace) so that hypervisors can defend themselves (by setting this
>> MSR to 0).
>
> Right---and with my QEMU hat on, this feature will have to be enabled
> manually on the command line because of the way QEMU supports running
> with old kernels. :(  This however does not impact the KVM patch.
>
> We may decide that, because CPUID faulting doesn't have a CPUID bit and
> is relatively a "fringe" feature, we are okay if the kernel enables this
> unconditionally and then userspace can arrange to block migration (in
> QEMU this would use a subsection).  David, Eduardo, opinions?

Sounds reasonable. Accurate CPU virtualization might be another reason
to disable this feature from userspace.

My worry is a kernel rollback, where migrating to an older kernel
version is unavoidable.

>
>>
>>> +       case MSR_MISC_FEATURES_ENABLES:
>>> +               msr_info->data = 0;
>>> +               if (vcpu->arch.cpuid_fault)
>>> +                       msr_info->data |= CPUID_FAULT_ENABLE;
>>> +               break;
>>
>> MSR_MISC_FEATURES_ENABLES should be added to emulated_msrs[] so that
>> the hypervisor will maintain the value of CPUID_FAULT_ENABLE across a
>> save/restore.
>
> This is definitely necessary.  Thanks David.
>
> Paolo
>
>>>         default:
>>>                 if (kvm_pmu_is_valid_msr(vcpu, msr_info->index))
>>>                         return kvm_pmu_get_msr(vcpu, msr_info->index, &msr_info->data);
>>>                 if (!ignore_msrs) {
>>>                         vcpu_unimpl(vcpu, "unhandled rdmsr: 0x%x\n", msr_info->index);
>>>                         return 1;
>>>                 } else {
>>>                         vcpu_unimpl(vcpu, "ignored rdmsr: 0x%x\n", msr_info->index);
>>> @@ -7493,16 +7507,18 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>>>         kvm_update_dr0123(vcpu);
>>>         vcpu->arch.dr6 = DR6_INIT;
>>>         kvm_update_dr6(vcpu);
>>>         vcpu->arch.dr7 = DR7_FIXED_1;
>>>         kvm_update_dr7(vcpu);
>>>
>>>         vcpu->arch.cr2 = 0;
>>>
>>> +       vcpu->arch.cpuid_fault = false;
>>> +
>>>         kvm_make_request(KVM_REQ_EVENT, vcpu);
>>>         vcpu->arch.apf.msr_val = 0;
>>>         vcpu->arch.st.msr_val = 0;
>>>
>>>         kvmclock_reset(vcpu);
>>>
>>>         kvm_clear_async_pf_completion_queue(vcpu);
>>>         kvm_async_pf_hash_reset(vcpu);
>>> --
>>> 2.10.2
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>>> the body of a message to majordomo@...r.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>