Subject: Re: [PATCH 5/6 v2] kvm: powerpc: booke: Add linux pte lookup like booke3s

From: Paul Mackerras <[email-masked]>
Date: Wed, 7 Aug 2013 11:47:54 +1000

On Tue, Aug 06, 2013 at 08:11:34PM -0500, Scott Wood wrote:
> On Wed, 2013-08-07 at 10:24 +1000, Paul Mackerras wrote:
> > On Tue, Aug 06, 2013 at 07:02:48AM +0000, Bhushan Bharat-R65777 wrote:
> > > 
> > > I am trying to me the Linux pte search and update generic so that this can be used for powerpc as well.
> > > 
> > > I am not sure which of the below two should be ok, please help
> > 
> > Given that the BookE code uses gfn_to_pfn_memslot() to get the host
> > pfn, and then kvm_set_pfn_dirty(pfn) on pages that you're going to let
> > the guest write to, I don't think you need to set the dirty and/or
> > accessed bits in the Linux PTE explicitly.  If you care about the
> > WIMGE bits you can do find_linux_pte_or_hugepte() and just look at the
> > PTE, but you really should be using mmu_notifier_retry() to guard
> > against concurrent changes to the Linux PTE.  See the HV KVM code or
> > patch 21 of my recent series to see how it's used. 
> 
> Hmm... we only get a callback on invalidate_range_start(), not
> invalidate_range_end() (and even if we did get a callback for the
> latter, it'd probably be racy).  So we may have a problem here
> regardless of getting WIMG from the PTE, unless it's guaranteed that
> hva_to_pfn() will fail after invalidate_range_start().

No, it's not guaranteed.  You have to use mmu_notifier_retry().  It
tells you if either (a) some sort of invalidation has happened since
you snapshotted kvm->mmu_notifier_seq, or (b) an
invalidate_range_start...end sequence is currently in progress.  In
either case you should discard any PTE or pfn information you
collected and retry.

> >  You probably should be calling kvm_set_pfn_accessed() as well.
> 
> Yeah...  I think it'll only affect the quality of page-out decisions (as
> opposed to corruption and such), but still it should be fixed.

Right.

Paul.