On Mon, Jun 17, 2013 at 07:59:15PM +0800, Xiao Guangrong wrote: > 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? Agreed - points are clear. Patchset looks good. -- 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
