Re: [PATCH] KVM: x86: Avoid zapping mmio sptes twice for generation wraparound

[Gleb Natapov <gleb@redhat.com>]

On Wed, Jul 03, 2013 at 10:57:35AM +0200, Paolo Bonzini wrote:
> Il 03/07/2013 10:53, Gleb Natapov ha scritto:
> >>> > > Please wait a while. I can not understand it very clearly.
> >>> > > 
> >>> > > This conditional check will cause caching a overflow value into mmio spte.
> >>> > > The simple case is that kvm adds new slots for many times, the mmio-gen is easily
> >>> > > more than MMIO_MAX_GEN.
> >>> > > 
> >> > 
> >> > Actually, the double zapping can be avoided by moving kvm_mmu_invalidate_mmio_sptes to
> >> > the end of install_new_memslots().
> >> > 
> > Exactly. Why should we hide it in obscure functions?
> 
> Because kvm_mmu_invalidate_mmio_sptes is an x86-specific function, and
> we already have a good arch API to hook into __kvm_set_memory_region.
> 
>From this patch it can be seen that the API is not good enough.
Generation update API is missing.

> Another possible implementation is to cache the last memslot generation,
> check it in kvm_arch_prepare/commit_memory_region, and call
> kvm_mmu_invalidate_mmio_sptes if it changed.  This would avoid the need
> to keep "((change == KVM_MR_DELETE) || (change == KVM_MR_MOVE))" in sync
> between the two places.  But I don't think it is a substantial improvement.
> 
Why are you trying to overcome API shortcomings instead of fixing the API.
 
--
			Gleb.