All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Graf <agraf@suse.de>
To: Paul Mackerras <paulus@samba.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	kvm-ppc@vger.kernel.org, kvm@vger.kernel.org
Subject: Re: [PATCH 20/23] KVM: PPC: Book3S PR: Better handling of host-side read-only pages
Date: Thu, 12 Sep 2013 18:01:37 -0500	[thread overview]
Message-ID: <A32A829C-D6AB-4BDA-A395-D59C963A6A5F@suse.de> (raw)
In-Reply-To: <20130806042706.GZ19254@iris.ozlabs.ibm.com>


On 05.08.2013, at 23:27, Paul Mackerras wrote:

> Currently we request write access to all pages that get mapped into the
> guest, even if the guest is only loading from the page.  This reduces
> the effectiveness of KSM because it means that we unshare every page we
> access.  Also, we always set the changed (C) bit in the guest HPTE if
> it allows writing, even for a guest load.
> 
> This fixes both these problems.  We pass an 'iswrite' flag to the
> mmu.xlate() functions and to kvmppc_mmu_map_page() to indicate whether
> the access is a load or a store.  The mmu.xlate() functions now only
> set C for stores.  kvmppc_gfn_to_pfn() now calls gfn_to_pfn_prot()
> instead of gfn_to_pfn() so that it can indicate whether we need write
> access to the page, and get back a 'writable' flag to indicate whether
> the page is writable or not.  If that 'writable' flag is clear, we then
> make the host HPTE read-only even if the guest HPTE allowed writing.
> 
> This means that we can get a protection fault when the guest writes to a
> page that it has mapped read-write but which is read-only on the host
> side (perhaps due to KSM having merged the page).  Thus we now call
> kvmppc_handle_pagefault() for protection faults as well as HPTE not found
> faults.  In kvmppc_handle_pagefault(), if the access was allowed by the
> guest HPTE and we thus need to install a new host HPTE, we then need to
> remove the old host HPTE if there is one.  This is done with a new
> function, kvmppc_mmu_unmap_page(), which uses kvmppc_mmu_pte_vflush() to
> find and remove the old host HPTE.

Have you measured how much performance we lose by mapping it twice? Usually Linux will mark user pages that are not written to yet as non-writable, no? That's why I assumed that "may_write" is the same as "guest wants to write" back when I wrote this.

I'm also afraid that a sequence like

  ld x,y
  std x,y

in the kernel will trap twice and slow us down heavily. But maybe I'm just being paranoid. Can you please measure bootup time with and without this, as well as a fork bomb (spawn /bin/echo 1000 times and time it) with and without so we get a feeling for its impact?


Thanks a lot!

Alex

WARNING: multiple messages have this Message-ID (diff)
From: Alexander Graf <agraf@suse.de>
To: Paul Mackerras <paulus@samba.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	kvm-ppc@vger.kernel.org, kvm@vger.kernel.org
Subject: Re: [PATCH 20/23] KVM: PPC: Book3S PR: Better handling of host-side read-only pages
Date: Thu, 12 Sep 2013 23:01:37 +0000	[thread overview]
Message-ID: <A32A829C-D6AB-4BDA-A395-D59C963A6A5F@suse.de> (raw)
In-Reply-To: <20130806042706.GZ19254@iris.ozlabs.ibm.com>


On 05.08.2013, at 23:27, Paul Mackerras wrote:

