[Qemu-devel] [PULL 0/3] OpenRISC patch queue for 1.7

[Qemu-devel] [PULL 0/3] OpenRISC patch queue for 1.7

[Jia Liu]

Hi Anthony,

This is my OpenRISC patch queue for 1.7, it have been well tested, please pull.


Thanks to Peter Maydell and Xi Wang, it fix OpenRISC sim broad and pic:
* Avoid using uninitialised variable 'entry'
* Fix masking in openrisc_pic_cpu_handler()
* Avoid undefined shift in openrisc_pic_cpu_handler()


The following changes since commit ecfe10c9a6f9bc77d0e4b7eb5d0f5d61e8fbaed8:

  Merge remote-tracking branch 'pmaydell/tags/pull-target-arm-20130820' into staging (2013-08-20 11:23:52 -0500)

are available in the git repository at:


  git://github.com/J-Liu/qemu.git or32

for you to fetch changes up to 7717f248eebdcfe6de400404d0cf65dcb3633308:

  hw/openrisc: Avoid undefined shift in openrisc_pic_cpu_handler() (2013-08-21 09:31:42 +0800)

----------------------------------------------------------------
Jia Liu (3):
      hw/openrisc: Avoid using uninitialised variable 'entry'
      hw/openrisc: Fix masking in openrisc_pic_cpu_handler()
      hw/openrisc: Avoid undefined shift in openrisc_pic_cpu_handler()

 hw/openrisc/openrisc_sim.c |  3 +--
 hw/openrisc/pic_cpu.c      | 17 ++++++++---------
 2 files changed, 9 insertions(+), 11 deletions(-)

[Qemu-devel] [PULL 1/3] hw/openrisc: Avoid using uninitialised variable 'entry'

[Jia Liu]

clang warns that cpu_openrisc_load_kernel() can use 'entry' uninitialized:

hw/openrisc/openrisc_sim.c:69:9: error: variable 'entry' is used uninitialized
whenever '&&' condition is false [-Werror,-Wsometimes-uninitialized]

    if (kernel_filename && !qtest_enabled()) {
        ^~~~~~~~~~~~~~~
hw/openrisc/openrisc_sim.c:91:19: note: uninitialized use occurs here
    cpu->env.pc = entry;
                  ^~~~~

Fix this by not attempting to change the CPU's starting PC unless
we actually loaded a kernel.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Jia Liu <proljc@gmail.com>
---
 hw/openrisc/openrisc_sim.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index a08f27c..28fa41d 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -86,9 +86,8 @@ static void cpu_openrisc_load_kernel(ram_addr_t ram_size,
                     kernel_filename);
             exit(1);
         }
+        cpu->env.pc = entry;
     }
-
-    cpu->env.pc = entry;
 }
 
 static void openrisc_sim_init(QEMUMachineInitArgs *args)
-- 
1.7.12.4 (Apple Git-37)

[Qemu-devel] [PULL 2/3] hw/openrisc: Fix masking in openrisc_pic_cpu_handler()

[Jia Liu]

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).

Signed-off-by: Xi Wang <xi.wang@gmail.com>
Acked-by: Jia Liu <proljc@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;
     }
 }
 
-- 
1.7.12.4 (Apple Git-37)

[Qemu-devel] [PULL 3/3] hw/openrisc: Avoid undefined shift in openrisc_pic_cpu_handler()

[Jia Liu]

In C99 signed shift (1 << 31) is undefined behavior, since the result
exceeds INT_MAX.  Use 1U instead and move the shift after the check.

Signed-off-by: Xi Wang <xi.wang@gmail.com>
Acked-by: Jia Liu <proljc@gmail.com>
---
 hw/openrisc/pic_cpu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/openrisc/pic_cpu.c b/hw/openrisc/pic_cpu.c
index 3fcee02..2af1d60 100644
--- a/hw/openrisc/pic_cpu.c
+++ b/hw/openrisc/pic_cpu.c
@@ -26,12 +26,14 @@ static void openrisc_pic_cpu_handler(void *opaque, int irq, int level)
 {
     OpenRISCCPU *cpu = (OpenRISCCPU *)opaque;
     CPUState *cs = CPU(cpu);
-    uint32_t irq_bit = 1 << irq;
+    uint32_t irq_bit;
 
     if (irq > 31 || irq < 0) {
         return;
     }
 
+    irq_bit = 1U << irq;
+
     if (level) {
         cpu->env.picsr |= irq_bit;
     } else {
-- 
1.7.12.4 (Apple Git-37)

Re: [Qemu-devel] [PULL 0/3] OpenRISC patch queue for 1.7

[Peter Maydell]

On 21 August 2013 03:06, Jia Liu <proljc@gmail.com> wrote:
> This is my OpenRISC patch queue for 1.7, it have been well tested, please pull.
>
>
> ----------------------------------------------------------------
> Jia Liu (3):
>       hw/openrisc: Avoid using uninitialised variable 'entry'
>       hw/openrisc: Fix masking in openrisc_pic_cpu_handler()
>       hw/openrisc: Avoid undefined shift in openrisc_pic_cpu_handler()

Hi -- a couple of minor notes for next time you put together a pull:

(1) your process for putting the git tree together has lost the
authorship -- these patches weren't written by you, but the
commits in your git repo (and thus now in master) have you as
the Author. (If you use 'git am' or Anthony's 'patches apply'
script it will get this right.)
(2) You should put your own Signed-off-by: line in each patchset,
as an indication that the patches have come through your tree.

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/3] OpenRISC patch queue for 1.7

[Jia Liu]

Hi Peter,

On Fri, Aug 23, 2013 at 10:09 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 21 August 2013 03:06, Jia Liu <proljc@gmail.com> wrote:
>> This is my OpenRISC patch queue for 1.7, it have been well tested, please pull.
>>
>>
>> ----------------------------------------------------------------
>> Jia Liu (3):
>>       hw/openrisc: Avoid using uninitialised variable 'entry'
>>       hw/openrisc: Fix masking in openrisc_pic_cpu_handler()
>>       hw/openrisc: Avoid undefined shift in openrisc_pic_cpu_handler()
>
> Hi -- a couple of minor notes for next time you put together a pull:
>
> (1) your process for putting the git tree together has lost the
> authorship -- these patches weren't written by you, but the
> commits in your git repo (and thus now in master) have you as
> the Author. (If you use 'git am' or Anthony's 'patches apply'
> script it will get this right.)

Thank you for reminding me, I'll use git am next time.

> (2) You should put your own Signed-off-by: line in each patchset,
> as an indication that the patches have come through your tree.
>
> thanks
> -- PMM

Regards,
Jia