On Tue, Aug 5, 2014 at 8:26 PM, Xiao Guangrong <xiaoguangrong@xxxxxxxxxxxxxxxxxx> wrote: > On 08/06/2014 06:39 AM, David Matlack wrote: >> On Mon, Aug 4, 2014 at 8:36 PM, Xiao Guangrong >> <xiaoguangrong@xxxxxxxxxxxxxxxxxx> wrote: >>> The memory barrier can't help us, consider this scenario: >>> >>> CPU 0 CPU 1 >>> page-fault >>> see gpa is not mapped in memslot >>> >>> create new memslot containing gpa from Qemu >>> update the slots's generation number >>> cache mmio info >>> >>> !!! later when vcpu accesses gpa again >>> it will cause mmio-exit. >> >> Ah! Thanks for catching my mistake. >> >>> The easy way to fix this is that we update slots's generation-number >>> after synchronize_srcu_expedited when memslot is being updated that >>> ensures other sides can see the new generation-number only after >>> finishing update. >> >> It would be possible for a vcpu to see an inconsistent kvm_memslots struct >> (new set of slots with an old generation number). Is that not an issue? > > In this case, checking generation-number will fail, we will goto the slow path > to handle mmio access - that's very rare, so i think it's ok. > >> >> We could just use the generation number that is stored in the >> spte. The only downside (that I can see) is that handle_abnormal_pfn() >> calls vcpu_cache_mmio_info() but does not have access to the spte. >> So presumably we'd have to do a page table walk. > > The issue is not only in vcpu_cache_mmio_info but also in > mark_mmio_spte() where we may cache new generation-number and old memslots > info. This is now a separate bug from the one I originally reported :). For now, I will resend a 3rd version of my patch with the fixes you suggested (the memory barrier is useless and no need to change to mmu_sync_roots). -- 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
