Re: [PATCH v7 03/14] KVM: X86: Don't track dirty for KVM_SET_[TSS_ADDR|IDENTITY_MAP_ADDR]

[Peter Xu]

On Mon, Mar 23, 2020 at 09:55:51AM -0700, Sean Christopherson wrote:
> On Mon, Mar 23, 2020 at 12:26:17PM -0400, Peter Xu wrote:
> > On Mon, Mar 23, 2020 at 08:42:16AM -0700, Sean Christopherson wrote:
> > > > > Regarding the HVA, it's a bit confusing saying that it's guaranteed to be
> > > > > valid, and then contradicting that in the second clause.  Maybe something
> > > > > like this to explain the GPA->HVA is guaranteed to be valid, but the
> > > > > HVA->HPA is not.
> > > > >  
> > > > > /*
> > > > >  * before use.  Note, KVM internal memory slots are guaranteed to remain valid
> > > > >  * and unchanged until the VM is destroyed, i.e. the GPA->HVA translation will
> > > > >  * not change.  However, the HVA is a user address, i.e. its accessibility is
> > > > >  * not guaranteed, and must be accessed via __copy_{to,from}_user().
> > > > >  */
> > > > 
> > > > Sure I can switch to this, though note that I still think the GPA->HVA
> > > > is not guaranteed logically because the userspace can unmap any HVA it
> > > > wants..
> > > 
> > > You're conflating the GPA->HVA translation with the validity of the HVA,
> > > i.e. the HVA->HPA and/or HVA->VMA translation/association.  GPA->HVA is
> > > guaranteed because userspace doesn't have access to the memslot which
> > > defines that transation.
> > 
> > Yes I completely agree if you mean the pure mapping of GPA->HVA.
> > 
> > I think it's a matter of how to define the "valid" when you say
> > "guaranteed to remain valid", because I don't think the mapping is
> > still valid from the most strict sense if e.g. the backing HVA does
> > not exist any more for that GPA->HVA mapping, then the memslot won't
> > be anything useful.
> 
> Yes.  That's why my proposed comment is worded to state that the _memslot_
> will remain valid.  It deliberately avoids mentioning "valid HVA".

OK, I see the point.  I did re-read the two versions again, I agree
yours is better, which I'll replace with.  Thanks!

-- 
Peter Xu