[PATCH] i386: inline asm cleanup

i386 Inline asm cleanup.  Use cr/dr accessor functions.

Also, a potential bugfix.  Also, some CR accessors really should be volatile.
Reads from CR0 (numeric state may change in an exception handler), writes to
CR4 (flipping CR4.TSD) and reads from CR2 (page fault) prevent instruction
re-ordering.  I did not add memory clobber to CR3 / CR4 / CR0 updates, as it
was not there to begin with, and in no case should kernel memory be clobbered,
except when doing a TLB flush, which already has memory clobber.

I noticed that page invalidation does not have a memory clobber.  I can't find
a bug as a result, but there is definitely a potential for a bug here:

#define __flush_tlb_single(addr) \
	__asm__ __volatile__("invlpg %0": :"m" (*(char *) addr))

Signed-off-by: Zachary Amsden <zach@vmware.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/arch/i386/kernel/cpu/common.c b/arch/i386/kernel/cpu/common.c
index 4553ffd94b1f9f2a645e75ba6982e80c40543cd0..361f2e7ccb12274385902f12bb9cd7f159ffaee1 100644 (file)
--- a/arch/i386/kernel/cpu/common.c
+++ b/arch/i386/kernel/cpu/common.c
@@ -642,12 +642,12 @@ void __devinit cpu_init(void)
        asm volatile ("xorl %eax, %eax; movl %eax, %fs; movl %eax, %gs");
 
        /* Clear all 6 debug registers: */
-
-#define CD(register) set_debugreg(0, register)
-
-       CD(0); CD(1); CD(2); CD(3); /* no db4 and db5 */; CD(6); CD(7);
-
-#undef CD
+       set_debugreg(0, 0);
+       set_debugreg(0, 1);
+       set_debugreg(0, 2);
+       set_debugreg(0, 3);
+       set_debugreg(0, 6);
+       set_debugreg(0, 7);
 
        /*
         * Force FPU initialization:

diff --git a/arch/i386/kernel/cpu/cpufreq/longhaul.c b/arch/i386/kernel/cpu/cpufreq/longhaul.c
index 04e3563da4fea19e2a7f0c2c0e0053855cda67f1..bf02b5026e6238ea13ff4743d856ecc5b9be5206 100644 (file)
--- a/arch/i386/kernel/cpu/cpufreq/longhaul.c
+++ b/arch/i386/kernel/cpu/cpufreq/longhaul.c
@@ -64,8 +64,6 @@ static int dont_scale_voltage;
 #define dprintk(msg...) cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "longhaul", msg)
 
 
-#define __hlt()     __asm__ __volatile__("hlt": : :"memory")
-
 /* Clock ratios multiplied by 10 */
 static int clock_ratio[32];
 static int eblcr_table[32];
@@ -168,11 +166,9 @@ static void do_powersaver(union msr_longhaul *longhaul,
        outb(0xFE,0x21);        /* TMR0 only */
        outb(0xFF,0x80);        /* delay */
 
-       local_irq_enable();
-
-       __hlt();
+       safe_halt();
        wrmsrl(MSR_VIA_LONGHAUL, longhaul->val);
-       __hlt();
+       halt();
 
        local_irq_disable();
 
@@ -251,9 +247,7 @@ static void longhaul_setstate(unsigned int clock_ratio_index)
                bcr2.bits.CLOCKMUL = clock_ratio_index;
                local_irq_disable();
                wrmsrl (MSR_VIA_BCR2, bcr2.val);
-               local_irq_enable();
-
-               __hlt();
+               safe_halt();
 
                /* Disable software clock multiplier */
                rdmsrl (MSR_VIA_BCR2, bcr2.val);

diff --git a/arch/i386/kernel/cpu/cyrix.c b/arch/i386/kernel/cpu/cyrix.c
index ba4b01138c8f62d9a01c5b3f2d10549f3edc54c5..ff87cc22b323aee87165e19be9688bc300dcc978 100644 (file)
--- a/arch/i386/kernel/cpu/cyrix.c
+++ b/arch/i386/kernel/cpu/cyrix.c
@@ -132,11 +132,7 @@ static void __init set_cx86_memwb(void)
        setCx86(CX86_CCR2, getCx86(CX86_CCR2) & ~0x04);
        /* set 'Not Write-through' */
        cr0 = 0x20000000;
-       __asm__("movl %%cr0,%%eax\n\t"
-               "orl %0,%%eax\n\t"
-               "movl %%eax,%%cr0\n"
-               : : "r" (cr0)
-               :"ax");
+       write_cr0(read_cr0() | cr0);
        /* CCR2 bit 2: lock NW bit and set WT1 */
        setCx86(CX86_CCR2, getCx86(CX86_CCR2) | 0x14 );
 }

