David VomLehn wrote:
On Wed, 2009-01-07 at 17:41 -0800, David Daney wrote:
This is a preliminary patch to allow the kernel to run in mapped
address space via a wired TLB entry. Probably in a future version I
would factor out the OCTEON specific parts to a separate patch.
Yes, please do the factoring.
Everything in good time. My real intent was to generate feedback about
the general ideas.
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 780b520..d9c46a4 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -1431,6 +1431,23 @@ config PAGE_SIZE_64KB
endchoice
+config MAPPED_KERNEL
+ bool "Mapped kernel"
+ help
+ Select this option if you want the kernel's code and data to
+ be in mapped memory. The kernel will be mapped using a
+ single wired TLB entry, thus reducing the number of
+ available TLB entries by one. Kernel modules will be able
+ to use a more efficient calling convention.
This is currently only supported on 64-bit processors, so this should
depend on CONFIG_64BIT.
It should be trivial to extend to 32-bit kernels as well. I may try it
on the mips32 based STB I have at home. But as it currently stands, you
are correct.
diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index 0bc2120..5468f6d 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
...
@@ -662,7 +670,7 @@ OBJCOPYFLAGS += --remove-section=.reginfo
CPPFLAGS_vmlinux.lds := \
$(KBUILD_CFLAGS) \
- -D"LOADADDR=$(load-y)" \
+ -D"LOADADDR=$(load-y)" $(PHYS_LOAD_ADDRESS) \
-D"JIFFIES=$(JIFFIES)" \
-D"DATAOFFSET=$(if $(dataoffset-y),$(dataoffset-y),0)"
It seems more consistent to just eliminate PHYS_LOAD_ADDRESS entirely
and add a line here reading:
-D"PHYSADDR=0x$(CONFIG_PHYS_LOAD_ADDRESS)" \
There is some macro trickery in vmlinux.lds.S that checks to see if it
is defined, so I cannot unconditionally define it.
diff --git a/arch/mips/include/asm/mach-cavium-octeon/kernel-entry-init.h b/arch/mips/include/asm/mach-cavium-octeon/kernel-entry-init.h
index 0b2b5eb..bf36d82 100644
--- a/arch/mips/include/asm/mach-cavium-octeon/kernel-entry-init.h
+++ b/arch/mips/include/asm/mach-cavium-octeon/kernel-entry-init.h
@@ -27,6 +27,56 @@
# a3 = address of boot descriptor block
.set push
.set arch=octeon
+#ifdef CONFIG_MAPPED_KERNEL
+ # Set up the TLB index 0 for wired access to kernel.
+ # Assume we were loaded with sufficient alignment so that we
+ # can cover the image with two pages.
This seems like a pretty big assumption. Possible ways to handle this:
o Generalize to handle n pages.
This is an optimization, burning through TLB entries is not going to
help things.
o Hang in a loop here if the assumption is not met
Possible.
o Check later on whether the assumption was true and print a message.
I'm not really sure how to do this last one, though.
Also possible.
This is all very low-level board code, if you want the optimization, you
need to load at a suitable physical address.
+ dla v0, _end
+ dla v1, _text
+ dsubu v0, v0, v1 # size of image
+ move v1, zero
+ li t1, -1 # shift count.
+1: dsrl v0, v0, 1 # mask into v1
+ dsll v1, v1, 1
+ daddiu t1, t1, 1
+ ori v1, v1, 1
+ bne v0, zero, 1b
+ daddiu t2, t1, -6
+ mtc0 v1, $5, 0 # PageMask
+ dla t3, 0xffffffffc0000000 # kernel address
I think this should be CKSSEG rather than a magic constant.
+ dmtc0 t3, $10, 0 # EntryHi
+ bal 1f
+1: dla v0, 0x7fffffff
Another magic constant; don't know if there is already a define that
really applies, though. Perhaps add something to asm-mips/inst.h?
Both are worth investigating. Each board could (and may have to) do it
differently.
diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index 1055348..b44bcf8 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -49,6 +49,8 @@
#include <asm/stacktrace.h>
#include <asm/irq.h>
+#include "../mm/uasm.h"
This looks like it would be a good idea to consider moving uasm.h to
include/asm-mips, or possibly splitting it into two header files, one of
which would move to include/asm-mips.
That was my thought as well. This being a quick-and-dirty hack, I took
the low road and did it this way.
+
extern void check_wait(void);
extern asmlinkage void r4k_wait(void);
extern asmlinkage void rollback_handle_int(void);
@@ -1295,9 +1297,18 @@ void *set_except_vector(int n, void *addr)
exception_handlers[n] = handler;
if (n == 0 && cpu_has_divec) {
- *(u32 *)(ebase + 0x200) = 0x08000000 |
- (0x03ffffff & (handler >> 2));
- local_flush_icache_range(ebase + 0x200, ebase + 0x204);
+ unsigned long jump_mask = ~((1 << 28) - 1);
The 28 is a magic constant specifying the number of bits of the offset
in a jump instruction. Perhaps define jump_mask in asm-mips/inst.h since
it is related to the instruction format?
Correct.
+ u32 *buf = (u32 *)(ebase + 0x200);
+ unsigned int k0 = 26;
You are using k0 as a constant by defining it as a variable. You could
just have a #define here, but my suggestion is that it would be better
to add defines to asm-mips/inst.h (something like "#define REG_K0 26"
might be suitable for meeting this particular need)
Yes, the constants should probably be factored out of the TLB and page
code. Then they could be used here as well.
[...]
- if (cpu_has_veic || cpu_has_vint)
+ if (cpu_has_veic || cpu_has_vint) {
ebase = (unsigned long) alloc_bootmem_low_pages(0x200 + VECTORSPACING*64);
- else {
+ } else {
Checkpatch will complain about this, and it doesn't really add value to
make the change.
IANACPL (I am not a checkpatch lawyer), but I think it is correct. If
one clause of an if has braces they both should. However that was left
over from my debugging and if it were to be changed, should be part of a
code cleanup patch.
diff --git a/arch/mips/mm/page.c b/arch/mips/mm/page.c
index 1417c64..0070aa0 100644
--- a/arch/mips/mm/page.c
+++ b/arch/mips/mm/page.c
@@ -687,3 +687,9 @@ void copy_page(void *to, void *from)
}
#endif /* CONFIG_SIBYTE_DMA_PAGEOPS */
+
+#ifdef CONFIG_MAPPED_KERNEL
+/* Initialized so it is not clobbered when .bss is zeroed. */
+unsigned long phys_to_kernel_offset = 1;
+unsigned long kernel_image_end = 1;
+#endif
Clearly there is some magic happening here, but the such wizardry needs
more documentation.
Clearly.
I can deduce that these must be overwritten before
we get to kernel_entry;
One of the first things done at kernel_entry is to zero out .bss, if you
want to communicate with things that happen before kernel_entry, you
cannot use .bss.
who sets these?
The code in kernel-entry-init.h
I don't know for sure what kernel_image_end is, but I am guessing that
it is the physical address of the end of the kernel.
It is the virtual address of the first page past the kernel's virtual
mapping. This is where we can start mapping modules.
David Daney
