[PATCH 2/6] reboot.c to use struct Xgt_desc_struct

[PATCH 2/6] reboot.c to use struct Xgt_desc_struct

[Rusty Russell]

arch/i386/kernel/reboot.c defines its own struct to describe an ldt
entry: it should use struct Xgt_desc_struct (currently load_ldt is a
macro, so doesn't complain: paravirt patches make it warn).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Index: working-2.6.18-rc2-hg-paravirt/arch/i386/kernel/reboot.c
===================================================================
--- working-2.6.18-rc2-hg-paravirt.orig/arch/i386/kernel/reboot.c	2006-07-22 07:58:11.000000000 +1000
+++ working-2.6.18-rc2-hg-paravirt/arch/i386/kernel/reboot.c	2006-07-22 07:58:34.000000000 +1000
@@ -145,14 +145,10 @@
 	0x000092000100ffffULL	/* 16-bit real-mode 64k data at 0x00000100 */
 };
 
-static struct
-{
-	unsigned short       size __attribute__ ((packed));
-	unsigned long long * base __attribute__ ((packed));
-}
-real_mode_gdt = { sizeof (real_mode_gdt_entries) - 1, real_mode_gdt_entries },
-real_mode_idt = { 0x3ff, NULL },
-no_idt = { 0, NULL };
+static struct Xgt_desc_struct
+real_mode_gdt = { sizeof (real_mode_gdt_entries) - 1, (long)real_mode_gdt_entries },
+real_mode_idt = { 0x3ff, 0 },
+no_idt = { 0, 0 };
 
 
 /* This is 16-bit protected mode code to disable paging and the cache,

-- 
Help! Save Australia from the worst of the DMCA: http://linux.org.au/law

[PATCH 6/6] cpuid neatening.

[Rusty Russell]

Roll all the cpuid asm into one __cpuid call.  It's a little neater,
and also means only one place to patch for paravirtualization.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Index: working-2.6.18-rc2-hg-paravirt/include/asm-i386/processor.h
===================================================================
--- working-2.6.18-rc2-hg-paravirt.orig/include/asm-i386/processor.h	2006-07-21 20:27:59.000000000 +1000
+++ working-2.6.18-rc2-hg-paravirt/include/asm-i386/processor.h	2006-07-21 21:50:49.000000000 +1000
@@ -143,6 +143,18 @@
 #define X86_EFLAGS_VIP	0x00100000 /* Virtual Interrupt Pending */
 #define X86_EFLAGS_ID	0x00200000 /* CPUID detection flag */
 
+static inline void __cpuid(unsigned int *eax, unsigned int *ebx,
+			   unsigned int *ecx, unsigned int *edx)
+{
+	/* ecx is often an input as well as an output. */
+	__asm__("cpuid"
+		: "=a" (*eax),
+		  "=b" (*ebx),
+		  "=c" (*ecx),
+		  "=d" (*edx)
+		: "0" (*eax), "2" (*ecx));
+}
+
 /*
  * Generic CPUID function
  * clear %ecx since some cpus (Cyrix MII) do not set or clear %ecx
@@ -150,24 +162,18 @@
  */
 static inline void cpuid(unsigned int op, unsigned int *eax, unsigned int *ebx, unsigned int *ecx, unsigned int *edx)
 {
-	__asm__("cpuid"
-		: "=a" (*eax),
-		  "=b" (*ebx),
-		  "=c" (*ecx),
-		  "=d" (*edx)
-		: "0" (op), "c"(0));
+	*eax = op;
+	*ecx = 0;
+	__cpuid(eax, ebx, ecx, edx);
 }
 
 /* Some CPUID calls want 'count' to be placed in ecx */
 static inline void cpuid_count(int op, int count, int *eax, int *ebx, int *ecx,
-	       	int *edx)
+			       int *edx)
 {
-	__asm__("cpuid"
-		: "=a" (*eax),
-		  "=b" (*ebx),
-		  "=c" (*ecx),
-		  "=d" (*edx)
-		: "0" (op), "c" (count));
+	*eax = op;
+	*ecx = count;
+	__cpuid(eax, ebx, ecx, edx);
 }
 
 /*
@@ -175,42 +181,30 @@
  */
 static inline unsigned int cpuid_eax(unsigned int op)
 {
-	unsigned int eax;
+	unsigned int eax, ebx, ecx, edx;
 
-	__asm__("cpuid"
-		: "=a" (eax)
-		: "0" (op)
-		: "bx", "cx", "dx");
+	cpuid(op, &eax, &ebx, &ecx, &edx);
 	return eax;
 }
 static inline unsigned int cpuid_ebx(unsigned int op)
 {
-	unsigned int eax, ebx;
+	unsigned int eax, ebx, ecx, edx;
 
-	__asm__("cpuid"
-		: "=a" (eax), "=b" (ebx)
-		: "0" (op)
-		: "cx", "dx" );
+	cpuid(op, &eax, &ebx, &ecx, &edx);
 	return ebx;
 }
 static inline unsigned int cpuid_ecx(unsigned int op)
 {
-	unsigned int eax, ecx;
+	unsigned int eax, ebx, ecx, edx;
 
-	__asm__("cpuid"
-		: "=a" (eax), "=c" (ecx)
-		: "0" (op)
-		: "bx", "dx" );
+	cpuid(op, &eax, &ebx, &ecx, &edx);
 	return ecx;
 }
 static inline unsigned int cpuid_edx(unsigned int op)
 {
-	unsigned int eax, edx;
+	unsigned int eax, ebx, ecx, edx;
 
-	__asm__("cpuid"
-		: "=a" (eax), "=d" (edx)
-		: "0" (op)
-		: "bx", "cx");
+	cpuid(op, &eax, &ebx, &ecx, &edx);
 	return edx;
 }
 

