[PATCH] i386: Omit pmap workaround on i486 or later.
As described in XXX comments, the workaround for memory mapping is
implemented for 80386 and it is unnecessary on i486 or later. Thus, it
is safe to omit that if the kernel is built for the recent (1989~)
processors.
Fuhito Inagawa pointed out the problem to me.
* i386/i386/trap.c (kernel_trap): Disable the workaround when the kernel
is built for i[456]86.
* i386/intel/pmap.c (pmap_protect, pmap_enter): Ditto.
* i386/intel/read_fault.c: Define intel_read_fault if and only if it is
necessary.
---
i386/i386/trap.c| 2 ++
i386/intel/pmap.c | 9 +
i386/intel/read_fault.c | 2 ++
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/i386/i386/trap.c b/i386/i386/trap.c
index 6470504..77aa6db 100644
--- a/i386/i386/trap.c
+++ b/i386/i386/trap.c
@@ -250,6 +250,7 @@ dump_ss(regs);
}
else
#endif /* MACH_KDB */
+#if !(__486__ || __586__ || __686__)
if ((code & T_PF_WRITE) == 0 &&
result == KERN_PROTECTION_FAILURE)
{
@@ -261,6 +262,7 @@ dump_ss(regs);
result = intel_read_fault(map,
trunc_page((vm_offset_t)subcode));
}
+#endif
if (result == KERN_SUCCESS) {
/*
diff --git a/i386/intel/pmap.c b/i386/intel/pmap.c
index e362b45..9a3a7ac 100644
--- a/i386/intel/pmap.c
+++ b/i386/intel/pmap.c
@@ -1711,17 +1711,17 @@ void pmap_protect(
return;
}
+#if !(__486__ || __586__ || __686__)
/*
* If write-protecting in the kernel pmap,
* remove the mappings; the i386 ignores
* the write-permission bit in kernel mode.
-*
-* XXX should be #if'd for i386
*/
if (map == kernel_pmap) {
pmap_remove(map, s, e);
return;
}
+#endif
SPLVM(spl);
simple_lock(&map->lock);
@@ -1812,14 +1812,14 @@ if (pmap_debug) printf("pmap(%lx, %lx)\n", v, pa);
if (pmap == kernel_pmap && (v < kernel_virtual_start || v >=
kernel_virtual_end))
panic("pmap_enter(%p, %p) falls in physical memory area!\n", v,
pa);
#endif
+
+#if !(__486__ || __586__ || __686__)
if (pmap == kernel_pmap && (prot & VM_PROT_WRITE) == 0
&& !wired /* hack for io_wire */ ) {
/*
* Because the 386 ignores write protection in kernel mode,
* we cannot enter a read-only kernel mapping, and must
* remove an existing mapping if changing it.
-*
-* XXX should be #if'd for i386
*/
PMAP_READ_LOCK(pmap, spl);
@@ -1836,6 +1836,7 @@ if (pmap_debug) printf("pmap(%lx, %lx)\n", v, pa);
PMAP_READ_UNLOCK(pmap, spl);
return;
}
+#endif
/*
* Must allocate a new pvlist entry while we're unlocked;
diff --git a/i386/intel/read_fault.c b/i386/intel/read_fault.c
index 4b1edce..f28cbbc 100644
--- a/i386/intel/read_fault.c
+++ b/i386/intel/read_fault.c
@@ -33,6 +33,7 @@
#include
+#if !(__486__ || __586__ || __686__)
/*
* Expansion of vm_fault for read fault in kernel mode.
* Must enter the mapping as writable, since the i386
@@ -174,3 +175,4 @@ intel_read_fault(
return (KERN_SUCCESS);
}
+#endif
--
2.7.3
Re: [PATCH] i386: Omit pmap workaround on i486 or later.
How about this patch?
Did I share this with you in a wrong way, or is it simply useless?
2016-06-18 22:04 GMT+09:00 Masanori Ogino :
> As described in XXX comments, the workaround for memory mapping is
> implemented for 80386 and it is unnecessary on i486 or later. Thus, it
> is safe to omit that if the kernel is built for the recent (1989~)
> processors.
>
> Fuhito Inagawa pointed out the problem to me.
>
> * i386/i386/trap.c (kernel_trap): Disable the workaround when the kernel
> is built for i[456]86.
> * i386/intel/pmap.c (pmap_protect, pmap_enter): Ditto.
> * i386/intel/read_fault.c: Define intel_read_fault if and only if it is
> necessary.
> ---
> i386/i386/trap.c| 2 ++
> i386/intel/pmap.c | 9 +
> i386/intel/read_fault.c | 2 ++
> 3 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/i386/i386/trap.c b/i386/i386/trap.c
> index 6470504..77aa6db 100644
> --- a/i386/i386/trap.c
> +++ b/i386/i386/trap.c
> @@ -250,6 +250,7 @@ dump_ss(regs);
> }
> else
> #endif /* MACH_KDB */
> +#if !(__486__ || __586__ || __686__)
> if ((code & T_PF_WRITE) == 0 &&
> result == KERN_PROTECTION_FAILURE)
> {
> @@ -261,6 +262,7 @@ dump_ss(regs);
> result = intel_read_fault(map,
> trunc_page((vm_offset_t)subcode));
> }
> +#endif
>
> if (result == KERN_SUCCESS) {
> /*
> diff --git a/i386/intel/pmap.c b/i386/intel/pmap.c
> index e362b45..9a3a7ac 100644
> --- a/i386/intel/pmap.c
> +++ b/i386/intel/pmap.c
> @@ -1711,17 +1711,17 @@ void pmap_protect(
> return;
> }
>
> +#if !(__486__ || __586__ || __686__)
> /*
> * If write-protecting in the kernel pmap,
> * remove the mappings; the i386 ignores
> * the write-permission bit in kernel mode.
> -*
> -* XXX should be #if'd for i386
> */
> if (map == kernel_pmap) {
> pmap_remove(map, s, e);
> return;
> }
> +#endif
>
> SPLVM(spl);
> simple_lock(&map->lock);
> @@ -1812,14 +1812,14 @@ if (pmap_debug) printf("pmap(%lx, %lx)\n", v, pa);
> if (pmap == kernel_pmap && (v < kernel_virtual_start || v >=
> kernel_virtual_end))
> panic("pmap_enter(%p, %p) falls in physical memory area!\n",
> v, pa);
> #endif
> +
> +#if !(__486__ || __586__ || __686__)
> if (pmap == kernel_pmap && (prot & VM_PROT_WRITE) == 0
> && !wired /* hack for io_wire */ ) {
> /*
> * Because the 386 ignores write protection in kernel mode,
> * we cannot enter a read-only kernel mapping, and must
> * remove an existing mapping if changing it.
> -*
> -* XXX should be #if'd for i386
> */
> PMAP_READ_LOCK(pmap, spl);
>
> @@ -1836,6 +1836,7 @@ if (pmap_debug) printf("pmap(%lx, %lx)\n", v, pa);
> PMAP_READ_UNLOCK(pmap, spl);
> return;
> }
> +#endif
>
> /*
> * Must allocate a new pvlist entry while we're unlocked;
> diff --git a/i386/intel/read_fault.c b/i386/intel/read_fault.c
> index 4b1edce..f28cbbc 100644
> --- a/i386/intel/read_fault.c
> +++ b/i386/intel/read_fault.c
> @@ -33,6 +33,7 @@
>
> #include
>
> +#if !(__486__ || __586__ || __686__)
> /*
> * Expansion of vm_fault for read fault in kernel mode.
> * Must enter the mapping as writable, since the i386
> @@ -174,3 +175,4 @@ intel_read_fault(
>
> return (KERN_SUCCESS);
> }
> +#endif
> --
> 2.7.3
>
--
Masanori Ogino
http://twitter.com/omasanori
http://gplus.to/omasanori
Re: [PATCH] i386: Omit pmap workaround on i486 or later.
Hello, 2016-07-08 17:43 GMT+09:00 Justus Winter : > Hi :) > > Masanori Ogino writes: >> How about this patch? >> Did I share this with you in a wrong way, or is it simply useless? >> >> 2016-06-18 22:04 GMT+09:00 Masanori Ogino : >>> As described in XXX comments, the workaround for memory mapping is >>> implemented for 80386 and it is unnecessary on i486 or later. Thus, it >>> is safe to omit that if the kernel is built for the recent (1989~) >>> processors. > > Sweet, thanks for the patch. I included it in my builds, and it seems > good. However, I did not understand the change in detail, could you > motivate it a little? Or if others agree with the change, I'll merge > it. OK. Let me explain it. According to the comment in pmap.c, line 1715 and below, i386 ignores the read-write bit of page table entries (PTEs) in kernel (a.k.a. supervisor or privileged) mode. This is why pmap_protect (pmap.c, line 1684~) and pmap_enter (pmap.c, line 1791~) treats the combination of read-only mode and kernel mode as a special case. Moreover, kernel_trap (trap.c, line 1521~) have to try read-write access to pmap first when a page fault occurs. (I couldn't find any page describing the i386 bug, though. Probably there were certain revisions with the bug but the others worked fine.) The patch is intended to remove the workaround to use hardware write-protection properly in kernel mode on recent processors and it might boost performance. Anyway, I found a bug in the patch now; the vm_fault call in trap.c, line 235 should check whether (code & T_PF_WRITE) is set or not on i486 or later. In other words, `VM_PROT_READ|VM_PROT_WRITE` in line 237 should be `(code & T_PF_WRITE) ? VM_PROT_READ|VM_PROT_WRITE : VM_PROT_READ` on recent processors. I will share the v2 patch after checking the code again. -- Masanori Ogino
Re: [PATCH] i386: Omit pmap workaround on i486 or later.
Hello, 2016-07-08 15:35 GMT+09:00 Samuel Thibault : > Masanori Ogino, on Fri 08 Jul 2016 10:43:00 +0900, wrote: >> Did I share this with you in a wrong way, or is it simply useless? > > Personnally, I simply currently have no time to spend on it :/ I see. No worry, I would introduce a bug to the kernel if you are hasty :) -- Masanori Ogino
Re: [PATCH] i386: Omit pmap workaround on i486 or later.
Awesome! Thank you so much for picking it up, and I am so sorry for my laziness. On Sun, Nov 8, 2020 at 11:13 AM Samuel Thibault wrote: > > Hello, > > I fixed the patch and pushed it, thanks! > > Samuel -- Masanori Ogino http://twitter.com/omasanori http://gplus.to/omasanori
