[Qemu-devel] [PATCH v3] pl190: fix read of VECTADDR

[Qemu-devel] [PATCH v3] pl190: fix read of VECTADDR

[Brendan Fennell]

Signed-off-by: Brendan Fennell <bfennell@skynet.ie>
---
 hw/pl190.c |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/hw/pl190.c b/hw/pl190.c
index cb50afb..b372da8 100644
--- a/hw/pl190.c
+++ b/hw/pl190.c
@@ -117,12 +117,18 @@ static uint64_t pl190_read(void *opaque, target_phys_addr_t offset,
         return s->protected;
     case 12: /* VECTADDR */
         /* Read vector address at the start of an ISR.  Increases the
-           current priority level to that of the current interrupt.  */
-        for (i = 0; i < s->priority; i++)
-          {
-            if ((s->level | s->soft_level) & s->prio_mask[i])
-              break;
-          }
+         * current priority level to that of the current interrupt.
+         *
+         * Since an enabled interrupt X at priority P causes prio_mask[Y]
+         * to have bit X set for all Y > P, this loop will stop with
+         * i == the priority of the highest priority set interrupt.
+         */
+        for (i = 0; i < s->priority; i++) {
+            if ((s->level | s->soft_level) & s->prio_mask[(i + 1)]) {
+                break;
+            }
+        }
+
         /* Reading this value with no pending interrupts is undefined.
            We return the default address.  */
         if (i == PL190_NUM_PRIO)
-- 
1.7.2.5

Re: [Qemu-devel] [PATCH v3] pl190: fix read of VECTADDR

[Peter Maydell]

On 20 August 2012 18:59, Brendan Fennell <bfennell@skynet.ie> wrote:
> Signed-off-by: Brendan Fennell <bfennell@skynet.ie>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

The () inside the [] aren't actually necessary, but I'm just
going to fix those as I take this patch into the arm-devs
queue, I think. I've also expanded the commit message a little:

=====
Reading VECTADDR was causing us to set the current priority to
the wrong value, the most obvious effect of which was that we
would return the vector for the wrong interrupt as the result
of the read.
=====

What guest did you see this problem with? Since the QEMU 1.2
release is now less than 2 weeks away, and this bug has been
present since the versatilepb platform was added 6 years ago,
I'm reluctant to squeeze it into this release unless it's
going to be hit by a lot of people. (1.3 is only 3 months
later so it's not a huge deal for a minor fix to miss 1.2.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3] pl190: fix read of VECTADDR

[Brendan Fennell]

On Mon, 20 Aug 2012, Peter Maydell wrote:

> On 20 August 2012 18:59, Brendan Fennell <bfennell@skynet.ie> wrote:
>> Signed-off-by: Brendan Fennell <bfennell@skynet.ie>
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> The () inside the [] aren't actually necessary, but I'm just
> going to fix those as I take this patch into the arm-devs
> queue, I think. I've also expanded the commit message a little:
>
> =====
> Reading VECTADDR was causing us to set the current priority to
> the wrong value, the most obvious effect of which was that we
> would return the vector for the wrong interrupt as the result
> of the read.
> =====
>
> What guest did you see this problem with? Since the QEMU 1.2
> release is now less than 2 weeks away, and this bug has been
> present since the versatilepb platform was added 6 years ago,
> I'm reluctant to squeeze it into this release unless it's
> going to be hit by a lot of people. (1.3 is only 3 months
> later so it's not a huge deal for a minor fix to miss 1.2.)

I'm working on a custom application, not a standard guest OS. I think it's 
safe to say this can wait for 1.3.

Brendan.

>
> thanks
> -- PMM
>
>