From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 In-Reply-To: <20170208080917.24320-9-khuey@kylehuey.com> References: <20170208080917.24320-1-khuey@kylehuey.com> <20170208080917.24320-9-khuey@kylehuey.com> From: Jim Mattson Date: Fri, 27 Jul 2018 10:15:32 -0700 Message-ID: Subject: Re: [PATCH v14 8/9] KVM: x86: virtualize cpuid faulting Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org To: Kyle Huey Cc: Robert O'Callahan , Thomas Gleixner , Andy Lutomirski , Ingo Molnar , "H. Peter Anvin" , the arch/x86 maintainers , Paolo Bonzini , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Jeff Dike , Richard Weinberger , Alexander Viro , Shuah Khan , Dave Hansen , Borislav Petkov , Peter Zijlstra , Boris Ostrovsky , Len Brown , "Rafael J. Wysocki" , Dmitry Safonov , David Matlack , Nadav Amit , Andi Kleen , LKML , user-mode-linux-devel@lists.sourceforge.net, user-mode-linux-user@lists.sourceforge.net, linux-fsdevel@vger.kernel.org, linux-kselftest@vger.kernel.org, kvm list List-ID: On Wed, Feb 8, 2017 at 12:09 AM, Kyle Huey wrote: > Hardware support for faulting on the cpuid instruction is not required to > emulate it, because cpuid triggers a VM exit anyways. KVM handles the relevant > MSRs (MSR_PLATFORM_INFO and MSR_MISC_FEATURES_ENABLE) and upon a > cpuid-induced VM exit checks the cpuid faulting state and the CPL. > kvm_require_cpl is even kind enough to inject the GP fault for us. > > Signed-off-by: Kyle Huey > Reviewed-by: David Matlack I have a couple of concerns about portions of this patch: 1) There are some backward compatibility issues: A) Suppose we have an old userspace that doesn't know it needs to zero MSR_PLATFORM_INFO to preserve existing behavior (to the extent possible). If a VM starts on a new kernel it could set the bit in MSR_MISC_FEATURES_ENABLES that enables CPUID faulting. On live-migration to an old kernel, that bit would be lost. B) With either an old userspace or a new userspace, as a VM migrates between old and new kernels, the behavior of RDMSR with ECX set to either MSR_PLATFORM_INFO or MSR_MISC_FEATURES_ENABLES will vary depending on which kernel the VM is currently running on. Ideally, I think this new functionality should be guarded by a KVM capability that has to be enabled from userspace. 2) This doesn't really play well with volume 3 of the SDM, section 18.7.3, where Intel instructs developers to use MSR_PLATFORM_INFO[15:8] to determine the TSC frequency for a variety of microarchitectures. When reads of this MSR raised #GP, it was pretty clear that one couldn't get the TSC frequency that way, but I don't think many consumers would specifically check for a 0 in that field when the RDMSR succeeds. If a guest hypervisor used that value in the computation of the TSC scaling factor for a VMCS12, for example, it might be surprised to get a #DE.