| Message ID | 20241118021820.4928-1-joel@jms.id.au (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | hw/aspeed: Correct minimum access size for all models | expand |
On Mon, Nov 18, 2024 at 10:18 AM Joel Stanley <joel@jms.id.au> wrote: > > Guest code was performing a byte load to the SCU MMIO region, leading to > the guest code crashing (it should be using proper accessors, but > that is not Qemu's bug). Hardware and the documentation[1] both agree that > byte loads are okay, so change all of the aspeed devices to accept a > minimum access size of 1. > > [1] See the 'ARM Address Space Mapping' table in the ASPEED docs. This > is section 6.1 in the ast2400 and ast2700, and 7.1 in the ast2500 and > ast2600 datasheets. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2636 > Signed-off-by: Joel Stanley <joel@jms.id.au> Reviewed-by: Troy Lee <leetroy@gmail.com> Troy Lee > --- > hw/fsi/aspeed_apb2opb.c | 2 +- > hw/gpio/aspeed_gpio.c | 4 ++-- > hw/intc/aspeed_vic.c | 2 +- > hw/misc/aspeed_scu.c | 4 ++-- > hw/misc/aspeed_sdmc.c | 2 +- > hw/misc/aspeed_xdma.c | 2 +- > hw/net/ftgmac100.c | 4 ++-- > hw/sd/aspeed_sdhci.c | 2 +- > hw/timer/aspeed_timer.c | 2 +- > hw/watchdog/wdt_aspeed.c | 2 +- > 10 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/hw/fsi/aspeed_apb2opb.c b/hw/fsi/aspeed_apb2opb.c > index 0e2cc143f105..855dccf6094c 100644 > --- a/hw/fsi/aspeed_apb2opb.c > +++ b/hw/fsi/aspeed_apb2opb.c > @@ -259,7 +259,7 @@ static const struct MemoryRegionOps aspeed_apb2opb_ops = { > .read = fsi_aspeed_apb2opb_read, > .write = fsi_aspeed_apb2opb_write, > .valid.max_access_size = 4, > - .valid.min_access_size = 4, > + .valid.min_access_size = 1, > .impl.max_access_size = 4, > .impl.min_access_size = 4, > .endianness = DEVICE_LITTLE_ENDIAN, > diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c > index a5b3f454e800..c8bb7e590696 100644 > --- a/hw/gpio/aspeed_gpio.c > +++ b/hw/gpio/aspeed_gpio.c > @@ -1372,7 +1372,7 @@ static const MemoryRegionOps aspeed_gpio_ops = { > .read = aspeed_gpio_read, > .write = aspeed_gpio_write, > .endianness = DEVICE_LITTLE_ENDIAN, > - .valid.min_access_size = 4, > + .valid.min_access_size = 1, > .valid.max_access_size = 4, > }; > > @@ -1380,7 +1380,7 @@ static const MemoryRegionOps aspeed_gpio_2700_ops = { > .read = aspeed_gpio_2700_read, > .write = aspeed_gpio_2700_write, > .endianness = DEVICE_LITTLE_ENDIAN, > - .valid.min_access_size = 4, > + .valid.min_access_size = 1, > .valid.max_access_size = 4, > }; > > diff --git a/hw/intc/aspeed_vic.c b/hw/intc/aspeed_vic.c > index 55fe51a6675f..8ee662064469 100644 > --- a/hw/intc/aspeed_vic.c > +++ b/hw/intc/aspeed_vic.c > @@ -286,7 +286,7 @@ static const MemoryRegionOps aspeed_vic_ops = { > .read = aspeed_vic_read, > .write = aspeed_vic_write, > .endianness = DEVICE_LITTLE_ENDIAN, > - .valid.min_access_size = 4, > + .valid.min_access_size = 1, > .valid.max_access_size = 4, > .valid.unaligned = false, > }; > diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c > index 2c919349cfc0..b7a62da45907 100644 > --- a/hw/misc/aspeed_scu.c > +++ b/hw/misc/aspeed_scu.c > @@ -436,7 +436,7 @@ static const MemoryRegionOps aspeed_ast2500_scu_ops = { > .read = aspeed_scu_read, > .write = aspeed_ast2500_scu_write, > .endianness = DEVICE_LITTLE_ENDIAN, > - .valid.min_access_size = 4, > + .valid.min_access_size = 1, > .valid.max_access_size = 4, > .valid.unaligned = false, > }; > @@ -777,7 +777,7 @@ static const MemoryRegionOps aspeed_ast2600_scu_ops = { > .read = aspeed_ast2600_scu_read, > .write = aspeed_ast2600_scu_write, > .endianness = DEVICE_LITTLE_ENDIAN, > - .valid.min_access_size = 4, > + .valid.min_access_size = 1, > .valid.max_access_size = 4, > .valid.unaligned = false, > }; > diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c > index 4bc9faf691d6..ba700b008e5e 100644 > --- a/hw/misc/aspeed_sdmc.c > +++ b/hw/misc/aspeed_sdmc.c > @@ -195,7 +195,7 @@ static const MemoryRegionOps aspeed_sdmc_ops = { > .read = aspeed_sdmc_read, > .write = aspeed_sdmc_write, > .endianness = DEVICE_LITTLE_ENDIAN, > - .valid.min_access_size = 4, > + .valid.min_access_size = 1, > .valid.max_access_size = 4, > }; > > diff --git a/hw/misc/aspeed_xdma.c b/hw/misc/aspeed_xdma.c > index 1dd32f72f453..f222c632c099 100644 > --- a/hw/misc/aspeed_xdma.c > +++ b/hw/misc/aspeed_xdma.c > @@ -114,7 +114,7 @@ static const MemoryRegionOps aspeed_xdma_ops = { > .read = aspeed_xdma_read, > .write = aspeed_xdma_write, > .endianness = DEVICE_NATIVE_ENDIAN, > - .valid.min_access_size = 4, > + .valid.min_access_size = 1, > .valid.max_access_size = 4, > }; > > diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c > index 478356ee3e10..c8f6e1138ed0 100644 > --- a/hw/net/ftgmac100.c > +++ b/hw/net/ftgmac100.c > @@ -1150,7 +1150,7 @@ static ssize_t ftgmac100_receive(NetClientState *nc, const uint8_t *buf, > static const MemoryRegionOps ftgmac100_ops = { > .read = ftgmac100_read, > .write = ftgmac100_write, > - .valid.min_access_size = 4, > + .valid.min_access_size = 1, > .valid.max_access_size = 4, > .endianness = DEVICE_LITTLE_ENDIAN, > }; > @@ -1158,7 +1158,7 @@ static const MemoryRegionOps ftgmac100_ops = { > static const MemoryRegionOps ftgmac100_high_ops = { > .read = ftgmac100_high_read, > .write = ftgmac100_high_write, > - .valid.min_access_size = 4, > + .valid.min_access_size = 1, > .valid.max_access_size = 4, > .endianness = DEVICE_LITTLE_ENDIAN, > }; > diff --git a/hw/sd/aspeed_sdhci.c b/hw/sd/aspeed_sdhci.c > index 98d5460905df..85e3f05e438f 100644 > --- a/hw/sd/aspeed_sdhci.c > +++ b/hw/sd/aspeed_sdhci.c > @@ -123,7 +123,7 @@ static const MemoryRegionOps aspeed_sdhci_ops = { > .read = aspeed_sdhci_read, > .write = aspeed_sdhci_write, > .endianness = DEVICE_NATIVE_ENDIAN, > - .valid.min_access_size = 4, > + .valid.min_access_size = 1, > .valid.max_access_size = 4, > }; > > diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c > index 149f7cc5a6aa..a116488aa2dd 100644 > --- a/hw/timer/aspeed_timer.c > +++ b/hw/timer/aspeed_timer.c > @@ -460,7 +460,7 @@ static const MemoryRegionOps aspeed_timer_ops = { > .read = aspeed_timer_read, > .write = aspeed_timer_write, > .endianness = DEVICE_LITTLE_ENDIAN, > - .valid.min_access_size = 4, > + .valid.min_access_size = 1, > .valid.max_access_size = 4, > .valid.unaligned = false, > }; > diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c > index 39c3f362a833..d9fd6fc9079f 100644 > --- a/hw/watchdog/wdt_aspeed.c > +++ b/hw/watchdog/wdt_aspeed.c > @@ -229,7 +229,7 @@ static const MemoryRegionOps aspeed_wdt_ops = { > .read = aspeed_wdt_read, > .write = aspeed_wdt_write, > .endianness = DEVICE_LITTLE_ENDIAN, > - .valid.min_access_size = 4, > + .valid.min_access_size = 1, > .valid.max_access_size = 4, > .valid.unaligned = false, > }; > -- > 2.45.2 >
diff --git a/hw/fsi/aspeed_apb2opb.c b/hw/fsi/aspeed_apb2opb.c index 0e2cc143f105..855dccf6094c 100644 --- a/hw/fsi/aspeed_apb2opb.c +++ b/hw/fsi/aspeed_apb2opb.c @@ -259,7 +259,7 @@ static const struct MemoryRegionOps aspeed_apb2opb_ops = { .read = fsi_aspeed_apb2opb_read, .write = fsi_aspeed_apb2opb_write, .valid.max_access_size = 4, - .valid.min_access_size = 4, + .valid.min_access_size = 1, .impl.max_access_size = 4, .impl.min_access_size = 4, .endianness = DEVICE_LITTLE_ENDIAN, diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c index a5b3f454e800..c8bb7e590696 100644 --- a/hw/gpio/aspeed_gpio.c +++ b/hw/gpio/aspeed_gpio.c @@ -1372,7 +1372,7 @@ static const MemoryRegionOps aspeed_gpio_ops = { .read = aspeed_gpio_read, .write = aspeed_gpio_write, .endianness = DEVICE_LITTLE_ENDIAN, - .valid.min_access_size = 4, + .valid.min_access_size = 1, .valid.max_access_size = 4, }; @@ -1380,7 +1380,7 @@ static const MemoryRegionOps aspeed_gpio_2700_ops = { .read = aspeed_gpio_2700_read, .write = aspeed_gpio_2700_write, .endianness = DEVICE_LITTLE_ENDIAN, - .valid.min_access_size = 4, + .valid.min_access_size = 1, .valid.max_access_size = 4, }; diff --git a/hw/intc/aspeed_vic.c b/hw/intc/aspeed_vic.c index 55fe51a6675f..8ee662064469 100644 --- a/hw/intc/aspeed_vic.c +++ b/hw/intc/aspeed_vic.c @@ -286,7 +286,7 @@ static const MemoryRegionOps aspeed_vic_ops = { .read = aspeed_vic_read, .write = aspeed_vic_write, .endianness = DEVICE_LITTLE_ENDIAN, - .valid.min_access_size = 4, + .valid.min_access_size = 1, .valid.max_access_size = 4, .valid.unaligned = false, }; diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c index 2c919349cfc0..b7a62da45907 100644 --- a/hw/misc/aspeed_scu.c +++ b/hw/misc/aspeed_scu.c @@ -436,7 +436,7 @@ static const MemoryRegionOps aspeed_ast2500_scu_ops = { .read = aspeed_scu_read, .write = aspeed_ast2500_scu_write, .endianness = DEVICE_LITTLE_ENDIAN, - .valid.min_access_size = 4, + .valid.min_access_size = 1, .valid.max_access_size = 4, .valid.unaligned = false, }; @@ -777,7 +777,7 @@ static const MemoryRegionOps aspeed_ast2600_scu_ops = { .read = aspeed_ast2600_scu_read, .write = aspeed_ast2600_scu_write, .endianness = DEVICE_LITTLE_ENDIAN, - .valid.min_access_size = 4, + .valid.min_access_size = 1, .valid.max_access_size = 4, .valid.unaligned = false, }; diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c index 4bc9faf691d6..ba700b008e5e 100644 --- a/hw/misc/aspeed_sdmc.c +++ b/hw/misc/aspeed_sdmc.c @@ -195,7 +195,7 @@ static const MemoryRegionOps aspeed_sdmc_ops = { .read = aspeed_sdmc_read, .write = aspeed_sdmc_write, .endianness = DEVICE_LITTLE_ENDIAN, - .valid.min_access_size = 4, + .valid.min_access_size = 1, .valid.max_access_size = 4, }; diff --git a/hw/misc/aspeed_xdma.c b/hw/misc/aspeed_xdma.c index 1dd32f72f453..f222c632c099 100644 --- a/hw/misc/aspeed_xdma.c +++ b/hw/misc/aspeed_xdma.c @@ -114,7 +114,7 @@ static const MemoryRegionOps aspeed_xdma_ops = { .read = aspeed_xdma_read, .write = aspeed_xdma_write, .endianness = DEVICE_NATIVE_ENDIAN, - .valid.min_access_size = 4, + .valid.min_access_size = 1, .valid.max_access_size = 4, }; diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c index 478356ee3e10..c8f6e1138ed0 100644 --- a/hw/net/ftgmac100.c +++ b/hw/net/ftgmac100.c @@ -1150,7 +1150,7 @@ static ssize_t ftgmac100_receive(NetClientState *nc, const uint8_t *buf, static const MemoryRegionOps ftgmac100_ops = { .read = ftgmac100_read, .write = ftgmac100_write, - .valid.min_access_size = 4, + .valid.min_access_size = 1, .valid.max_access_size = 4, .endianness = DEVICE_LITTLE_ENDIAN, }; @@ -1158,7 +1158,7 @@ static const MemoryRegionOps ftgmac100_ops = { static const MemoryRegionOps ftgmac100_high_ops = { .read = ftgmac100_high_read, .write = ftgmac100_high_write, - .valid.min_access_size = 4, + .valid.min_access_size = 1, .valid.max_access_size = 4, .endianness = DEVICE_LITTLE_ENDIAN, }; diff --git a/hw/sd/aspeed_sdhci.c b/hw/sd/aspeed_sdhci.c index 98d5460905df..85e3f05e438f 100644 --- a/hw/sd/aspeed_sdhci.c +++ b/hw/sd/aspeed_sdhci.c @@ -123,7 +123,7 @@ static const MemoryRegionOps aspeed_sdhci_ops = { .read = aspeed_sdhci_read, .write = aspeed_sdhci_write, .endianness = DEVICE_NATIVE_ENDIAN, - .valid.min_access_size = 4, + .valid.min_access_size = 1, .valid.max_access_size = 4, }; diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c index 149f7cc5a6aa..a116488aa2dd 100644 --- a/hw/timer/aspeed_timer.c +++ b/hw/timer/aspeed_timer.c @@ -460,7 +460,7 @@ static const MemoryRegionOps aspeed_timer_ops = { .read = aspeed_timer_read, .write = aspeed_timer_write, .endianness = DEVICE_LITTLE_ENDIAN, - .valid.min_access_size = 4, + .valid.min_access_size = 1, .valid.max_access_size = 4, .valid.unaligned = false, }; diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c index 39c3f362a833..d9fd6fc9079f 100644 --- a/hw/watchdog/wdt_aspeed.c +++ b/hw/watchdog/wdt_aspeed.c @@ -229,7 +229,7 @@ static const MemoryRegionOps aspeed_wdt_ops = { .read = aspeed_wdt_read, .write = aspeed_wdt_write, .endianness = DEVICE_LITTLE_ENDIAN, - .valid.min_access_size = 4, + .valid.min_access_size = 1, .valid.max_access_size = 4, .valid.unaligned = false, };
Guest code was performing a byte load to the SCU MMIO region, leading to the guest code crashing (it should be using proper accessors, but that is not Qemu's bug). Hardware and the documentation[1] both agree that byte loads are okay, so change all of the aspeed devices to accept a minimum access size of 1. [1] See the 'ARM Address Space Mapping' table in the ASPEED docs. This is section 6.1 in the ast2400 and ast2700, and 7.1 in the ast2500 and ast2600 datasheets. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2636 Signed-off-by: Joel Stanley <joel@jms.id.au> --- hw/fsi/aspeed_apb2opb.c | 2 +- hw/gpio/aspeed_gpio.c | 4 ++-- hw/intc/aspeed_vic.c | 2 +- hw/misc/aspeed_scu.c | 4 ++-- hw/misc/aspeed_sdmc.c | 2 +- hw/misc/aspeed_xdma.c | 2 +- hw/net/ftgmac100.c | 4 ++-- hw/sd/aspeed_sdhci.c | 2 +- hw/timer/aspeed_timer.c | 2 +- hw/watchdog/wdt_aspeed.c | 2 +- 10 files changed, 13 insertions(+), 13 deletions(-)