Message ID | 1480434248-27138-15-git-send-email-clg@kaod.org |
---|---|
State | New |
Headers | show |
On 29 November 2016 at 15:43, Cédric Le Goater <clg@kaod.org> wrote: > Change the routines prototype to use a 'AspeedSMCFlash *' instead of > 'AspeedSMCState *'. The result will help in making future changes > clearer. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > Reviewed-by: Joel Stanley <joel@jms.id.au> > Reviewed-by: Andrew Jeffery <andrew@aj.id.au> This patch breaks 'make check' because the palmetto-bmc model now segfaults on startup: gdb --args ./build/x86/arm-softmmu/qemu-system-arm -M palmetto-bmc GNU gdb (Ubuntu 7.11.1-0ubuntu1~16.04) 7.11.1 [...] (gdb) r Starting program: /home/petmay01/linaro/qemu-from-laptop/qemu/build/x86/arm-softmmu/qemu-system-arm -M palmetto-bmc [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". [New Thread 0x7fffd1270700 (LWP 30296)] [New Thread 0x7fffcfb97700 (LWP 30297)] [New Thread 0x7fffcf396700 (LWP 30298)] [New Thread 0x7fffceb95700 (LWP 30299)] [New Thread 0x7fffbf709700 (LWP 30300)] Thread 1 "qemu-system-arm" received signal SIGSEGV, Segmentation fault. 0x0000555555b2fce8 in aspeed_smc_flash_update_cs (fl=0x0) at /home/petmay01/linaro/qemu-from-laptop/qemu/hw/ssi/aspeed_smc.c:413 413 AspeedSMCState *s = fl->controller; (gdb) bt #0 0x0000555555b2fce8 in aspeed_smc_flash_update_cs (fl=0x0) at /home/petmay01/linaro/qemu-from-laptop/qemu/hw/ssi/aspeed_smc.c:413 #1 0x0000555555b2fd6a in aspeed_smc_update_cs (s=0x7fffcc3924c0) at /home/petmay01/linaro/qemu-from-laptop/qemu/hw/ssi/aspeed_smc.c:422 #2 0x0000555555b2febc in aspeed_smc_reset (d=0x7fffcc3924c0) at /home/petmay01/linaro/qemu-from-laptop/qemu/hw/ssi/aspeed_smc.c:447 #3 0x0000555555b304a6 in aspeed_smc_realize (dev=0x7fffcc3924c0, errp=0x7fffffffdcf0) at /home/petmay01/linaro/qemu-from-laptop/qemu/hw/ssi/aspeed_smc.c:556 #4 0x0000555555a19329 in device_set_realized (obj=0x7fffcc3924c0, value=true, errp=0x7fffffffde98) at /home/petmay01/linaro/qemu-from-laptop/qemu/hw/core/qdev.c:918 #5 0x0000555555c17e50 in property_set_bool (obj=0x7fffcc3924c0, v= 0x555556ea9d90, name=0x555555d639e7 "realized", opaque=0x5555569cafb0, errp=0x7fffffffde98) at /home/petmay01/linaro/qemu-from-laptop/qemu/qom/object.c:1854 #6 0x0000555555c1635b in object_property_set (obj=0x7fffcc3924c0, v= 0x555556ea9d90, name=0x555555d639e7 "realized", errp=0x7fffffffde98) at /home/petmay01/linaro/qemu-from-laptop/qemu/qom/object.c:1088 #7 0x0000555555c19183 in object_property_set_qobject (obj=0x7fffcc3924c0, value=0x555556ea9c80, name=0x555555d639e7 "realized", errp=0x7fffffffde98) at /home/petmay01/linaro/qemu-from-laptop/qemu/qom/qom-qobject.c:27 #8 0x0000555555c165fe in object_property_set_bool (obj=0x7fffcc3924c0, value=true, name=0x555555d639e7 "realized", errp=0x7fffffffde98) at /home/petmay01/linaro/qemu-from-laptop/qemu/qom/object.c:1157 #9 0x00005555558da673 in aspeed_soc_realize (dev=0x7fffcc371010, errp=0x7fffffffdf20) at /home/petmay01/linaro/qemu-from-laptop/qemu/hw/arm/aspeed_soc.c:256 #10 0x0000555555a19329 in device_set_realized (obj=0x7fffcc371010, value=true, errp=0x555556849518 <error_abort>) at /home/petmay01/linaro/qemu-from-laptop/qemu/hw/core/qdev.c:918 #11 0x0000555555c17e50 in property_set_bool (obj=0x7fffcc371010, v= 0x5555569cc5b0, name=0x555555d63b5c "realized", opaque=0x5555569be320, errp=0x555556849518 <error_abort>) at /home/petmay01/linaro/qemu-from-laptop/qemu/qom/object.c:1854 #12 0x0000555555c1635b in object_property_set (obj=0x7fffcc371010, v= 0x5555569cc5b0, name=0x555555d63b5c "realized", errp=0x555556849518 <error_abort>) at /home/petmay01/linaro/qemu-from-laptop/qemu/qom/object.c:1088 #13 0x0000555555c19183 in object_property_set_qobject (obj=0x7fffcc371010, value=0x5555569cc3a0, name=0x555555d63b5c "realized", errp=0x555556849518 <error_abort>) at /home/petmay01/linaro/qemu-from-laptop/qemu/qom/qom-qobject.c:27 #14 0x0000555555c165fe in object_property_set_bool (obj=0x7fffcc371010, value=true, name=0x555555d63b5c "realized", errp=0x555556849518 <error_abort>) at /home/petmay01/linaro/qemu-from-laptop/qemu/qom/object.c:1157 #15 0x00005555558dadfe in aspeed_board_init (machine=0x55555693d8a0, cfg=0x555556184fe0 <aspeed_boards>) at /home/petmay01/linaro/qemu-from-laptop/qemu/hw/arm/aspeed.c:152 #16 0x00005555558daf9c in palmetto_bmc_init (machine=0x55555693d8a0) at /home/petmay01/linaro/qemu-from-laptop/qemu/hw/arm/aspeed.c:182 #17 0x000055555596af75 in main (argc=3, argv=0x7fffffffe4c8, envp=0x7fffffffe4e8) at /home/petmay01/linaro/qemu-from-laptop/qemu/vl.c:4548 Calling reset from realize is probably a bad idea (reset gets called later anyway), and calling qemu_set_irq() from reset isn't recommended either. These may or may not be the cause of the crash though. thanks -- PMM
On 12/14/2016 06:09 PM, Peter Maydell wrote: > On 29 November 2016 at 15:43, Cédric Le Goater <clg@kaod.org> wrote: >> Change the routines prototype to use a 'AspeedSMCFlash *' instead of >> 'AspeedSMCState *'. The result will help in making future changes >> clearer. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> Reviewed-by: Joel Stanley <joel@jms.id.au> >> Reviewed-by: Andrew Jeffery <andrew@aj.id.au> > > This patch breaks 'make check' because the palmetto-bmc > model now segfaults on startup: > > gdb --args ./build/x86/arm-softmmu/qemu-system-arm -M palmetto-bmc > GNU gdb (Ubuntu 7.11.1-0ubuntu1~16.04) 7.11.1 > [...] > (gdb) r > Starting program: > /home/petmay01/linaro/qemu-from-laptop/qemu/build/x86/arm-softmmu/qemu-system-arm > -M palmetto-bmc > [Thread debugging using libthread_db enabled] > Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". > [New Thread 0x7fffd1270700 (LWP 30296)] > [New Thread 0x7fffcfb97700 (LWP 30297)] > [New Thread 0x7fffcf396700 (LWP 30298)] > [New Thread 0x7fffceb95700 (LWP 30299)] > [New Thread 0x7fffbf709700 (LWP 30300)] > > Thread 1 "qemu-system-arm" received signal SIGSEGV, Segmentation fault. > 0x0000555555b2fce8 in aspeed_smc_flash_update_cs (fl=0x0) > at /home/petmay01/linaro/qemu-from-laptop/qemu/hw/ssi/aspeed_smc.c:413 > 413 AspeedSMCState *s = fl->controller; > (gdb) bt > #0 0x0000555555b2fce8 in aspeed_smc_flash_update_cs (fl=0x0) > at /home/petmay01/linaro/qemu-from-laptop/qemu/hw/ssi/aspeed_smc.c:413 > #1 0x0000555555b2fd6a in aspeed_smc_update_cs (s=0x7fffcc3924c0) > at /home/petmay01/linaro/qemu-from-laptop/qemu/hw/ssi/aspeed_smc.c:422 > #2 0x0000555555b2febc in aspeed_smc_reset (d=0x7fffcc3924c0) > at /home/petmay01/linaro/qemu-from-laptop/qemu/hw/ssi/aspeed_smc.c:447 > #3 0x0000555555b304a6 in aspeed_smc_realize (dev=0x7fffcc3924c0, > errp=0x7fffffffdcf0) at > /home/petmay01/linaro/qemu-from-laptop/qemu/hw/ssi/aspeed_smc.c:556 > #4 0x0000555555a19329 in device_set_realized (obj=0x7fffcc3924c0, > value=true, errp=0x7fffffffde98) at > /home/petmay01/linaro/qemu-from-laptop/qemu/hw/core/qdev.c:918 > #5 0x0000555555c17e50 in property_set_bool (obj=0x7fffcc3924c0, v= > 0x555556ea9d90, name=0x555555d639e7 "realized", > opaque=0x5555569cafb0, errp=0x7fffffffde98) at > /home/petmay01/linaro/qemu-from-laptop/qemu/qom/object.c:1854 > #6 0x0000555555c1635b in object_property_set (obj=0x7fffcc3924c0, v= > 0x555556ea9d90, name=0x555555d639e7 "realized", errp=0x7fffffffde98) > at /home/petmay01/linaro/qemu-from-laptop/qemu/qom/object.c:1088 > #7 0x0000555555c19183 in object_property_set_qobject > (obj=0x7fffcc3924c0, value=0x555556ea9c80, name=0x555555d639e7 > "realized", errp=0x7fffffffde98) > at /home/petmay01/linaro/qemu-from-laptop/qemu/qom/qom-qobject.c:27 > #8 0x0000555555c165fe in object_property_set_bool > (obj=0x7fffcc3924c0, value=true, name=0x555555d639e7 "realized", > errp=0x7fffffffde98) > at /home/petmay01/linaro/qemu-from-laptop/qemu/qom/object.c:1157 > #9 0x00005555558da673 in aspeed_soc_realize (dev=0x7fffcc371010, > errp=0x7fffffffdf20) at > /home/petmay01/linaro/qemu-from-laptop/qemu/hw/arm/aspeed_soc.c:256 > #10 0x0000555555a19329 in device_set_realized (obj=0x7fffcc371010, > value=true, errp=0x555556849518 <error_abort>) > at /home/petmay01/linaro/qemu-from-laptop/qemu/hw/core/qdev.c:918 > #11 0x0000555555c17e50 in property_set_bool (obj=0x7fffcc371010, v= > 0x5555569cc5b0, name=0x555555d63b5c "realized", > opaque=0x5555569be320, errp=0x555556849518 <error_abort>) > at /home/petmay01/linaro/qemu-from-laptop/qemu/qom/object.c:1854 > #12 0x0000555555c1635b in object_property_set (obj=0x7fffcc371010, v= > 0x5555569cc5b0, name=0x555555d63b5c "realized", > errp=0x555556849518 <error_abort>) at > /home/petmay01/linaro/qemu-from-laptop/qemu/qom/object.c:1088 > #13 0x0000555555c19183 in object_property_set_qobject > (obj=0x7fffcc371010, value=0x5555569cc3a0, name=0x555555d63b5c > "realized", errp=0x555556849518 <error_abort>) > at /home/petmay01/linaro/qemu-from-laptop/qemu/qom/qom-qobject.c:27 > #14 0x0000555555c165fe in object_property_set_bool > (obj=0x7fffcc371010, value=true, name=0x555555d63b5c "realized", > errp=0x555556849518 <error_abort>) > at /home/petmay01/linaro/qemu-from-laptop/qemu/qom/object.c:1157 > #15 0x00005555558dadfe in aspeed_board_init (machine=0x55555693d8a0, > cfg=0x555556184fe0 <aspeed_boards>) > at /home/petmay01/linaro/qemu-from-laptop/qemu/hw/arm/aspeed.c:152 > #16 0x00005555558daf9c in palmetto_bmc_init (machine=0x55555693d8a0) > at /home/petmay01/linaro/qemu-from-laptop/qemu/hw/arm/aspeed.c:182 > #17 0x000055555596af75 in main (argc=3, argv=0x7fffffffe4c8, > envp=0x7fffffffe4e8) > at /home/petmay01/linaro/qemu-from-laptop/qemu/vl.c:4548 > > Calling reset from realize is probably a bad idea (reset gets > called later anyway), and calling qemu_set_irq() from reset > isn't recommended either. These may or may not be the cause > of the crash though. It it a bad split with the following patch so I will merge them in the next version and try to cleanup up the qemu_set_irq(). Thanks, C.
diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index 78f5aed53247..66622f198a2f 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -328,19 +328,30 @@ static const MemoryRegionOps aspeed_smc_flash_default_ops = { }, }; -static inline int aspeed_smc_flash_mode(const AspeedSMCState *s, int cs) +static inline int aspeed_smc_flash_mode(const AspeedSMCFlash *fl) { - return s->regs[s->r_ctrl0 + cs] & CTRL_CMD_MODE_MASK; + AspeedSMCState *s = fl->controller; + + return s->regs[s->r_ctrl0 + fl->id] & CTRL_CMD_MODE_MASK; +} + +static inline bool aspeed_smc_is_usermode(const AspeedSMCFlash *fl) +{ + return aspeed_smc_flash_mode(fl) == CTRL_USERMODE; } -static inline bool aspeed_smc_is_usermode(const AspeedSMCState *s, int cs) +static inline bool aspeed_smc_is_ce_stop_active(const AspeedSMCFlash *fl) { - return aspeed_smc_flash_mode(s, cs) == CTRL_USERMODE; + AspeedSMCState *s = fl->controller; + + return s->regs[s->r_ctrl0 + fl->id] & CTRL_CE_STOP_ACTIVE; } -static inline bool aspeed_smc_is_writable(const AspeedSMCState *s, int cs) +static inline bool aspeed_smc_is_writable(const AspeedSMCFlash *fl) { - return s->regs[s->r_conf] & (1 << (s->conf_enable_w0 + cs)); + AspeedSMCState *s = fl->controller; + + return s->regs[s->r_conf] & (1 << (s->conf_enable_w0 + fl->id)); } static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned size) @@ -350,7 +361,7 @@ static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned size) uint64_t ret = 0; int i; - if (aspeed_smc_is_usermode(s, fl->id)) { + if (aspeed_smc_is_usermode(fl)) { for (i = 0; i < size; i++) { ret |= ssi_transfer(s->spi, 0x0) << (8 * i); } @@ -370,13 +381,13 @@ static void aspeed_smc_flash_write(void *opaque, hwaddr addr, uint64_t data, const AspeedSMCState *s = fl->controller; int i; - if (!aspeed_smc_is_writable(s, fl->id)) { + if (!aspeed_smc_is_writable(fl)) { qemu_log_mask(LOG_GUEST_ERROR, "%s: flash is not writable at 0x%" HWADDR_PRIx "\n", __func__, addr); return; } - if (!aspeed_smc_is_usermode(s, fl->id)) { + if (!aspeed_smc_is_usermode(fl)) { qemu_log_mask(LOG_UNIMP, "%s: usermode not implemented\n", __func__); return; @@ -397,9 +408,10 @@ static const MemoryRegionOps aspeed_smc_flash_ops = { }, }; -static bool aspeed_smc_is_ce_stop_active(const AspeedSMCState *s, int cs) +static void aspeed_smc_flash_update_cs(AspeedSMCFlash *fl) { - return s->regs[s->r_ctrl0 + cs] & CTRL_CE_STOP_ACTIVE; + AspeedSMCState *s = fl->controller; + qemu_set_irq(s->cs_lines[fl->id], aspeed_smc_is_ce_stop_active(fl)); } static void aspeed_smc_update_cs(const AspeedSMCState *s) @@ -407,7 +419,7 @@ static void aspeed_smc_update_cs(const AspeedSMCState *s) int i; for (i = 0; i < s->num_cs; ++i) { - qemu_set_irq(s->cs_lines[i], aspeed_smc_is_ce_stop_active(s, i)); + aspeed_smc_flash_update_cs(&s->flashes[i]); } } diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h index bdfbcc0ffa7d..c64954f6103f 100644 --- a/include/hw/ssi/aspeed_smc.h +++ b/include/hw/ssi/aspeed_smc.h @@ -47,7 +47,7 @@ typedef struct AspeedSMCController { } AspeedSMCController; typedef struct AspeedSMCFlash { - const struct AspeedSMCState *controller; + struct AspeedSMCState *controller; uint8_t id; uint32_t size;