* [PATCH-for-10.0 0/2] hw/misc/aspeed_scu: Correct minimum access size for AST2500 / AST2600
@ 2025-03-31 23:04 Philippe Mathieu-Daudé
2025-03-31 23:04 ` [PATCH-for-10.0 1/2] hw/misc/aspeed_scu: Set MemoryRegionOps::impl::access_size to 32-bit Philippe Mathieu-Daudé
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-31 23:04 UTC (permalink / raw)
To: qemu-devel
Cc: Steven Lee, Peter Maydell, Jamin Lin, Troy Lee, Joel Stanley,
Cédric Le Goater, qemu-arm, Andrew Jeffery,
Philippe Mathieu-Daudé
Mark SCU MemoryRegionOps read/write handler implementations
as 32-bit, then allow down to 8-bit accesses.
Joel Stanley (1):
hw/misc/aspeed_scu: Correct minimum access size for AST2500 / AST2600
Philippe Mathieu-Daudé (1):
hw/misc/aspeed_scu: Set MemoryRegionOps::impl::access_size to 32-bit
hw/misc/aspeed_scu.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
--
2.47.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH-for-10.0 1/2] hw/misc/aspeed_scu: Set MemoryRegionOps::impl::access_size to 32-bit
2025-03-31 23:04 [PATCH-for-10.0 0/2] hw/misc/aspeed_scu: Correct minimum access size for AST2500 / AST2600 Philippe Mathieu-Daudé
@ 2025-03-31 23:04 ` Philippe Mathieu-Daudé
2025-04-01 1:07 ` Andrew Jeffery
2025-03-31 23:04 ` [PATCH-for-10.0 2/2] hw/misc/aspeed_scu: Correct minimum access size for AST2500 / AST2600 Philippe Mathieu-Daudé
2025-04-01 9:43 ` [PATCH-for-10.0 0/2] " Cédric Le Goater
2 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-31 23:04 UTC (permalink / raw)
To: qemu-devel
Cc: Steven Lee, Peter Maydell, Jamin Lin, Troy Lee, Joel Stanley,
Cédric Le Goater, qemu-arm, Andrew Jeffery,
Philippe Mathieu-Daudé
All MemoryRegionOps::read/write() handlers switch over a 32-bit
aligned value, because converted using TO_REG(), which is defined
as:
#define TO_REG(offset) ((offset) >> 2)
So all implementations are 32-bit.
Set min/max access_size accordingly.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/misc/aspeed_scu.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index 76cfd916716..6703f3f9691 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -427,6 +427,10 @@ static const MemoryRegionOps aspeed_ast2400_scu_ops = {
.read = aspeed_scu_read,
.write = aspeed_ast2400_scu_write,
.endianness = DEVICE_LITTLE_ENDIAN,
+ .impl = {
+ .min_access_size = 4,
+ .max_access_size = 4,
+ },
.valid = {
.min_access_size = 1,
.max_access_size = 4,
@@ -437,6 +441,8 @@ static const MemoryRegionOps aspeed_ast2500_scu_ops = {
.read = aspeed_scu_read,
.write = aspeed_ast2500_scu_write,
.endianness = DEVICE_LITTLE_ENDIAN,
+ .impl.min_access_size = 4,
+ .impl.max_access_size = 4,
.valid.min_access_size = 4,
.valid.max_access_size = 4,
.valid.unaligned = false,
@@ -779,6 +785,8 @@ static const MemoryRegionOps aspeed_ast2600_scu_ops = {
.read = aspeed_ast2600_scu_read,
.write = aspeed_ast2600_scu_write,
.endianness = DEVICE_LITTLE_ENDIAN,
+ .impl.min_access_size = 4,
+ .impl.max_access_size = 4,
.valid.min_access_size = 4,
.valid.max_access_size = 4,
.valid.unaligned = false,
@@ -906,6 +914,8 @@ static const MemoryRegionOps aspeed_ast2700_scu_ops = {
.read = aspeed_ast2700_scu_read,
.write = aspeed_ast2700_scu_write,
.endianness = DEVICE_LITTLE_ENDIAN,
+ .impl.min_access_size = 4,
+ .impl.max_access_size = 4,
.valid.min_access_size = 1,
.valid.max_access_size = 8,
.valid.unaligned = false,
@@ -1028,6 +1038,8 @@ static const MemoryRegionOps aspeed_ast2700_scuio_ops = {
.read = aspeed_ast2700_scuio_read,
.write = aspeed_ast2700_scuio_write,
.endianness = DEVICE_LITTLE_ENDIAN,
+ .impl.min_access_size = 4,
+ .impl.max_access_size = 4,
.valid.min_access_size = 1,
.valid.max_access_size = 8,
.valid.unaligned = false,
--
2.47.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH-for-10.0 2/2] hw/misc/aspeed_scu: Correct minimum access size for AST2500 / AST2600
2025-03-31 23:04 [PATCH-for-10.0 0/2] hw/misc/aspeed_scu: Correct minimum access size for AST2500 / AST2600 Philippe Mathieu-Daudé
2025-03-31 23:04 ` [PATCH-for-10.0 1/2] hw/misc/aspeed_scu: Set MemoryRegionOps::impl::access_size to 32-bit Philippe Mathieu-Daudé
@ 2025-03-31 23:04 ` Philippe Mathieu-Daudé
2025-04-01 1:09 ` Andrew Jeffery
2025-04-01 9:43 ` [PATCH-for-10.0 0/2] " Cédric Le Goater
2 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-31 23:04 UTC (permalink / raw)
To: qemu-devel
Cc: Steven Lee, Peter Maydell, Jamin Lin, Troy Lee, Joel Stanley,
Cédric Le Goater, qemu-arm, Andrew Jeffery,
Philippe Mathieu-Daudé
From: Joel Stanley <joel@jms.id.au>
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 SCU 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>
Message-ID: <20241118021820.4928-1-joel@jms.id.au>
[PMD: Rebased, only including SCU changes]
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/misc/aspeed_scu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index 6703f3f9691..1af1a35a081 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -443,7 +443,7 @@ static const MemoryRegionOps aspeed_ast2500_scu_ops = {
.endianness = DEVICE_LITTLE_ENDIAN,
.impl.min_access_size = 4,
.impl.max_access_size = 4,
- .valid.min_access_size = 4,
+ .valid.min_access_size = 1,
.valid.max_access_size = 4,
.valid.unaligned = false,
};
@@ -787,7 +787,7 @@ static const MemoryRegionOps aspeed_ast2600_scu_ops = {
.endianness = DEVICE_LITTLE_ENDIAN,
.impl.min_access_size = 4,
.impl.max_access_size = 4,
- .valid.min_access_size = 4,
+ .valid.min_access_size = 1,
.valid.max_access_size = 4,
.valid.unaligned = false,
};
--
2.47.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH-for-10.0 1/2] hw/misc/aspeed_scu: Set MemoryRegionOps::impl::access_size to 32-bit
2025-03-31 23:04 ` [PATCH-for-10.0 1/2] hw/misc/aspeed_scu: Set MemoryRegionOps::impl::access_size to 32-bit Philippe Mathieu-Daudé
@ 2025-04-01 1:07 ` Andrew Jeffery
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Jeffery @ 2025-04-01 1:07 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Steven Lee, Peter Maydell, Jamin Lin, Troy Lee, Joel Stanley,
Cédric Le Goater, qemu-arm
On Tue, 2025-04-01 at 01:04 +0200, Philippe Mathieu-Daudé wrote:
> All MemoryRegionOps::read/write() handlers switch over a 32-bit
> aligned value, because converted using TO_REG(), which is defined
> as:
>
> #define TO_REG(offset) ((offset) >> 2)
>
> So all implementations are 32-bit.
> Set min/max access_size accordingly.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Andrew Jeffery <andrew@codeconstruct.com.au>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH-for-10.0 2/2] hw/misc/aspeed_scu: Correct minimum access size for AST2500 / AST2600
2025-03-31 23:04 ` [PATCH-for-10.0 2/2] hw/misc/aspeed_scu: Correct minimum access size for AST2500 / AST2600 Philippe Mathieu-Daudé
@ 2025-04-01 1:09 ` Andrew Jeffery
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Jeffery @ 2025-04-01 1:09 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Steven Lee, Peter Maydell, Jamin Lin, Troy Lee, Joel Stanley,
Cédric Le Goater, qemu-arm
On Tue, 2025-04-01 at 01:04 +0200, Philippe Mathieu-Daudé wrote:
> From: Joel Stanley <joel@jms.id.au>
>
> 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 SCU 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>
> Message-ID: <20241118021820.4928-1-joel@jms.id.au>
> [PMD: Rebased, only including SCU changes]
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Andrew Jeffery <andrew@codeconstruct.com.au>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH-for-10.0 0/2] hw/misc/aspeed_scu: Correct minimum access size for AST2500 / AST2600
2025-03-31 23:04 [PATCH-for-10.0 0/2] hw/misc/aspeed_scu: Correct minimum access size for AST2500 / AST2600 Philippe Mathieu-Daudé
2025-03-31 23:04 ` [PATCH-for-10.0 1/2] hw/misc/aspeed_scu: Set MemoryRegionOps::impl::access_size to 32-bit Philippe Mathieu-Daudé
2025-03-31 23:04 ` [PATCH-for-10.0 2/2] hw/misc/aspeed_scu: Correct minimum access size for AST2500 / AST2600 Philippe Mathieu-Daudé
@ 2025-04-01 9:43 ` Cédric Le Goater
2 siblings, 0 replies; 6+ messages in thread
From: Cédric Le Goater @ 2025-04-01 9:43 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Steven Lee, Peter Maydell, Jamin Lin, Troy Lee, Joel Stanley,
qemu-arm, Andrew Jeffery
On 4/1/25 01:04, Philippe Mathieu-Daudé wrote:
> Mark SCU MemoryRegionOps read/write handler implementations
> as 32-bit, then allow down to 8-bit accesses.
>
> Joel Stanley (1):
> hw/misc/aspeed_scu: Correct minimum access size for AST2500 / AST2600
>
> Philippe Mathieu-Daudé (1):
> hw/misc/aspeed_scu: Set MemoryRegionOps::impl::access_size to 32-bit
>
> hw/misc/aspeed_scu.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
Applied to aspeed-next.
Thanks,
C.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-04-01 9:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-31 23:04 [PATCH-for-10.0 0/2] hw/misc/aspeed_scu: Correct minimum access size for AST2500 / AST2600 Philippe Mathieu-Daudé
2025-03-31 23:04 ` [PATCH-for-10.0 1/2] hw/misc/aspeed_scu: Set MemoryRegionOps::impl::access_size to 32-bit Philippe Mathieu-Daudé
2025-04-01 1:07 ` Andrew Jeffery
2025-03-31 23:04 ` [PATCH-for-10.0 2/2] hw/misc/aspeed_scu: Correct minimum access size for AST2500 / AST2600 Philippe Mathieu-Daudé
2025-04-01 1:09 ` Andrew Jeffery
2025-04-01 9:43 ` [PATCH-for-10.0 0/2] " Cédric Le Goater
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).