From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH] KVM: x86: Keep masked bits unmodified on kvm_set_shared_msr Date: Thu, 21 Aug 2014 14:31:53 +0200 Message-ID: <53F5E6B9.2090307@redhat.com> References: <1408536711-32643-1-git-send-email-namit@cs.technion.ac.il> <20140821080536.GA30303@kernel> <53F5DE7E.1000506@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Wanpeng Li , Nadav Amit , kvm@vger.kernel.org To: Nadav Amit Return-path: Received: from mail-wi0-f180.google.com ([209.85.212.180]:53788 "EHLO mail-wi0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755104AbaHUMb5 (ORCPT ); Thu, 21 Aug 2014 08:31:57 -0400 Received: by mail-wi0-f180.google.com with SMTP id n3so8367408wiv.13 for ; Thu, 21 Aug 2014 05:31:56 -0700 (PDT) In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: Il 21/08/2014 14:19, Nadav Amit ha scritto: >> > >> > He meant they are passed as zero in the WRMSR but actually they're not >> > zeroed. They're set to the value that is passed to kvm_set_shared_msr, >> > and this value is massaged elsewhere to do mix guest and host bugs. See >> > update_transition_efer. >> > >> > So I'm removing this patch, it's wrong. > I stand corrected - they are massaged in update_transition_efer. > > The question is whether this massaging is specific to EFER, or a general one. > Currently update_transition_efer does: > > guest_efer &= ~ignore_bits; > guest_efer |= host_efer & ignore_bits; > vmx->guest_msrs[efer_offset].data = guest_efer; > > I think this is a general behaviour - taking the masked bits from the > host, and the rest from the guest. Therefore, it makes sense to put > this logic into kvm_set_shared_msr. I understand the EFER is > currently the only MSR which is only partially masked. Nonetheless, > kvm_set_shared_msr can be useful for other purposes. Yes, I agree. But right now it's not particularly interesting to do it: you're not using the functionality in e.g. the MISC_ENABLE patch, so it's just a matter of defining the semantics of the .data field, basically. Paolo