Re: [PATCH] Cleanup f00f bug code

Brian Gerst
2001-09-29T13:40:09Z

[-- Attachment #1: Type: text/plain, Size: 889 bytes --]

Mikael Pettersson wrote:
>
> On Wed, 26 Sep 2001 21:52:09 -0400, Brian Gerst wrote:
>
> >This patch changes the f00f bug workaround code to use a fixed mapping
> >instead of touching the page tables directly. It also removes the
> >assumption that only 686's don't have the bug. It has been tested for
> >normal operation of the relocated IDT, but I don't have a processor with
> >the bug to do a real test. Could someone with such a processor please
> >verify that the workaround still works?
>
> It doesn't. 2.4.9-ac16 + your patch booted ok on my P5MMX, and the
> kernel printed that it applied the F0 0F workaround, but executing
> F0 0F C7 C8 in user-space caused an instant lockup.
>
> /Mikael

Here is an updated patch that Mikael has verified as working. There was
a bug in set_pte_phys() which forced the read/write bit set in the page
table entry.

--

Brian Gerst

[-- Attachment #2: diff-f00f-3 --]
[-- Type: text/plain, Size: 4947 bytes --]

diff -urN linux-2.4.10/arch/i386/config.in linux/arch/i386/config.in
--- linux-2.4.10/arch/i386/config.in Mon Sep 24 09:24:08 2001
+++ linux/arch/i386/config.in Thu Sep 27 20:15:31 2001
@@ -52,6 +52,7 @@
define_int CONFIG_X86_L1_CACHE_SHIFT 4
define_bool CONFIG_RWSEM_GENERIC_SPINLOCK y
define_bool CONFIG_RWSEM_XCHGADD_ALGORITHM n
+ define_bool CONFIG_X86_F00F_BUG y
else
define_bool CONFIG_X86_WP_WORKS_OK y
define_bool CONFIG_X86_INVLPG y
@@ -66,17 +67,20 @@
define_int CONFIG_X86_L1_CACHE_SHIFT 4
define_bool CONFIG_X86_USE_STRING_486 y
define_bool CONFIG_X86_ALIGNMENT_16 y
+ define_bool CONFIG_X86_F00F_BUG y
fi
if [ "$CONFIG_M586" = "y" ]; then
define_int CONFIG_X86_L1_CACHE_SHIFT 5
define_bool CONFIG_X86_USE_STRING_486 y
define_bool CONFIG_X86_ALIGNMENT_16 y
+ define_bool CONFIG_X86_F00F_BUG y
fi
if [ "$CONFIG_M586TSC" = "y" ]; then
define_int CONFIG_X86_L1_CACHE_SHIFT 5
define_bool CONFIG_X86_USE_STRING_486 y
define_bool CONFIG_X86_ALIGNMENT_16 y
define_bool CONFIG_X86_TSC y
+ define_bool CONFIG_X86_F00F_BUG y
fi
if [ "$CONFIG_M586MMX" = "y" ]; then
define_int CONFIG_X86_L1_CACHE_SHIFT 5
@@ -84,6 +88,7 @@
define_bool CONFIG_X86_ALIGNMENT_16 y
define_bool CONFIG_X86_TSC y
define_bool CONFIG_X86_GOOD_APIC y
+ define_bool CONFIG_X86_F00F_BUG y
fi
if [ "$CONFIG_M686" = "y" ]; then
define_int CONFIG_X86_L1_CACHE_SHIFT 5
diff -urN linux-2.4.10/arch/i386/kernel/setup.c linux/arch/i386/kernel/setup.c
--- linux-2.4.10/arch/i386/kernel/setup.c Mon Sep 24 09:24:08 2001
+++ linux/arch/i386/kernel/setup.c Fri Sep 28 19:11:51 2001
@@ -1712,13 +1712,10 @@

static void __init init_intel(struct cpuinfo_x86 *c)
{
-#ifndef CONFIG_M686
- static int f00f_workaround_enabled = 0;
-#endif
char *p = NULL;
unsigned int l1i = 0, l1d = 0, l2 = 0, l3 = 0; /* Cache sizes */

-#ifndef CONFIG_M686
+#ifdef CONFIG_X86_F00F_BUG
/*
* All current models of Pentium and Pentium with MMX technology CPUs
* have the F0 0F bug, which lets nonpriviledged users lock up the system.
@@ -1726,6 +1723,8 @@
*/
c->f00f_bug = 0;
if ( c->x86 == 5 ) {
+ static int f00f_workaround_enabled = 0;
+
c->f00f_bug = 1;
if ( !f00f_workaround_enabled ) {
trap_init_f00f_bug();
diff -urN linux-2.4.10/arch/i386/kernel/traps.c linux/arch/i386/kernel/traps.c
--- linux-2.4.10/arch/i386/kernel/traps.c Mon Sep 24 09:24:08 2001
+++ linux/arch/i386/kernel/traps.c Fri Sep 28 19:11:46 2001
@@ -719,35 +719,17 @@

#endif /* CONFIG_MATH_EMULATION */

-#ifndef CONFIG_M686
+#ifdef CONFIG_X86_F00F_BUG
void __init trap_init_f00f_bug(void)
{
- unsigned long page;
- pgd_t * pgd;
- pmd_t * pmd;
- pte_t * pte;
-
- /*
- * Allocate a new page in virtual address space,
- * move the IDT into it and write protect this page.
- */
- page = (unsigned long) vmalloc(PAGE_SIZE);
- pgd = pgd_offset(&init_mm, page);
- pmd = pmd_offset(pgd, page);
- pte = pte_offset(pmd, page);
- __free_page(pte_page(*pte));
- *pte = mk_pte_phys(__pa(&idt_table), PAGE_KERNEL_RO);
- /*
- * Not that any PGE-capable kernel should have the f00f bug ...
- */
- __flush_tlb_all();
+ set_fixmap_readonly(FIX_F00F_IDT, __pa(&idt_table));

/*
* "idt" is magic - it overlaps the idt_descr
* variable so that updating idt will automatically
* update the idt descriptor..
*/
- idt = (struct desc_struct *)page;
+ idt = (struct desc_struct *) fix_to_virt(FIX_F00F_IDT);
__asm__ __volatile__("lidt %0": "=m" (idt_descr));
}
#endif
diff -urN linux-2.4.10/arch/i386/mm/init.c linux/arch/i386/mm/init.c
--- linux-2.4.10/arch/i386/mm/init.c Mon Sep 24 09:24:08 2001
+++ linux/arch/i386/mm/init.c Fri Sep 28 16:28:13 2001
@@ -125,9 +125,8 @@
extern char __init_begin, __init_end;

static inline void set_pte_phys (unsigned long vaddr,
- unsigned long phys, pgprot_t flags)
+ unsigned long phys, pgprot_t prot)
{
- pgprot_t prot;
pgd_t *pgd;
pmd_t *pmd;
pte_t *pte;
@@ -145,7 +144,6 @@
pte = pte_offset(pmd, vaddr);
if (pte_val(*pte))
pte_ERROR(*pte);
- pgprot_val(prot) = pgprot_val(PAGE_KERNEL) | pgprot_val(flags);
set_pte(pte, mk_pte_phys(phys, prot));

/*
diff -urN linux-2.4.10/include/asm-i386/fixmap.h linux/include/asm-i386/fixmap.h
--- linux-2.4.10/include/asm-i386/fixmap.h Wed Sep 26 20:34:06 2001
+++ linux/include/asm-i386/fixmap.h Fri Sep 28 18:52:48 2001
@@ -51,6 +51,9 @@
#ifdef CONFIG_X86_LOCAL_APIC
FIX_APIC_BASE, /* local (CPU) APIC) -- required for SMP or not */
#endif
+#ifdef CONFIG_X86_F00F_BUG
+ FIX_F00F_IDT, /* Virtual mapping for IDT */
+#endif
#ifdef CONFIG_X86_IO_APIC
FIX_IO_APIC_BASE_0,
FIX_IO_APIC_BASE_END = FIX_IO_APIC_BASE_0 + MAX_IO_APICS-1,
@@ -73,6 +76,8 @@

#define set_fixmap(idx, phys) \
__set_fixmap(idx, phys, PAGE_KERNEL)
+#define set_fixmap_readonly(idx, phys) \
+ __set_fixmap(idx, phys, PAGE_KERNEL_RO)
/*
* Some hardware wants to get fixmapped without caching.
*/