On 11/13/2012 07:12 AM, Marcelo Tosatti wrote:
> On Mon, Nov 05, 2012 at 08:10:08PM +0800, Xiao Guangrong wrote:
>> In order to detecting spte remapping, we can simply check whether the
>> spte has already been pointing to the pfn even if the spte is not the
>> last spte for middle spte is pointing to the kernel pfn which can not
>> be mapped to userspace
>>
>> Also, update slot and stat.lpages iff the spte is not remapped
>>
>> Signed-off-by: Xiao Guangrong <xiaoguangrong@xxxxxxxxxxxxxxxxxx>
>> ---
>> arch/x86/kvm/mmu.c | 40 +++++++++++++---------------------------
>> 1 files changed, 13 insertions(+), 27 deletions(-)
>>
>> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
>> index 692ebb1..4ea731e 100644
>> --- a/arch/x86/kvm/mmu.c
>> +++ b/arch/x86/kvm/mmu.c
>> @@ -2420,8 +2420,7 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
>> pfn_t pfn, bool speculative,
>> bool host_writable)
>> {
>> - int was_rmapped = 0;
>> - int rmap_count;
>> + bool was_rmapped = false;
>>
>> pgprintk("%s: spte %llx access %x write_fault %d"
>> " user_fault %d gfn %llx\n",
>> @@ -2429,25 +2428,13 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
>> write_fault, user_fault, gfn);
>>
>> if (is_rmap_spte(*sptep)) {
>> - /*
>> - * If we overwrite a PTE page pointer with a 2MB PMD, unlink
>> - * the parent of the now unreachable PTE.
>> - */
>> - if (level > PT_PAGE_TABLE_LEVEL &&
>> - !is_large_pte(*sptep)) {
>> - struct kvm_mmu_page *child;
>> - u64 pte = *sptep;
>> + if (pfn != spte_to_pfn(*sptep)) {
>> + struct kvm_mmu_page *sp = page_header(__pa(sptep));
>>
>> - child = page_header(pte & PT64_BASE_ADDR_MASK);
>> - drop_parent_pte(child, sptep);
>> - kvm_flush_remote_tlbs(vcpu->kvm);
>
> How come its safe to drop this case?
We use "if (pfn != spte_to_pfn(*sptep))" to simplify the thing.
There are two cases:
1) the sptep is not the last mapping.
under this case, sptep must point to a shadow page table, that means
spte_to_pfn(*sptep)) is used by KVM module, and 'pfn' is used by userspace.
so, 'if' condition must be satisfied, the sptep will be dropped.
Actually, This is the origin case:
| if (level > PT_PAGE_TABLE_LEVEL &&
| !is_large_pte(*sptep))"
2) the sptep is the last mapping.
under this case, the level of spte (sp.level) must equal the 'level' which
we pass to mmu_set_spte. If they point to the same pfn, it is 'remap', otherwise
we drop it.
I think this is safe. :)
--
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
