* [Qemu-devel] [PATCH] qemu-m68k: add support for interrupt masking/unmasking
@ 2015-03-22 9:09 Waldemar Brodkorb
2015-03-22 9:49 ` Stefan Weil
2015-03-22 11:40 ` Peter Maydell
0 siblings, 2 replies; 4+ messages in thread
From: Waldemar Brodkorb @ 2015-03-22 9:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Thomas Petazzoni
Fixes following problem, when trying to boot linux:
qemu: hardware error: mcf_intc_write: Bad write offset 28
CPU #0:
D0 = 000000ff A0 = 402ea5dc F0 = 0000000000000000 ( 0)
D1 = 00000004 A1 = 402ea5e0 F1 = 0000000000000000 ( 0)
D2 = 00000040 A2 = 40040752 F2 = 0000000000000000 ( 0)
D3 = 00000000 A3 = 40040a98 F3 = 0000000000000000 ( 0)
D4 = 00000000 A4 = 400407b4 F4 = 0000000000000000 ( 0)
D5 = 00000000 A5 = 00000000 F5 = 0000000000000000 ( 0)
D6 = 00000000 A6 = 40195ff8 F6 = 0000000000000000 ( 0)
D7 = 00000000 A7 = 40195fd0 F7 = 0000000000000000 ( 0)
PC = 401b2058 SR = 2704 --Z-- FPRESULT = 0
Aborted
System started via:
qemu-system-m68k -nographic -nographic -M mcf5208evb -cpu m5208 -kernel kernel
Patch originally posted here:
http://lists.busybox.net/pipermail/buildroot/2012-April/052585.html
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Tested-by: Waldemar Brodkorb <wbx@openadk.org>
Signed-off-by: Waldemar Brodkorb <wbx@openadk.org>
---
hw/m68k/mcf_intc.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/hw/m68k/mcf_intc.c b/hw/m68k/mcf_intc.c
index 621423c..1d161b1 100644
--- a/hw/m68k/mcf_intc.c
+++ b/hw/m68k/mcf_intc.c
@@ -65,6 +65,10 @@ static uint64_t mcf_intc_read(void *opaque, hwaddr addr,
return (uint32_t)(s->ifr >> 32);
case 0x14:
return (uint32_t)s->ifr;
+ /* Reading from SIMR and CIMR return 0 */
+ case 0x1c:
+ case 0x1d:
+ return 0;
case 0xe0: /* SWIACK. */
return s->active_vector;
case 0xe1: case 0xe2: case 0xe3: case 0xe4:
@@ -102,6 +106,20 @@ static void mcf_intc_write(void *opaque, hwaddr addr,
case 0x0c:
s->imr = (s->imr & 0xffffffff00000000ull) | (uint32_t)val;
break;
+ /* SIMR allows to easily mask interrupts */
+ case 0x1c:
+ if (val & 0x40)
+ s->imr = ~0ull;
+ else
+ s->imr |= (1 << (val & 0x3f));
+ break;
+ /* CIMR allows to easily unmask interrupts */
+ case 0x1d:
+ if (val & 0x40)
+ s->imr = 0ull;
+ else
+ s->imr &= ~(1 << (val & 0x3f));
+ break;
default:
hw_error("mcf_intc_write: Bad write offset %d\n", offset);
break;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] qemu-m68k: add support for interrupt masking/unmasking
2015-03-22 9:09 [Qemu-devel] [PATCH] qemu-m68k: add support for interrupt masking/unmasking Waldemar Brodkorb
@ 2015-03-22 9:49 ` Stefan Weil
2015-03-22 11:43 ` Peter Maydell
2015-03-22 11:40 ` Peter Maydell
1 sibling, 1 reply; 4+ messages in thread
From: Stefan Weil @ 2015-03-22 9:49 UTC (permalink / raw)
To: Waldemar Brodkorb, qemu-devel; +Cc: Thomas Petazzoni
Technically this implementation looks reasonable. I added some remarks
below.
Am 22.03.2015 um 10:09 schrieb Waldemar Brodkorb:
> Fixes following problem, when trying to boot linux:
> qemu: hardware error: mcf_intc_write: Bad write offset 28
>
> CPU #0:
> D0 = 000000ff A0 = 402ea5dc F0 = 0000000000000000 ( 0)
> D1 = 00000004 A1 = 402ea5e0 F1 = 0000000000000000 ( 0)
> D2 = 00000040 A2 = 40040752 F2 = 0000000000000000 ( 0)
> D3 = 00000000 A3 = 40040a98 F3 = 0000000000000000 ( 0)
> D4 = 00000000 A4 = 400407b4 F4 = 0000000000000000 ( 0)
> D5 = 00000000 A5 = 00000000 F5 = 0000000000000000 ( 0)
> D6 = 00000000 A6 = 40195ff8 F6 = 0000000000000000 ( 0)
> D7 = 00000000 A7 = 40195fd0 F7 = 0000000000000000 ( 0)
> PC = 401b2058 SR = 2704 --Z-- FPRESULT = 0
> Aborted
>
> System started via:
> qemu-system-m68k -nographic -nographic -M mcf5208evb -cpu m5208 -kernel kernel
>
> Patch originally posted here:
> http://lists.busybox.net/pipermail/buildroot/2012-April/052585.html
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Tested-by: Waldemar Brodkorb <wbx@openadk.org>
> Signed-off-by: Waldemar Brodkorb <wbx@openadk.org>
> ---
> hw/m68k/mcf_intc.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/hw/m68k/mcf_intc.c b/hw/m68k/mcf_intc.c
> index 621423c..1d161b1 100644
> --- a/hw/m68k/mcf_intc.c
> +++ b/hw/m68k/mcf_intc.c
> @@ -65,6 +65,10 @@ static uint64_t mcf_intc_read(void *opaque, hwaddr addr,
> return (uint32_t)(s->ifr >> 32);
> case 0x14:
> return (uint32_t)s->ifr;
> + /* Reading from SIMR and CIMR return 0 */
Maybe this comment is not needed if the following code is changed (see
below).
> + case 0x1c:
Add /* SIMR */ comment behind case statement like it was done for SWIACK.
Then either add a /* fall through */ comment or a return 0 (to satisfy
static
code analyzers).
> + case 0x1d:
Dto. for CIMR.
> + return 0;
> case 0xe0: /* SWIACK. */
> return s->active_vector;
> case 0xe1: case 0xe2: case 0xe3: case 0xe4:
> @@ -102,6 +106,20 @@ static void mcf_intc_write(void *opaque, hwaddr addr,
> case 0x0c:
> s->imr = (s->imr & 0xffffffff00000000ull) | (uint32_t)val;
> break;
> + /* SIMR allows to easily mask interrupts */
> + case 0x1c:
> + if (val & 0x40)
> + s->imr = ~0ull;
UINT64_MAX
> + else
> + s->imr |= (1 << (val & 0x3f));
The QEMU coding style requires {}.
> + break;
> + /* CIMR allows to easily unmask interrupts */
> + case 0x1d:
> + if (val & 0x40)
> + s->imr = 0ull;
> + else
> + s->imr &= ~(1 << (val & 0x3f));
Dto.
> + break;
> default:
> hw_error("mcf_intc_write: Bad write offset %d\n", offset);
> break;
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] qemu-m68k: add support for interrupt masking/unmasking
2015-03-22 9:49 ` Stefan Weil
@ 2015-03-22 11:43 ` Peter Maydell
0 siblings, 0 replies; 4+ messages in thread
From: Peter Maydell @ 2015-03-22 11:43 UTC (permalink / raw)
To: Stefan Weil; +Cc: Waldemar Brodkorb, Thomas Petazzoni, QEMU Developers
On 22 March 2015 at 09:49, Stefan Weil <sw@weilnetz.de> wrote:
> Am 22.03.2015 um 10:09 schrieb Waldemar Brodkorb:
>>
>> + case 0x1c:
>
>
> Add /* SIMR */ comment behind case statement like it was done for SWIACK.
> Then either add a /* fall through */ comment or a return 0 (to satisfy
> static
> code analyzers).
No, I don't believe you need a /* fall through */ comment here:
analysers are smart enough to realize that two 'case foo:'
lines with no code at all between them means a deliberate
fall through. It's only where you have
case foo:
some_code();
case bar:
more_code();
that you need to add the comment to indicate that the fall through
was intentional.
Just writing
case 0x1c: /* SIMR */
case 0x1d: /* CIMR */
return 0;
should be good enough.
-- PMM
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] qemu-m68k: add support for interrupt masking/unmasking
2015-03-22 9:09 [Qemu-devel] [PATCH] qemu-m68k: add support for interrupt masking/unmasking Waldemar Brodkorb
2015-03-22 9:49 ` Stefan Weil
@ 2015-03-22 11:40 ` Peter Maydell
1 sibling, 0 replies; 4+ messages in thread
From: Peter Maydell @ 2015-03-22 11:40 UTC (permalink / raw)
To: Waldemar Brodkorb; +Cc: Thomas Petazzoni, QEMU Developers
On 22 March 2015 at 09:09, Waldemar Brodkorb <wbx@openadk.org> wrote:
> Fixes following problem, when trying to boot linux:
> qemu: hardware error: mcf_intc_write: Bad write offset 28
>
> CPU #0:
> D0 = 000000ff A0 = 402ea5dc F0 = 0000000000000000 ( 0)
> D1 = 00000004 A1 = 402ea5e0 F1 = 0000000000000000 ( 0)
> D2 = 00000040 A2 = 40040752 F2 = 0000000000000000 ( 0)
> D3 = 00000000 A3 = 40040a98 F3 = 0000000000000000 ( 0)
> D4 = 00000000 A4 = 400407b4 F4 = 0000000000000000 ( 0)
> D5 = 00000000 A5 = 00000000 F5 = 0000000000000000 ( 0)
> D6 = 00000000 A6 = 40195ff8 F6 = 0000000000000000 ( 0)
> D7 = 00000000 A7 = 40195fd0 F7 = 0000000000000000 ( 0)
> PC = 401b2058 SR = 2704 --Z-- FPRESULT = 0
> Aborted
>
> System started via:
> qemu-system-m68k -nographic -nographic -M mcf5208evb -cpu m5208 -kernel kernel
>
> Patch originally posted here:
> http://lists.busybox.net/pipermail/buildroot/2012-April/052585.html
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Tested-by: Waldemar Brodkorb <wbx@openadk.org>
> Signed-off-by: Waldemar Brodkorb <wbx@openadk.org>
> ---
> hw/m68k/mcf_intc.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/hw/m68k/mcf_intc.c b/hw/m68k/mcf_intc.c
> index 621423c..1d161b1 100644
> --- a/hw/m68k/mcf_intc.c
> +++ b/hw/m68k/mcf_intc.c
> @@ -65,6 +65,10 @@ static uint64_t mcf_intc_read(void *opaque, hwaddr addr,
> return (uint32_t)(s->ifr >> 32);
> case 0x14:
> return (uint32_t)s->ifr;
> + /* Reading from SIMR and CIMR return 0 */
> + case 0x1c:
> + case 0x1d:
> + return 0;
> case 0xe0: /* SWIACK. */
> return s->active_vector;
> case 0xe1: case 0xe2: case 0xe3: case 0xe4:
> @@ -102,6 +106,20 @@ static void mcf_intc_write(void *opaque, hwaddr addr,
> case 0x0c:
> s->imr = (s->imr & 0xffffffff00000000ull) | (uint32_t)val;
> break;
> + /* SIMR allows to easily mask interrupts */
> + case 0x1c:
> + if (val & 0x40)
> + s->imr = ~0ull;
> + else
> + s->imr |= (1 << (val & 0x3f));
This is undefined behaviour for large values of
'val' because 1 is only an int type and we might
try to shift it by more than 30. You need a ULL
suffix on the 1.
> + break;
> + /* CIMR allows to easily unmask interrupts */
> + case 0x1d:
> + if (val & 0x40)
> + s->imr = 0ull;
> + else
> + s->imr &= ~(1 << (val & 0x3f));
> + break;
> default:
> hw_error("mcf_intc_write: Bad write offset %d\n", offset);
> break;
thanks
-- PMM
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-03-22 11:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-22 9:09 [Qemu-devel] [PATCH] qemu-m68k: add support for interrupt masking/unmasking Waldemar Brodkorb
2015-03-22 9:49 ` Stefan Weil
2015-03-22 11:43 ` Peter Maydell
2015-03-22 11:40 ` Peter Maydell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.