All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Mackerras <paulus@samba.org>
To: Scott Wood <scottwood@freescale.com>
Cc: Bhushan Bharat-R65777 <R65777@freescale.com>,
	Wood Scott-B07421 <B07421@freescale.com>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	"agraf@suse.de" <agraf@suse.de>,
	"kvm-ppc@vger.kernel.org" <kvm-ppc@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH 5/6 v2] kvm: powerpc: booke: Add linux pte lookup like booke3s
Date: Wed, 07 Aug 2013 01:47:54 +0000	[thread overview]
Message-ID: <20130807014754.GA31007@iris.ozlabs.ibm.com> (raw)
In-Reply-To: <1375837894.5600.64.camel@snotra.buserror.net>

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.

WARNING: multiple messages have this Message-ID (diff)
From: Paul Mackerras <paulus@samba.org>
To: Scott Wood <scottwood@freescale.com>
Cc: Wood Scott-B07421 <B07421@freescale.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"agraf@suse.de" <agraf@suse.de>,
	"kvm-ppc@vger.kernel.org" <kvm-ppc@vger.kernel.org>,
	Bhushan Bharat-R65777 <R65777@freescale.com>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH 5/6 v2] kvm: powerpc: booke: Add linux pte lookup like booke3s
Date: Wed, 7 Aug 2013 11:47:54 +1000	[thread overview]
Message-ID: <20130807014754.GA31007@iris.ozlabs.ibm.com> (raw)
In-Reply-To: <1375837894.5600.64.camel@snotra.buserror.net>

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.

WARNING: multiple messages have this Message-ID (diff)
From: Paul Mackerras <paulus@samba.org>
To: Scott Wood <scottwood@freescale.com>
Cc: Bhushan Bharat-R65777 <R65777@freescale.com>,
	Wood Scott-B07421 <B07421@freescale.com>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	"agraf@suse.de" <agraf@suse.de>,
	"kvm-ppc@vger.kernel.org" <kvm-ppc@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH 5/6 v2] kvm: powerpc: booke: Add linux pte lookup like booke3s
Date: Wed, 7 Aug 2013 11:47:54 +1000	[thread overview]
Message-ID: <20130807014754.GA31007@iris.ozlabs.ibm.com> (raw)
In-Reply-To: <1375837894.5600.64.camel@snotra.buserror.net>

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.

  reply	other threads:[~2013-08-07  1:47 UTC|newest]

