On Thu, 20 Jun 2013 23:29:22 +0200 Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote: > > @@ -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 As Xiao's comment says, this is intentional because when we delete a memory slot, we increase slots->generation twice: first for setting the invalid flag, then for actually commiting the slot. To avoid this double zap_all(), we need to adjust the generation to make it really wraparound when we hit: kvm_current_mmio_generation(kvm) == MMIO_MAX_GEN - 1 > > > + printk_ratelimited(KERN_INFO "kvm: zapping shadow pages for mmio generation wraparound\n"); > > kvm_mmu_invalidate_zap_all_pages(kvm); > > + } > > } BTW, I'm now thinking of another way to know this information. If it's possible to check slots->generation, plus current_mmio_gen if needed, on some events, administrators can know how many times such wraparounds happened before. They do not need to do tracing all the time for that, just hitting one event later makes it possible to know if the guest was doing something problematic before. I'm now looking for the best trace point for that. Probably somewhere in arch_commit_memory(). Takuya -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html