[PATCH] Add new location of Linux direct-map to the places to look for writable mappings

[PATCH] Add new location of Linux direct-map to the places to look for writable mappings

[George Dunlap]

Add new location of Linux direct-map to the places to look for
writable mappings.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

diff -r dbac9ee4d761 xen/arch/x86/mm/shadow/common.c
--- a/xen/arch/x86/mm/shadow/common.c	Mon Sep 08 16:02:13 2008 +0100
+++ b/xen/arch/x86/mm/shadow/common.c	Fri Sep 12 16:42:32 2008 +0100
@@ -2385,9 +2385,11 @@ int sh_remove_write_access(struct vcpu *
                           + ((fault_addr & VADDR_MASK) >> 27), 3); break;
             }

-            /* 64bit Linux direct map at 0xffff810000000000; older kernels
-             * had it at 0x0000010000000000UL */
+            /* 64bit Linux direct map at 0xffff880000000000; older kernels
+             * had it at 0xffff880000000000, and older kernels yet had it
+             * at 0x0000010000000000UL */
             gfn = mfn_to_gfn(v->domain, gmfn);
+            GUESS(0xffff880000000000UL + (gfn << PAGE_SHIFT), 4);
             GUESS(0xffff810000000000UL + (gfn << PAGE_SHIFT), 4);
             GUESS(0x0000010000000000UL + (gfn << PAGE_SHIFT), 4);

RE: [PATCH] Add new location of Linux direct-map to the places to look for writable mappings

[Daniel Magenheimer]

I haven't even looked at this code so sorry for my
possibly naive comment, but isn't this just asking for
trouble to hardcode constants that apply to specific
OS's?  Isn't there a way to "sense" that this address is
used a lot and add it to a dynamic list that can be
checked?  Else sooner or later some user is going to say
"I tried Xen on xxx OS and performance sucked and it
was fine on (unnamed virtualization platform)".  But that 
user might not be as diligent about reporting to
xen-devel as Todd was.

> -----Original Message-----
> From: George Dunlap [mailto:George.Dunlap@eu.citrix.com]
> Sent: Friday, September 12, 2008 9:39 AM
> To: xen-devel mailing list
> Subject: [Xen-devel] [PATCH] Add new location of Linux 
> direct-map to the
> places to look for writable mappings
> 
> 
> Add new location of Linux direct-map to the places to look for
> writable mappings.
> 
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> 
> diff -r dbac9ee4d761 xen/arch/x86/mm/shadow/common.c
> --- a/xen/arch/x86/mm/shadow/common.c	Mon Sep 08 16:02:13 2008 +0100
> +++ b/xen/arch/x86/mm/shadow/common.c	Fri Sep 12 16:42:32 2008 +0100
> @@ -2385,9 +2385,11 @@ int sh_remove_write_access(struct vcpu *
>                            + ((fault_addr & VADDR_MASK) >> 
> 27), 3); break;
>              }
> 
> -            /* 64bit Linux direct map at 0xffff810000000000; 
> older kernels
> -             * had it at 0x0000010000000000UL */
> +            /* 64bit Linux direct map at 0xffff880000000000; 
> older kernels
> +             * had it at 0xffff880000000000, and older 
> kernels yet had it
> +             * at 0x0000010000000000UL */
>              gfn = mfn_to_gfn(v->domain, gmfn);
> +            GUESS(0xffff880000000000UL + (gfn << PAGE_SHIFT), 4);
>              GUESS(0xffff810000000000UL + (gfn << PAGE_SHIFT), 4);
>              GUESS(0x0000010000000000UL + (gfn << PAGE_SHIFT), 4);
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

RE: [PATCH] Add new location of Linux direct-map to theplaces to look for writable mappings

[Ian Pratt]

> I haven't even looked at this code so sorry for my
> possibly naive comment, but isn't this just asking for
> trouble to hardcode constants that apply to specific
> OS's?  Isn't there a way to "sense" that this address is
> used a lot and add it to a dynamic list that can be
> checked?  Else sooner or later some user is going to say
> "I tried Xen on xxx OS and performance sucked and it
> was fine on (unnamed virtualization platform)".  But that
> user might not be as diligent about reporting to
> xen-devel as Todd was.

There are three different mechanisms that different OSes use to update
their pagetables: 1:1 direct map regions, linear recursive map regions,
and kmap-style regions. It would be possible to come up with logic to
automatically detect and identify these regions, but its non trivial.
The test might be quite expensive, so you'd want to run it for a while
and then make a decision on the type and location. If you started
getting too many 'misses' you'd want to re-evaluate the decision as the
guest kernel may have changed mode of operation (perhaps someone's done
a kexec, or the boot time behaviour was different from runtime). 