diff --git a/arch/i386/kernel/efi.c b/arch/i386/kernel/efi.c
index 850648ae8305c17249307985790af915157f94e2..921fdb15fc9be234282090e9a3616d084a61217f 100644 (file)
--- a/arch/i386/kernel/efi.c
+++ b/arch/i386/kernel/efi.c
@@ -79,7 +79,7 @@ static void efi_call_phys_prelog(void)
         * directory. If I have PSE, I just need to duplicate one entry in
         * page directory.
         */
-       __asm__ __volatile__("movl %%cr4, %0":"=r"(cr4));
+       cr4 = read_cr4();
 
        if (cr4 & X86_CR4_PSE) {
                efi_bak_pg_dir_pointer[0].pgd =
@@ -115,7 +115,7 @@ static void efi_call_phys_epilog(void)
        cpu_gdt_descr[0].address =
                (unsigned long) __va(cpu_gdt_descr[0].address);
        __asm__ __volatile__("lgdt %0":"=m"(cpu_gdt_descr));
-       __asm__ __volatile__("movl %%cr4, %0":"=r"(cr4));
+       cr4 = read_cr4();
 
        if (cr4 & X86_CR4_PSE) {
                swapper_pg_dir[pgd_index(0)].pgd =

diff --git a/arch/i386/kernel/machine_kexec.c b/arch/i386/kernel/machine_kexec.c
index cb699a2aa1f84a915c8a1105fe66f19548adec10..f19f6d34bcbff9506c1a619a34ff50e7c246bd74 100644 (file)
--- a/arch/i386/kernel/machine_kexec.c
+++ b/arch/i386/kernel/machine_kexec.c
@@ -17,13 +17,7 @@
 #include <asm/apic.h>
 #include <asm/cpufeature.h>
 #include <asm/desc.h>
-
-static inline unsigned long read_cr3(void)
-{
-       unsigned long cr3;
-       asm volatile("movl %%cr3,%0": "=r"(cr3));
-       return cr3;
-}
+#include <asm/system.h>
 
 #define PAGE_ALIGNED __attribute__ ((__aligned__(PAGE_SIZE)))
 

diff --git a/arch/i386/kernel/process.c b/arch/i386/kernel/process.c
index e3f362e8af5b33e194ac5b7bad3a9ec294f0a030..761d4ed47ef38e126aa964a531d99d63732d22fb 100644 (file)
--- a/arch/i386/kernel/process.c
+++ b/arch/i386/kernel/process.c
@@ -313,16 +313,12 @@ void show_regs(struct pt_regs * regs)
        printk(" DS: %04x ES: %04x\n",
                0xffff & regs->xds,0xffff & regs->xes);
 
-       __asm__("movl %%cr0, %0": "=r" (cr0));
-       __asm__("movl %%cr2, %0": "=r" (cr2));
-       __asm__("movl %%cr3, %0": "=r" (cr3));
-       /* This could fault if %cr4 does not exist */
-       __asm__("1: movl %%cr4, %0              \n"
-               "2:                             \n"
-               ".section __ex_table,\"a\"      \n"
-               ".long 1b,2b                    \n"
-               ".previous                      \n"
-               : "=r" (cr4): "0" (0));
+       cr0 = read_cr0();
+       cr2 = read_cr2();
+       cr3 = read_cr3();
+       if (current_cpu_data.x86 > 4) {
+               cr4 = read_cr4();
+       }
        printk("CR0: %08lx CR2: %08lx CR3: %08lx CR4: %08lx\n", cr0, cr2, cr3, cr4);
        show_trace(NULL, &regs->esp);
 }

diff --git a/arch/i386/kernel/smp.c b/arch/i386/kernel/smp.c
index cec4bde67161254106267d6c6e4715aaeea93a3a..48b55db3680f3eaac2b1a67274343677ceb1a32d 100644 (file)
--- a/arch/i386/kernel/smp.c
+++ b/arch/i386/kernel/smp.c
@@ -576,7 +576,7 @@ static void stop_this_cpu (void * dummy)
        local_irq_disable();
        disable_local_APIC();
        if (cpu_data[smp_processor_id()].hlt_works_ok)
-               for(;;) __asm__("hlt");
+               for(;;) halt();
        for (;;);
 }
 

