@samitouri / QOSamiQemu / commits / 80c5be9458

hw/ssi/aspeed_smc: Convert mem ops to read/write_with_attrs for error handling

Error conditions (invalid flash mode, unwritable flash) now return MEMTX_ERROR instead of silently succeeding or returning undefined values. This allows the memory subsystem to properly propagate transaction errors to the guest, improving QEMU reliability. Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3335 Reviewed-by: Jamin Lin <jamin_lin@aspeedtech.com> Link: https://lore.kernel.org/qemu-devel/20260323125545.577653-2-clg@redhat.com Signed-off-by: Cédric Le Goater <clg@redhat.com>

Cédric Le Goater committed Mar 23, 2026 at 13:55 UTC 80c5be945877ea3f258679c6042df8f0efd77202
1 file changed +28 -21
hw/ssi/aspeed_smc.c
+28 -21
@@ -493,17 +493,18 @@ static void aspeed_smc_flash_setup(AspeedSMCFlash *fl, uint32_t addr)
493 }
494 }
495
496 -static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned size)
496 +static MemTxResult aspeed_smc_flash_read(void *opaque, hwaddr addr,
497 + uint64_t *data, unsigned size, MemTxAttrs attrs)
498 {
499 AspeedSMCFlash *fl = opaque;
500 AspeedSMCState *s = fl->controller;
500 - uint64_t ret = 0;
501 int i;
502
503 + *data = 0;
504 switch (aspeed_smc_flash_mode(fl)) {
505 case CTRL_USERMODE:
506 for (i = 0; i < size; i++) {
506 - ret |= (uint64_t) ssi_transfer(s->spi, 0x0) << (8 * i);
507 + *data |= (uint64_t) ssi_transfer(s->spi, 0x0) << (8 * i);
508 }
509 break;
510 case CTRL_READMODE:
@@ -512,18 +513,19 @@ static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned size)
513 aspeed_smc_flash_setup(fl, addr);
514
515 for (i = 0; i < size; i++) {
515 - ret |= (uint64_t) ssi_transfer(s->spi, 0x0) << (8 * i);
516 + *data |= (uint64_t) ssi_transfer(s->spi, 0x0) << (8 * i);
517 }
518
519 aspeed_smc_flash_unselect(fl);
520 break;
521 default:
522 aspeed_smc_error("invalid flash mode %d", aspeed_smc_flash_mode(fl));
523 + return MEMTX_ERROR;
524 }
525
524 - trace_aspeed_smc_flash_read(fl->cs, addr, size, ret,
526 + trace_aspeed_smc_flash_read(fl->cs, addr, size, *data,
527 aspeed_smc_flash_mode(fl));
526 - return ret;
528 + return MEMTX_OK;
529 }
530
531 /*
@@ -624,8 +626,8 @@ static bool aspeed_smc_do_snoop(AspeedSMCFlash *fl, uint64_t data,
626 return false;
627 }
628
627 -static void aspeed_smc_flash_write(void *opaque, hwaddr addr, uint64_t data,
628 - unsigned size)
629 +static MemTxResult aspeed_smc_flash_write(void *opaque, hwaddr addr,
630 + uint64_t data, unsigned size, MemTxAttrs attrs)
631 {
632 AspeedSMCFlash *fl = opaque;
633 AspeedSMCState *s = fl->controller;
@@ -636,7 +638,7 @@ static void aspeed_smc_flash_write(void *opaque, hwaddr addr, uint64_t data,
638
639 if (!aspeed_smc_is_writable(fl)) {
640 aspeed_smc_error("flash is not writable at 0x%" HWADDR_PRIx, addr);
639 - return;
641 + return MEMTX_ERROR;
642 }
643
644 switch (aspeed_smc_flash_mode(fl)) {
@@ -661,12 +663,15 @@ static void aspeed_smc_flash_write(void *opaque, hwaddr addr, uint64_t data,
663 break;
664 default:
665 aspeed_smc_error("invalid flash mode %d", aspeed_smc_flash_mode(fl));
666 + return MEMTX_ERROR;
667 }
668 +
669 + return MEMTX_OK;
670 }
671
672 static const MemoryRegionOps aspeed_smc_flash_ops = {
668 - .read = aspeed_smc_flash_read,
669 - .write = aspeed_smc_flash_write,
673 + .read_with_attrs = aspeed_smc_flash_read,
674 + .write_with_attrs = aspeed_smc_flash_write,
675 .endianness = DEVICE_LITTLE_ENDIAN,
676 .valid = {
677 .min_access_size = 1,
@@ -754,7 +759,8 @@ static void aspeed_smc_reset(DeviceState *d)
759 s->snoop_dummies = 0;
760 }
761
757 -static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size)
762 +static MemTxResult aspeed_smc_read(void *opaque, hwaddr addr, uint64_t *data,
763 + unsigned int size, MemTxAttrs attrs)
764 {
765 AspeedSMCState *s = ASPEED_SMC(opaque);
766 AspeedSMCClass *asc = ASPEED_SMC_GET_CLASS(opaque);
@@ -782,12 +788,13 @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size)
788
789 trace_aspeed_smc_read(addr << 2, size, s->regs[addr]);
790
785 - return s->regs[addr];
791 + *data = s->regs[addr];
792 } else {
793 qemu_log_mask(LOG_UNIMP, "%s: not implemented: 0x%" HWADDR_PRIx "\n",
794 __func__, addr);
789 - return -1;
795 + *data = -1;
796 }
797 + return MEMTX_OK;
798 }
799
800 static uint8_t aspeed_smc_hclk_divisor(uint8_t hclk_mask)
@@ -1108,8 +1115,8 @@ static void aspeed_2600_smc_dma_ctrl(AspeedSMCState *s, uint32_t dma_ctrl)
1115 s->regs[R_DMA_CTRL] &= ~(DMA_CTRL_REQUEST | DMA_CTRL_GRANT);
1116 }
1117
1111 -static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data,
1112 - unsigned int size)
1118 +static MemTxResult aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data,
1119 + unsigned int size, MemTxAttrs attrs)
1120 {
1121 AspeedSMCState *s = ASPEED_SMC(opaque);
1122 AspeedSMCClass *asc = ASPEED_SMC_GET_CLASS(s);
@@ -1159,13 +1166,13 @@ static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data,
1166 } else {
1167 qemu_log_mask(LOG_UNIMP, "%s: not implemented: 0x%" HWADDR_PRIx "\n",
1168 __func__, addr);
1162 - return;
1169 }
1170 + return MEMTX_OK;
1171 }
1172
1173 static const MemoryRegionOps aspeed_smc_ops = {
1167 - .read = aspeed_smc_read,
1168 - .write = aspeed_smc_write,
1174 + .read_with_attrs = aspeed_smc_read,
1175 + .write_with_attrs = aspeed_smc_write,
1176 .endianness = DEVICE_LITTLE_ENDIAN,
1177 };
1178
@@ -2007,8 +2014,8 @@ static const uint32_t aspeed_2700_fmc_resets[ASPEED_SMC_R_MAX] = {
2014 };
2015
2016 static const MemoryRegionOps aspeed_2700_smc_flash_ops = {
2010 - .read = aspeed_smc_flash_read,
2011 - .write = aspeed_smc_flash_write,
2017 + .read_with_attrs = aspeed_smc_flash_read,
2018 + .write_with_attrs = aspeed_smc_flash_write,
2019 .endianness = DEVICE_LITTLE_ENDIAN,
2020 .valid = {
2021 .min_access_size = 1,