Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934250AbcKWWIS (ORCPT ); Wed, 23 Nov 2016 17:08:18 -0500 Received: from mail-qt0-f172.google.com ([209.85.216.172]:34761 "EHLO mail-qt0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934039AbcKWWIM (ORCPT ); Wed, 23 Nov 2016 17:08:12 -0500 MIME-Version: 1.0 In-Reply-To: References: <1479863680-117511-1-git-send-email-dmatlack@google.com> <1479863680-117511-4-git-send-email-dmatlack@google.com> <1168524783.1374944.1479891977484.JavaMail.zimbra@redhat.com> From: David Matlack Date: Wed, 23 Nov 2016 14:07:40 -0800 Message-ID: Subject: Re: [PATCH 3/4] KVM: nVMX: accurate emulation of MSR_IA32_CR{0,4}_FIXED1 To: Paolo Bonzini Cc: kvm list , "linux-kernel@vger.kernel.org" , Jim Mattson , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2378 Lines: 52 On Wed, Nov 23, 2016 at 11:24 AM, Paolo Bonzini wrote: > > > On 23/11/2016 20:16, David Matlack wrote: >> > Oh, I thought userspace would do that! Doing it in KVM is fine as well, >> > but then do we need to give userspace access to CR{0,4}_FIXED{0,1} at all? >> >> I think it should be safe for userspace to skip restoring CR4_FIXED1, >> since it is 100% generated based on CPUID. But I'd prefer to keep it >> accessible from userspace, for consistency with the other VMX MSRs and >> for flexibility. The auditing should ensure userspace doesn't restore >> a CR4_FIXED1 that is inconsistent with CPUID. > > Or would it just allow userspace to put anything into it, even if it's > inconsistent with CPUID, as long as it's consistent with the host? It would not allow anything inconsistent with guest CPUID. The auditing on restore of CR4_FIXED1 compares the new value with vmx->nested.nested_vmx_cr4_fixed1, which is updated as part of setting the guest's CPUID. > >> Userspace should restore CR0_FIXED1 in case future CPUs change which >> bits of CR0 are valid in VMX operation. Userspace should also restore >> CR{0,4}_FIXED0 so we have the flexibility to change the defaults in >> KVM. Both of these situations seem unlikely but we might as well play >> it safe, the cost is small. > > I disagree, there is always a cost. Besides the fact that it's > unlikely that there'll be any future CR0 bits at all, any changes would > most likely be keyed by a new CPUID bit (the same as CR4) or execution > control (the same as unrestricted guest). That's true. So CR0_FIXED1 would not need to be accessible from userspace either. This patch would need to be a little different then: vmx_cpuid_update should also update vmx->nested.nested_vmx_cr0_fixed1 to 0xffffffff. A downside of this scheme is we'd have to remember to update nested_vmx_cr4_fixed1_update() before giving VMs new CPUID bits. If we forget, a VM could end up with different values for CR{0,4}_FIXED0 for the same CPUID depending on which version of KVM you're running on. Hm, now I'm thinking you were right in the beginning. Userspace should generate CR{0,4}_FIXED1, not the kernel. And KVM should allow userspace to save/restore them. > > In the end, since we assume that userspace (any) has no idea of what to > do with it, I see no good reason to make the MSRs available. > > Paolo