diff --git a/arch/i386/mm/fault.c b/arch/i386/mm/fault.c
index 61d9e34af5a6c665e5c4cb5fc78dd4d9a31f8290..411b8500ad1b07736c92412fb3eaced4e5baf4f8 100644 (file)
--- a/arch/i386/mm/fault.c
+++ b/arch/i386/mm/fault.c
@@ -233,7 +233,7 @@ fastcall void do_page_fault(struct pt_regs *regs, unsigned long error_code)
        int write, si_code;
 
        /* get the address */
-       __asm__("movl %%cr2,%0":"=r" (address));
+        address = read_cr2();
 
        if (notify_die(DIE_PAGE_FAULT, "page fault", regs, error_code, 14,
                                        SIGSEGV) == NOTIFY_STOP)
@@ -453,7 +453,7 @@ no_context:
        printk(" at virtual address %08lx\n",address);
        printk(KERN_ALERT " printing eip:\n");
        printk("%08lx\n", regs->eip);
-       asm("movl %%cr3,%0":"=r" (page));
+       page = read_cr3();
        page = ((unsigned long *) __va(page))[address >> 22];
        printk(KERN_ALERT "*pde = %08lx\n", page);
        /*
@@ -526,7 +526,7 @@ vmalloc_fault:
                pmd_t *pmd, *pmd_k;
                pte_t *pte_k;
 
-               asm("movl %%cr3,%0":"=r" (pgd_paddr));
+               pgd_paddr = read_cr3();
                pgd = index + (pgd_t *)__va(pgd_paddr);
                pgd_k = init_mm.pgd + index;
 

diff --git a/arch/i386/mm/pageattr.c b/arch/i386/mm/pageattr.c
index cb3da6baa704208d4faf2977d411190d9a2aa88f..bce06a79eafa556ecac212c3167bf02f34bce234 100644 (file)
--- a/arch/i386/mm/pageattr.c
+++ b/arch/i386/mm/pageattr.c
@@ -62,7 +62,7 @@ static void flush_kernel_map(void *dummy)
 { 
        /* Could use CLFLUSH here if the CPU supports it (Hammer,P4) */
        if (boot_cpu_data.x86_model >= 4) 
-               asm volatile("wbinvd":::"memory"); 
+               wbinvd();
        /* Flush all to work around Errata in early athlons regarding 
         * large page flushing. 
         */

diff --git a/arch/i386/power/cpu.c b/arch/i386/power/cpu.c
index c547c1af6fa1a86ea8e25f4dd62332c3f3302faa..4e19c43e09541bbbb3d429bdcbd1acd0556d3ea2 100644 (file)
--- a/arch/i386/power/cpu.c
+++ b/arch/i386/power/cpu.c
@@ -57,10 +57,10 @@ void __save_processor_state(struct saved_context *ctxt)
        /*
         * control registers 
         */
-       asm volatile ("movl %%cr0, %0" : "=r" (ctxt->cr0));
-       asm volatile ("movl %%cr2, %0" : "=r" (ctxt->cr2));
-       asm volatile ("movl %%cr3, %0" : "=r" (ctxt->cr3));
-       asm volatile ("movl %%cr4, %0" : "=r" (ctxt->cr4));
+       ctxt->cr0 = read_cr0();
+       ctxt->cr2 = read_cr2();
+       ctxt->cr3 = read_cr3();
+       ctxt->cr4 = read_cr4();
 }
 
 void save_processor_state(void)
@@ -109,10 +109,10 @@ void __restore_processor_state(struct saved_context *ctxt)
        /*
         * control registers
         */
