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 <[email protected]>

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,
> 
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to