From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932463AbdKDR3I convert rfc822-to-8bit (ORCPT ); Sat, 4 Nov 2017 13:29:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41280 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932111AbdKDR3H (ORCPT ); Sat, 4 Nov 2017 13:29:07 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com D9AF4C04AC54 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=pbonzini@redhat.com Date: Sat, 4 Nov 2017 13:29:06 -0400 (EDT) From: Paolo Bonzini To: "Gustavo A. R. Silva" Cc: Radim =?utf-8?B?S3LEjW3DocWZ?= , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org Message-ID: <256976066.17297233.1509816546047.JavaMail.zimbra@redhat.com> In-Reply-To: <20171103225819.GA3482@embeddedor.com> References: <20171103225819.GA3482@embeddedor.com> Subject: Re: [PATCH] KVM: VMX: replace move_msr_up with swap macro MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT X-Originating-IP: [94.39.170.107, 10.4.196.18, 10.4.195.25] Thread-Topic: replace move_msr_up with swap macro Thread-Index: 0Vf7ILBU3GzfXKKervw4qLBsa7qkCQ== X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Sat, 04 Nov 2017 17:29:07 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org ----- Original Message ----- > From: "Gustavo A. R. Silva" > To: "Paolo Bonzini" , "Radim Krčmář" , "Thomas Gleixner" > , "Ingo Molnar" , "H. Peter Anvin" , x86@kernel.org > Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, "Gustavo A. R. Silva" > Sent: Friday, November 3, 2017 11:58:19 PM > Subject: [PATCH] KVM: VMX: replace move_msr_up with swap macro > > Function move_msr_up is used to _manually_ swap MSR entries in MSR array. > This function can be removed and replaced using the swap macro instead. > > This code was detected with the help of Coccinelle. I think move_msr_up should instead change into a function like void mark_msr_for_save(struct vcpu_vmx *vmx, int index) { swap(vmx->guest_msrs[index], vmx->guest_msrs[vmx->save_nmsrs]); vmx->save_nmsrs++; } Using swap is useful, but it is also hiding what's going on exactly (in addition, using ++ inside a macro argument might be calling for trouble). Paolo > > Signed-off-by: Gustavo A. R. Silva > --- > The new lines are over 80 characters, but I think in this case that is > preferable over splitting them. > > arch/x86/kvm/vmx.c | 24 ++++++------------------ > 1 file changed, 6 insertions(+), 18 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index e6c8ffa..210e491 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -2544,18 +2544,6 @@ static bool vmx_invpcid_supported(void) > return cpu_has_vmx_invpcid() && enable_ept; > } > > -/* > - * Swap MSR entry in host/guest MSR entry array. > - */ > -static void move_msr_up(struct vcpu_vmx *vmx, int from, int to) > -{ > - struct shared_msr_entry tmp; > - > - tmp = vmx->guest_msrs[to]; > - vmx->guest_msrs[to] = vmx->guest_msrs[from]; > - vmx->guest_msrs[from] = tmp; > -} > - > static void vmx_set_msr_bitmap(struct kvm_vcpu *vcpu) > { > unsigned long *msr_bitmap; > @@ -2600,28 +2588,28 @@ static void setup_msrs(struct vcpu_vmx *vmx) > if (is_long_mode(&vmx->vcpu)) { > index = __find_msr_index(vmx, MSR_SYSCALL_MASK); > if (index >= 0) > - move_msr_up(vmx, index, save_nmsrs++); > + swap(vmx->guest_msrs[index], vmx->guest_msrs[save_nmsrs++]); > index = __find_msr_index(vmx, MSR_LSTAR); > if (index >= 0) > - move_msr_up(vmx, index, save_nmsrs++); > + swap(vmx->guest_msrs[index], vmx->guest_msrs[save_nmsrs++]); > index = __find_msr_index(vmx, MSR_CSTAR); > if (index >= 0) > - move_msr_up(vmx, index, save_nmsrs++); > + swap(vmx->guest_msrs[index], vmx->guest_msrs[save_nmsrs++]); > index = __find_msr_index(vmx, MSR_TSC_AUX); > if (index >= 0 && guest_cpuid_has(&vmx->vcpu, X86_FEATURE_RDTSCP)) > - move_msr_up(vmx, index, save_nmsrs++); > + swap(vmx->guest_msrs[index], vmx->guest_msrs[save_nmsrs++]); > /* > * MSR_STAR is only needed on long mode guests, and only > * if efer.sce is enabled. > */ > index = __find_msr_index(vmx, MSR_STAR); > if ((index >= 0) && (vmx->vcpu.arch.efer & EFER_SCE)) > - move_msr_up(vmx, index, save_nmsrs++); > + swap(vmx->guest_msrs[index], vmx->guest_msrs[save_nmsrs++]); > } > #endif > index = __find_msr_index(vmx, MSR_EFER); > if (index >= 0 && update_transition_efer(vmx, index)) > - move_msr_up(vmx, index, save_nmsrs++); > + swap(vmx->guest_msrs[index], vmx->guest_msrs[save_nmsrs++]); > > vmx->save_nmsrs = save_nmsrs; > > -- > 2.7.4 > >