Anyhow, it's certainly possible to do this automatically, but given that
the mechanism and location used by kernels typically doesn't change the
hard-coded heuristic has served us pretty well to date. Feel free to
send patches :-)

What would also be interesting would be a shadow pagetable mode that
stored 'back pointers' to enable one to easily find all the shadow PTEs
that point to a page. This would be useful for some of the more fancy
shadow pagetable modes that are interesting from a research POV, as well
as coping with guests that use some other mechanism for updating PTEs
(if such guests exist).

Ian
 






 
> > -----Original Message-----
> > From: George Dunlap [mailto:George.Dunlap@eu.citrix.com]
> > Sent: Friday, September 12, 2008 9:39 AM
> > To: xen-devel mailing list
> > Subject: [Xen-devel] [PATCH] Add new location of Linux
> > direct-map to the
> > places to look for writable mappings
> >
> >
> > Add new location of Linux direct-map to the places to look for
> > writable mappings.
> >
> > Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> >
> > diff -r dbac9ee4d761 xen/arch/x86/mm/shadow/common.c
> > --- a/xen/arch/x86/mm/shadow/common.c	Mon Sep 08 16:02:13 2008
> +0100
> > +++ b/xen/arch/x86/mm/shadow/common.c	Fri Sep 12 16:42:32 2008
> +0100
> > @@ -2385,9 +2385,11 @@ int sh_remove_write_access(struct vcpu *
> >                            + ((fault_addr & VADDR_MASK) >>
> > 27), 3); break;
> >              }
> >
> > -            /* 64bit Linux direct map at 0xffff810000000000;
> > older kernels
> > -             * had it at 0x0000010000000000UL */
> > +            /* 64bit Linux direct map at 0xffff880000000000;
> > older kernels
> > +             * had it at 0xffff880000000000, and older
> > kernels yet had it
> > +             * at 0x0000010000000000UL */
> >              gfn = mfn_to_gfn(v->domain, gmfn);
> > +            GUESS(0xffff880000000000UL + (gfn << PAGE_SHIFT), 4);
> >              GUESS(0xffff810000000000UL + (gfn << PAGE_SHIFT), 4);
> >              GUESS(0x0000010000000000UL + (gfn << PAGE_SHIFT), 4);
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel
> >
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] Add new location of Linux direct-map to theplaces to look for writable mappings

[Gianluca Guida]

Sorry for the double-quote, I never received bizarrely Dan's email and I 
have only Ian's answer.

>> I haven't even looked at this code so sorry for my
>> possibly naive comment, but isn't this just asking for
>> trouble to hardcode constants that apply to specific
>> OS's?  Isn't there a way to "sense" that this address is
>> used a lot and add it to a dynamic list that can be
>> checked?  Else sooner or later some user is going to say
>> "I tried Xen on xxx OS and performance sucked and it
>> was fine on (unnamed virtualization platform)".  But that
>> user might not be as diligent about reporting to
>> xen-devel as Todd was.

I would just like to note that this disastrous benchmark result was due 
to two consecutive problems, the OS heuristic being only a secondary issue.

Usually, removing write access to an out-of-sync page shouldn't need at 
all the OS heuristic guessing, since it is a very frequent operation and 
that would affect performances (as we all have seen). Fixup tables exist 
exactly for this purpose, acting basically as reverse map for writable 
mappings of pagetables.

What was happening is that the fixup tables were failing, thus the 
research of the writable mappings was falling back to guest heuristic. 
This is bad per se, but it was working well enough until 2.6.27.

So, while I agree that having hard-coded addresses into the hypervisor 
is not nice, in case of failure of this mechanism the results shouldn't 
be as bad as the one we've seen. That was all my fault. :)

Thanks,
Gianluca

Re: [PATCH] Add new location of Linux direct-map to theplaces to look for writable mappings

[Tim Deegan]

At 20:19 +0100 on 12 Sep (1221250763), Ian Pratt wrote:
> What would also be interesting would be a shadow pagetable mode that
> stored 'back pointers' to enable one to easily find all the shadow PTEs
> that point to a page. This would be useful for some of the more fancy
> shadow pagetable modes that are interesting from a research POV, as well
> as coping with guests that use some other mechanism for updating PTEs
> (if such guests exist).

