qemu-m68k: add support for interrupt masking/unmasking
PATCH 1/1 qemu-m68k: add support for interrupt masking/unmasking
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
--- hw/m68k/mcf_intc.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
hw/m68k/mcf_intc.c +18 -0
@@ -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 ;
Signature · 1 line
1.7.10.4
Technically this implementation looks reasonable. I added some remarks below.
Am 22.03.2015 um 10:09 schrieb Waldemar Brodkorb:
Quoted 33 lines
Fixes following problem, when trying to boot linux:
qemu: hardware error:mcf_intc_write: Bad write offset 28CPU #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 = 0Aborted
System started via:
qemu-system-m68k -nographic -nographic -M mcf5208evb -cpu m5208 -kernel kernelPatch originally posted here:
http://lists.busybox.net/pipermail/buildroot/2012-April/052585.htmlSigned-off-by Thomas Petazzoni <[email protected]>Tested-by Waldemar Brodkorb <[email protected]>Signed-off-by Waldemar Brodkorb <[email protected]>--- hw/m68k/mcf_intc.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)hw/m68k/mcf_intc.c +0
-0
hw/m68k/mcf_intc.c static uint64_t mcf_intc_read(void *opaque, hwaddr addr, · line 68
Quoted 4 lines
hw/m68k/mcf_intc.c +1
-0@@ -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).
hw/m68k/mcf_intc.c static uint64_t mcf_intc_read(void *opaque, hwaddr addr, · line 69
Quoted 1 line
hw/m68k/mcf_intc.c +1
-0+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).
hw/m68k/mcf_intc.c static uint64_t mcf_intc_read(void *opaque, hwaddr addr, · line 70
Quoted 1 line
hw/m68k/mcf_intc.c +1
-0+case 0x1d :
Dto. for CIMR.
hw/m68k/mcf_intc.c static void mcf_intc_write(void *opaque, hwaddr addr, · line 112
Quoted 12 lines
hw/m68k/mcf_intc.c +5
-0+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
hw/m68k/mcf_intc.c static void mcf_intc_write(void *opaque, hwaddr addr, · line 114
Quoted 2 lines
hw/m68k/mcf_intc.c +2
-0+else + s->imr |= (1 << (val &0x3f ));
The QEMU coding style requires {}.
hw/m68k/mcf_intc.c static void mcf_intc_write(void *opaque, hwaddr addr, · line 121
Quoted 7 lines
hw/m68k/mcf_intc.c +7
-0+break ; +/* CIMR allows to easily unmask interrupts */ +case 0x1d : +if (val &0x40 ) + s->imr =0ull ; +else + s->imr &= ~(1 << (val &0x3f ));
Dto.
Quoted 4 lines
hw/m68k/mcf_intc.c +1
-0+break ;default :hw_error ("mcf_intc_write: Bad write offset %d\n" , offset);break ;
Quoted 9 lines · Stefan Weil
Am 22.03.2015 um 10:09 schrieb Waldemar Brodkorb:
Quoted 2 lines
+ 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
Quoted 44 lines · Waldemar Brodkorb
Fixes following problem, when trying to boot linux:
qemu: hardware error:mcf_intc_write: Bad write offset 28CPU #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 = 0Aborted
System started via:
qemu-system-m68k -nographic -nographic -M mcf5208evb -cpu m5208 -kernel kernelPatch originally posted here:
http://lists.busybox.net/pipermail/buildroot/2012-April/052585.htmlSigned-off-by Thomas Petazzoni <[email protected]>Tested-by Waldemar Brodkorb <[email protected]>Signed-off-by Waldemar Brodkorb <[email protected]>--- hw/m68k/mcf_intc.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)hw/m68k/mcf_intc.c +4
-0@@ -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 :
hw/m68k/mcf_intc.c static void mcf_intc_write(void *opaque, hwaddr addr, · line 114
Quoted 9 lines
hw/m68k/mcf_intc.c +6
-0@@ -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.
hw/m68k/mcf_intc.c static void mcf_intc_write(void *opaque, hwaddr addr, · line 125
Quoted 11 lines
hw/m68k/mcf_intc.c +8
-0+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