| Message ID | 20200206112645.21275-2-clg@kaod.org |
|---|---|
| State | New |
| Headers | show |
| Series | aspeed/smc: fix data corruption on witherspoon machines | expand |
On Thu, 6 Feb 2020, at 21:56, Cédric Le Goater wrote: > Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
n On Thu, 6 Feb 2020 at 11:27, Cédric Le Goater <clg@kaod.org> wrote: > > Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> > --- > Makefile.objs | 1 + > hw/ssi/aspeed_smc.c | 17 +++++++++++++++++ > hw/ssi/trace-events | 9 +++++++++ > 3 files changed, 27 insertions(+) > create mode 100644 hw/ssi/trace-events > > diff --git a/Makefile.objs b/Makefile.objs > index 26b9cff95436..9e4ba95794e9 100644 > --- a/Makefile.objs > +++ b/Makefile.objs > @@ -168,6 +168,7 @@ trace-events-subdirs += hw/scsi > trace-events-subdirs += hw/sd > trace-events-subdirs += hw/sparc > trace-events-subdirs += hw/sparc64 > +trace-events-subdirs += hw/ssi > trace-events-subdirs += hw/timer > trace-events-subdirs += hw/tpm > trace-events-subdirs += hw/usb > diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c > index 23c8d2f06245..e5621bf728ca 100644 > --- a/hw/ssi/aspeed_smc.c > +++ b/hw/ssi/aspeed_smc.c > @@ -31,6 +31,7 @@ > #include "qapi/error.h" > #include "exec/address-spaces.h" > #include "qemu/units.h" > +#include "trace.h" > > #include "hw/irq.h" > #include "hw/qdev-properties.h" > @@ -513,6 +514,8 @@ static void aspeed_smc_flash_set_segment(AspeedSMCState *s, int cs, > > s->ctrl->reg_to_segment(s, new, &seg); > > + trace_aspeed_smc_flash_set_segment(cs, new, seg.addr, seg.addr + seg.size); > + > /* The start address of CS0 is read-only */ > if (cs == 0 && seg.addr != s->ctrl->flash_window_base) { > qemu_log_mask(LOG_GUEST_ERROR, > @@ -753,6 +756,8 @@ static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned size) > __func__, aspeed_smc_flash_mode(fl)); > } > > + trace_aspeed_smc_flash_read(fl->id, addr, size, ret, > + aspeed_smc_flash_mode(fl)); > return ret; > } > > @@ -808,6 +813,9 @@ static bool aspeed_smc_do_snoop(AspeedSMCFlash *fl, uint64_t data, > AspeedSMCState *s = fl->controller; > uint8_t addr_width = aspeed_smc_flash_is_4byte(fl) ? 4 : 3; > > + trace_aspeed_smc_do_snoop(fl->id, s->snoop_index, s->snoop_dummies, > + (uint8_t) data & 0xff); > + > if (s->snoop_index == SNOOP_OFF) { > return false; /* Do nothing */ > > @@ -858,6 +866,9 @@ static void aspeed_smc_flash_write(void *opaque, hwaddr addr, uint64_t data, > AspeedSMCState *s = fl->controller; > int i; > > + trace_aspeed_smc_flash_write(fl->id, addr, size, data, > + aspeed_smc_flash_mode(fl)); > + > if (!aspeed_smc_is_writable(fl)) { > qemu_log_mask(LOG_GUEST_ERROR, "%s: flash is not writable at 0x%" > HWADDR_PRIx "\n", __func__, addr); > @@ -972,6 +983,9 @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size) > (s->ctrl->has_dma && addr == R_DMA_CHECKSUM) || > (addr >= R_SEG_ADDR0 && addr < R_SEG_ADDR0 + s->ctrl->max_slaves) || > (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->ctrl->max_slaves)) { > + > + trace_aspeed_smc_read(addr, size, s->regs[addr]); > + > return s->regs[addr]; > } else { > qemu_log_mask(LOG_UNIMP, "%s: not implemented: 0x%" HWADDR_PRIx "\n", > @@ -1091,6 +1105,7 @@ static void aspeed_smc_dma_checksum(AspeedSMCState *s) > __func__, s->regs[R_DMA_FLASH_ADDR]); > return; > } > + trace_aspeed_smc_dma_checksum(s->regs[R_DMA_FLASH_ADDR], data); > > /* > * When the DMA is on-going, the DMA registers are updated > @@ -1225,6 +1240,8 @@ static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data, > > addr >>= 2; > > + trace_aspeed_smc_write(addr, size, data); > + > if (addr == s->r_conf || > (addr >= s->r_timings && > addr < s->r_timings + s->ctrl->nregs_timings) || > diff --git a/hw/ssi/trace-events b/hw/ssi/trace-events > new file mode 100644 > index 000000000000..ffe531a500aa > --- /dev/null > +++ b/hw/ssi/trace-events > @@ -0,0 +1,9 @@ > +# aspeed_smc.c > + > +aspeed_smc_flash_set_segment(int cs, uint64_t reg, uint64_t start, uint64_t end) "CS%d segreg=0x%"PRIx64" [ 0x%"PRIx64" - 0x%"PRIx64" ]" > +aspeed_smc_flash_read(int cs, uint64_t addr, uint32_t size, uint64_t data, int mode) "CS%d @0x%" PRIx64 " size %u: 0x%" PRIx64" mode:%d" > +aspeed_smc_do_snoop(int cs, int index, int dummies, int data) "CS%d index:0x%x dummies:%d data:0x%x" > +aspeed_smc_flash_write(int cs, uint64_t addr, uint32_t size, uint64_t data, int mode) "CS%d @0x%" PRIx64 " size %u: 0x%" PRIx64" mode:%d" > +aspeed_smc_read(uint64_t addr, uint32_t size, uint64_t data) "@0x%" PRIx64 " size %u: 0x%" PRIx64 > +aspeed_smc_dma_checksum(uint32_t addr, uint32_t data) "0x%08x: 0x%08x" > +aspeed_smc_write(uint64_t addr, uint32_t size, uint64_t data) "@0x%" PRIx64 " size %u: 0x%" PRIx64 > -- > 2.21.1 >
On 2/6/20 12:26 PM, Cédric Le Goater wrote: > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > Makefile.objs | 1 + > hw/ssi/aspeed_smc.c | 17 +++++++++++++++++ > hw/ssi/trace-events | 9 +++++++++ > 3 files changed, 27 insertions(+) > create mode 100644 hw/ssi/trace-events > > diff --git a/Makefile.objs b/Makefile.objs > index 26b9cff95436..9e4ba95794e9 100644 > --- a/Makefile.objs > +++ b/Makefile.objs > @@ -168,6 +168,7 @@ trace-events-subdirs += hw/scsi > trace-events-subdirs += hw/sd > trace-events-subdirs += hw/sparc > trace-events-subdirs += hw/sparc64 > +trace-events-subdirs += hw/ssi > trace-events-subdirs += hw/timer > trace-events-subdirs += hw/tpm > trace-events-subdirs += hw/usb > diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c > index 23c8d2f06245..e5621bf728ca 100644 > --- a/hw/ssi/aspeed_smc.c > +++ b/hw/ssi/aspeed_smc.c > @@ -31,6 +31,7 @@ > #include "qapi/error.h" > #include "exec/address-spaces.h" > #include "qemu/units.h" > +#include "trace.h" > > #include "hw/irq.h" > #include "hw/qdev-properties.h" > @@ -513,6 +514,8 @@ static void aspeed_smc_flash_set_segment(AspeedSMCState *s, int cs, > > s->ctrl->reg_to_segment(s, new, &seg); > > + trace_aspeed_smc_flash_set_segment(cs, new, seg.addr, seg.addr + seg.size); > + > /* The start address of CS0 is read-only */ > if (cs == 0 && seg.addr != s->ctrl->flash_window_base) { > qemu_log_mask(LOG_GUEST_ERROR, > @@ -753,6 +756,8 @@ static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned size) > __func__, aspeed_smc_flash_mode(fl)); > } > > + trace_aspeed_smc_flash_read(fl->id, addr, size, ret, > + aspeed_smc_flash_mode(fl)); > return ret; > } > > @@ -808,6 +813,9 @@ static bool aspeed_smc_do_snoop(AspeedSMCFlash *fl, uint64_t data, > AspeedSMCState *s = fl->controller; > uint8_t addr_width = aspeed_smc_flash_is_4byte(fl) ? 4 : 3; > > + trace_aspeed_smc_do_snoop(fl->id, s->snoop_index, s->snoop_dummies, > + (uint8_t) data & 0xff); > + > if (s->snoop_index == SNOOP_OFF) { > return false; /* Do nothing */ > > @@ -858,6 +866,9 @@ static void aspeed_smc_flash_write(void *opaque, hwaddr addr, uint64_t data, > AspeedSMCState *s = fl->controller; > int i; > > + trace_aspeed_smc_flash_write(fl->id, addr, size, data, > + aspeed_smc_flash_mode(fl)); > + > if (!aspeed_smc_is_writable(fl)) { > qemu_log_mask(LOG_GUEST_ERROR, "%s: flash is not writable at 0x%" > HWADDR_PRIx "\n", __func__, addr); > @@ -972,6 +983,9 @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size) > (s->ctrl->has_dma && addr == R_DMA_CHECKSUM) || > (addr >= R_SEG_ADDR0 && addr < R_SEG_ADDR0 + s->ctrl->max_slaves) || > (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->ctrl->max_slaves)) { > + > + trace_aspeed_smc_read(addr, size, s->regs[addr]); > + > return s->regs[addr]; > } else { > qemu_log_mask(LOG_UNIMP, "%s: not implemented: 0x%" HWADDR_PRIx "\n", > @@ -1091,6 +1105,7 @@ static void aspeed_smc_dma_checksum(AspeedSMCState *s) > __func__, s->regs[R_DMA_FLASH_ADDR]); > return; > } > + trace_aspeed_smc_dma_checksum(s->regs[R_DMA_FLASH_ADDR], data); > > /* > * When the DMA is on-going, the DMA registers are updated > @@ -1225,6 +1240,8 @@ static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data, > > addr >>= 2; > > + trace_aspeed_smc_write(addr, size, data); > + > if (addr == s->r_conf || > (addr >= s->r_timings && > addr < s->r_timings + s->ctrl->nregs_timings) || > diff --git a/hw/ssi/trace-events b/hw/ssi/trace-events > new file mode 100644 > index 000000000000..ffe531a500aa > --- /dev/null > +++ b/hw/ssi/trace-events > @@ -0,0 +1,9 @@ > +# aspeed_smc.c > + > +aspeed_smc_flash_set_segment(int cs, uint64_t reg, uint64_t start, uint64_t end) "CS%d segreg=0x%"PRIx64" [ 0x%"PRIx64" - 0x%"PRIx64" ]" > +aspeed_smc_flash_read(int cs, uint64_t addr, uint32_t size, uint64_t data, int mode) "CS%d @0x%" PRIx64 " size %u: 0x%" PRIx64" mode:%d" > +aspeed_smc_do_snoop(int cs, int index, int dummies, int data) "CS%d index:0x%x dummies:%d data:0x%x" > +aspeed_smc_flash_write(int cs, uint64_t addr, uint32_t size, uint64_t data, int mode) "CS%d @0x%" PRIx64 " size %u: 0x%" PRIx64" mode:%d" > +aspeed_smc_read(uint64_t addr, uint32_t size, uint64_t data) "@0x%" PRIx64 " size %u: 0x%" PRIx64 > +aspeed_smc_dma_checksum(uint32_t addr, uint32_t data) "0x%08x: 0x%08x" > +aspeed_smc_write(uint64_t addr, uint32_t size, uint64_t data) "@0x%" PRIx64 " size %u: 0x%" PRIx64 > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
diff --git a/Makefile.objs b/Makefile.objs index 26b9cff95436..9e4ba95794e9 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -168,6 +168,7 @@ trace-events-subdirs += hw/scsi trace-events-subdirs += hw/sd trace-events-subdirs += hw/sparc trace-events-subdirs += hw/sparc64 +trace-events-subdirs += hw/ssi trace-events-subdirs += hw/timer trace-events-subdirs += hw/tpm trace-events-subdirs += hw/usb diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index 23c8d2f06245..e5621bf728ca 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -31,6 +31,7 @@ #include "qapi/error.h" #include "exec/address-spaces.h" #include "qemu/units.h" +#include "trace.h" #include "hw/irq.h" #include "hw/qdev-properties.h" @@ -513,6 +514,8 @@ static void aspeed_smc_flash_set_segment(AspeedSMCState *s, int cs, s->ctrl->reg_to_segment(s, new, &seg); + trace_aspeed_smc_flash_set_segment(cs, new, seg.addr, seg.addr + seg.size); + /* The start address of CS0 is read-only */ if (cs == 0 && seg.addr != s->ctrl->flash_window_base) { qemu_log_mask(LOG_GUEST_ERROR, @@ -753,6 +756,8 @@ static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned size) __func__, aspeed_smc_flash_mode(fl)); } + trace_aspeed_smc_flash_read(fl->id, addr, size, ret, + aspeed_smc_flash_mode(fl)); return ret; } @@ -808,6 +813,9 @@ static bool aspeed_smc_do_snoop(AspeedSMCFlash *fl, uint64_t data, AspeedSMCState *s = fl->controller; uint8_t addr_width = aspeed_smc_flash_is_4byte(fl) ? 4 : 3; + trace_aspeed_smc_do_snoop(fl->id, s->snoop_index, s->snoop_dummies, + (uint8_t) data & 0xff); + if (s->snoop_index == SNOOP_OFF) { return false; /* Do nothing */ @@ -858,6 +866,9 @@ static void aspeed_smc_flash_write(void *opaque, hwaddr addr, uint64_t data, AspeedSMCState *s = fl->controller; int i; + trace_aspeed_smc_flash_write(fl->id, addr, size, data, + aspeed_smc_flash_mode(fl)); + if (!aspeed_smc_is_writable(fl)) { qemu_log_mask(LOG_GUEST_ERROR, "%s: flash is not writable at 0x%" HWADDR_PRIx "\n", __func__, addr); @@ -972,6 +983,9 @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size) (s->ctrl->has_dma && addr == R_DMA_CHECKSUM) || (addr >= R_SEG_ADDR0 && addr < R_SEG_ADDR0 + s->ctrl->max_slaves) || (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->ctrl->max_slaves)) { + + trace_aspeed_smc_read(addr, size, s->regs[addr]); + return s->regs[addr]; } else { qemu_log_mask(LOG_UNIMP, "%s: not implemented: 0x%" HWADDR_PRIx "\n", @@ -1091,6 +1105,7 @@ static void aspeed_smc_dma_checksum(AspeedSMCState *s) __func__, s->regs[R_DMA_FLASH_ADDR]); return; } + trace_aspeed_smc_dma_checksum(s->regs[R_DMA_FLASH_ADDR], data); /* * When the DMA is on-going, the DMA registers are updated @@ -1225,6 +1240,8 @@ static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data, addr >>= 2; + trace_aspeed_smc_write(addr, size, data); + if (addr == s->r_conf || (addr >= s->r_timings && addr < s->r_timings + s->ctrl->nregs_timings) || diff --git a/hw/ssi/trace-events b/hw/ssi/trace-events new file mode 100644 index 000000000000..ffe531a500aa --- /dev/null +++ b/hw/ssi/trace-events @@ -0,0 +1,9 @@ +# aspeed_smc.c + +aspeed_smc_flash_set_segment(int cs, uint64_t reg, uint64_t start, uint64_t end) "CS%d segreg=0x%"PRIx64" [ 0x%"PRIx64" - 0x%"PRIx64" ]" +aspeed_smc_flash_read(int cs, uint64_t addr, uint32_t size, uint64_t data, int mode) "CS%d @0x%" PRIx64 " size %u: 0x%" PRIx64" mode:%d" +aspeed_smc_do_snoop(int cs, int index, int dummies, int data) "CS%d index:0x%x dummies:%d data:0x%x" +aspeed_smc_flash_write(int cs, uint64_t addr, uint32_t size, uint64_t data, int mode) "CS%d @0x%" PRIx64 " size %u: 0x%" PRIx64" mode:%d" +aspeed_smc_read(uint64_t addr, uint32_t size, uint64_t data) "@0x%" PRIx64 " size %u: 0x%" PRIx64 +aspeed_smc_dma_checksum(uint32_t addr, uint32_t data) "0x%08x: 0x%08x" +aspeed_smc_write(uint64_t addr, uint32_t size, uint64_t data) "@0x%" PRIx64 " size %u: 0x%" PRIx64
Signed-off-by: Cédric Le Goater <clg@kaod.org> --- Makefile.objs | 1 + hw/ssi/aspeed_smc.c | 17 +++++++++++++++++ hw/ssi/trace-events | 9 +++++++++ 3 files changed, 27 insertions(+) create mode 100644 hw/ssi/trace-events