On Mon, 18 Nov 2024 at 02:19, Joel Stanley <j...@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 <j...@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(-)
>
> 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,

Have you reviewed all the device read/write function
implementations for these devices to check whether
(a) changing the .valid value does the right thing, or
(b) whether there are cases where we should instead
be updating the implementation and setting the .impl
min access size ?

thanks
-- PMM

Reply via email to