On Mon, 04 Feb 2013 22:57:43 +0100
Stefan Weil <s...@weilnetz.de> wrote:
> Am 04.02.2013 22:23, schrieb Anthony Liguori:
> > As best I can tell, this is a false positive.
> >
> > [aliguori@ccnode4 qemu-s390]$ make
> > CC s390x-softmmu/target-s390x/helper.o
> > /home/aliguori/git/qemu/target-s390x/helper.c: In function ‘do_interrupt’:
> > /home/aliguori/git/qemu/target-s390x/helper.c:673:17: error: ‘addr’ may
> > be used uninitialized in this function [-Werror=maybe-uninitialized]
> > /home/aliguori/git/qemu/target-s390x/helper.c:620:20: note: ‘addr’ was
> > declared here
> > /home/aliguori/git/qemu/target-s390x/helper.c:673:17: error: ‘mask’ may
> > be used uninitialized in this function [-Werror=maybe-uninitialized]
> > /home/aliguori/git/qemu/target-s390x/helper.c:620:14: note: ‘mask’ was
> > declared here
> > cc1: all warnings being treated as errors
> > make[1]: *** [target-s390x/helper.o] Error 1
> > make: *** [subdir-s390x-softmmu] Error 2
> >
>
> Yes, this is a false positive. A better compiler will complain when your
> patch was applied because addr, mask are assigned values which are
> never used...
>
> Would it be possible to completely eliminate variable "found" and
> move the DPRINTF, load_psw statements into the for loop (just before
> the break statement)?
We could move the lpsw. However, this made me notice another problem:
We stop scanning subsequent iscs if we found an interrupt to inject...
This is not a problem for current virtio-ccw based Linux guests since
they never use anything else than isc 3, but we'll probably want the
following patch.
From 8b2f40e8eaac16b55a72ab1e36a4c5de0b016495 Mon Sep 17 00:00:00 2001
From: Cornelia Huck <cornelia.h...@de.ibm.com>
Date: Tue, 5 Feb 2013 10:14:49 +0100
Subject: [PATCH] s390: Keep I/O interrupts enabled for all iscs.
do_io_interrupt() would stop scanning further iscs if it found
an I/O interrupt it could inject. This might cause the pending
interrupt indication for I/O interrupts to be reset although there
might be queued I/O interrupts for subsequent iscs.
Fix this by reordering the logic: Inject the I/O interrupt immediately
and continue searching all iscs for queued interrupts.
Signed-off-by: Cornelia Huck <cornelia.h...@de.ibm.com>
---
target-s390x/helper.c | 40 +++++++++++++++++++++-------------------
1 file changed, 21 insertions(+), 19 deletions(-)
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index 043feb2..9f9088b 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -617,7 +617,6 @@ static void do_ext_interrupt(CPUS390XState *env)
static void do_io_interrupt(CPUS390XState *env)
{
- uint64_t mask = 0, addr = 0;
LowCore *lowcore;
IOIntQueue *q;
uint8_t isc;
@@ -642,36 +641,39 @@ static void do_io_interrupt(CPUS390XState *env)
disable = 0;
continue;
}
- found = 1;
- lowcore = cpu_map_lowcore(env);
+ if (!found) {
+ uint64_t mask, addr;
- lowcore->subchannel_id = cpu_to_be16(q->id);
- lowcore->subchannel_nr = cpu_to_be16(q->nr);
- lowcore->io_int_parm = cpu_to_be32(q->parm);
- lowcore->io_int_word = cpu_to_be32(q->word);
- lowcore->io_old_psw.mask = cpu_to_be64(get_psw_mask(env));
- lowcore->io_old_psw.addr = cpu_to_be64(env->psw.addr);
- mask = be64_to_cpu(lowcore->io_new_psw.mask);
- addr = be64_to_cpu(lowcore->io_new_psw.addr);
+ found = 1;
+ lowcore = cpu_map_lowcore(env);
- cpu_unmap_lowcore(lowcore);
+ lowcore->subchannel_id = cpu_to_be16(q->id);
+ lowcore->subchannel_nr = cpu_to_be16(q->nr);
+ lowcore->io_int_parm = cpu_to_be32(q->parm);
+ lowcore->io_int_word = cpu_to_be32(q->word);
+ lowcore->io_old_psw.mask = cpu_to_be64(get_psw_mask(env));
+ lowcore->io_old_psw.addr = cpu_to_be64(env->psw.addr);
+ mask = be64_to_cpu(lowcore->io_new_psw.mask);
+ addr = be64_to_cpu(lowcore->io_new_psw.addr);
- env->io_index[isc]--;
+ cpu_unmap_lowcore(lowcore);
+
+ env->io_index[isc]--;
+
+ DPRINTF("%s: %" PRIx64 " %" PRIx64 "\n", __func__,
+ env->psw.mask, env->psw.addr);
+ load_psw(env, mask, addr);
+ }
if (env->io_index[isc] >= 0) {
disable = 0;
}
- break;
+ continue;
}
if (disable) {
env->pending_int &= ~INTERRUPT_IO;
}
- if (found) {
- DPRINTF("%s: %" PRIx64 " %" PRIx64 "\n", __func__,
- env->psw.mask, env->psw.addr);
- load_psw(env, mask, addr);
- }
}
static void do_mchk_interrupt(CPUS390XState *env)
--
1.7.12.4
