George Dunlap
2008-Sep-12 15:38 UTC
[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
Daniel Magenheimer
2008-Sep-12 15:57 UTC
RE: [Xen-devel] [PATCH] Add new location of Linux direct-map to the places to look for writable mappings
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 >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Pratt
2008-Sep-12 19:19 UTC
RE: [Xen-devel] [PATCH] Add new location of Linux direct-map to theplaces to look for writable mappings
> 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_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianluca Guida
2008-Sep-12 20:32 UTC
Re: [Xen-devel] [PATCH] Add new location of Linux direct-map to theplaces to look for writable mappings
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2008-Sep-12 23:49 UTC
Re: [Xen-devel] [PATCH] Add new location of Linux direct-map to the places to look for writable mappings
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Tim Deegan
2008-Sep-15 08:31 UTC
Re: [Xen-devel] [PATCH] Add new location of Linux direct-map to theplaces to look for writable mappings
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.] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
George Dunlap
2008-Sep-15 10:56 UTC
Re: [Xen-devel] [PATCH] Add new location of Linux direct-map to theplaces to look for writable mappings
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 >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel