mark pages in p2m_ram_paging_out state read-only

mark pages in p2m_ram_paging_out state read-only

[Olaf Hering]

I was wondering why ept_p2m_type_to_flags() removes all permissions from
a gfn in state p2m_ram_paging_out. If the guest happens to read or
execute from that page while the pager writes that gfn to disk, wouldnt
it be enough to remove the write bit to prevent writes from the guest?
If the page is read-only the guest could continue to make progress until
the gfn is really evicted and the p2mt changes to p2m_ram_paged.

I havent actually tried the patch below, but is there any reason it
would break the guest?


diff -r ed809663f829 xen/arch/x86/mm/p2m-ept.c
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -75,7 +75,6 @@ static void ept_p2m_type_to_flags(ept_en
         case p2m_invalid:
         case p2m_mmio_dm:
         case p2m_populate_on_demand:
-        case p2m_ram_paging_out:
         case p2m_ram_paged:
         case p2m_ram_paging_in:
         case p2m_ram_paging_in_start:
@@ -92,6 +91,7 @@ static void ept_p2m_type_to_flags(ept_en
             break;
         case p2m_ram_logdirty:
         case p2m_ram_ro:
+        case p2m_ram_paging_out:
         case p2m_ram_shared:
             entry->r = entry->x = 1;
             entry->w = 0;

Re: mark pages in p2m_ram_paging_out state read-only

[Tim Deegan]

At 17:53 +0100 on 14 Nov (1321293182), Olaf Hering wrote:
> 
> I was wondering why ept_p2m_type_to_flags() removes all permissions from
> a gfn in state p2m_ram_paging_out. If the guest happens to read or
> execute from that page while the pager writes that gfn to disk, wouldnt
> it be enough to remove the write bit to prevent writes from the guest?
> If the page is read-only the guest could continue to make progress until
> the gfn is really evicted and the p2mt changes to p2m_ram_paged.
> 
> I havent actually tried the patch below, but is there any reason it
> would break the guest?

As long as we change the p2m type before scrubbing or freeing the page,
that seems like it shuold be fine. 

Tim.

Re: mark pages in p2m_ram_paging_out state read-only

[Andres Lagar-Cavilla]

> Date: Thu, 24 Nov 2011 11:38:02 +0000
> From: Tim Deegan <tim@xen.org>
> To: Olaf Hering <olaf@aepfle.de>
> Cc: xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] mark pages in p2m_ram_paging_out state
> 	read-only
> Message-ID: <20111124113802.GB77868@ocelot.phlegethon.org>
> Content-Type: text/plain; charset=iso-8859-1
>
> At 17:53 +0100 on 14 Nov (1321293182), Olaf Hering wrote:
>>
>> I was wondering why ept_p2m_type_to_flags() removes all permissions from
>> a gfn in state p2m_ram_paging_out. If the guest happens to read or
>> execute from that page while the pager writes that gfn to disk, wouldnt
>> it be enough to remove the write bit to prevent writes from the guest?
>> If the page is read-only the guest could continue to make progress until
>> the gfn is really evicted and the p2mt changes to p2m_ram_paged.
>>
>> I havent actually tried the patch below, but is there any reason it
>> would break the guest?
>
> As long as we change the p2m type before scrubbing or freeing the page,
> that seems like it shuold be fine.

Is this a good idea? If the guest is accessing the page, then paging out
should stop immediately. We're risking complex races for a tiny tiny gain.

Andres
>
> Tim.
>
>
>
> ------------------------------
>
> Message: 7
> Date: Thu, 24 Nov 2011 11:38:19 +0000
> From: Ian Jackson <Ian.Jackson@eu.citrix.com>
> To: Andres Lagar-Cavilla <andres@lagarcavilla.org>
> Cc: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>
> Subject: Re: [Xen-devel] [PATCH 00 of 14] MM Fixes
> Message-ID: <20174.11435.306730.583922@mariner.uk.xensource.com>
> Content-Type: text/plain; charset="us-ascii"
>
> Andres Lagar-Cavilla writes ("[Xen-devel] [PATCH 00 of 14] MM Fixes"):
>> This patch series includes a number of bug fixes, targetting
>> primarily the mm layer. Many of these fixes also lay groundwork
>> for future patches.
>
> Thanks.  Something seems to have eaten patches 12,13,14.
>
> Can you please confirm that you sent them, and tell me their
> messageids, and any information you can tell me about their
> transmission ?
>
> Ideally I would like log entries from the final hop mail server on
> your side showing the messages being handed over to the MX's for
> lists.xensource.com, but looking at the headers of your other messages
> they came through "homiemail-***.***.dreamhost.com" so that may
> involve talking to whoever they are.
>
> Thanks,
> Ian.
>
>
>
> ------------------------------
>
> Message: 8
> Date: Thu, 24 Nov 2011 11:58:16 +0000
> From: Tim Deegan <tim@xen.org>
> To: ???? <327801865@qq.com>
> Cc: Xen-devel <Xen-devel@lists.xensource.com>
> Subject: Re: [Xen-devel] HVM (hypercall_grant_table_op) Problem
> Message-ID: <20111124115816.GD77868@ocelot.phlegethon.org>
> Content-Type: text/plain; charset=iso-8859-1
>
> Hi,
>
> At 00:52 +0800 on 18 Nov (1321577533), ???? wrote:
>> Hi:
>>      I modified the netfont.c of Linux HVM domU installed PVonHVM.In it,
>> I call hypercall_grant_table_op
>>  (GNTTABOP_map_grant_ref...), then dom0 shutdown and restart at once.
>
> Do you have a serial line attached to the machine to capture the console
> output when this happens?  Without that it's hard to knwo what the bug is.
>
>>      From above, I conclude that I can map a HVM's page to another HVM,
>> just like two PVs.
>>  Am I wrong? Who can give me some suggestion?
>
> Yes, HVM guests can now map granted frames, but not quite 'just like pv'.
> The grant hypercall maps the granted frame into the HVM guest's p2m map,
> instead of into the pagetables.  That is, you pass in a PFN, and when
> the grant succeeds, you still need to map that PFN in your pagetables to
> access the page.
>
> The checkin that added the feature came with this comment:
>
>   After some discussion, here's a second version of the patch I posted a
>   couple of weeks back to map grant references into HVM guests.  As
>   before, this is done by modifying the P2M map, but this time there's
>   no new hypercall to do it.  Instead, the existing GNTTABOP_map is
>   overloaded to perform a P2M mapping if called from a shadow mode
>   translate guest.  This matches the IA64 API.
>
> http://xenbits.xen.org/hg/xen-unstable.hg/rev/c0cb307d927f
>
> Tim.
>
>
>
> ------------------------------
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
>
> End of Xen-devel Digest, Vol 81, Issue 328
> ******************************************
>

Re: mark pages in p2m_ram_paging_out state read-only

[Tim Deegan]

At 09:09 -0800 on 24 Nov (1322125786), Andres Lagar-Cavilla wrote:
> > Date: Thu, 24 Nov 2011 11:38:02 +0000
> > From: Tim Deegan <tim@xen.org>
> > To: Olaf Hering <olaf@aepfle.de>
> > Cc: xen-devel@lists.xensource.com
> > Subject: Re: [Xen-devel] mark pages in p2m_ram_paging_out state
> > 	read-only
> > Message-ID: <20111124113802.GB77868@ocelot.phlegethon.org>
> > Content-Type: text/plain; charset=iso-8859-1
> >
> > At 17:53 +0100 on 14 Nov (1321293182), Olaf Hering wrote:
> >>
> >> I was wondering why ept_p2m_type_to_flags() removes all permissions from
> >> a gfn in state p2m_ram_paging_out. If the guest happens to read or
> >> execute from that page while the pager writes that gfn to disk, wouldnt
> >> it be enough to remove the write bit to prevent writes from the guest?
> >> If the page is read-only the guest could continue to make progress until
> >> the gfn is really evicted and the p2mt changes to p2m_ram_paged.
> >>
> >> I havent actually tried the patch below, but is there any reason it
> >> would break the guest?
> >
> > As long as we change the p2m type before scrubbing or freeing the page,
> > that seems like it shuold be fine.
> 
> Is this a good idea? If the guest is accessing the page, then paging out
> should stop immediately. We're risking complex races for a tiny tiny gain.

I don't think that this would introduce any new races, but yes - this is
probably a hint that this page is a poor candidate to be paged out.
We might as well abandon the page-out rather than probably having to
page it back in immediately.

Tim.

Re: mark pages in p2m_ram_paging_out state read-only

[Olaf Hering]

On Thu, Nov 24, Tim Deegan wrote:

> > Is this a good idea? If the guest is accessing the page, then paging out
> > should stop immediately. We're risking complex races for a tiny tiny gain.
> 
> I don't think that this would introduce any new races, but yes - this is
> probably a hint that this page is a poor candidate to be paged out.
> We might as well abandon the page-out rather than probably having to
> page it back in immediately.

In the current state there will be a stall in the guest due to the
fault, until the page comes back. 
With this change the guest can still make progress. 
Can the read-execute access be detected anyway to stop paging?

Re: mark pages in p2m_ram_paging_out state read-only

[Olaf Hering]

On Thu, Nov 24, Olaf Hering wrote:

> On Thu, Nov 24, Tim Deegan wrote:
> 
> > > Is this a good idea? If the guest is accessing the page, then paging out
> > > should stop immediately. We're risking complex races for a tiny tiny gain.
> > 
> > I don't think that this would introduce any new races, but yes - this is
> > probably a hint that this page is a poor candidate to be paged out.
> > We might as well abandon the page-out rather than probably having to
> > page it back in immediately.
> 
> In the current state there will be a stall in the guest due to the
> fault, until the page comes back. 
> With this change the guest can still make progress. 
> Can the read-execute access be detected anyway to stop paging?

(I hit send too soon..)

Right now nominate would cause an immediate fault. populate would be
called at some point, hopefully before evict. If populate were smart
enough to bring the page back from p2m_ram_paging_out to p2m_ram_rw and
let the callers know about that, the guest could proceed right away. I
have already added some code to populate to detect p2m_ram_paging_out.
Perhaps p2m_mem_paging_populate shouldnt be void. It could return -EBUSY
to its callers, they should get a valid mfn for their gfn and handle the
gfn as valid ram.

Olaf

Re: mark pages in p2m_ram_paging_out state read-only

[Andres Lagar-Cavilla]

> On Thu, Nov 24, Tim Deegan wrote:
>
>> > Is this a good idea? If the guest is accessing the page, then paging
>> out
>> > should stop immediately. We're risking complex races for a tiny tiny
>> gain.
>>
>> I don't think that this would introduce any new races, but yes - this is
>> probably a hint that this page is a poor candidate to be paged out.
>> We might as well abandon the page-out rather than probably having to
>> page it back in immediately.
>
> In the current state there will be a stall in the guest due to the
> fault, until the page comes back.
> With this change the guest can still make progress.
> Can the read-execute access be detected anyway to stop paging?
>
The stall is minuscule. A "minor fault" since the page hasn't been paged
out yet (in the condition this patch is concerned with) and thus there is
no IO readback.

The current code handles this nicely with its state machine. Further, it
lets the pager know with an additional event in the ring, which is
something I'd like to keep in place.

There are extra round-trip(s) to the pager, as is. Imho, it's not an
unreasonable price for a fringe condition.

Andres