Re: [09/36] AArch64: Exception handling

[Catalin Marinas]

Hi Christopher,

On Thu, Aug 09, 2012 at 06:05:36PM +0100, Christopher Covington wrote:
> On 01/-10/-28163 02:59 PM, Catalin Marinas wrote:
> > +/*
> > + * Exception vectors.
> > + */
> > +	.macro	ventry	label
> > +	.align	7
> > +	b	\label
> > +	.endm
> > +
> > +	.align	11
> > +ENTRY(vectors)
> > +	ventry	el1_sync_invalid		// Synchronous EL1t
> > +	ventry	el1_irq_invalid			// IRQ EL1t
> > +	ventry	el1_fiq_invalid			// FIQ EL1t
> > +	ventry	el1_error_invalid		// Error EL1t
> > +
> > +	ventry	el1_sync			// Synchronous EL1h
> > +	ventry	el1_irq				// IRQ EL1h
> > +	ventry	el1_fiq_invalid			// FIQ EL1h
> > +	ventry	el1_error_invalid		// Error EL1h
> > +
> > +	ventry	el0_sync			// Synchronous 64-bit EL0
> > +	ventry	el0_irq				// IRQ 64-bit EL0
> > +	ventry	el0_fiq_invalid			// FIQ 64-bit EL0
> > +	ventry	el0_error_invalid		// Error 64-bit EL0
> > +
> > +#ifdef CONFIG_AARCH32_EMULATION
> > +	ventry	el0_sync_compat			// Synchronous 32-bit EL0
> > +	ventry	el0_irq_compat			// IRQ 32-bit EL0
> > +	ventry	el0_fiq_invalid_compat		// FIQ 32-bit EL0
> > +	ventry	el0_error_invalid_compat	// Error 32-bit EL0
> > +#else
> > +	ventry	el0_sync_invalid		// Synchronous 32-bit EL0
> > +	ventry	el0_irq_invalid			// IRQ 32-bit EL0
> > +	ventry	el0_fiq_invalid			// FIQ 32-bit EL0
> > +	ventry	el0_error_invalid		// Error 32-bit EL0
> > +#endif
> > +END(vectors)
> > +
> > +/*
> > + * Invalid mode handlers
> > + */
> > +	.macro	inv_entry, el, reason, regsize = 64
> > +	kernel_entry el, \regsize
> > +	mov	x0, sp
> > +	mov	x1, #\reason
> > +	mrs	x2, esr_el1
> > +	b	bad_mode
> > +	.endm
> 
> The code seems to indicate that the invalid mode handlers have
> different alignment requirements than the valid mode handlers, which
> puzzles me.

I don't see any difference. The whole vector must be 2K aligned while
each individual entry is found every 128 bytes (to allow for more
instructions, though we only use a branch).

The inv_entry macro (as the kernel_entry one) is used in code being
branched to from the vector and not inside the vector.

> > +el0_sync_invalid:
> > +	inv_entry 0, BAD_SYNC
> > +ENDPROC(el0_sync_invalid)
> 
> Plain labels, the ENTRY macro, the END macro and the ENDPROC macro are
> used variously throughout this file, and I wonder if a greater amount
> of consistency might be attainable. The description of the ENDPROC
> macro in include/linux/linkage.h makes me think its use might not be
> completely warranted in blocks of assembly that don't end with a
> return instruction.

We use ENTRY only when we want to export the symbol as it contains the
.globl directive. The ENDPROC is used to mark a function and it's in
general useful for debugging information it generates.

> > +	.align	6
> > +el0_irq:
> > +	kernel_entry 0
> > +el0_irq_naked:
> > +	disable_step x1
> > +	isb
> > +	enable_dbg
> > +#ifdef CONFIG_TRACE_IRQFLAGS
> > +	bl	trace_hardirqs_off
> > +#endif
> > +	get_thread_info tsk
> > +#ifdef CONFIG_PREEMPT
> > +	ldr	x24, [tsk, #TI_PREEMPT]		// get preempt count
> > +	add	x23, x24, #1			// increment it
> > +	str	x23, [tsk, #TI_PREEMPT]
> > +#endif
> > +	irq_handler
> > +#ifdef CONFIG_PREEMPT
> > +	ldr	x0, [tsk, #TI_PREEMPT]
> > +	str	x24, [tsk, #TI_PREEMPT]
> > +	cmp	x0, x23
> > +	b.eq	1f
> > +	mov	x1, #0
> > +	str	x1, [x1]			// BUG
> 
> It looks like the error handling here isn't quite complete.

We trigger a bug by storing to 0 and the kernel will panic, giving the
full trace. I don't think we can do more in terms of error handling
here.

Regards.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/