-       asm volatile ("movl %0, %%cr4" :: "r" (ctxt->cr4));
-       asm volatile ("movl %0, %%cr3" :: "r" (ctxt->cr3));
-       asm volatile ("movl %0, %%cr2" :: "r" (ctxt->cr2));
-       asm volatile ("movl %0, %%cr0" :: "r" (ctxt->cr0));
+       write_cr4(ctxt->cr4);
+       write_cr3(ctxt->cr3);
+       write_cr2(ctxt->cr2);
+       write_cr2(ctxt->cr0);
 
        /*
         * now restore the descriptor tables to their proper values

diff --git a/include/asm-i386/agp.h b/include/asm-i386/agp.h
index b82f5f3ab887b4307fa2e248c2122bdfe8d32bfc..9075083bab76bd2ed3cd26daaaaa63887dbf5255 100644 (file)
--- a/include/asm-i386/agp.h
+++ b/include/asm-i386/agp.h
@@ -19,7 +19,7 @@ int unmap_page_from_agp(struct page *page);
 /* Could use CLFLUSH here if the cpu supports it. But then it would
    need to be called for each cacheline of the whole page so it may not be 
    worth it. Would need a page for it. */
-#define flush_agp_cache() asm volatile("wbinvd":::"memory")
+#define flush_agp_cache() wbinvd()
 
 /* Convert a physical address to an address suitable for the GART. */
 #define phys_to_gart(x) (x)

diff --git a/include/asm-i386/bugs.h b/include/asm-i386/bugs.h
index 6789fc275da36cc3f5da7da3efc03609662f7bdb..ea54540638d211f2bb99fc4693e603a664b48dad 100644 (file)
--- a/include/asm-i386/bugs.h
+++ b/include/asm-i386/bugs.h
@@ -118,7 +118,10 @@ static void __init check_hlt(void)
                printk("disabled\n");
                return;
        }
-       __asm__ __volatile__("hlt ; hlt ; hlt ; hlt");
+       halt();
+       halt();
+       halt();
+       halt();
        printk("OK.\n");
 }
 

diff --git a/include/asm-i386/processor.h b/include/asm-i386/processor.h
index d0d8b0160090427328fc384baed9ab77872ecb88..7e17d3b4f65a52ff934c8bbeb0d488ea24c120a6 100644 (file)
--- a/include/asm-i386/processor.h
+++ b/include/asm-i386/processor.h
@@ -203,9 +203,7 @@ static inline unsigned int cpuid_edx(unsigned int op)
        return edx;
 }
 
-#define load_cr3(pgdir) \
-       asm volatile("movl %0,%%cr3": :"r" (__pa(pgdir)))
-
+#define load_cr3(pgdir) write_cr3(__pa(pgdir))
 
 /*
  * Intel CPU features in CR4
@@ -232,22 +230,20 @@ extern unsigned long mmu_cr4_features;
 
 static inline void set_in_cr4 (unsigned long mask)
 {
+       unsigned cr4;
        mmu_cr4_features |= mask;
-       __asm__("movl %%cr4,%%eax\n\t"
-               "orl %0,%%eax\n\t"
-               "movl %%eax,%%cr4\n"
-               : : "irg" (mask)
-               :"ax");
+       cr4 = read_cr4();
+       cr4 |= mask;
+       write_cr4(cr4);
 }
 
 static inline void clear_in_cr4 (unsigned long mask)
 {
+       unsigned cr4;
        mmu_cr4_features &= ~mask;
-       __asm__("movl %%cr4,%%eax\n\t"
-               "andl %0,%%eax\n\t"
-               "movl %%eax,%%cr4\n"
-               : : "irg" (~mask)
-               :"ax");
+       cr4 = read_cr4();
+       cr4 &= ~mask;
+       write_cr4(cr4);
 }
 
 /*

diff --git a/include/asm-i386/system.h b/include/asm-i386/system.h
index 3db717a244f0c4df75a06102c522e6cdf19f82b1..8048a5e018cd1953fb097e3531d0fea6cbc83254 100644 (file)
--- a/include/asm-i386/system.h
+++ b/include/asm-i386/system.h
@@ -107,13 +107,33 @@ static inline unsigned long _get_base(char * addr)
 #define clts() __asm__ __volatile__ ("clts")
 #define read_cr0() ({ \
        unsigned int __dummy; \
-       __asm__( \
+       __asm__ __volatile__( \
                "movl %%cr0,%0\n\t" \
                :"=r" (__dummy)); \
        __dummy; \
 })
 #define write_cr0(x) \
-       __asm__("movl %0,%%cr0": :"r" (x));
+       __asm__ __volatile__("movl %0,%%cr0": :"r" (x));
+
+#define read_cr2() ({ \
+       unsigned int __dummy; \
+       __asm__ __volatile__( \
+               "movl %%cr2,%0\n\t" \
+               :"=r" (__dummy)); \
+       __dummy; \
+})
+#define write_cr2(x) \
+       __asm__ __volatile__("movl %0,%%cr2": :"r" (x));
+
+#define read_cr3() ({ \
+       unsigned int __dummy; \
+       __asm__ ( \
+               "movl %%cr3,%0\n\t" \
+               :"=r" (__dummy)); \
+       __dummy; \
+})
+#define write_cr3(x) \
+       __asm__ __volatile__("movl %0,%%cr3": :"r" (x));
 
 #define read_cr4() ({ \
        unsigned int __dummy; \
@@ -123,7 +143,7 @@ static inline unsigned long _get_base(char * addr)
        __dummy; \
 })
 #define write_cr4(x) \
-       __asm__("movl %0,%%cr4": :"r" (x));
+       __asm__ __volatile__("movl %0,%%cr4": :"r" (x));
 #define stts() write_cr0(8 | read_cr0())
 
 #endif /* __KERNEL__ */
