Re: [PATCH 4/5] KVM: PPC: Book3S HV: Don't give the guest RW access to RO pages

From: Alexander Graf <agraf@suse.de>
To: Paul Mackerras <paulus@samba.org>
Cc: "kvm-ppc@vger.kernel.org" <kvm-ppc@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>
Subject: Re: [PATCH 4/5] KVM: PPC: Book3S HV: Don't give the guest RW access to RO pages
Date: Sat, 24 Nov 2012 10:05:37 +0100	[thread overview]
Message-ID: <E3BDC278-EA1D-4B28-BA91-A2676BFA7232@suse.de> (raw)
In-Reply-To: <20121123221342.GC23537@bloggs.ozlabs.ibm.com>

On 23.11.2012, at 23:13, Paul Mackerras <paulus@samba.org> wrote:

> On Fri, Nov 23, 2012 at 04:47:45PM +0100, Alexander Graf wrote:
>> 
>> On 22.11.2012, at 10:28, Paul Mackerras wrote:
>> 
>>> Currently, if the guest does an H_PROTECT hcall requesting that the
>>> permissions on a HPT entry be changed to allow writing, we make the
>>> requested change even if the page is marked read-only in the host
>>> Linux page tables.  This is a problem since it would for instance
>>> allow a guest to modify a page that KSM has decided can be shared
>>> between multiple guests.
>>> 
>>> To fix this, if the new permissions for the page allow writing, we need
>>> to look up the memslot for the page, work out the host virtual address,
>>> and look up the Linux page tables to get the PTE for the page.  If that
>>> PTE is read-only, we reduce the HPTE permissions to read-only.
>> 
>> How does KSM handle this usually? If you reduce the permissions to R/O, how do you ever get a R/W page from a deduplicated one?
> 
> The scenario goes something like this:
> 
> 1. Guest creates an HPTE with RO permissions.
> 2. KSM decides the page is identical to another page and changes the
>   HPTE to point to a shared copy.  Permissions are still RO.
> 3. Guest decides it wants write access to the page and does an
>   H_PROTECT hcall to change the permissions on the HPTE to RW.
> 
> The bug is that we actually make the requested change in step 3.
> Instead we should leave it at RO, then when the guest tries to write
> to the page, we take a hypervisor page fault, copy the page and give
> the guest write access to its own copy of the page.
> 
> So what this patch does is add code to H_PROTECT so that if the guest
> is requesting RW access, we check the Linux PTE to see if the
> underlying guest page is RO, and if so reduce the permissions in the
> HPTE to RO.

But this will be guest visible, because now H_PROTECT doesn't actually mark the page R/W in the HTAB, right?

So the flow with this patch is:

  - guest page permission fault
  - guest does H_PROTECT to mark page r/w
  - H_PROTECT doesn't do anything
  - guest returns from permission handler, triggers write fault

2 questions here:

How does the host know that the page is actually r/w?

How does this work on 970? I thought page faults always go straight to the guest there.

Alex