> Currently we request write access to all pages that get mapped into the
> guest, even if the guest is only loading from the page.  This reduces
> the effectiveness of KSM because it means that we unshare every page we
> access.  Also, we always set the changed (C) bit in the guest HPTE if
> it allows writing, even for a guest load.
> 
> This fixes both these problems.  We pass an 'iswrite' flag to the
> mmu.xlate() functions and to kvmppc_mmu_map_page() to indicate whether
> the access is a load or a store.  The mmu.xlate() functions now only
> set C for stores.  kvmppc_gfn_to_pfn() now calls gfn_to_pfn_prot()
> instead of gfn_to_pfn() so that it can indicate whether we need write
> access to the page, and get back a 'writable' flag to indicate whether
> the page is writable or not.  If that 'writable' flag is clear, we then
> make the host HPTE read-only even if the guest HPTE allowed writing.
> 
> This means that we can get a protection fault when the guest writes to a
> page that it has mapped read-write but which is read-only on the host
> side (perhaps due to KSM having merged the page).  Thus we now call
> kvmppc_handle_pagefault() for protection faults as well as HPTE not found
> faults.  In kvmppc_handle_pagefault(), if the access was allowed by the
> guest HPTE and we thus need to install a new host HPTE, we then need to
> remove the old host HPTE if there is one.  This is done with a new
> function, kvmppc_mmu_unmap_page(), which uses kvmppc_mmu_pte_vflush() to
> find and remove the old host HPTE.

Have you measured how much performance we lose by mapping it twice? Usually Linux will mark user pages that are not written to yet as non-writable, no? That's why I assumed that "may_write" is the same as "guest wants to write" back when I wrote this.

I'm also afraid that a sequence like

  ld x,y
  std x,y

in the kernel will trap twice and slow us down heavily. But maybe I'm just being paranoid. Can you please measure bootup time with and without this, as well as a fork bomb (spawn /bin/echo 1000 times and time it) with and without so we get a feeling for its impact?


Thanks a lot!

Alex


  reply	other threads:[~2013-09-12 23:01 UTC|newest]

