diff mbox

[v2,1/2] hw/openrisc: fix masking in openrisc_pic_cpu_handler()

Message ID 1376459728-2029-1-git-send-email-xi.wang@gmail.com
State New
Headers show

Commit Message

Xi Wang Aug. 14, 2013, 5:55 a.m. UTC
Consider the masking of PICSR and PICMR:

    ((cpu->env.picsr && (1 << i)) && (cpu->env.picmr && (1 << i)))

To correctly mask bits, we should use the bitwise AND "&" rather than
the logical AND "&&".  Also, the loop is not necessary for masking.
Simply use (cpu->env.picsr & cpu->env.picmr).

Cc: Jia Liu <proljc@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Xi Wang <xi.wang@gmail.com>
---
 hw/openrisc/pic_cpu.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

Comments

Jia Liu Aug. 14, 2013, 1:16 p.m. UTC | #1
Hi Xi,

You should use git format-patch with "-s" and "--cover-letter" next time,
and manually edit cover letter for a brief description of your patch set.
More info. at http://qemu-project.org/Contribute/SubmitAPatch .

On Wed, Aug 14, 2013 at 1:55 PM, Xi Wang <xi.wang@gmail.com> wrote:
> Consider the masking of PICSR and PICMR:
>
>     ((cpu->env.picsr && (1 << i)) && (cpu->env.picmr && (1 << i)))
>
> To correctly mask bits, we should use the bitwise AND "&" rather than
> the logical AND "&&".  Also, the loop is not necessary for masking.
> Simply use (cpu->env.picsr & cpu->env.picmr).
>
> Cc: Jia Liu <proljc@gmail.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Xi Wang <xi.wang@gmail.com>
> ---
>  hw/openrisc/pic_cpu.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/hw/openrisc/pic_cpu.c b/hw/openrisc/pic_cpu.c
> index ca0b7c1..3fcee02 100644
> --- a/hw/openrisc/pic_cpu.c
> +++ b/hw/openrisc/pic_cpu.c
> @@ -26,7 +26,6 @@ static void openrisc_pic_cpu_handler(void *opaque, int irq, int level)
>  {
>      OpenRISCCPU *cpu = (OpenRISCCPU *)opaque;
>      CPUState *cs = CPU(cpu);
> -    int i;
>      uint32_t irq_bit = 1 << irq;
>
>      if (irq > 31 || irq < 0) {
> @@ -39,13 +38,11 @@ static void openrisc_pic_cpu_handler(void *opaque, int irq, int level)
>          cpu->env.picsr &= ~irq_bit;
>      }
>
> -    for (i = 0; i < 32; i++) {
> -        if ((cpu->env.picsr && (1 << i)) && (cpu->env.picmr && (1 << i))) {
> -            cpu_interrupt(cs, CPU_INTERRUPT_HARD);
> -        } else {
> -            cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
> -            cpu->env.picsr &= ~(1 << i);
> -        }
> +    if (cpu->env.picsr & cpu->env.picmr) {
> +        cpu_interrupt(cs, CPU_INTERRUPT_HARD);
> +    } else {
> +        cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
> +        cpu->env.picsr = 0;
>      }
>  }

Thank you very much for fixing this bug.

Acked-by: Jia Liu <proljc@gmail.com>

>
> --
> 1.8.1.2
>

Regards,
Jia.
diff mbox

Patch

diff --git a/hw/openrisc/pic_cpu.c b/hw/openrisc/pic_cpu.c
index ca0b7c1..3fcee02 100644
--- a/hw/openrisc/pic_cpu.c
+++ b/hw/openrisc/pic_cpu.c
@@ -26,7 +26,6 @@  static void openrisc_pic_cpu_handler(void *opaque, int irq, int level)
 {
     OpenRISCCPU *cpu = (OpenRISCCPU *)opaque;
     CPUState *cs = CPU(cpu);
-    int i;
     uint32_t irq_bit = 1 << irq;
 
     if (irq > 31 || irq < 0) {
@@ -39,13 +38,11 @@  static void openrisc_pic_cpu_handler(void *opaque, int irq, int level)
         cpu->env.picsr &= ~irq_bit;
     }
 
-    for (i = 0; i < 32; i++) {
-        if ((cpu->env.picsr && (1 << i)) && (cpu->env.picmr && (1 << i))) {
-            cpu_interrupt(cs, CPU_INTERRUPT_HARD);
-        } else {
-            cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
-            cpu->env.picsr &= ~(1 << i);
-        }
+    if (cpu->env.picsr & cpu->env.picmr) {
+        cpu_interrupt(cs, CPU_INTERRUPT_HARD);
+    } else {
+        cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
+        cpu->env.picsr = 0;
     }
 }