DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 0E425C059744
From: Bandan Das <bsd@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, david@redhat.com
Subject: Re: [PATCH 4/6] kvm: nVMX: support EPT accessed/dirty bits
References: <1490867732-16743-1-git-send-email-pbonzini@redhat.com>
        <1490867732-16743-5-git-send-email-pbonzini@redhat.com>
        <jpgefwyldqj.fsf@linux.bootlegged.copy>
        <317734077.12913155.1491954869446.JavaMail.zimbra@redhat.com>
Date: Wed, 12 Apr 2017 19:02:50 -0400
Message-ID: <jpg8tn5cjqd.fsf@linux.bootlegged.copy>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2376
Lines: 64

Paolo Bonzini <pbonzini@redhat.com> writes:

> ----- Original Message -----
>> From: "Bandan Das" <bsd@redhat.com>
>> To: "Paolo Bonzini" <pbonzini@redhat.com>
>> Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, david@redhat.com
>> Sent: Wednesday, April 12, 2017 7:35:16 AM
>> Subject: Re: [PATCH 4/6] kvm: nVMX: support EPT accessed/dirty bits
>> 
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>> ...
>> >  	accessed_dirty = have_ad ? PT_GUEST_ACCESSED_MASK : 0;
>> > +
>> > +	/*
>> > +	 * FIXME: on Intel processors, loads of the PDPTE registers for PAE
>> > paging
>> > +	 * by the MOV to CR instruction are treated as reads and do not cause the
>> > +	 * processor to set the dirty flag in tany EPT paging-structure entry.
>> > +	 */
>> 
>> Minor typo: "in any EPT paging-structure entry".
>> 
>> > +	nested_access = (have_ad ? PFERR_WRITE_MASK : 0) | PFERR_USER_MASK;
>> > +
>> >  	pt_access = pte_access = ACC_ALL;
>> >  	++walker->level;
>> >  
>> > @@ -338,7 +337,7 @@ static int FNAME(walk_addr_generic)(struct guest_walker
>> > *walker,
>> >  		walker->pte_gpa[walker->level - 1] = pte_gpa;
>> >  
>> >  		real_gfn = mmu->translate_gpa(vcpu, gfn_to_gpa(table_gfn),
>> > -					      PFERR_USER_MASK|PFERR_WRITE_MASK,
>> > +					      nested_access,
>> >  					      &walker->fault);
>> 
>> I can't seem to understand the significance of this change (or for that
>> matter what was before this change).
>> 
>> mmu->translate_gpa() just returns gfn_to_gpa(table_gfn), right ?
>
> For EPT it is, you're right it's fishy.  The "nested_access" should be
> computed in translate_nested_gpa, which is where kvm->arch.nested_mmu
> (non-EPT) requests to access kvm->arch.mmu (EPT).

Thanks for the clarification. Is it the case when L1 runs L2 without
EPT ? I can't figure out the case where translate_nested_gpa will actually
be called. FNAME(walk_addr_nested) calls walk_addr_generic
with &vcpu->arch.nested_mmu and init_kvm_nested_mmu() sets gva_to_gpa()
with the appropriate "_nested" functions. But the gva_to_gpa() pointers
don't seem to get invoked at all for the nested case.

BTW, just noticed that setting PFERR_USER_MASK is redundant since
translate_nested_gpa does it too.

Bandan

> In practice we need to define a new function
> vcpu->arch.mmu.gva_to_gpa_nested that computes the nested_access
> and calls cpu->arch.mmu.gva_to_gpa.
>
> Thanks,
>
> Paolo