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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <D15C0FFF-A437-46A9-AF3E-1E31BD938B8B@gmail.com>
Date:	Thu, 3 Oct 2013 14:46:03 +0800
From:	Xiao Guangrong <xiaoguangrong.eric@...il.com>
To:	Marcelo Tosatti <mtosatti@...hat.com>
Cc:	Xiao Guangrong <xiaoguangrong@...ux.vnet.ibm.com>, gleb@...hat.com,
	avi.kivity@...il.com, pbonzini@...hat.com,
	linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [PATCH v2 05/15] KVM: MMU: flush tlb out of mmu lock when write-protect the sptes


On Oct 1, 2013, at 7:05 AM, Marcelo Tosatti <mtosatti@...hat.com> 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@...ux.vnet.ibm.com>
>> ---
>> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/