On Mon, Aug 4, 2014 at 8:36 PM, Xiao Guangrong
<xiaoguangrong@xxxxxxxxxxxxxxxxxx> wrote:
> On 08/05/2014 05:10 AM, David Matlack wrote:
>>
>> This patch fixes the issue by doing the following:
>> - Tag the mmio cache with the memslot generation and use it to
>> validate mmio cache lookups.
>> - Extend vcpu_clear_mmio_info to clear mmio_gfn in addition to
>> mmio_gva, since both can be used to fast path mmio faults.
>> - In mmu_sync_roots, unconditionally clear the mmio cache since
>> even direct_map (e.g. tdp) hosts use it.
>
> It's not needed.
>
> direct map only uses gpa (and never cache gva) and
> vcpu_clear_mmio_info only clears gva.
Ok thanks for the clarification.
>> +static inline void vcpu_cache_mmio_info(struct kvm_vcpu *vcpu,
>> + gva_t gva, gfn_t gfn, unsigned access)
>> +{
>> + vcpu->arch.mmio_gen = kvm_current_mmio_generation(vcpu->kvm);
>> +
>> + /*
>> + * Ensure that the mmio_gen is set before the rest of the cache entry.
>> + * Otherwise we might see a new generation number attached to an old
>> + * cache entry if creating/deleting a memslot races with mmio caching.
>> + * The inverse case is possible (old generation number with new cache
>> + * info), but that is safe. The next access will just miss the cache
>> + * when it should have hit.
>> + */
>> + smp_wmb();
>
> 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?
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.
--
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
