On Wed, Jul 19, 2017 at 05:37:49AM +0300, Artturi Alm wrote: > On Mon, Jul 10, 2017 at 04:19:12PM +0300, Artturi Alm wrote: > > Hi, > > > > > > this diff does seem bigger than it is, because this does move the exception > > handler entrys from arm/exceptions.S to arm/vectors.S, while removing > > a round of useless indirection that was needed more before VBAR, which > > can be found supported even on some V6ses w/extensions(ARM11), so this is > > nothing new or anything that anyone should be afraid of, imo. > > i > > +Before anyone goes liek "you just broke FIQs!", no, i didn't, and depending > > on defines to it, the FIQs might try to get ran on .data section, which is > > not > > OK, imo., and i have some doubt that no-one has ever enabled that > > FIQ-support > > on OpenBSD/armv7, so no use-case = no fiq fix in this diff, > > which would likely be less than 10lines of code w/some thought. > > > > shortly; i added these +++++ to locore0.S: > > > > mcr CP15_DACR(r0) > > > > + /* set VBAR */ > > + ldr r0, =vectors > > + mcr CP15_VBAR(r0) > > + > > /* Enable MMU */ > > mrc CP15_SCTLR(r0) > > + bic r0, r0, #CPU_CONTROL_VECRELOC /* our vectors are at VBAR */ > > orr r0, r0, #CPU_CONTROL_MMU_ENABLE > > mcr CP15_SCTLR(r0) > > CPWAIT(r0) > > > > > > and removed the initialization/indirection/special-casing that did still > > exist > > for the exception handlers/vector_page/systempage. > > > > > > Works for me on cubieb2&wandb. > > -Artturi > > > > > > Hi, > > updated diff below, which does not move/use the C_OBJECT()-define, > and potentially fixes FIQs, if someone was to enable them. > Removes arm/arm/fiq_subr.S as a side-effect to touching FIQs, sry. > > -Artturi >
Hi, let's try the minimal diff, for which i don't think i could shove the actual improvements into any smaller diff not resulting in unused leftovers/broken build. 13 files changed, 47 insertions(+), 290 deletions(-) i can shrink the insertions a bit, if you don't care about my fix for the multi-line comment in vm_machdep or w/e. what i'm asking is a comment about static vectors in general/answer for "why not?", errr, i mean why insist on making a special case out of vectors when it clearly is not needed and does nothing but complicate the armv7 MD-code. -Artturi diff --git a/sys/arch/arm/arm/arm32_machdep.c b/sys/arch/arm/arm/arm32_machdep.c index 44ae69fa7f9..09b171373aa 100644 --- a/sys/arch/arm/arm/arm32_machdep.c +++ b/sys/arch/arm/arm/arm32_machdep.c @@ -115,62 +115,6 @@ void prefetch_abort_handler (trapframe_t *frame); extern void configure (void); /* - * arm32_vector_init: - * - * Initialize the vector page, and select whether or not to - * relocate the vectors. - * - * NOTE: We expect the vector page to be mapped at its expected - * destination. - */ -void -arm32_vector_init(vaddr_t va, int which) -{ - extern unsigned int page0[], page0_data[]; - unsigned int *vectors = (unsigned int *) va; - unsigned int *vectors_data = vectors + (page0_data - page0); - int vec; - - /* - * Loop through the vectors we're taking over, and copy the - * vector's insn and data word. - */ - for (vec = 0; vec < ARM_NVEC; vec++) { - if ((which & (1 << vec)) == 0) { - /* Don't want to take over this vector. */ - continue; - } - vectors[vec] = page0[vec]; - vectors_data[vec] = page0_data[vec]; - } - - /* Now sync the vectors. */ - cpu_icache_sync_range(va, (ARM_NVEC * 2) * sizeof(u_int)); - - vector_page = va; - - if (va == ARM_VECTORS_HIGH) { - /* - * Assume the MD caller knows what it's doing here, and - * really does want the vector page relocated. - * - * Note: This has to be done here (and not just in - * cpu_setup()) because the vector page needs to be - * accessible *before* main() is called. - * Think ddb(9) ... - * - * NOTE: If the CPU control register is not readable, - * this will totally fail! We'll just assume that - * any system that has high vector support has a - * readable CPU control register, for now. If we - * ever encounter one that does not, we'll have to - * rethink this. - */ - cpu_control(CPU_CONTROL_VECRELOC, CPU_CONTROL_VECRELOC); - } -} - -/* * Debug function just to park the CPU */ @@ -228,9 +172,6 @@ cpu_startup() paddr_t minaddr; paddr_t maxaddr; - /* Lock down zero page */ - vector_page_setprot(PROT_READ | PROT_EXEC); - /* * Give pmap a chance to set up a few more things now the vm * is initialised diff --git a/sys/arch/arm/arm/arm_machdep.c b/sys/arch/arm/arm/arm_machdep.c index f5871f3afb5..041507714cd 100644 --- a/sys/arch/arm/arm/arm_machdep.c +++ b/sys/arch/arm/arm/arm_machdep.c @@ -87,18 +87,6 @@ #include <machine/bus.h> /* - * The ARM architecture places the vector page at address 0. - * Later ARM architecture versions, however, allow it to be - * relocated to a high address (0xffff0000). This is primarily - * to support the Fast Context Switch Extension. - * - * This variable contains the address of the vector page. It - * defaults to 0; it only needs to be initialized if we enable - * relocated vectors. - */ -vaddr_t vector_page; - -/* * Clear registers on exec */ diff --git a/sys/arch/arm/arm/cpufunc.c b/sys/arch/arm/arm/cpufunc.c index c91108e7066..7f77d89045b 100644 --- a/sys/arch/arm/arm/cpufunc.c +++ b/sys/arch/arm/arm/cpufunc.c @@ -402,9 +402,6 @@ armv7_setup() | CPU_CONTROL_IC_ENABLE | CPU_CONTROL_AFE; - if (vector_page == ARM_VECTORS_HIGH) - cpuctrl |= CPU_CONTROL_VECRELOC; - /* * Check for the Virtualization Extensions and enable UWXN of * those are included. diff --git a/sys/arch/arm/arm/exception.S b/sys/arch/arm/arm/exception.S index f1bceac6864..680743a2585 100644 --- a/sys/arch/arm/arm/exception.S +++ b/sys/arch/arm/arm/exception.S @@ -91,36 +91,13 @@ ASENTRY_NP(swi_entry) * Handler for the Prefetch Abort exception. */ ASENTRY_NP(prefetch_abort_entry) -#ifdef __XSCALE__ - nop /* Make absolutely sure any pending */ - nop /* imprecise aborts have occurred. */ -#endif sub lr, lr, #0x00000004 /* Adjust the lr */ PUSHFRAMEINSVC - ldr r1, Lprefetch_abort_handler_address adr lr, exception_exit mov r0, sp /* pass the stack pointer as r0 */ - ldr pc, [r1] - -Lprefetch_abort_handler_address: - .word _C_LABEL(prefetch_abort_handler_address) - - .data - .global _C_LABEL(prefetch_abort_handler_address) - -_C_LABEL(prefetch_abort_handler_address): - .word abortprefetch - - .text -abortprefetch: - adr r0, abortprefetchmsg - b _C_LABEL(panic) - -abortprefetchmsg: - .asciz "abortprefetch" - .align 2 + b prefetch_abort_handler /* * data_abort_entry: @@ -128,36 +105,14 @@ abortprefetchmsg: * Handler for the Data Abort exception. */ ASENTRY_NP(data_abort_entry) -#ifdef __XSCALE__ - nop /* Make absolutely sure any pending */ - nop /* imprecise aborts have occurred. */ -#endif sub lr, lr, #0x00000008 /* Adjust the lr */ PUSHFRAMEINSVC /* Push trap frame and switch */ /* to SVC32 mode */ - ldr r1, Ldata_abort_handler_address adr lr, exception_exit mov r0, sp /* pass the stack pointer as r0 */ - ldr pc, [r1] - -Ldata_abort_handler_address: - .word _C_LABEL(data_abort_handler_address) - - .data - .global _C_LABEL(data_abort_handler_address) -_C_LABEL(data_abort_handler_address): - .word abortdata - - .text -abortdata: - adr r0, abortdatamsg - b _C_LABEL(panic) - -abortdatamsg: - .asciz "abortdata" - .align 2 + b prefetch_abort_handler /* * address_exception_entry: @@ -206,43 +161,9 @@ exception_exit: * look like direct entry from the vector. */ ASENTRY_NP(undefined_entry) - stmfd sp!, {r0, r1} - ldr r0, Lundefined_handler_indirection - ldr r1, [sp], #0x0004 - str r1, [r0, #0x0000] - ldr r1, [sp], #0x0004 - str r1, [r0, #0x0004] - ldmia r0, {r0, r1, pc} - -Lundefined_handler_indirection: - .word Lundefined_handler_indirection_data - -/* - * assembly bounce code for calling the kernel - * undefined instruction handler. This uses - * a standard trap frame and is called in SVC mode. - */ - -ENTRY_NP(undefinedinstruction_bounce) + sub lr, lr, #0x00000004 /* Adjust the lr */ PUSHFRAMEINSVC mov r0, sp adr lr, exception_exit - b _C_LABEL(undefinedinstruction) - - .data - .align 2 - -/* - * Indirection data - * 2 words use for preserving r0 and r1 - * 3rd word contains the undefined handler address. - */ - -Lundefined_handler_indirection_data: - .word 0 - .word 0 - - .global _C_LABEL(undefined_handler_address) -_C_LABEL(undefined_handler_address): - .word _C_LABEL(undefinedinstruction_bounce) + b undefinedinstruction diff --git a/sys/arch/arm/arm/fault.c b/sys/arch/arm/arm/fault.c index 7ed4ce466b3..749119d5087 100644 --- a/sys/arch/arm/arm/fault.c +++ b/sys/arch/arm/arm/fault.c @@ -261,13 +261,15 @@ data_abort_handler(trapframe_t *tf) va = trunc_page((vaddr_t)far); /* - * It is only a kernel address space fault iff: - * 1. user == 0 and - * 2. pcb_onfault not set or - * 3. pcb_onfault set and not LDRT/LDRBT/STRT/STRBT instruction. + * It is only a kernel address space fault if: + * 1. user == 0 && + * 2. va >= VM_MIN_KERNEL_ADDRESS && ( + * 3. pcb_onfault == NULL || + * 4. *(tf->tf_pc) != LDR{B}T/STR{B}T instruction ) + * (T-suffix = load/store with privileges as unprivileged) */ - if (user == 0 && (va >= VM_MIN_KERNEL_ADDRESS || - (va < VM_MIN_ADDRESS && vector_page == ARM_VECTORS_LOW)) && + if (user == 0 && + va >= VM_MIN_KERNEL_ADDRESS && __predict_true((pcb->pcb_onfault == NULL || ((*(u_int *)tf->tf_pc) & 0x05200000) != 0x04200000))) { map = kernel_map; @@ -590,8 +592,7 @@ prefetch_abort_handler(trapframe_t *tf) p->p_addr->u_pcb.pcb_tf = tf; /* Ok validate the address, can only execute in USER space */ - if (__predict_false(far >= VM_MAXUSER_ADDRESS || - (far < VM_MIN_ADDRESS && vector_page == ARM_VECTORS_LOW))) { + if (__predict_false(far >= VM_MAXUSER_ADDRESS)) { sv.sival_ptr = (u_int32_t *)far; trapsignal(p, SIGSEGV, 0, SEGV_ACCERR, sv); goto out; diff --git a/sys/arch/arm/arm/fiq.c b/sys/arch/arm/arm/fiq.c index 7201ee5607e..aad531a6c8f 100644 --- a/sys/arch/arm/arm/fiq.c +++ b/sys/arch/arm/arm/fiq.c @@ -55,24 +55,15 @@ extern char fiq_nullhandler[], fiq_nullhandler_end[]; * fiq_installhandler: * * Actually install the FIQ handler down at the FIQ vector. - * - * Note: If the FIQ is invoked via an extra layer of - * indirection, the actual FIQ code store lives in the - * data segment, so there is no need to manipulate - * the vector page's protection. */ static void fiq_installhandler(void *func, size_t size) { -#if !defined(__ARM_FIQ_INDIRECT) - vector_page_setprot(PROT_READ | PROT_WRITE | PROT_EXEC); -#endif + vector_page_setprot(PROT_READ | PROT_WRITE); memcpy(fiqvector, func, size); -#if !defined(__ARM_FIQ_INDIRECT) vector_page_setprot(PROT_READ | PROT_EXEC); -#endif cpu_icache_sync_range((vaddr_t) fiqvector, size); } diff --git a/sys/arch/arm/arm/fiq_subr.S b/sys/arch/arm/arm/fiq_subr.S index 7b805ec06ab..e8e4d5f5595 100644 --- a/sys/arch/arm/arm/fiq_subr.S +++ b/sys/arch/arm/arm/fiq_subr.S @@ -90,3 +90,11 @@ ENTRY(fiq_setregs) _C_LABEL(fiq_nullhandler): subs pc, lr, #4 _C_LABEL(fiq_nullhandler_end): + + .text + .global fiqvector + /* PAGE_SIZE align, so only single pte needs editing */ + .align 11 /* from exec to writable and back */ +fiqvector: + subs pc, lr, #4 + .org fiqvector + 0x100 diff --git a/sys/arch/arm/arm/pmap7.c b/sys/arch/arm/arm/pmap7.c index f99ee582e00..ccc16adda28 100644 --- a/sys/arch/arm/arm/pmap7.c +++ b/sys/arch/arm/arm/pmap7.c @@ -158,14 +158,6 @@ * the active domain on that cpu). I guess there are lots more tlb * shootdown issues too... * - * o If the vector_page is at 0x00000000 instead of 0xffff0000, then - * MP systems will lose big-time because of the MMU domain hack. - * The only way this can be solved (apart from moving the vector - * page to 0xffff0000) is to reserve the first 1MB of user address - * space for kernel use only. This would require re-linking all - * applications so that the text section starts above this 1MB - * boundary. - * * o Tracking which VM space is resident in the cache/tlb has not yet * been implemented for MP systems. * @@ -421,8 +413,6 @@ vaddr_t virtual_avail; vaddr_t virtual_end; vaddr_t pmap_curmaxkvaddr; -extern pv_addr_t systempage; - static __inline boolean_t pmap_is_current(pmap_t pm) { @@ -2149,6 +2139,10 @@ vector_page_setprot(int prot) { struct l2_bucket *l2b; pt_entry_t *ptep; + vaddr_t vbar, vector_page; + + __asm volatile("mrc p15, 0, %0, c12, c0, 0\n" : "=r"(vbar)); + vector_page = trunc_page(vbar); l2b = pmap_get_l2_bucket(pmap_kernel(), vector_page); KDASSERT(l2b != NULL); diff --git a/sys/arch/arm/arm/vectors.S b/sys/arch/arm/arm/vectors.S index 608335d719d..bc7af7f78fe 100644 --- a/sys/arch/arm/arm/vectors.S +++ b/sys/arch/arm/arm/vectors.S @@ -35,70 +35,23 @@ #include "assym.h" #include <machine/asm.h> -/* - * These are the exception vectors copied down to page 0. - * - * Note that FIQs are special; rather than using a level of - * indirection, we actually copy the FIQ code down into the - * vector page. - */ - .text - .align 2 - .global _C_LABEL(page0), _C_LABEL(page0_data), _C_LABEL(page0_end) - .global _C_LABEL(fiqvector) + .global vectors + .p2align 5 /* VBAR(=vector base address) has to be 32bit aligned */ -_C_LABEL(page0): - ldr pc, .Lreset_target - ldr pc, .Lundefined_target - ldr pc, .Lswi_target - ldr pc, .Lprefetch_abort_target - ldr pc, .Ldata_abort_target - ldr pc, .Laddress_exception_target - ldr pc, .Lirq_target -#ifdef __ARM_FIQ_INDIRECT - ldr pc, .Lfiq_target -#else -.Lfiqvector: - .set _C_LABEL(fiqvector), . - _C_LABEL(page0) +/* + * These are the exception vectors. + */ +vectors: + b reset_entry + b undefined_entry + b swi_entry + b prefetch_abort_entry + b data_abort_entry + b address_exception_entry + b irq_entry +#ifndef FIQ subs pc, lr, #4 - .org .Lfiqvector + 0x100 -#endif - -_C_LABEL(page0_data): -.Lreset_target: - .word reset_entry - -.Lundefined_target: - .word undefined_entry - -.Lswi_target: - .word swi_entry - -.Lprefetch_abort_target: - .word prefetch_abort_entry - -.Ldata_abort_target: - .word data_abort_entry - -.Laddress_exception_target: - .word address_exception_entry - -.Lirq_target: - .word irq_entry - -#ifdef __ARM_FIQ_INDIRECT -.Lfiq_target: - .word _C_LABEL(fiqvector) #else - .word 0 /* pad it out */ -#endif -_C_LABEL(page0_end): - -#ifdef __ARM_FIQ_INDIRECT - .data - .align 2 -_C_LABEL(fiqvector): - subs pc, lr, #4 - .org _C_LABEL(fiqvector) + 0x100 + b fiqvector #endif diff --git a/sys/arch/arm/arm/vm_machdep.c b/sys/arch/arm/arm/vm_machdep.c index 9b2a00a92f5..44f7c9e36c5 100644 --- a/sys/arch/arm/arm/vm_machdep.c +++ b/sys/arch/arm/arm/vm_machdep.c @@ -62,8 +62,6 @@ #include <machine/reg.h> #include <machine/vmparam.h> -extern pv_addr_t systempage; - int process_read_regs (struct proc *p, struct reg *regs); int process_read_fpregs (struct proc *p, struct fpreg *regs); diff --git a/sys/arch/arm/include/cpu.h b/sys/arch/arm/include/cpu.h index 0477b88883f..27cd964be83 100644 --- a/sys/arch/arm/include/cpu.h +++ b/sys/arch/arm/include/cpu.h @@ -157,10 +157,6 @@ extern int cpu_do_powersave; #define PROC_PC(p) ((p)->p_addr->u_pcb.pcb_tf->tf_pc) #define PROC_STACK(p) ((p)->p_addr->u_pcb.pcb_tf->tf_usr_sp) -/* The address of the vector page. */ -extern vaddr_t vector_page; -void arm32_vector_init(vaddr_t, int); - #define ARM_VEC_RESET (1 << 0) #define ARM_VEC_UNDEFINED (1 << 1) #define ARM_VEC_SWI (1 << 2) diff --git a/sys/arch/armv7/armv7/armv7_machdep.c b/sys/arch/armv7/armv7/armv7_machdep.c index aa1c549b29b..975f9885d55 100644 --- a/sys/arch/armv7/armv7/armv7_machdep.c +++ b/sys/arch/armv7/armv7/armv7_machdep.c @@ -164,7 +164,6 @@ int max_processes = 64; /* Default number */ #endif /* !PMAP_STATIC_L1S */ /* Physical and virtual addresses for some global pages */ -pv_addr_t systempage; pv_addr_t irqstack; pv_addr_t undstack; pv_addr_t abtstack; @@ -178,8 +177,7 @@ extern u_int undefined_handler_address; uint32_t board_id; -#define KERNEL_PT_SYS 0 /* Page table for mapping proc0 zero page */ -#define KERNEL_PT_KERNEL 1 /* Page table for mapping kernel */ +#define KERNEL_PT_KERNEL 0 /* Page table for mapping kernel */ #define KERNEL_PT_KERNEL_NUM 32 #define KERNEL_PT_VMDATA (KERNEL_PT_KERNEL+KERNEL_PT_KERNEL_NUM) /* Page tables for mapping kernel VM */ @@ -543,15 +541,6 @@ initarm(void *arg0, void *arg1, void *arg2, paddr_t loadaddr) if (!kernel_l1pt.pv_pa || (kernel_l1pt.pv_pa & (L1_TABLE_SIZE-1)) != 0) panic("initarm: Failed to align the kernel page directory"); - /* - * Allocate a page for the system page mapped to V0x00000000 - * This page will just contain the system vectors and can be - * shared by all processes. - */ - vector_page = ARM_VECTORS_HIGH; - alloc_pages(systempage.pv_pa, 1); - systempage.pv_va = vector_page; - /* Allocate stacks for all modes */ valloc_pages(irqstack, IRQ_STACK_SIZE); valloc_pages(abtstack, ABT_STACK_SIZE); @@ -600,9 +589,6 @@ initarm(void *arg0, void *arg1, void *arg2, paddr_t loadaddr) l1pagetable = kernel_l1pt.pv_pa; /* Map the L2 pages tables in the L1 page table */ - pmap_link_l2pt(l1pagetable, vector_page & ~(0x00400000 - 1), - &kernel_pt_table[KERNEL_PT_SYS]); - for (loop = 0; loop < KERNEL_PT_KERNEL_NUM; loop++) pmap_link_l2pt(l1pagetable, KERNEL_BASE + loop * 0x00400000, &kernel_pt_table[KERNEL_PT_KERNEL + loop]); @@ -664,10 +650,6 @@ initarm(void *arg0, void *arg1, void *arg2, paddr_t loadaddr) /* Map the Mini-Data cache clean area. */ - /* Map the vector page. */ - pmap_map_entry(l1pagetable, vector_page, systempage.pv_pa, - PROT_READ | PROT_WRITE, PTE_CACHE); - /* Map the FDT. */ pmap_map_chunk(l1pagetable, fdt.pv_va, fdt.pv_pa, round_page(fdt_get_size((void *)fdt.pv_pa)), @@ -694,8 +676,6 @@ initarm(void *arg0, void *arg1, void *arg2, paddr_t loadaddr) proc0paddr = (struct user *)kernelstack.pv_va; proc0.p_addr = proc0paddr; - arm32_vector_init(vector_page, ARM_VEC_ALL); - /* * Pages were allocated during the secondary bootstrap for the * stacks for different CPU modes. @@ -712,20 +692,6 @@ initarm(void *arg0, void *arg1, void *arg2, paddr_t loadaddr) set_stackptr(PSR_UND32_MODE, undstack.pv_va + UND_STACK_SIZE * PAGE_SIZE); - /* - * Well we should set a data abort handler. - * Once things get going this will change as we will need a proper - * handler. - * Until then we will use a handler that just panics but tells us - * why. - * Initialisation of the vectors will just panic on a data abort. - * This just fills in a slighly better one. - */ - - data_abort_handler_address = (u_int)data_abort_handler; - prefetch_abort_handler_address = (u_int)prefetch_abort_handler; - undefined_handler_address = (u_int)undefinedinstruction_bounce; - /* Now we can reinit the FDT, using the virtual address. */ fdt_init((void *)fdt.pv_va); @@ -770,8 +736,6 @@ initarm(void *arg0, void *arg1, void *arg2, paddr_t loadaddr) pmap_bootstrap((pd_entry_t *)kernel_l1pt.pv_va, KERNEL_VM_BASE, KERNEL_VM_BASE + KERNEL_VM_SIZE); - vector_page_setprot(PROT_READ | PROT_EXEC); - /* * Restore proper bus_space operation, now that pmap is initialized. */ diff --git a/sys/arch/armv7/armv7/locore0.S b/sys/arch/armv7/armv7/locore0.S index 2a4e98cbe8c..85cd961f3b8 100644 --- a/sys/arch/armv7/armv7/locore0.S +++ b/sys/arch/armv7/armv7/locore0.S @@ -160,8 +160,13 @@ _C_LABEL(bootstrap_start): mov r0, #DOMAIN_CLIENT /* We only use domain 0 */ mcr CP15_DACR(r0) + /* set VBAR */ + ldr r0, =vectors + mcr CP15_VBAR(r0) + /* Enable MMU */ mrc CP15_SCTLR(r0) + bic r0, r0, #CPU_CONTROL_VECRELOC /* our vectors are at VBAR */ orr r0, r0, #CPU_CONTROL_MMU_ENABLE mcr CP15_SCTLR(r0) CPWAIT(r0)