@@ -447,6 +467,8 @@ struct alt_instr {
 #define local_irq_enable()     __asm__ __volatile__("sti": : :"memory")
 /* used in the idle loop; sti takes one instruction cycle to complete */
 #define safe_halt()            __asm__ __volatile__("sti; hlt": : :"memory")
+/* used when interrupts are already enabled or to shutdown the processor */
+#define halt()                 __asm__ __volatile__("hlt": : :"memory")
 
 #define irqs_disabled()                        \
 ({                                     \

diff --git a/include/asm-i386/xor.h b/include/asm-i386/xor.h
index f80e2dbe1b56a541fc1be0e37dff529ffb46a08c..23c86cef3b258a8611e765bd168e1c89054a19f6 100644 (file)
--- a/include/asm-i386/xor.h
+++ b/include/asm-i386/xor.h
@@ -535,14 +535,14 @@ static struct xor_block_template xor_block_p5_mmx = {
 
 #define XMMS_SAVE do {                         \
        preempt_disable();                      \
+       cr0 = read_cr0();                       \
+       clts();                                 \
        __asm__ __volatile__ (                  \
-               "movl %%cr0,%0          ;\n\t"  \
-               "clts                   ;\n\t"  \
-               "movups %%xmm0,(%1)     ;\n\t"  \
-               "movups %%xmm1,0x10(%1) ;\n\t"  \
-               "movups %%xmm2,0x20(%1) ;\n\t"  \
-               "movups %%xmm3,0x30(%1) ;\n\t"  \
-               : "=&r" (cr0)                   \
+               "movups %%xmm0,(%0)     ;\n\t"  \
+               "movups %%xmm1,0x10(%0) ;\n\t"  \
+               "movups %%xmm2,0x20(%0) ;\n\t"  \
+               "movups %%xmm3,0x30(%0) ;\n\t"  \
+               :                               \
                : "r" (xmm_save)                \
                : "memory");                    \
 } while(0)
@@ -550,14 +550,14 @@ static struct xor_block_template xor_block_p5_mmx = {
 #define XMMS_RESTORE do {                      \
        __asm__ __volatile__ (                  \
                "sfence                 ;\n\t"  \
-               "movups (%1),%%xmm0     ;\n\t"  \
-               "movups 0x10(%1),%%xmm1 ;\n\t"  \
-               "movups 0x20(%1),%%xmm2 ;\n\t"  \
-               "movups 0x30(%1),%%xmm3 ;\n\t"  \
-               "movl   %0,%%cr0        ;\n\t"  \
+               "movups (%0),%%xmm0     ;\n\t"  \
+               "movups 0x10(%0),%%xmm1 ;\n\t"  \
+               "movups 0x20(%0),%%xmm2 ;\n\t"  \
+               "movups 0x30(%0),%%xmm3 ;\n\t"  \
                :                               \
-               : "r" (cr0), "r" (xmm_save)     \
+               : "r" (xmm_save)                \
                : "memory");                    \
+       write_cr0(cr0);                         \
        preempt_enable();                       \
 } while(0)