-- 
Help! Save Australia from the worst of the DMCA: http://linux.org.au/law

RE: [PATCH 6/6] cpuid neatening.

[David Schwartz]

> +static inline void __cpuid(unsigned int *eax, unsigned int *ebx,
> +			   unsigned int *ecx, unsigned int *edx)
> +{
> +	/* ecx is often an input as well as an output. */
> +	__asm__("cpuid"
> +		: "=a" (*eax),
> +		  "=b" (*ebx),
> +		  "=c" (*ecx),
> +		  "=d" (*edx)
> +		: "0" (*eax), "2" (*ecx));
> +}
> +

	Shouldn't that be:

__asm__("cpuid"
: "+a" (*eax),
  "=b" (*ebx),
  "+c" (*ecx),
  "=d" (*edx)
);

	DS

Re: [PATCH 6/6] cpuid neatening.

[H. Peter Anvin]

Rusty Russell wrote:
> Roll all the cpuid asm into one __cpuid call.  It's a little neater,
> and also means only one place to patch for paravirtualization.

The whole point of those is to avoid the unnecessary write to memory and 
pick it back up again.  This patch reintroduces that ugliness.

You don't want to make it the obvious multi-return macro, either, 
because that code is known to break some old versions of gcc.

	-hpa

Re: [PATCH 6/6] cpuid neatening.

[Andi Kleen]

On Mon, Jul 24, 2006 at 04:58:07PM -0700, H. Peter Anvin wrote:
> Rusty Russell wrote:
> >Roll all the cpuid asm into one __cpuid call.  It's a little neater,
> >and also means only one place to patch for paravirtualization.
> 
> The whole point of those is to avoid the unnecessary write to memory and 
> pick it back up again.  This patch reintroduces that ugliness.

Modern gcc should optimize this when it is inlined. If it didn't
most abstracted C++ code would be quite unhappy.

Also as far as I know there is only a single time critical CPUID
in the code and it ignores all output arguments.

-Andi

RE: [PATCH 6/6] cpuid neatening.

[Rusty Russell]

> > +static inline void __cpuid(unsigned int *eax, unsigned int *ebx,
> > +			   unsigned int *ecx, unsigned int *edx)
> > +{
> > +	/* ecx is often an input as well as an output. */
> > +	__asm__("cpuid"
> > +		: "=a" (*eax),
> > +		  "=b" (*ebx),
> > +		  "=c" (*ecx),
> > +		  "=d" (*edx)
> > +		: "0" (*eax), "2" (*ecx));
> > +}
> > +
> 
> 	Shouldn't that be:
> 
> __asm__("cpuid"
> : "+a" (*eax),
>   "=b" (*ebx),
>   "+c" (*ecx),
>   "=d" (*edx)
> );

Perhaps?  I copied the existing code...

Rusty.
-- 
Help! Save Australia from the worst of the DMCA: http://linux.org.au/law