Thread overview: 80+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-01 11:12 [PATCH 0/6 v2] kvm: powerpc: use cache attributes from linux pte Bharat Bhushan
2013-08-01 11:24 ` Bharat Bhushan
2013-08-01 11:12 ` [PATCH 1/6 v2] powerpc: book3e: _PAGE_LENDIAN must be _PAGE_ENDIAN Bharat Bhushan
2013-08-01 11:24   ` Bharat Bhushan
2013-08-01 11:12   ` Bharat Bhushan
2013-08-01 11:12 ` [PATCH 2/6 v2] kvm: powerpc: allow guest control "E" attribute in mas2 Bharat Bhushan
2013-08-01 11:24   ` Bharat Bhushan
2013-08-01 11:12   ` Bharat Bhushan
2013-08-01 11:12 ` [PATCH 3/6 v2] kvm: powerpc: allow guest control "G" " Bharat Bhushan
2013-08-01 11:24   ` Bharat Bhushan
2013-08-01 11:12   ` Bharat Bhushan
2013-08-02  6:39   ` "“tiejun.chen”"
2013-08-02  6:39     ` "“tiejun.chen”"
2013-08-02  6:39     ` "“tiejun.chen”"
2013-08-01 11:12 ` [PATCH 4/6 v2] powerpc: move linux pte/hugepte search to more generic file Bharat Bhushan
2013-08-01 11:24   ` Bharat Bhushan
2013-08-01 11:12   ` Bharat Bhushan
2013-08-01 11:12 ` [PATCH 5/6 v2] kvm: powerpc: booke: Add linux pte lookup like booke3s Bharat Bhushan
2013-08-01 11:24   ` Bharat Bhushan
2013-08-01 11:12   ` Bharat Bhushan
2013-08-02  6:37   ` "“tiejun.chen”"
2013-08-02  6:37     ` "“tiejun.chen”"
2013-08-02  6:37     ` "“tiejun.chen”"
2013-08-02 22:58   ` Scott Wood
2013-08-02 22:58     ` Scott Wood
2013-08-02 22:58     ` Scott Wood
2013-08-02 23:16     ` Benjamin Herrenschmidt
2013-08-02 23:16       ` Benjamin Herrenschmidt
2013-08-02 23:16       ` Benjamin Herrenschmidt
2013-08-03  2:58       ` Bhushan Bharat-R65777
2013-08-03  2:58         ` Bhushan Bharat-R65777
2013-08-03  2:58         ` Bhushan Bharat-R65777
2013-08-03  4:24         ` Benjamin Herrenschmidt
2013-08-03  4:24           ` Benjamin Herrenschmidt
2013-08-03  4:24           ` Benjamin Herrenschmidt
2013-08-05 14:27           ` Bhushan Bharat-R65777
2013-08-05 14:27             ` Bhushan Bharat-R65777
2013-08-05 14:27             ` Bhushan Bharat-R65777
2013-08-05 19:19             ` Scott Wood
2013-08-05 19:19               ` Scott Wood
2013-08-05 19:19               ` Scott Wood
2013-08-06  1:12               ` Bhushan Bharat-R65777
2013-08-06  1:12                 ` Bhushan Bharat-R65777
2013-08-06  1:12                 ` Bhushan Bharat-R65777
2013-08-06  7:02               ` Bhushan Bharat-R65777
2013-08-06  7:02                 ` Bhushan Bharat-R65777
2013-08-06  7:02                 ` Bhushan Bharat-R65777
2013-08-07  0:24                 ` Paul Mackerras
2013-08-07  0:24                   ` Paul Mackerras
2013-08-07  0:24                   ` Paul Mackerras
2013-08-07  1:11                   ` Scott Wood
2013-08-07  1:11                     ` Scott Wood
2013-08-07  1:11                     ` Scott Wood
2013-08-07  1:47                     ` Paul Mackerras [this message]
2013-08-07  1:47                       ` Paul Mackerras
2013-08-07  1:47                       ` Paul Mackerras
2013-08-06 14:46               ` Bhushan Bharat-R65777
2013-08-06 14:46                 ` Bhushan Bharat-R65777
2013-08-06 14:46                 ` Bhushan Bharat-R65777
2013-08-01 11:12 ` [PATCH 6/6 v2] kvm: powerpc: use caching attributes as per linux pte Bharat Bhushan
2013-08-01 11:24   ` Bharat Bhushan
2013-08-01 11:12   ` Bharat Bhushan
2013-08-02  6:24   ` "“tiejun.chen”"
2013-08-02  6:24     ` "“tiejun.chen”"
2013-08-02  6:24     ` "“tiejun.chen”"
2013-08-02 23:34   ` Scott Wood
2013-08-02 23:34     ` Scott Wood
2013-08-02 23:34     ` Scott Wood
2013-08-03  3:11     ` Bhushan Bharat-R65777
2013-08-03  3:11       ` Bhushan Bharat-R65777
2013-08-03  3:11       ` Bhushan Bharat-R65777
2013-08-03  4:25       ` Benjamin Herrenschmidt
2013-08-03  4:25         ` Benjamin Herrenschmidt
2013-08-03  4:25         ` Benjamin Herrenschmidt
2013-08-05 16:28         ` Scott Wood
2013-08-05 16:28           ` Scott Wood
2013-08-05 16:28           ` Scott Wood
2013-08-05 16:30       ` Scott Wood
2013-08-05 16:30         ` Scott Wood
2013-08-05 16:30         ` Scott Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130807014754.GA31007@iris.ozlabs.ibm.com \
    --to=paulus@samba.org \
    --cc=B07421@freescale.com \
    --cc=R65777@freescale.com \
    --cc=agraf@suse.de \
    --cc=benh@kernel.crashing.org \
    --cc=kvm-ppc@vger.kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=scottwood@freescale.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.