Re: [PATCH v2] KVM: MMU: Inform users of mmio generation wraparound

[Paolo Bonzini <pbonzini@redhat.com>]

Il 20/06/2013 18:34, Takuya Yoshikawa ha scritto:
> From: Takuya Yoshikawa <yoshikawa_takuya_b1@lab.ntt.co.jp>
> 
> Without this information, users will just see unexpected performance
> problems and there is little chance we will get good reports from them:
> note that mmio generation is increased even when we just start, or stop,
> dirty logging for some memory slot, in which case users cannot expect
> all shadow pages to be zapped.
> 
> printk_ratelimited() is used for this taking into account the problems
> that we can see the information many times when we start multiple VMs
> and guests can trigger this by reading ROM in a loop for example.
> 
> Signed-off-by: Takuya Yoshikawa <yoshikawa_takuya_b1@lab.ntt.co.jp>
> ---
>  Interestingly, I saw this information printed twice every time.
>  Looks like current_mmio_gen can become mmio_max_gen...
>  arch/x86/kvm/mmu.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index c60c5da..54e3968 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -4385,8 +4385,10 @@ void kvm_mmu_invalidate_mmio_sptes(struct kvm *kvm)
>  	 * The max value is MMIO_MAX_GEN - 1 since it is not called
>  	 * when mark memslot invalid.
>  	 */
> -	if (unlikely(kvm_current_mmio_generation(kvm) >= (MMIO_MAX_GEN - 1)))
> +	if (unlikely(kvm_current_mmio_generation(kvm) >= (MMIO_MAX_GEN - 1))) {

There is an off-by-one here (that I can fix in a separate patch).  The
right test is >= MMIO_MAX_GEN, since we have:

#define MMIO_MAX_GEN                    ((1 << MMIO_GEN_SHIFT) - 1)

Paolo

> +		printk_ratelimited(KERN_INFO "kvm: zapping shadow pages for mmio generation wraparound\n");
>  		kvm_mmu_invalidate_zap_all_pages(kvm);
> +	}
>  }
>  
>  static int mmu_shrink(struct shrinker *shrink, struct shrink_control *sc)
>