* [PATCH 0/4] hw/misc/a9scu: Verify CPU count is valid and simplify a bit
@ 2020-09-01 14:40 Philippe Mathieu-Daudé
2020-09-01 14:40 ` [PATCH 1/4] hw/misc/a9scu: Do not allow invalid CPU count Philippe Mathieu-Daudé
` (5 more replies)
0 siblings, 6 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-01 14:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, qemu-arm, Philippe Mathieu-Daudé
Trivial patches:
- verify the A9 CPU count is in range,
- simplify using MemoryRegionOps valid/impl,
- log unimplemented registers.
Philippe Mathieu-Daudé (4):
hw/misc/a9scu: Do not allow invalid CPU count
hw/misc/a9scu: Simplify setting MemoryRegionOps::valid fields
hw/misc/a9scu: Simplify setting MemoryRegionOps::impl fields
hw/misc/a9scu: Report unimplemented accesses with qemu_log_mask(UNIMP)
hw/misc/a9scu.c | 59 +++++++++++++++++++++++--------------------------
1 file changed, 28 insertions(+), 31 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/4] hw/misc/a9scu: Do not allow invalid CPU count
2020-09-01 14:40 [PATCH 0/4] hw/misc/a9scu: Verify CPU count is valid and simplify a bit Philippe Mathieu-Daudé
@ 2020-09-01 14:40 ` Philippe Mathieu-Daudé
2020-09-01 14:40 ` [PATCH 2/4] hw/misc/a9scu: Simplify setting MemoryRegionOps::valid fields Philippe Mathieu-Daudé
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-01 14:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, qemu-arm, Philippe Mathieu-Daudé
Per the datasheet (DDI0407 r2p0):
"The SCU connects one to four Cortex-A9 processors to
the memory system through the AXI interfaces."
Change the instance_init() handler to a device_realize()
one so we can verify the property is in range, and return
an error to the caller if not.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/misc/a9scu.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/hw/misc/a9scu.c b/hw/misc/a9scu.c
index 324371a1c00..915f127761e 100644
--- a/hw/misc/a9scu.c
+++ b/hw/misc/a9scu.c
@@ -12,8 +12,11 @@
#include "hw/misc/a9scu.h"
#include "hw/qdev-properties.h"
#include "migration/vmstate.h"
+#include "qapi/error.h"
#include "qemu/module.h"
+#define A9_SCU_CPU_MAX 4
+
static uint64_t a9_scu_read(void *opaque, hwaddr offset,
unsigned size)
{
@@ -105,12 +108,17 @@ static void a9_scu_reset(DeviceState *dev)
s->control = 0;
}
-static void a9_scu_init(Object *obj)
+static void a9_scu_realize(DeviceState *dev, Error **errp)
{
- A9SCUState *s = A9_SCU(obj);
- SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+ A9SCUState *s = A9_SCU(dev);
+ SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
- memory_region_init_io(&s->iomem, obj, &a9_scu_ops, s,
+ if (!s->num_cpu || s->num_cpu > A9_SCU_CPU_MAX) {
+ error_setg(errp, "Illegal CPU count: %u", s->num_cpu);
+ return;
+ }
+
+ memory_region_init_io(&s->iomem, OBJECT(s), &a9_scu_ops, s,
"a9-scu", 0x100);
sysbus_init_mmio(sbd, &s->iomem);
}
@@ -138,13 +146,13 @@ static void a9_scu_class_init(ObjectClass *klass, void *data)
device_class_set_props(dc, a9_scu_properties);
dc->vmsd = &vmstate_a9_scu;
dc->reset = a9_scu_reset;
+ dc->realize = a9_scu_realize;
}
static const TypeInfo a9_scu_info = {
.name = TYPE_A9_SCU,
.parent = TYPE_SYS_BUS_DEVICE,
.instance_size = sizeof(A9SCUState),
- .instance_init = a9_scu_init,
.class_init = a9_scu_class_init,
};
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/4] hw/misc/a9scu: Simplify setting MemoryRegionOps::valid fields
2020-09-01 14:40 [PATCH 0/4] hw/misc/a9scu: Verify CPU count is valid and simplify a bit Philippe Mathieu-Daudé
2020-09-01 14:40 ` [PATCH 1/4] hw/misc/a9scu: Do not allow invalid CPU count Philippe Mathieu-Daudé
@ 2020-09-01 14:40 ` Philippe Mathieu-Daudé
2020-09-01 14:40 ` [PATCH 3/4] hw/misc/a9scu: Simplify setting MemoryRegionOps::impl fields Philippe Mathieu-Daudé
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-01 14:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, qemu-arm, Philippe Mathieu-Daudé
Per the datasheet (DDI0407 r2p0):
"All SCU registers are byte accessible" and are 32-bit aligned.
Set MemoryRegionOps::valid min/max fields and simplify the write()
handler.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/misc/a9scu.c | 21 +++++----------------
1 file changed, 5 insertions(+), 16 deletions(-)
diff --git a/hw/misc/a9scu.c b/hw/misc/a9scu.c
index 915f127761e..3f3dcc414fe 100644
--- a/hw/misc/a9scu.c
+++ b/hw/misc/a9scu.c
@@ -52,23 +52,8 @@ static void a9_scu_write(void *opaque, hwaddr offset,
uint64_t value, unsigned size)
{
A9SCUState *s = (A9SCUState *)opaque;
- uint32_t mask;
+ uint32_t mask = MAKE_64BIT_MASK(0, size * 8);
uint32_t shift;
- switch (size) {
- case 1:
- mask = 0xff;
- break;
- case 2:
- mask = 0xffff;
- break;
- case 4:
- mask = 0xffffffff;
- break;
- default:
- fprintf(stderr, "Invalid size %u in write to a9 scu register %x\n",
- size, (unsigned)offset);
- return;
- }
switch (offset) {
case 0x00: /* Control */
@@ -99,6 +84,10 @@ static void a9_scu_write(void *opaque, hwaddr offset,
static const MemoryRegionOps a9_scu_ops = {
.read = a9_scu_read,
.write = a9_scu_write,
+ .valid = {
+ .min_access_size = 1,
+ .max_access_size = 4,
+ },
.endianness = DEVICE_NATIVE_ENDIAN,
};
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/4] hw/misc/a9scu: Simplify setting MemoryRegionOps::impl fields
2020-09-01 14:40 [PATCH 0/4] hw/misc/a9scu: Verify CPU count is valid and simplify a bit Philippe Mathieu-Daudé
2020-09-01 14:40 ` [PATCH 1/4] hw/misc/a9scu: Do not allow invalid CPU count Philippe Mathieu-Daudé
2020-09-01 14:40 ` [PATCH 2/4] hw/misc/a9scu: Simplify setting MemoryRegionOps::valid fields Philippe Mathieu-Daudé
@ 2020-09-01 14:40 ` Philippe Mathieu-Daudé
2020-09-01 14:41 ` [PATCH 4/4] hw/misc/a9scu: Report unimplemented accesses with qemu_log_mask(UNIMP) Philippe Mathieu-Daudé
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-01 14:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, qemu-arm, Philippe Mathieu-Daudé
This model implementation is designed for 32-bit accesses.
We can simplify setting the MemoryRegionOps::impl min/max
fields to 32-bit (memory::access_with_adjusted_size() will
take care of the 8/16-bit accesses).
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/misc/a9scu.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/hw/misc/a9scu.c b/hw/misc/a9scu.c
index 3f3dcc414fe..47f948341f7 100644
--- a/hw/misc/a9scu.c
+++ b/hw/misc/a9scu.c
@@ -28,12 +28,6 @@ static uint64_t a9_scu_read(void *opaque, hwaddr offset,
return (((1 << s->num_cpu) - 1) << 4) | (s->num_cpu - 1);
case 0x08: /* CPU Power Status */
return s->status;
- case 0x09: /* CPU status. */
- return s->status >> 8;
- case 0x0a: /* CPU status. */
- return s->status >> 16;
- case 0x0b: /* CPU status. */
- return s->status >> 24;
case 0x0c: /* Invalidate All Registers In Secure State */
return 0;
case 0x40: /* Filtering Start Address Register */
@@ -52,8 +46,6 @@ static void a9_scu_write(void *opaque, hwaddr offset,
uint64_t value, unsigned size)
{
A9SCUState *s = (A9SCUState *)opaque;
- uint32_t mask = MAKE_64BIT_MASK(0, size * 8);
- uint32_t shift;
switch (offset) {
case 0x00: /* Control */
@@ -62,9 +54,7 @@ static void a9_scu_write(void *opaque, hwaddr offset,
case 0x4: /* Configuration: RO */
break;
case 0x08: case 0x09: case 0x0A: case 0x0B: /* Power Control */
- shift = (offset - 0x8) * 8;
- s->status &= ~(mask << shift);
- s->status |= ((value & mask) << shift);
+ s->status = value;
break;
case 0x0c: /* Invalidate All Registers In Secure State */
/* no-op as we do not implement caches */
@@ -84,6 +74,10 @@ static void a9_scu_write(void *opaque, hwaddr offset,
static const MemoryRegionOps a9_scu_ops = {
.read = a9_scu_read,
.write = a9_scu_write,
+ .impl = {
+ .min_access_size = 4,
+ .max_access_size = 4,
+ },
.valid = {
.min_access_size = 1,
.max_access_size = 4,
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/4] hw/misc/a9scu: Report unimplemented accesses with qemu_log_mask(UNIMP)
2020-09-01 14:40 [PATCH 0/4] hw/misc/a9scu: Verify CPU count is valid and simplify a bit Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2020-09-01 14:40 ` [PATCH 3/4] hw/misc/a9scu: Simplify setting MemoryRegionOps::impl fields Philippe Mathieu-Daudé
@ 2020-09-01 14:41 ` Philippe Mathieu-Daudé
2020-09-01 21:34 ` [PATCH 0/4] hw/misc/a9scu: Verify CPU count is valid and simplify a bit Richard Henderson
2020-09-07 15:38 ` Peter Maydell
5 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-01 14:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, qemu-arm, Philippe Mathieu-Daudé
Report unimplemented register accesses using qemu_log_mask(UNIMP).
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/misc/a9scu.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/hw/misc/a9scu.c b/hw/misc/a9scu.c
index 47f948341f7..a375ebc9878 100644
--- a/hw/misc/a9scu.c
+++ b/hw/misc/a9scu.c
@@ -13,6 +13,7 @@
#include "hw/qdev-properties.h"
#include "migration/vmstate.h"
#include "qapi/error.h"
+#include "qemu/log.h"
#include "qemu/module.h"
#define A9_SCU_CPU_MAX 4
@@ -38,6 +39,8 @@ static uint64_t a9_scu_read(void *opaque, hwaddr offset,
case 0x54: /* SCU Non-secure Access Control Register */
/* unimplemented, fall through */
default:
+ qemu_log_mask(LOG_UNIMP, "%s: Unsupported offset 0x%"HWADDR_PRIx"\n",
+ __func__, offset);
return 0;
}
}
@@ -67,6 +70,9 @@ static void a9_scu_write(void *opaque, hwaddr offset,
case 0x54: /* SCU Non-secure Access Control Register */
/* unimplemented, fall through */
default:
+ qemu_log_mask(LOG_UNIMP, "%s: Unsupported offset 0x%"HWADDR_PRIx
+ " value 0x%"PRIx64"\n",
+ __func__, offset, value);
break;
}
}
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 0/4] hw/misc/a9scu: Verify CPU count is valid and simplify a bit
2020-09-01 14:40 [PATCH 0/4] hw/misc/a9scu: Verify CPU count is valid and simplify a bit Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2020-09-01 14:41 ` [PATCH 4/4] hw/misc/a9scu: Report unimplemented accesses with qemu_log_mask(UNIMP) Philippe Mathieu-Daudé
@ 2020-09-01 21:34 ` Richard Henderson
2020-09-07 15:38 ` Peter Maydell
5 siblings, 0 replies; 7+ messages in thread
From: Richard Henderson @ 2020-09-01 21:34 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel; +Cc: Peter Maydell, qemu-arm
On 9/1/20 7:40 AM, Philippe Mathieu-Daudé wrote:
> Trivial patches:
> - verify the A9 CPU count is in range,
> - simplify using MemoryRegionOps valid/impl,
> - log unimplemented registers.
>
> Philippe Mathieu-Daudé (4):
> hw/misc/a9scu: Do not allow invalid CPU count
> hw/misc/a9scu: Simplify setting MemoryRegionOps::valid fields
> hw/misc/a9scu: Simplify setting MemoryRegionOps::impl fields
> hw/misc/a9scu: Report unimplemented accesses with qemu_log_mask(UNIMP)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/4] hw/misc/a9scu: Verify CPU count is valid and simplify a bit
2020-09-01 14:40 [PATCH 0/4] hw/misc/a9scu: Verify CPU count is valid and simplify a bit Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2020-09-01 21:34 ` [PATCH 0/4] hw/misc/a9scu: Verify CPU count is valid and simplify a bit Richard Henderson
@ 2020-09-07 15:38 ` Peter Maydell
5 siblings, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2020-09-07 15:38 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu-arm, QEMU Developers
On Tue, 1 Sep 2020 at 15:41, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Trivial patches:
> - verify the A9 CPU count is in range,
> - simplify using MemoryRegionOps valid/impl,
> - log unimplemented registers.
>
> Philippe Mathieu-Daudé (4):
> hw/misc/a9scu: Do not allow invalid CPU count
> hw/misc/a9scu: Simplify setting MemoryRegionOps::valid fields
> hw/misc/a9scu: Simplify setting MemoryRegionOps::impl fields
> hw/misc/a9scu: Report unimplemented accesses with qemu_log_mask(UNIMP)
>
> hw/misc/a9scu.c | 59 +++++++++++++++++++++++--------------------------
> 1 file changed, 28 insertions(+), 31 deletions(-)
Applied to target-arm.next, thanks.
-- PMM
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-09-07 15:39 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-01 14:40 [PATCH 0/4] hw/misc/a9scu: Verify CPU count is valid and simplify a bit Philippe Mathieu-Daudé
2020-09-01 14:40 ` [PATCH 1/4] hw/misc/a9scu: Do not allow invalid CPU count Philippe Mathieu-Daudé
2020-09-01 14:40 ` [PATCH 2/4] hw/misc/a9scu: Simplify setting MemoryRegionOps::valid fields Philippe Mathieu-Daudé
2020-09-01 14:40 ` [PATCH 3/4] hw/misc/a9scu: Simplify setting MemoryRegionOps::impl fields Philippe Mathieu-Daudé
2020-09-01 14:41 ` [PATCH 4/4] hw/misc/a9scu: Report unimplemented accesses with qemu_log_mask(UNIMP) Philippe Mathieu-Daudé
2020-09-01 21:34 ` [PATCH 0/4] hw/misc/a9scu: Verify CPU count is valid and simplify a bit Richard Henderson
2020-09-07 15:38 ` Peter Maydell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.