Re: [PATCH v2 05/15] KVM: MMU: flush tlb out of mmu lock when write-protect the sptes

[Xiao Guangrong]

On Oct 1, 2013, at 7:05 AM, Marcelo Tosatti <mtosatti@xxxxxxxxxx> wrote:

> On Thu, Sep 05, 2013 at 06:29:08PM +0800, Xiao Guangrong wrote:
>> Now we can flush all the TLBs out of the mmu lock without TLB corruption when
>> write-proect the sptes, it is because:
>> - we have marked large sptes readonly instead of dropping them that means we
>>  just change the spte from writable to readonly so that we only need to care
>>  the case of changing spte from present to present (changing the spte from
>>  present to nonpresent will flush all the TLBs immediately), in other words,
>>  the only case we need to care is mmu_spte_update()
>> 
>> - in mmu_spte_update(), we have checked
>>  SPTE_HOST_WRITEABLE | PTE_MMU_WRITEABLE instead of PT_WRITABLE_MASK, that
>>  means it does not depend on PT_WRITABLE_MASK anymore
>> 
>> Signed-off-by: Xiao Guangrong <xiaoguangrong@xxxxxxxxxxxxxxxxxx>
>> ---
>> arch/x86/kvm/mmu.c | 18 ++++++++++++++----
>> arch/x86/kvm/x86.c |  9 +++++++--
>> 2 files changed, 21 insertions(+), 6 deletions(-)
>> 
>> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
>> index 7488229..a983570 100644
>> --- a/arch/x86/kvm/mmu.c
>> +++ b/arch/x86/kvm/mmu.c
>> @@ -4320,15 +4320,25 @@ void kvm_mmu_slot_remove_write_access(struct kvm *kvm, int slot)
>> 			if (*rmapp)
>> 				__rmap_write_protect(kvm, rmapp, false);
>> 
>> -			if (need_resched() || spin_needbreak(&kvm->mmu_lock)) {
>> -				kvm_flush_remote_tlbs(kvm);
>> +			if (need_resched() || spin_needbreak(&kvm->mmu_lock))
>> 				cond_resched_lock(&kvm->mmu_lock);
>> -			}
>> 		}
>> 	}
>> 
>> -	kvm_flush_remote_tlbs(kvm);
>> 	spin_unlock(&kvm->mmu_lock);
>> +
>> +	/*
>> +	 * We can flush all the TLBs out of the mmu lock without TLB
>> +	 * corruption since we just change the spte from writable to
>> +	 * readonly so that we only need to care the case of changing
>> +	 * spte from present to present (changing the spte from present
>> +	 * to nonpresent will flush all the TLBs immediately), in other
>> +	 * words, the only case we care is mmu_spte_update() where we
>> +	 * haved checked SPTE_HOST_WRITEABLE | SPTE_MMU_WRITEABLE
>> +	 * instead of PT_WRITABLE_MASK, that means it does not depend
>> +	 * on PT_WRITABLE_MASK anymore.
>> +	 */
>> +	kvm_flush_remote_tlbs(kvm);
>> }
> 
> What about need_remote_flush? 

It is safe because before calling need_remote_flush mmu_pte_write_new_pte is called to
update the spte which will finally call set_spte() where the tlb flush has been properly checked.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/