Thread overview: 136+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-06  4:12 [PATCH 00/23] Allow PR and HV KVM to coexist in one kernel Paul Mackerras
2013-08-06  4:12 ` Paul Mackerras
2013-08-06  4:13 ` [PATCH 01/23] KVM: PPC: Book3S: Fix compile error in XICS emulation Paul Mackerras
2013-08-06  4:13   ` Paul Mackerras
2013-08-28 22:51   ` Alexander Graf
2013-08-28 22:51     ` Alexander Graf
2013-08-06  4:14 ` [PATCH 02/23] KVM: PPC: Book3S PR: Don't corrupt guest state when kernel uses VMX Paul Mackerras
2013-08-06  4:14   ` Paul Mackerras
2013-08-08 15:49   ` Aneesh Kumar K.V
2013-08-08 15:49     ` Aneesh Kumar K.V
2013-08-28 22:51   ` Alexander Graf
2013-08-28 22:51     ` Alexander Graf
2013-08-06  4:15 ` [PATCH 03/23] KVM: PPC: Book3S PR: Make instruction fetch fallback work for system calls Paul Mackerras
2013-08-06  4:15   ` Paul Mackerras
2013-08-28 22:51   ` Alexander Graf
2013-08-28 22:51     ` Alexander Graf
2013-08-06  4:16 ` [PATCH 04/23] KVM: PPC: Book3S PR: Keep volatile reg values in vcpu rather than shadow_vcpu Paul Mackerras
2013-08-06  4:16   ` Paul Mackerras
2013-08-11 11:06   ` Aneesh Kumar K.V
2013-08-11 11:18     ` Aneesh Kumar K.V
2013-08-28 22:00   ` Alexander Graf
2013-08-28 22:00     ` Alexander Graf
2013-08-29  5:04     ` Paul Mackerras
2013-08-29  5:04       ` Paul Mackerras
2013-08-29 12:46       ` Alexander Graf
2013-08-29 12:46         ` Alexander Graf
2013-08-06  4:18 ` [PATCH 05/23] KVM: PPC: Book3S PR: Rework kvmppc_mmu_book3s_64_xlate() Paul Mackerras
2013-08-06  4:18   ` Paul Mackerras
2013-08-28 22:51   ` Alexander Graf
2013-08-28 22:51     ` Alexander Graf
2013-08-06  4:18 ` [PATCH 06/23] KVM: PPC: Book3S PR: Allow guest to use 64k pages Paul Mackerras
2013-08-06  4:18   ` Paul Mackerras
2013-08-28 22:56   ` Alexander Graf
2013-08-28 22:56     ` Alexander Graf
2013-08-29  5:17     ` Paul Mackerras
2013-08-29  5:17       ` Paul Mackerras
2013-08-29 12:48       ` Alexander Graf
2013-08-29 12:48         ` Alexander Graf
2013-08-06  4:19 ` [PATCH 07/23] KVM: PPC: Book3S PR: Use 64k host pages where possible Paul Mackerras
2013-08-06  4:19   ` Paul Mackerras
2013-08-28 23:24   ` Alexander Graf
2013-08-28 23:24     ` Alexander Graf
2013-08-29  5:23     ` Paul Mackerras
2013-08-29  5:23       ` Paul Mackerras
2013-08-29 12:43       ` Alexander Graf
2013-08-29 12:43         ` Alexander Graf
2013-08-06  4:20 ` [PATCH 08/23] KVM: PPC: Book3S PR: Handle PP0 page-protection bit in guest HPTEs Paul Mackerras
2013-08-06  4:20   ` Paul Mackerras
2013-08-06  4:20 ` [PATCH 09/23] KVM: PPC: Book3S PR: Correct errors in H_ENTER implementation Paul Mackerras
2013-08-06  4:20   ` Paul Mackerras
2013-08-06  4:21 ` [PATCH 10/23] KVM: PPC: Book3S PR: Make HPT accesses and updates SMP-safe Paul Mackerras
2013-08-06  4:21   ` Paul Mackerras
2013-08-06  4:21 ` [PATCH 11/23] KVM: PPC: Book3S PR: Allocate kvm_vcpu structs from kvm_vcpu_cache Paul Mackerras
2013-08-06  4:21   ` Paul Mackerras
2013-08-12 10:03   ` Aneesh Kumar K.V
2013-08-12 10:15     ` Aneesh Kumar K.V
2013-08-06  4:22 ` [PATCH 12/23] KVM: PPC: Book3S HV: Better handling of exceptions that happen in real mode Paul Mackerras
2013-08-06  4:22   ` Paul Mackerras
2013-08-06  4:22 ` [PATCH 13/23] KVM: PPC: Book3S: Move skip-interrupt handlers to common code Paul Mackerras
2013-08-06  4:22   ` Paul Mackerras
2013-08-06  4:23 ` [PATCH 14/23] KVM: PPC: Book3S PR: Delay disabling relocation-on interrupts Paul Mackerras
2013-08-06  4:23   ` Paul Mackerras
2013-08-30 16:30   ` Alexander Graf
2013-08-30 16:30     ` Alexander Graf
2013-08-30 22:55     ` Paul Mackerras
2013-08-30 22:55       ` Paul Mackerras
2013-08-30 23:13       ` Alexander Graf
2013-08-30 23:13         ` Alexander Graf
2013-08-31  5:42         ` Paul Mackerras
2013-08-31  5:42           ` Paul Mackerras
2013-08-06  4:24 ` [PATCH 15/23] KVM: PPC: Book3S: Rename symbols that exist in both PR and HV KVM Paul Mackerras
2013-08-06  4:24   ` Paul Mackerras
2013-08-06  4:24 ` [PATCH 16/23] KVM: PPC: Book3S: Merge implementations of KVM_PPC_GET_SMMU_INFO ioctl Paul Mackerras
2013-08-06  4:24   ` Paul Mackerras
2013-08-06  4:25 ` [PATCH 17/23] KVM: PPC: Book3S HV: Factorize kvmppc_core_vcpu_create_hv() Paul Mackerras
2013-08-06  4:25   ` Paul Mackerras
2013-08-06  4:25 ` [PATCH 18/23] KVM: PPC: Book3S: Allow both PR and HV KVM to be selected Paul Mackerras
2013-08-06  4:25   ` Paul Mackerras
2013-08-06  4:26 ` [PATCH 19/23] KVM: PPC: Book3S: Select PR vs HV separately for each guest Paul Mackerras
2013-08-06  4:26   ` Paul Mackerras
2013-09-12 22:56   ` Alexander Graf
2013-09-12 22:56     ` Alexander Graf
2013-09-13  0:17     ` Paul Mackerras
2013-09-13  0:17       ` Paul Mackerras
2013-09-13  1:31       ` Benjamin Herrenschmidt
2013-09-13  1:31         ` Benjamin Herrenschmidt
2013-09-13  4:18         ` Alexander Graf
2013-09-13  4:18           ` Alexander Graf
2013-09-14 18:33         ` Aneesh Kumar K.V
2013-09-14 18:45           ` Aneesh Kumar K.V
2013-09-14 20:22           ` Alexander Graf
2013-09-14 20:22             ` Alexander Graf
2013-09-15  9:16             ` Aneesh Kumar K.V
2013-09-15  9:28               ` Aneesh Kumar K.V
2013-09-15 11:55               ` Alexander Graf
2013-09-15 11:55                 ` Alexander Graf
2013-09-13  4:17       ` Alexander Graf
2013-09-13  4:17         ` Alexander Graf
2013-09-18 12:05         ` Paul Mackerras
2013-09-18 12:05           ` Paul Mackerras
2013-09-19  7:31           ` Alexander Graf
2013-09-19  7:31             ` Alexander Graf
2013-08-06  4:27 ` [PATCH 20/23] KVM: PPC: Book3S PR: Better handling of host-side read-only pages Paul Mackerras
2013-08-06  4:27   ` Paul Mackerras
2013-09-12 23:01   ` Alexander Graf [this message]
2013-09-12 23:01     ` Alexander Graf
2013-09-13  0:23     ` Paul Mackerras
2013-09-13  0:23       ` Paul Mackerras
2013-09-14  5:24     ` Paul Mackerras
2013-09-14  5:24       ` Paul Mackerras
2013-09-14 20:23       ` Alexander Graf
2013-09-14 20:23         ` Alexander Graf
2013-09-16  4:12         ` Paul Mackerras
2013-09-16  4:12           ` Paul Mackerras
2013-09-16 12:47           ` Alexander Graf
2013-09-16 12:47             ` Alexander Graf
2013-08-06  4:27 ` [PATCH 21/23] KVM: PPC: Book3S PR: Use mmu_notifier_retry() in kvmppc_mmu_map_page() Paul Mackerras
2013-08-06  4:27   ` Paul Mackerras
2013-08-07  4:13   ` Bhushan Bharat-R65777
2013-08-07  4:13     ` Bhushan Bharat-R65777
2013-08-07  4:28     ` Paul Mackerras
2013-08-07  4:28       ` Paul Mackerras
2013-08-07  5:18       ` Bhushan Bharat-R65777
2013-08-07  5:18         ` Bhushan Bharat-R65777
2013-08-07  5:17   ` Bhushan Bharat-R65777
2013-08-07  5:17     ` Bhushan Bharat-R65777
2013-08-07  8:27     ` Paul Mackerras
2013-08-07  8:27       ` Paul Mackerras
2013-08-07  8:31       ` Bhushan Bharat-R65777
2013-08-07  8:31         ` Bhushan Bharat-R65777
2013-08-08 12:06         ` Paul Mackerras
2013-08-08 12:06           ` Paul Mackerras
2013-08-06  4:27 ` [PATCH 22/23] KVM: PPC: Book3S PR: Mark pages accessed, and dirty if being written Paul Mackerras
2013-08-06  4:27   ` Paul Mackerras
2013-08-06  4:28 ` [PATCH 23/23] KVM: PPC: Book3S PR: Reduce number of shadow PTEs invalidated by MMU notifiers Paul Mackerras
2013-08-06  4:28   ` Paul Mackerras

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=A32A829C-D6AB-4BDA-A395-D59C963A6A5F@suse.de \
    --to=agraf@suse.de \
    --cc=benh@kernel.crashing.org \
    --cc=kvm-ppc@vger.kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=paulus@samba.org \
    /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.