Skip to contentSkip to conversation

reboot.c to use struct Xgt_desc_struct

PATCH In review Rusty Russell 4 participants 5 messages Jul 22, 2006 – Jul 25, 2006 2 messages not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

#1 lore raw

PATCH 2/6 reboot.c to use struct Xgt_desc_struct

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).

arch/i386/kernel/reboot.c +4 -8
Index: working-2.6.18-rc2-hg-paravirt/arch/i386/kernel/reboot.c
===================================================================
@@ -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,
Signature · 1 line
Help! Save Australia from the worst of the DMCA: http://linux.org.au/law

A message that is not in the archive (sent off-list or not yet received): [email protected]

#3 lore raw

PATCH 6/6 cpuid neatening.

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

include/asm-i386/processor.h +27 -33
Index: working-2.6.18-rc2-hg-paravirt/include/asm-i386/processor.h
===================================================================
@@ -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;
 }
 
Signature · 1 line
Help! Save Australia from the worst of the DMCA: http://linux.org.au/law
#4 lore raw

RE: [PATCH 6/6] cpuid neatening.

include/asm-i386/processor.h line 157

Quoted 13 lines
include/asm-i386/processor.h +12 -0
+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

#5 lore raw

Re: [PATCH 6/6] cpuid neatening.

In reply to Rusty Russell
Quoted 2 lines · 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.

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

#6 lore raw

Re: [PATCH 6/6] cpuid neatening.

Quoted 6 lines · H. Peter Anvin
Quoted 2 lines · 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.

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