Re: [PATCH 4/8] unify paravirt parts of system.h
On Monday, 17 of December 2007, Pavel Machek wrote: > On Mon 2007-12-17 01:27:29, Rafael J. Wysocki wrote: > > On Saturday, 15 of December 2007, Ingo Molnar wrote: > > > > > > * Pavel Machek <[EMAIL PROTECTED]> wrote: > > > > > > > > Linux never uses that register. The only user is suspend > > > > > save/restore, but that' bogus because it wasn't ever initialized by > > > > > Linux in the first place. It could be probably all safely removed. > > > > > > > > It probably is safe to remove... but we currently support '2.8.95 > > > > kernel loads/resumes 2.6.24 image'... which would break if 2.8 uses > > > > cr8. > > > > > > > > So please keep it if it is not a big problem. > > > > > > hm, so __save_processor_state() is in essence an ABI? Could you please > > > also send a patch that documents this prominently, in the structure > > > itself? > > > > Hmm, I'm not sure if it really is an ABI part. It doesn't communicate > > anything > > outside of the kernel in which it is defined. > > Well, it is not "application binary interface", but it is > "kernel-to-kernel binary interface"... Hm, rather a kernel-to-itself interface. ;-) Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] unify paravirt parts of system.h
On Mon 2007-12-17 01:27:29, Rafael J. Wysocki wrote: > On Saturday, 15 of December 2007, Ingo Molnar wrote: > > > > * Pavel Machek <[EMAIL PROTECTED]> wrote: > > > > > > Linux never uses that register. The only user is suspend > > > > save/restore, but that' bogus because it wasn't ever initialized by > > > > Linux in the first place. It could be probably all safely removed. > > > > > > It probably is safe to remove... but we currently support '2.8.95 > > > kernel loads/resumes 2.6.24 image'... which would break if 2.8 uses > > > cr8. > > > > > > So please keep it if it is not a big problem. > > > > hm, so __save_processor_state() is in essence an ABI? Could you please > > also send a patch that documents this prominently, in the structure > > itself? > > Hmm, I'm not sure if it really is an ABI part. It doesn't communicate > anything > outside of the kernel in which it is defined. Well, it is not "application binary interface", but it is "kernel-to-kernel binary interface"... > The problem is, though, that if kernel A is used for resuming kernel B, and > kernel B doesn't save/restore everything it will need after the resume, then > things will break if kernel A modifies that. So, yes, we'll need to document > that explicitly. Agreed. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] unify paravirt parts of system.h
On Saturday, 15 of December 2007, Ingo Molnar wrote: > > * Pavel Machek <[EMAIL PROTECTED]> wrote: > > > > Linux never uses that register. The only user is suspend > > > save/restore, but that' bogus because it wasn't ever initialized by > > > Linux in the first place. It could be probably all safely removed. > > > > It probably is safe to remove... but we currently support '2.8.95 > > kernel loads/resumes 2.6.24 image'... which would break if 2.8 uses > > cr8. > > > > So please keep it if it is not a big problem. > > hm, so __save_processor_state() is in essence an ABI? Could you please > also send a patch that documents this prominently, in the structure > itself? Hmm, I'm not sure if it really is an ABI part. It doesn't communicate anything outside of the kernel in which it is defined. The problem is, though, that if kernel A is used for resuming kernel B, and kernel B doesn't save/restore everything it will need after the resume, then things will break if kernel A modifies that. So, yes, we'll need to document that explicitly. Greetings, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] unify paravirt parts of system.h
On Saturday, 15 of December 2007, Ingo Molnar wrote: * Pavel Machek [EMAIL PROTECTED] wrote: Linux never uses that register. The only user is suspend save/restore, but that' bogus because it wasn't ever initialized by Linux in the first place. It could be probably all safely removed. It probably is safe to remove... but we currently support '2.8.95 kernel loads/resumes 2.6.24 image'... which would break if 2.8 uses cr8. So please keep it if it is not a big problem. hm, so __save_processor_state() is in essence an ABI? Could you please also send a patch that documents this prominently, in the structure itself? Hmm, I'm not sure if it really is an ABI part. It doesn't communicate anything outside of the kernel in which it is defined. The problem is, though, that if kernel A is used for resuming kernel B, and kernel B doesn't save/restore everything it will need after the resume, then things will break if kernel A modifies that. So, yes, we'll need to document that explicitly. Greetings, Rafael -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] unify paravirt parts of system.h
On Mon 2007-12-17 01:27:29, Rafael J. Wysocki wrote: On Saturday, 15 of December 2007, Ingo Molnar wrote: * Pavel Machek [EMAIL PROTECTED] wrote: Linux never uses that register. The only user is suspend save/restore, but that' bogus because it wasn't ever initialized by Linux in the first place. It could be probably all safely removed. It probably is safe to remove... but we currently support '2.8.95 kernel loads/resumes 2.6.24 image'... which would break if 2.8 uses cr8. So please keep it if it is not a big problem. hm, so __save_processor_state() is in essence an ABI? Could you please also send a patch that documents this prominently, in the structure itself? Hmm, I'm not sure if it really is an ABI part. It doesn't communicate anything outside of the kernel in which it is defined. Well, it is not application binary interface, but it is kernel-to-kernel binary interface... The problem is, though, that if kernel A is used for resuming kernel B, and kernel B doesn't save/restore everything it will need after the resume, then things will break if kernel A modifies that. So, yes, we'll need to document that explicitly. Agreed. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] unify paravirt parts of system.h
On Monday, 17 of December 2007, Pavel Machek wrote: On Mon 2007-12-17 01:27:29, Rafael J. Wysocki wrote: On Saturday, 15 of December 2007, Ingo Molnar wrote: * Pavel Machek [EMAIL PROTECTED] wrote: Linux never uses that register. The only user is suspend save/restore, but that' bogus because it wasn't ever initialized by Linux in the first place. It could be probably all safely removed. It probably is safe to remove... but we currently support '2.8.95 kernel loads/resumes 2.6.24 image'... which would break if 2.8 uses cr8. So please keep it if it is not a big problem. hm, so __save_processor_state() is in essence an ABI? Could you please also send a patch that documents this prominently, in the structure itself? Hmm, I'm not sure if it really is an ABI part. It doesn't communicate anything outside of the kernel in which it is defined. Well, it is not application binary interface, but it is kernel-to-kernel binary interface... Hm, rather a kernel-to-itself interface. ;-) Rafael -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] unify paravirt parts of system.h
On Sat 2007-12-15 14:26:38, Andi Kleen wrote: > > It probably is safe to remove... but we currently support '2.8.95 > > kernel loads/resumes 2.6.24 image'... which would break if 2.8 uses > > cr8. > > No it won't. 2.8 would just restore some random useless value. Restoring random value seems wrong. Putting random values into cpu registers can break stuff, right? Even if 2.6.24 image being restored did not set %cr8 itself, it may depend on %cr8 to have "sane" value. > If 2.8 wants to use CR8 it would have to re-initialize it We are talking "2.8 restores 2.6 image" here. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] unify paravirt parts of system.h
Pavel Machek wrote: It probably is safe to remove... but we currently support '2.8.95 kernel loads/resumes 2.6.24 image'... which would break if 2.8 uses cr8. So please keep it if it is not a big problem. Note that CR8 is an alias for the TPR in the APIC. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] unify paravirt parts of system.h
> It probably is safe to remove... but we currently support '2.8.95 > kernel loads/resumes 2.6.24 image'... which would break if 2.8 uses > cr8. No it won't. 2.8 would just restore some random useless value. If 2.8 wants to use CR8 it would have to re-initialize it -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] unify paravirt parts of system.h
* Pavel Machek <[EMAIL PROTECTED]> wrote: > > Linux never uses that register. The only user is suspend > > save/restore, but that' bogus because it wasn't ever initialized by > > Linux in the first place. It could be probably all safely removed. > > It probably is safe to remove... but we currently support '2.8.95 > kernel loads/resumes 2.6.24 image'... which would break if 2.8 uses > cr8. > > So please keep it if it is not a big problem. hm, so __save_processor_state() is in essence an ABI? Could you please also send a patch that documents this prominently, in the structure itself? Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] unify paravirt parts of system.h
* Pavel Machek [EMAIL PROTECTED] wrote: Linux never uses that register. The only user is suspend save/restore, but that' bogus because it wasn't ever initialized by Linux in the first place. It could be probably all safely removed. It probably is safe to remove... but we currently support '2.8.95 kernel loads/resumes 2.6.24 image'... which would break if 2.8 uses cr8. So please keep it if it is not a big problem. hm, so __save_processor_state() is in essence an ABI? Could you please also send a patch that documents this prominently, in the structure itself? Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] unify paravirt parts of system.h
It probably is safe to remove... but we currently support '2.8.95 kernel loads/resumes 2.6.24 image'... which would break if 2.8 uses cr8. No it won't. 2.8 would just restore some random useless value. If 2.8 wants to use CR8 it would have to re-initialize it -Andi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] unify paravirt parts of system.h
Pavel Machek wrote: It probably is safe to remove... but we currently support '2.8.95 kernel loads/resumes 2.6.24 image'... which would break if 2.8 uses cr8. So please keep it if it is not a big problem. Note that CR8 is an alias for the TPR in the APIC. -hpa -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] unify paravirt parts of system.h
On Sat 2007-12-15 14:26:38, Andi Kleen wrote: It probably is safe to remove... but we currently support '2.8.95 kernel loads/resumes 2.6.24 image'... which would break if 2.8 uses cr8. No it won't. 2.8 would just restore some random useless value. Restoring random value seems wrong. Putting random values into cpu registers can break stuff, right? Even if 2.6.24 image being restored did not set %cr8 itself, it may depend on %cr8 to have sane value. If 2.8 wants to use CR8 it would have to re-initialize it We are talking 2.8 restores 2.6 image here. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] unify paravirt parts of system.h
On Tue 2007-12-04 20:34:32, Andi Kleen wrote:
> On Tue, Dec 04, 2007 at 09:18:33PM +0200, Avi Kivity wrote:
> > Glauber de Oliveira Costa wrote:
> >> This patch moves the i386 control registers manipulation functions,
> >> wbinvd, and clts functions to system.h. They are essentially the same
> >> as in x86_64, except for the cr8 register, which we add.
> >>
> >> +
> >> +static inline unsigned long native_read_cr8(void)
> >> +{
> >> + unsigned long cr8;
> >> + asm volatile("mov %%cr8,%0" : "=r" (cr8), "=m" (__force_order));
> >> + return cr8;
> >> +}
> >> +
> >>
> >
> > There is no cr8 register on i386. This had better be protected by an
> > #ifdef.
> >
> > (you're likely not getting an error since it's a static inline, so the asm
> > is never emitted)
>
> Linux never uses that register. The only user is suspend save/restore,
> but that' bogus because it wasn't ever initialized by Linux in the first
> place. It could be probably all safely removed.
It probably is safe to remove... but we currently support '2.8.95
kernel loads/resumes 2.6.24 image'... which would break if 2.8 uses
cr8.
So please keep it if it is not a big problem.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] unify paravirt parts of system.h
On Tue 2007-12-04 20:34:32, Andi Kleen wrote:
On Tue, Dec 04, 2007 at 09:18:33PM +0200, Avi Kivity wrote:
Glauber de Oliveira Costa wrote:
This patch moves the i386 control registers manipulation functions,
wbinvd, and clts functions to system.h. They are essentially the same
as in x86_64, except for the cr8 register, which we add.
+
+static inline unsigned long native_read_cr8(void)
+{
+ unsigned long cr8;
+ asm volatile(mov %%cr8,%0 : =r (cr8), =m (__force_order));
+ return cr8;
+}
+
There is no cr8 register on i386. This had better be protected by an
#ifdef.
(you're likely not getting an error since it's a static inline, so the asm
is never emitted)
Linux never uses that register. The only user is suspend save/restore,
but that' bogus because it wasn't ever initialized by Linux in the first
place. It could be probably all safely removed.
It probably is safe to remove... but we currently support '2.8.95
kernel loads/resumes 2.6.24 image'... which would break if 2.8 uses
cr8.
So please keep it if it is not a big problem.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] unify paravirt parts of system.h
On Tuesday 04 December 2007 11:41, Glauber de Oliveira Costa wrote: > On Dec 4, 2007 5:18 PM, Avi Kivity <[EMAIL PROTECTED]> wrote: > > There is no cr8 register on i386. This had better be protected by an > > #ifdef. > > Sure. I mentioned it in the changelog. I, however, am not sure If I > agree it should be enclosed > in ifdefs. Me and Jeremy discussed it a while ago, and we seem to > agree that for those functions > that are exclusive of one architecture, there were no need for ifdefs. > Any usage by the other arch > is a bug. > > But I admit that I'm not particularly biased here, and I can change > it, if there's agreement that > an ifdef here is the way to go. > > > (you're likely not getting an error since it's a static inline, so the > > asm is never emitted) > > Which also means it does not affect the binary in anyway. No bigger > code, no nothing. If future changes will mistakenly make 32-bit x86 call native_read_cr8(), you will get no warning. (Hmmm. Maybe as will complain, I'm not sure). If it explicitly ifdefed out for 32 bits, it's easier to detect misuse. -- vda -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] unify paravirt parts of system.h
On Tue, Dec 04, 2007 at 09:18:33PM +0200, Avi Kivity wrote:
> Glauber de Oliveira Costa wrote:
>> This patch moves the i386 control registers manipulation functions,
>> wbinvd, and clts functions to system.h. They are essentially the same
>> as in x86_64, except for the cr8 register, which we add.
>>
>> +
>> +static inline unsigned long native_read_cr8(void)
>> +{
>> +unsigned long cr8;
>> +asm volatile("mov %%cr8,%0" : "=r" (cr8), "=m" (__force_order));
>> +return cr8;
>> +}
>> +
>>
>
> There is no cr8 register on i386. This had better be protected by an
> #ifdef.
>
> (you're likely not getting an error since it's a static inline, so the asm
> is never emitted)
Linux never uses that register. The only user is suspend save/restore,
but that' bogus because it wasn't ever initialized by Linux in the first
place. It could be probably all safely removed.
-Andi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] unify paravirt parts of system.h
On Dec 4, 2007 5:18 PM, Avi Kivity <[EMAIL PROTECTED]> wrote:
> Glauber de Oliveira Costa wrote:
> > This patch moves the i386 control registers manipulation functions,
> > wbinvd, and clts functions to system.h. They are essentially the same
> > as in x86_64, except for the cr8 register, which we add.
> >
> > +
> > +static inline unsigned long native_read_cr8(void)
> > +{
> > + unsigned long cr8;
> > + asm volatile("mov %%cr8,%0" : "=r" (cr8), "=m" (__force_order));
> > + return cr8;
> > +}
> > +
> >
>
> There is no cr8 register on i386. This had better be protected by an
> #ifdef.
Sure. I mentioned it in the changelog. I, however, am not sure If I
agree it should be enclosed
in ifdefs. Me and Jeremy discussed it a while ago, and we seem to
agree that for those functions
that are exclusive of one architecture, there were no need for ifdefs.
Any usage by the other arch
is a bug.
But I admit that I'm not particularly biased here, and I can change
it, if there's agreement that
an ifdef here is the way to go.
> (you're likely not getting an error since it's a static inline, so the
> asm is never emitted)
>
Which also means it does not affect the binary in anyway. No bigger
code, no nothing.
My current approach is to save the ifdefs for pieces that in fact can
save us some resources
in the final image. But again... I can change it.
--
Glauber de Oliveira Costa.
"Free as in Freedom"
http://glommer.net
"The less confident you are, the more serious you have to act."
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] unify paravirt parts of system.h
Glauber de Oliveira Costa wrote:
This patch moves the i386 control registers manipulation functions,
wbinvd, and clts functions to system.h. They are essentially the same
as in x86_64, except for the cr8 register, which we add.
+
+static inline unsigned long native_read_cr8(void)
+{
+ unsigned long cr8;
+ asm volatile("mov %%cr8,%0" : "=r" (cr8), "=m" (__force_order));
+ return cr8;
+}
+
There is no cr8 register on i386. This had better be protected by an
#ifdef.
(you're likely not getting an error since it's a static inline, so the
asm is never emitted)
--
Any sufficiently difficult bug is indistinguishable from a feature.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/8] unify paravirt parts of system.h
This patch moves the i386 control registers manipulation functions,
wbinvd, and clts functions to system.h. They are essentially the same
as in x86_64, except for the cr8 register, which we add.
With this, system.h paravirt comes for free in x86_64.
Signed-off-by: Glauber de Oliveira Costa <[EMAIL PROTECTED]>
---
include/asm-x86/system.h| 124 +++
include/asm-x86/system_32.h | 94
include/asm-x86/system_64.h | 73 -
3 files changed, 124 insertions(+), 167 deletions(-)
diff --git a/include/asm-x86/system.h b/include/asm-x86/system.h
index 1ac6088..f1fdc55 100644
--- a/include/asm-x86/system.h
+++ b/include/asm-x86/system.h
@@ -3,6 +3,8 @@
#include
+#include
+
#ifdef CONFIG_X86_32
# include "system_32.h"
#else
@@ -38,6 +40,8 @@ __asm__ __volatile__ ("movw %%dx,%1\n\t" \
#define set_base(ldt, base) _set_base(((char *)&(ldt)) , (base))
#define set_limit(ldt, limit) _set_limit(((char *)&(ldt)) , ((limit)-1))
+extern void load_gs_index(unsigned);
+
/*
* Load a segment. Fall back on loading the zero
* segment if something goes wrong..
@@ -72,6 +76,126 @@ static inline unsigned long get_limit(unsigned long segment)
:"=r" (__limit):"r" (segment));
return __limit+1;
}
+
+static inline void native_clts(void)
+{
+ asm volatile ("clts");
+}
+
+/*
+ * Volatile isn't enough to prevent the compiler from reordering the
+ * read/write functions for the control registers and messing everything up.
+ * A memory clobber would solve the problem, but would prevent reordering of
+ * all loads stores around it, which can hurt performance. Solution is to
+ * use a variable and mimic reads and writes to it to enforce serialization
+ */
+static unsigned long __force_order;
+
+static inline unsigned long native_read_cr0(void)
+{
+ unsigned long val;
+ asm volatile("mov %%cr0,%0\n\t" :"=r" (val), "=m" (__force_order));
+ return val;
+}
+
+static inline void native_write_cr0(unsigned long val)
+{
+ asm volatile("mov %0,%%cr0": :"r" (val), "m" (__force_order));
+}
+
+static inline unsigned long native_read_cr2(void)
+{
+ unsigned long val;
+ asm volatile("mov %%cr2,%0\n\t" :"=r" (val), "=m" (__force_order));
+ return val;
+}
+
+static inline void native_write_cr2(unsigned long val)
+{
+ asm volatile("mov %0,%%cr2": :"r" (val), "m" (__force_order));
+}
+
+static inline unsigned long native_read_cr3(void)
+{
+ unsigned long val;
+ asm volatile("mov %%cr3,%0\n\t" :"=r" (val), "=m" (__force_order));
+ return val;
+}
+
+static inline void native_write_cr3(unsigned long val)
+{
+ asm volatile("mov %0,%%cr3": :"r" (val), "m" (__force_order));
+}
+
+static inline unsigned long native_read_cr4(void)
+{
+ unsigned long val;
+ asm volatile("mov %%cr4,%0\n\t" :"=r" (val), "=m" (__force_order));
+ return val;
+}
+
+static inline unsigned long native_read_cr4_safe(void)
+{
+ unsigned long val;
+ /* This could fault if %cr4 does not exist. In x86_64, a cr4 always
+* exists, so it will never fail. */
+#ifdef CONFIG_X86_32
+ asm volatile("1: mov %%cr4, %0 \n"
+ "2: \n"
+ ".section __ex_table,\"a\" \n"
+ ".long 1b,2b\n"
+ ".previous \n"
+ : "=r" (val), "=m" (__force_order) : "0" (0));
+#else
+ val = native_read_cr4();
+#endif
+ return val;
+}
+
+static inline void native_write_cr4(unsigned long val)
+{
+ asm volatile("mov %0,%%cr4": :"r" (val), "m" (__force_order));
+}
+
+static inline unsigned long native_read_cr8(void)
+{
+ unsigned long cr8;
+ asm volatile("mov %%cr8,%0" : "=r" (cr8), "=m" (__force_order));
+ return cr8;
+}
+
+static inline void native_write_cr8(unsigned long val)
+{
+ asm volatile("mov %0,%%cr8" : : "r" (val));
+}
+
+static inline void native_wbinvd(void)
+{
+ asm volatile("wbinvd": : :"memory");
+}
+#ifdef CONFIG_PARAVIRT
+#include
+#else
+#define read_cr0() (native_read_cr0())
+#define write_cr0(x) (native_write_cr0(x))
+#define read_cr2() (native_read_cr2())
+#define write_cr2(x) (native_write_cr2(x))
+#define read_cr3() (native_read_cr3())
+#define write_cr3(x) (native_write_cr3(x))
+#define read_cr4() (native_read_cr4())
+#define read_cr4_safe()(native_read_cr4_safe())
+#define write_cr4(x) (native_write_cr4(x))
+#define read_cr8() (native_read_cr8())
+#define write_cr8(x) (native_write_cr8(x))
+#define wbinvd() (native_wbinvd())
+
+/* Clear the 'TS' bit */
+#define clts() (native_clts())
+
+#endif/* CONFIG_PARAVIRT */
+
+#define stts() write_cr0(8 | read_cr0())
+
#endif /* __KERNEL__ */
static inline void clflush(void *__p)
diff --git a/include/asm-x86/system_32.h b/include/asm-x86/system_32.h
index
[PATCH 4/8] unify paravirt parts of system.h
This patch moves the i386 control registers manipulation functions,
wbinvd, and clts functions to system.h. They are essentially the same
as in x86_64, except for the cr8 register, which we add.
With this, system.h paravirt comes for free in x86_64.
Signed-off-by: Glauber de Oliveira Costa [EMAIL PROTECTED]
---
include/asm-x86/system.h| 124 +++
include/asm-x86/system_32.h | 94
include/asm-x86/system_64.h | 73 -
3 files changed, 124 insertions(+), 167 deletions(-)
diff --git a/include/asm-x86/system.h b/include/asm-x86/system.h
index 1ac6088..f1fdc55 100644
--- a/include/asm-x86/system.h
+++ b/include/asm-x86/system.h
@@ -3,6 +3,8 @@
#include asm/asm.h
+#include linux/kernel.h
+
#ifdef CONFIG_X86_32
# include system_32.h
#else
@@ -38,6 +40,8 @@ __asm__ __volatile__ (movw %%dx,%1\n\t \
#define set_base(ldt, base) _set_base(((char *)(ldt)) , (base))
#define set_limit(ldt, limit) _set_limit(((char *)(ldt)) , ((limit)-1))
+extern void load_gs_index(unsigned);
+
/*
* Load a segment. Fall back on loading the zero
* segment if something goes wrong..
@@ -72,6 +76,126 @@ static inline unsigned long get_limit(unsigned long segment)
:=r (__limit):r (segment));
return __limit+1;
}
+
+static inline void native_clts(void)
+{
+ asm volatile (clts);
+}
+
+/*
+ * Volatile isn't enough to prevent the compiler from reordering the
+ * read/write functions for the control registers and messing everything up.
+ * A memory clobber would solve the problem, but would prevent reordering of
+ * all loads stores around it, which can hurt performance. Solution is to
+ * use a variable and mimic reads and writes to it to enforce serialization
+ */
+static unsigned long __force_order;
+
+static inline unsigned long native_read_cr0(void)
+{
+ unsigned long val;
+ asm volatile(mov %%cr0,%0\n\t :=r (val), =m (__force_order));
+ return val;
+}
+
+static inline void native_write_cr0(unsigned long val)
+{
+ asm volatile(mov %0,%%cr0: :r (val), m (__force_order));
+}
+
+static inline unsigned long native_read_cr2(void)
+{
+ unsigned long val;
+ asm volatile(mov %%cr2,%0\n\t :=r (val), =m (__force_order));
+ return val;
+}
+
+static inline void native_write_cr2(unsigned long val)
+{
+ asm volatile(mov %0,%%cr2: :r (val), m (__force_order));
+}
+
+static inline unsigned long native_read_cr3(void)
+{
+ unsigned long val;
+ asm volatile(mov %%cr3,%0\n\t :=r (val), =m (__force_order));
+ return val;
+}
+
+static inline void native_write_cr3(unsigned long val)
+{
+ asm volatile(mov %0,%%cr3: :r (val), m (__force_order));
+}
+
+static inline unsigned long native_read_cr4(void)
+{
+ unsigned long val;
+ asm volatile(mov %%cr4,%0\n\t :=r (val), =m (__force_order));
+ return val;
+}
+
+static inline unsigned long native_read_cr4_safe(void)
+{
+ unsigned long val;
+ /* This could fault if %cr4 does not exist. In x86_64, a cr4 always
+* exists, so it will never fail. */
+#ifdef CONFIG_X86_32
+ asm volatile(1: mov %%cr4, %0 \n
+ 2: \n
+ .section __ex_table,\a\ \n
+ .long 1b,2b\n
+ .previous \n
+ : =r (val), =m (__force_order) : 0 (0));
+#else
+ val = native_read_cr4();
+#endif
+ return val;
+}
+
+static inline void native_write_cr4(unsigned long val)
+{
+ asm volatile(mov %0,%%cr4: :r (val), m (__force_order));
+}
+
+static inline unsigned long native_read_cr8(void)
+{
+ unsigned long cr8;
+ asm volatile(mov %%cr8,%0 : =r (cr8), =m (__force_order));
+ return cr8;
+}
+
+static inline void native_write_cr8(unsigned long val)
+{
+ asm volatile(mov %0,%%cr8 : : r (val));
+}
+
+static inline void native_wbinvd(void)
+{
+ asm volatile(wbinvd: : :memory);
+}
+#ifdef CONFIG_PARAVIRT
+#include asm/paravirt.h
+#else
+#define read_cr0() (native_read_cr0())
+#define write_cr0(x) (native_write_cr0(x))
+#define read_cr2() (native_read_cr2())
+#define write_cr2(x) (native_write_cr2(x))
+#define read_cr3() (native_read_cr3())
+#define write_cr3(x) (native_write_cr3(x))
+#define read_cr4() (native_read_cr4())
+#define read_cr4_safe()(native_read_cr4_safe())
+#define write_cr4(x) (native_write_cr4(x))
+#define read_cr8() (native_read_cr8())
+#define write_cr8(x) (native_write_cr8(x))
+#define wbinvd() (native_wbinvd())
+
+/* Clear the 'TS' bit */
+#define clts() (native_clts())
+
+#endif/* CONFIG_PARAVIRT */
+
+#define stts() write_cr0(8 | read_cr0())
+
#endif /* __KERNEL__ */
static inline void clflush(void *__p)
diff --git a/include/asm-x86/system_32.h b/include/asm-x86/system_32.h
index a0641a3..6c69567 100644
--- a/include/asm-x86/system_32.h
+++
Re: [PATCH 4/8] unify paravirt parts of system.h
Glauber de Oliveira Costa wrote:
This patch moves the i386 control registers manipulation functions,
wbinvd, and clts functions to system.h. They are essentially the same
as in x86_64, except for the cr8 register, which we add.
+
+static inline unsigned long native_read_cr8(void)
+{
+ unsigned long cr8;
+ asm volatile(mov %%cr8,%0 : =r (cr8), =m (__force_order));
+ return cr8;
+}
+
There is no cr8 register on i386. This had better be protected by an
#ifdef.
(you're likely not getting an error since it's a static inline, so the
asm is never emitted)
--
Any sufficiently difficult bug is indistinguishable from a feature.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] unify paravirt parts of system.h
On Dec 4, 2007 5:18 PM, Avi Kivity [EMAIL PROTECTED] wrote:
Glauber de Oliveira Costa wrote:
This patch moves the i386 control registers manipulation functions,
wbinvd, and clts functions to system.h. They are essentially the same
as in x86_64, except for the cr8 register, which we add.
+
+static inline unsigned long native_read_cr8(void)
+{
+ unsigned long cr8;
+ asm volatile(mov %%cr8,%0 : =r (cr8), =m (__force_order));
+ return cr8;
+}
+
There is no cr8 register on i386. This had better be protected by an
#ifdef.
Sure. I mentioned it in the changelog. I, however, am not sure If I
agree it should be enclosed
in ifdefs. Me and Jeremy discussed it a while ago, and we seem to
agree that for those functions
that are exclusive of one architecture, there were no need for ifdefs.
Any usage by the other arch
is a bug.
But I admit that I'm not particularly biased here, and I can change
it, if there's agreement that
an ifdef here is the way to go.
(you're likely not getting an error since it's a static inline, so the
asm is never emitted)
Which also means it does not affect the binary in anyway. No bigger
code, no nothing.
My current approach is to save the ifdefs for pieces that in fact can
save us some resources
in the final image. But again... I can change it.
--
Glauber de Oliveira Costa.
Free as in Freedom
http://glommer.net
The less confident you are, the more serious you have to act.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] unify paravirt parts of system.h
On Tue, Dec 04, 2007 at 09:18:33PM +0200, Avi Kivity wrote:
Glauber de Oliveira Costa wrote:
This patch moves the i386 control registers manipulation functions,
wbinvd, and clts functions to system.h. They are essentially the same
as in x86_64, except for the cr8 register, which we add.
+
+static inline unsigned long native_read_cr8(void)
+{
+unsigned long cr8;
+asm volatile(mov %%cr8,%0 : =r (cr8), =m (__force_order));
+return cr8;
+}
+
There is no cr8 register on i386. This had better be protected by an
#ifdef.
(you're likely not getting an error since it's a static inline, so the asm
is never emitted)
Linux never uses that register. The only user is suspend save/restore,
but that' bogus because it wasn't ever initialized by Linux in the first
place. It could be probably all safely removed.
-Andi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] unify paravirt parts of system.h
On Tuesday 04 December 2007 11:41, Glauber de Oliveira Costa wrote: On Dec 4, 2007 5:18 PM, Avi Kivity [EMAIL PROTECTED] wrote: There is no cr8 register on i386. This had better be protected by an #ifdef. Sure. I mentioned it in the changelog. I, however, am not sure If I agree it should be enclosed in ifdefs. Me and Jeremy discussed it a while ago, and we seem to agree that for those functions that are exclusive of one architecture, there were no need for ifdefs. Any usage by the other arch is a bug. But I admit that I'm not particularly biased here, and I can change it, if there's agreement that an ifdef here is the way to go. (you're likely not getting an error since it's a static inline, so the asm is never emitted) Which also means it does not affect the binary in anyway. No bigger code, no nothing. If future changes will mistakenly make 32-bit x86 call native_read_cr8(), you will get no warning. (Hmmm. Maybe as will complain, I'm not sure). If it explicitly ifdefed out for 32 bits, it's easier to detect misuse. -- vda -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
