From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH] kvm: x86: vmx: remove MMIO_GEN_MASK Date: Tue, 18 Nov 2014 11:15:09 +0100 Message-ID: <546B1C2D.3030001@redhat.com> References: <1416302078-6324-1-git-send-email-tiejun.chen@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: Tiejun Chen Return-path: Received: from mx1.redhat.com ([209.132.183.28]:50572 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751974AbaKRKPP (ORCPT ); Tue, 18 Nov 2014 05:15:15 -0500 In-Reply-To: <1416302078-6324-1-git-send-email-tiejun.chen@intel.com> Sender: kvm-owner@vger.kernel.org List-ID: On 18/11/2014 10:14, Tiejun Chen wrote: > Actually MMIO_MAX_GEN is same as MMIO_GEN_MASK, and > especially in all cases we already AND this so its > also unnecessary to generate such a WARN_ON(). > > Signed-off-by: Tiejun Chen Good idea, but I actually prefer keeping MMIO_GEN_MASK since it is often used together with an "&". Then, the WARN_ON is better written as - WARN_ON(gen > MMIO_MAX_GEN); + WARN_ON(gen & ~MMIO_GEN_MASK); Paolo > --- > arch/x86/kvm/mmu.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c > index ac1c4de..7712345 100644 > --- a/arch/x86/kvm/mmu.c > +++ b/arch/x86/kvm/mmu.c > @@ -213,15 +213,12 @@ EXPORT_SYMBOL_GPL(kvm_mmu_set_mmio_spte_mask); > #define MMIO_GEN_SHIFT 20 > #define MMIO_GEN_LOW_SHIFT 10 > #define MMIO_GEN_LOW_MASK ((1 << MMIO_GEN_LOW_SHIFT) - 2) > -#define MMIO_GEN_MASK ((1 << MMIO_GEN_SHIFT) - 1) > #define MMIO_MAX_GEN ((1 << MMIO_GEN_SHIFT) - 1) > > static u64 generation_mmio_spte_mask(unsigned int gen) > { > u64 mask; > > - WARN_ON(gen > MMIO_MAX_GEN); > - > mask = (gen & MMIO_GEN_LOW_MASK) << MMIO_SPTE_GEN_LOW_SHIFT; > mask |= ((u64)gen >> MMIO_GEN_LOW_SHIFT) << MMIO_SPTE_GEN_HIGH_SHIFT; > return mask; > @@ -240,7 +237,7 @@ static unsigned int get_mmio_spte_generation(u64 spte) > > static unsigned int kvm_current_mmio_generation(struct kvm *kvm) > { > - return kvm_memslots(kvm)->generation & MMIO_GEN_MASK; > + return kvm_memslots(kvm)->generation & MMIO_MAX_GEN; > } > > static void mark_mmio_spte(struct kvm *kvm, u64 *sptep, u64 gfn, >