| Message ID | 4DEB900F.5070206@embedded-brains.de |
|---|---|
| State | New |
| Headers | show |
On 5 June 2011 15:17, Sebastian Huber <sebastian.huber@embedded-brains.de> wrote: > On 05/06/11 15:44, Peter Maydell wrote: >> In any case, M profile exception priority handling is sufficiently >> complicated that any change which only looks at PRIMASK (which is >> effectively what the change to look at CPSR_I here is doing) is >> almost certainly wrong. I think that whatever is raising the >> interrupt should be looking at the CPU priority and not raising it >> in the first place. > Yes. Please have a look at: > > http://lists.nongnu.org/archive/html/qemu-devel/2011-05/msg03132.html That patch does correct an error in the decode of the basepri registers (and so we might as well apply it), but it does not affect the fact that the underlying v7m.basepri field is not actually used for anything. >> (It looks suspiciously as if most of the v7M priority handling >> is simply missing from QEMU, ie you have bigger problems than >> can be fixed by a small patch like this...) > Yes, but the current behaviour is definitely not right. Since the > PRIMASK is mapped to the I bit in the CPSR I guessed that this was the > right place to fix it. I agree that the current behaviour is not right. However, to fix this problem you need to work on a larger scale than attempting to apply two line patches which fix your particular use case. -- PMM
On 05/06/11 16:57, Peter Maydell wrote: > On 5 June 2011 15:17, Sebastian Huber > <sebastian.huber@embedded-brains.de> wrote: > >> On 05/06/11 15:44, Peter Maydell wrote: [...] >>> (It looks suspiciously as if most of the v7M priority handling >>> is simply missing from QEMU, ie you have bigger problems than >>> can be fixed by a small patch like this...) >>> > >> Yes, but the current behaviour is definitely not right. Since the >> PRIMASK is mapped to the I bit in the CPSR I guessed that this was the >> right place to fix it. >> > I agree that the current behaviour is not right. However, to fix > this problem you need to work on a larger scale than attempting > to apply two line patches which fix your particular use case. > I agree, but you have to start somewhere. What is "this problem"? Is that we have no execution priority (in the sense of the ARMv7 architecture, B1.3.2 Exceptions), but instead use a mapping to CPSR_I and CPSR_F?
On 5 June 2011 17:32, Sebastian Huber <sebastian.huber@embedded-brains.de> wrote: > On 05/06/11 16:57, Peter Maydell wrote: >> I agree that the current behaviour is not right. However, to fix >> this problem you need to work on a larger scale than attempting >> to apply two line patches which fix your particular use case. > > I agree, but you have to start somewhere. What is "this problem"? Is > that we have no execution priority (in the sense of the ARMv7 > architecture, B1.3.2 Exceptions), but instead use a mapping to CPSR_I > and CPSR_F? There is some notion of priority, see gic_update() in hw/arm_gic.c; but it is only within the gic and is not dealing with v7M specific issues. At the moment I am mostly just warning you that you're entering difficult territory; if I have time to read the qemu v7m code more carefully next week I may have more concrete opinions. -- PMM
From 917f2491c1dc2525b24c635afe4459e55700149c Mon Sep 17 00:00:00 2001 From: Sebastian Huber <sebastian.huber@embedded-brains.de> Date: Sun, 5 Jun 2011 14:57:17 +0200 Subject: [PATCH] Fixed interrupt handling for ARMv7M. Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de> --- cpu-exec.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cpu-exec.c b/cpu-exec.c index 6ddd8dd..d1e9816 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -470,8 +470,8 @@ int cpu_exec(CPUState *env1) We avoid this by disabling interrupts when pc contains a magic address. */ if (interrupt_request & CPU_INTERRUPT_HARD - && ((IS_M(env) && env->regs[15] < 0xfffffff0) - || !(env->uncached_cpsr & CPSR_I))) { + && !(env->uncached_cpsr & CPSR_I) + && (!IS_M(env) || env->regs[15] < 0xfffffff0)) { env->exception_index = EXCP_IRQ; do_interrupt(env); next_tb = 0; -- 1.7.1