Sorry for the delay reply since i was on vacation. On 06/15/2013 10:22 AM, Takuya Yoshikawa wrote: > On Thu, 13 Jun 2013 21:08:21 -0300 > Marcelo Tosatti <mtosatti@xxxxxxxxxx> wrote: > >> On Fri, Jun 07, 2013 at 04:51:22PM +0800, Xiao Guangrong wrote: > >> - Where is the generation number increased? > > Looks like when a new slot is installed in update_memslots() because > it's based on slots->generation. This is not restricted to "create" > and "move". Yes. It reuses slots->generation to avoid unnecessary synchronizations (RCU, memory barrier). Increasing mmio generation number in the case of "create" and "move" is ok - it is no addition work unless mmio generation number is overflow which is hardly triggered (since the valid mmio generation number is large enough and zap_all is scale well now.) and the mmio spte is updated only when it is used in the future. > >> - Should use spinlock breakable code in kvm_mmu_zap_mmio_sptes() >> (picture guest with 512GB of RAM, even walking all those pages is >> expensive) (ah, patch to remove kvm_mmu_zap_mmio_sptes does that). >> - Is -13 enough to test wraparound? Its highly likely the guest has >> not began executing by the time 13 kvm_set_memory_calls are made >> (so no sptes around). Perhaps -2000 is more sensible (should confirm >> though). > > In the future, after we've tested enough, we should change the testing > code to be executed only for some debugging configs. Especially, if we > change zap_mmio_sptes() to zap_all_shadows(), very common guests, even > without huge memory like 512GB, can see the effect induced by sudden page > faults unnecessarily. > > If necessary, developers can test the wraparound code by lowering the > max_gen itself anyway. I agree. > >> - Why remove "if (change == KVM_MR_CREATE) || (change >> == KVM_MR_MOVE)" from kvm_arch_commit_memory_region? >> Its instructive. > > There may be a chance that we miss generation wraparounds if we don't > check other cases: seems unlikely, but theoretically possible. > > In short, all memory slot changes make mmio sptes stored in shadow pages > obsolete, or zapped for wraparounds, in the new way -- am I right? Yes. You are definitely right. :) Takuya-san, thank you very much for you answering the questions for me and thanks all of you for patiently reviewing my patches. Marcelo, your points? -- 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