ISTR we decided not to do this when we were planning shadow2, because it
doubles the memory overhead and adds extra bookkeeping to the fast
paths.  Not to say it wouldn't be interesting, though. :)

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Citrix Systems (R&D) Ltd.
[Company #02300071, SL9 0DZ, UK.]

Re: [PATCH] Add new location of Linux direct-map to theplaces to look for writable mappings

[George Dunlap]

On Fri, Sep 12, 2008 at 8:19 PM, Ian Pratt <Ian.Pratt@eu.citrix.com> wrote:
> There are three different mechanisms that different OSes use to update
> their pagetables: 1:1 direct map regions, linear recursive map regions,
> and kmap-style regions. It would be possible to come up with logic to
> automatically detect and identify these regions, but its non trivial.
> The test might be quite expensive, so you'd want to run it for a while
> and then make a decision on the type and location. If you started
> getting too many 'misses' you'd want to re-evaluate the decision as the
> guest kernel may have changed mode of operation (perhaps someone's done
> a kexec, or the boot time behaviour was different from runtime).

Even automatically detecting this is still fairly OS-specific: it
relies on the fact that Linux normally does direct-mapping, and
Windows normally does linear recursive maps.  If FooOS came out and
used the formula (LIMIT-gpfn) instead of (BASE+gpfn), for instance,
we'd be back to square one.

If it were less expensive (detection path on every set_l1e()
basically), or the exact addresses tended to move around a lot, that
might be a good option.  But as Ian said, it would add a significant
amount of code to a common path, and the addresses really don't change
all that often.

Thanks for bringing this up though, Dan -- It's good to revisit these
kinds of decisions every so often to make sure that they're still
valid.

 -George

>
> Anyhow, it's certainly possible to do this automatically, but given that
> the mechanism and location used by kernels typically doesn't change the
> hard-coded heuristic has served us pretty well to date. Feel free to
> send patches :-)
>
> What would also be interesting would be a shadow pagetable mode that
> stored 'back pointers' to enable one to easily find all the shadow PTEs
> that point to a page. This would be useful for some of the more fancy
> shadow pagetable modes that are interesting from a research POV, as well
> as coping with guests that use some other mechanism for updating PTEs
> (if such guests exist).
>
> Ian
>
>
>
>
>
>
>
>
>> > -----Original Message-----
>> > From: George Dunlap [mailto:George.Dunlap@eu.citrix.com]
>> > Sent: Friday, September 12, 2008 9:39 AM
>> > To: xen-devel mailing list
>> > Subject: [Xen-devel] [PATCH] Add new location of Linux
>> > direct-map to the
>> > places to look for writable mappings
>> >
>> >
>> > Add new location of Linux direct-map to the places to look for
>> > writable mappings.
>> >
>> > Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
>> >
>> > diff -r dbac9ee4d761 xen/arch/x86/mm/shadow/common.c
>> > --- a/xen/arch/x86/mm/shadow/common.c       Mon Sep 08 16:02:13 2008
>> +0100
>> > +++ b/xen/arch/x86/mm/shadow/common.c       Fri Sep 12 16:42:32 2008
>> +0100
>> > @@ -2385,9 +2385,11 @@ int sh_remove_write_access(struct vcpu *
>> >                            + ((fault_addr & VADDR_MASK) >>
>> > 27), 3); break;
>> >              }
>> >
>> > -            /* 64bit Linux direct map at 0xffff810000000000;
>> > older kernels
>> > -             * had it at 0x0000010000000000UL */
>> > +            /* 64bit Linux direct map at 0xffff880000000000;
>> > older kernels
>> > +             * had it at 0xffff880000000000, and older
>> > kernels yet had it
>> > +             * at 0x0000010000000000UL */
>> >              gfn = mfn_to_gfn(v->domain, gmfn);
>> > +            GUESS(0xffff880000000000UL + (gfn << PAGE_SHIFT), 4);
>> >              GUESS(0xffff810000000000UL + (gfn << PAGE_SHIFT), 4);
>> >              GUESS(0x0000010000000000UL + (gfn << PAGE_SHIFT), 4);
>> >
>> > _______________________________________________
>> > Xen-devel mailing list
>> > Xen-devel@lists.xensource.com
>> > http://lists.xensource.com/xen-devel
>> >
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

Re: [PATCH] Add new location of Linux direct-map to the places to look for writable mappings

[Jeremy Fitzhardinge]

George Dunlap wrote:
> Add new location of Linux direct-map to the places to look for
> writable mappings.
>
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
>
> diff -r dbac9ee4d761 xen/arch/x86/mm/shadow/common.c
> --- a/xen/arch/x86/mm/shadow/common.c	Mon Sep 08 16:02:13 2008 +0100
> +++ b/xen/arch/x86/mm/shadow/common.c	Fri Sep 12 16:42:32 2008 +0100
> @@ -2385,9 +2385,11 @@ int sh_remove_write_access(struct vcpu *
>                            + ((fault_addr & VADDR_MASK) >> 27), 3); break;
>              }
>
> -            /* 64bit Linux direct map at 0xffff810000000000; older kernels
> -             * had it at 0x0000010000000000UL */
> +            /* 64bit Linux direct map at 0xffff880000000000; older kernels
> +             * had it at 0xffff880000000000, and older kernels yet had it
>   

Should be "older kernels had it at 0xffff810000000000".

    J