Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755877AbbEERRx (ORCPT ); Tue, 5 May 2015 13:17:53 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50371 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755023AbbEERRv (ORCPT ); Tue, 5 May 2015 13:17:51 -0400 Date: Tue, 5 May 2015 19:17:47 +0200 From: Radim =?utf-8?B?S3LEjW3DocWZ?= To: Paolo Bonzini Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, bsd@redhat.com, guangrong.xiao@linux.intel.com, Yang Zhang , wanpeng.li@linux.intel.com Subject: Re: [PATCH 12/13] KVM: x86: add KVM_MEM_X86_SMRAM memory slot flag Message-ID: <20150505171747.GB17198@potion.brq.redhat.com> References: <1430393772-27208-1-git-send-email-pbonzini@redhat.com> <1430393772-27208-13-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1430393772-27208-13-git-send-email-pbonzini@redhat.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2612 Lines: 58 2015-04-30 13:36+0200, Paolo Bonzini: > This adds an arch-specific memslot flag that hides slots unless the > VCPU is in system management mode. > > Some care is needed in order to limit the overhead of x86_gfn_to_memslot > when compared with gfn_to_memslot. Thankfully, we have __gfn_to_memslot > and search_memslots which are the same, so we can add some extra output > to search_memslots. The compiler will optimize it as dead code in > __gfn_to_memslot, and will use it to thread jumps in x86_gfn_to_memslot. > > Signed-off-by: Paolo Bonzini > --- > diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt > @@ -19,10 +19,23 @@ > > #include > #include > +#include "kvm_cache_regs.h" > > struct kvm_memory_slot *x86_gfn_to_memslot(struct kvm_vcpu *vcpu, gfn_t gfn) > { > - struct kvm_memory_slot *slot = gfn_to_memslot(vcpu->kvm, gfn); > + /* By using search_memslots directly the compiler can optimize away > + * the "if (found)" check below. > + * > + * It cannot do the same for gfn_to_memslot because it is not inlined, > + * and it also cannot do the same for __gfn_to_memslot because the > + * kernel is compiled with -fno-delete-null-pointer-checks. > + */ > + bool found; > + struct kvm_memslots *memslots = kvm_memslots(vcpu->kvm); > + struct kvm_memory_slot *slot = search_memslots(memslots, gfn, &found); > + > + if (found && unlikely(slot->flags & KVM_MEM_X86_SMRAM) && !is_smm(vcpu)) > + return NULL; Patch [10/13] made me sad and IIUIC, the line above is the only reason for it ... what about renaming and changing kvm_* memory function to vcpu_* and create bool kvm_arch_vcpu_can_access_slot(vcpu, slot) which could also be inline in arch/*/include/asm/kvm_host.h thanks to the way we build. We could be passing both kvm and vcpu in internal memslot operations and not checking if vcpu is NULL. This should allow all possible operations with little code duplication and the compiler could also optimize the case where vcpu is NULL. Another option is adding something like "vcpu kvm_arch_fake_vcpu(kvm)" for cases where the access doesn't have an associated vcpu, so it would always succeed. (Might not be generic enough.) I prefer everything to copy-pasting code, so I'll try to come up with more ideas if you don't like these :) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/