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(-)
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(-)
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.2On Mon, 18 Nov 2024 at 02:19, 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>
> ---
> 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
On Mon, 18 Nov 2024 at 20:40, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 18 Nov 2024 at 02:19, 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>
> > ---
> > 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
I read the implementation of the read/write memory ops and I believe
it does the right thing. We want devices to accept reads that are of
any size, instead of returning an error.
> (b) whether there are cases where we should instead
> be updating the implementation and setting the .impl
> min access size ?
Reading the documentation for impl vs valid, we definitely want to set
valid to 1. There should be no machine check when performing byte
reads.
I don't think we want to change .impl.min from the default of 1.
I'm not sure if I've missed something that you're trying to point out.
Are there gotchas about setting valid.min=1 that I should know about?
Cheers,
Joel
On Tue, 19 Nov 2024 at 02:53, Joel Stanley <joel@jms.id.au> wrote: > > On Mon, 18 Nov 2024 at 20:40, Peter Maydell <peter.maydell@linaro.org> wrote: > > 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 > > I read the implementation of the read/write memory ops and I believe > it does the right thing. We want devices to accept reads that are of > any size, instead of returning an error. > > > (b) whether there are cases where we should instead > > be updating the implementation and setting the .impl > > min access size ? > > Reading the documentation for impl vs valid, we definitely want to set > valid to 1. There should be no machine check when performing byte > reads. > > I don't think we want to change .impl.min from the default of 1. > > I'm not sure if I've missed something that you're trying to point out. > Are there gotchas about setting valid.min=1 that I should know about? The "gotcha" is that the memory system's implementation of the size 1 and 2 reads when .impl.min is 4 and .valid.min is 1 (as for instance with aspeed_apb2opb_ops after this patch) is "read 4 bytes, return the relevant portion" and the implementation of size 1 and 2 writes is "pad the small value with zeroes at either end appropriately so it is 4 bytes and write that". That is often the right thing for the required behaviour of the device registers, but it is also quite common that it is the wrong behaviour. For instance for some devices the write of a byte is supposed to only modify that byte, and leave the other bytes of a 4-byte register alone. Or if the device has bit-clears-on-read behaviour for a register then the default handling will incorrectly do that for bits that the guest didn't actually read. Conversely if the device leaves the .impl.min at its default 1 and moves .valid.min from 4 to 1 (as with eg ftgmac100_ops) the device will now be called for byte reads and writes at any address in its range. If a write to, say, byte 3 of a 32-bit register is supposed to update bits [31:24], that won't happen unless the write function is changed (usually if there's a switch on offset the write to something that's not at a multiple-of-4 will end up in the default "log an error" code path). What this adds up to is that it's a bit misleading to have a single patch which changes the minimum access size for lots of devices at once, because for each device you need to look at QEMU's implementation of the read and write functions together with the spec of the device, and confirm that the right way to implement "byte writes are supported" for this particular device is to change .valid.min, and that you don't also need to make changes to the read or write function code at the same time or adjust .impl.min. Putting them all in one patch with no discussion in the commit message of the device behaviour and implementation was just a bit of a yellow flag to me that maybe this complexity wasn't considered. If we get this wrong for one of these devices, it's also likely to be rather easier to bisect the problem if bisection can track it down to "we made this change to aspeed_timer" rather than "we made this change to a dozen devices all at once". -- PMM
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
>
© 2016 - 2024 Red Hat, Inc.