* [PATCH 0/9] hw/arm/aspeed_ast10x0: Map more peripherals & few more fixes
@ 2022-12-29 15:23 Philippe Mathieu-Daudé
2022-12-29 15:23 ` [PATCH 1/9] hw/watchdog/wdt_aspeed: Map the whole MMIO range Philippe Mathieu-Daudé
` (10 more replies)
0 siblings, 11 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-29 15:23 UTC (permalink / raw)
To: qemu-devel
Cc: Steven Lee, Peter Delevoryas, Peter Delevoryas, qemu-arm,
Cleber Rosa, Peter Maydell, Beraldo Leal,
Wainer dos Santos Moschetta, Troy Lee, Andrew Jeffery,
Joel Stanley, Cédric Le Goater, Peter Delevoryas,
Philippe Mathieu-Daudé,
Jamin Lin
Trying to fix some bugs triggered running Zephyr.
Still 2 bugs:
1/
uart:~$ sensor get SYSCLK
[00:00:23.592,000] <err> os: ***** USAGE FAULT *****
[00:00:23.593,000] <err> os: Illegal use of the EPSR
[00:00:23.593,000] <err> os: r0/a1: 0x00033448 r1/a2: 0x00000000 r2/a3: 0x00047f50
[00:00:23.593,000] <err> os: r3/a4: 0x00000000 r12/ip: 0x00000000 r14/lr: 0x00000fbd
[00:00:23.593,000] <err> os: xpsr: 0x60000000
[00:00:23.593,000] <err> os: Faulting instruction address (r15/pc): 0x00000000
[00:00:23.593,000] <err> os: >>> ZEPHYR FATAL ERROR 0: CPU exception on CPU 0
[00:00:23.594,000] <err> os: Current thread: 0x38248 (shell_uart)
[00:00:23.601,000] <err> os: Halting system
2/
uart:~$ mcuboot
[00:01:04.990,000] <err> os: ***** BUS FAULT *****
[00:01:04.990,000] <err> os: Instruction bus error
[00:01:04.991,000] <err> os: r0/a1: 0x00000000 r1/a2: 0x000ffff0 r2/a3: 0x00047ef0
[00:01:04.991,000] <err> os: r3/a4: 0x00000010 r12/ip: 0x6df7ecb5 r14/lr: 0x000188ed
[00:01:04.991,000] <err> os: xpsr: 0x61000000
[00:01:04.991,000] <err> os: Faulting instruction address (r15/pc): 0x6df7ecb4
[00:01:04.991,000] <err> os: >>> ZEPHYR FATAL ERROR 0: CPU exception on CPU 0
[00:01:04.991,000] <err> os: Current thread: 0x38248 (shell_uart)
[00:01:04.994,000] <err> os: Halting system
----------------
IN:
PMSA MPU lookup for reading at 0x0001d400 mmu_idx 65 -> Hit (prot rwx)
0x0001d5a2: 6869 ldr r1, [r5, #4]
0x0001d5a4: 4421 add r1, r4
0x0001d5a6: 6883 ldr r3, [r0, #8]
0x0001d5a8: 681c ldr r4, [r3]
0x0001d5aa: 463a mov r2, r7
0x0001d5ac: 4633 mov r3, r6
0x0001d5ae: 46a4 mov ip, r4
0x0001d5b0: e8bd 41f0 pop.w {r4, r5, r6, r7, r8, lr}
0x0001d5b4: 4760 bx ip
PMSA MPU lookup for reading at 0x00000008 mmu_idx 65 -> Hit (prot rwx)
PMSA MPU lookup for execute at 0x6df7ecb4 mmu_idx 65 -> Hit (prot rwx)
Taking exception 3 [Prefetch Abort] on CPU 0
...at fault address 0x6df7ecb4
...with CFSR.IBUSERR
PMSA MPU lookup for writing at 0x00047ec8 mmu_idx 65 -> Hit (prot rwx)
PMSA MPU lookup for writing at 0x00047ecc mmu_idx 65 -> Hit (prot rwx)
PMSA MPU lookup for writing at 0x00047ed0 mmu_idx 65 -> Hit (prot rwx)
PMSA MPU lookup for writing at 0x00047ed4 mmu_idx 65 -> Hit (prot rwx)
PMSA MPU lookup for writing at 0x00047ed8 mmu_idx 65 -> Hit (prot rwx)
PMSA MPU lookup for writing at 0x00047edc mmu_idx 65 -> Hit (prot rwx)
PMSA MPU lookup for writing at 0x00047ee0 mmu_idx 65 -> Hit (prot rwx)
PMSA MPU lookup for writing at 0x00047ee4 mmu_idx 65 -> Hit (prot rwx)
...taking pending nonsecure exception 5
...loading from element 5 of non-secure vector table at 0x14
...loaded new PC 0xa0cd
----------------
HACE isn't really functional there. I probably screwed smth while wiring
the peripheral. Not obvious without access to the datasheet.
Philippe Mathieu-Daudé (9):
hw/watchdog/wdt_aspeed: Map the whole MMIO range
hw/arm/aspeed: Use the IEC binary prefix definitions
hw/arm/aspeed_ast10x0: Add various unimplemented peripherals
hw/arm/aspeed_ast10x0: Map I3C peripheral
hw/arm/aspeed_ast10x0: Map the secure SRAM
hw/arm/aspeed_ast10x0: Map HACE peripheral
hw/misc/aspeed_hace: Do not crash if address_space_map() failed
hw/arm/aspeed_ast10x0: Add TODO comment to use Cortex-M4F
tests/avocado: Test Aspeed Zephyr SDK v00.01.08 on AST1030 board
hw/arm/aspeed_ast10x0.c | 84 ++++++++++++++++++++++++++++++--
hw/arm/aspeed_ast2600.c | 5 +-
hw/arm/aspeed_soc.c | 6 +--
hw/misc/aspeed_hace.c | 21 +++++---
hw/watchdog/wdt_aspeed.c | 12 +++--
include/hw/arm/aspeed_soc.h | 14 ++++++
include/hw/watchdog/wdt_aspeed.h | 2 +-
tests/avocado/machine_aspeed.py | 41 +++++++++++++++-
8 files changed, 163 insertions(+), 22 deletions(-)
--
2.38.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 1/9] hw/watchdog/wdt_aspeed: Map the whole MMIO range
2022-12-29 15:23 [PATCH 0/9] hw/arm/aspeed_ast10x0: Map more peripherals & few more fixes Philippe Mathieu-Daudé
@ 2022-12-29 15:23 ` Philippe Mathieu-Daudé
2022-12-29 20:42 ` Peter Delevoryas
2023-01-02 10:21 ` Cédric Le Goater
2022-12-29 15:23 ` [PATCH 2/9] hw/arm/aspeed: Use the IEC binary prefix definitions Philippe Mathieu-Daudé
` (9 subsequent siblings)
10 siblings, 2 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-29 15:23 UTC (permalink / raw)
To: qemu-devel
Cc: Steven Lee, Peter Delevoryas, Peter Delevoryas, qemu-arm,
Cleber Rosa, Peter Maydell, Beraldo Leal,
Wainer dos Santos Moschetta, Troy Lee, Andrew Jeffery,
Joel Stanley, Cédric Le Goater, Peter Delevoryas,
Philippe Mathieu-Daudé,
Jamin Lin
Avoid confusing two different things:
- the WDT I/O region size ('iosize')
- at which offset the SoC map the WDT ('offset')
While it is often the same, we can map smaller region sizes at
larger offsets.
Here we are interested in the I/O region size. Rename as 'iosize'
and map the whole range, not only the first implemented registers.
Unimplemented registers accesses are already logged as guest-errors.
Otherwise when booting the demo in [*] we get:
aspeed.io: unimplemented device write (size 4, offset 0x1851a8, value 0x030f1ff1)
aspeed.io: unimplemented device write (size 4, offset 0x1851ac, value 0x03fffff1)
aspeed.io: unimplemented device write (size 4, offset 0x185128, value 0x030f1ff1)
aspeed.io: unimplemented device write (size 4, offset 0x18512c, value 0x03fffff1)
[*] https://github.com/AspeedTech-BMC/zephyr/releases/tag/v00.01.07
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/arm/aspeed_ast10x0.c | 2 +-
hw/arm/aspeed_ast2600.c | 2 +-
hw/arm/aspeed_soc.c | 2 +-
hw/watchdog/wdt_aspeed.c | 12 +++++++-----
include/hw/watchdog/wdt_aspeed.h | 2 +-
5 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index 4d0b9b115f..122b3fd3f3 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -325,7 +325,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
return;
}
aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->wdt[i]), 0,
- sc->memmap[ASPEED_DEV_WDT] + i * awc->offset);
+ sc->memmap[ASPEED_DEV_WDT] + i * awc->iosize);
}
/* GPIO */
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index cd75465c2b..a79e05ddbd 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -472,7 +472,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
return;
}
aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->wdt[i]), 0,
- sc->memmap[ASPEED_DEV_WDT] + i * awc->offset);
+ sc->memmap[ASPEED_DEV_WDT] + i * awc->iosize);
}
/* RAM */
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index b05b9dd416..2c0924d311 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -393,7 +393,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
return;
}
aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->wdt[i]), 0,
- sc->memmap[ASPEED_DEV_WDT] + i * awc->offset);
+ sc->memmap[ASPEED_DEV_WDT] + i * awc->iosize);
}
/* RAM */
diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
index d753693a2e..c1311ce74c 100644
--- a/hw/watchdog/wdt_aspeed.c
+++ b/hw/watchdog/wdt_aspeed.c
@@ -260,6 +260,7 @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
{
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
AspeedWDTState *s = ASPEED_WDT(dev);
+ AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(dev);
assert(s->scu);
@@ -270,8 +271,9 @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
*/
s->pclk_freq = PCLK_HZ;
+ assert(awc->iosize >= ASPEED_WDT_REGS_MAX * 4);
memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_wdt_ops, s,
- TYPE_ASPEED_WDT, ASPEED_WDT_REGS_MAX * 4);
+ TYPE_ASPEED_WDT, awc->iosize);
sysbus_init_mmio(sbd, &s->iomem);
}
@@ -309,7 +311,7 @@ static void aspeed_2400_wdt_class_init(ObjectClass *klass, void *data)
AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass);
dc->desc = "ASPEED 2400 Watchdog Controller";
- awc->offset = 0x20;
+ awc->iosize = 0x20;
awc->ext_pulse_width_mask = 0xff;
awc->reset_ctrl_reg = SCU_RESET_CONTROL1;
awc->wdt_reload = aspeed_wdt_reload;
@@ -346,7 +348,7 @@ static void aspeed_2500_wdt_class_init(ObjectClass *klass, void *data)
AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass);
dc->desc = "ASPEED 2500 Watchdog Controller";
- awc->offset = 0x20;
+ awc->iosize = 0x20;
awc->ext_pulse_width_mask = 0xfffff;
awc->reset_ctrl_reg = SCU_RESET_CONTROL1;
awc->reset_pulse = aspeed_2500_wdt_reset_pulse;
@@ -369,7 +371,7 @@ static void aspeed_2600_wdt_class_init(ObjectClass *klass, void *data)
AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass);
dc->desc = "ASPEED 2600 Watchdog Controller";
- awc->offset = 0x40;
+ awc->iosize = 0x40;
awc->ext_pulse_width_mask = 0xfffff; /* TODO */
awc->reset_ctrl_reg = AST2600_SCU_RESET_CONTROL1;
awc->reset_pulse = aspeed_2500_wdt_reset_pulse;
@@ -392,7 +394,7 @@ static void aspeed_1030_wdt_class_init(ObjectClass *klass, void *data)
AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass);
dc->desc = "ASPEED 1030 Watchdog Controller";
- awc->offset = 0x80;
+ awc->iosize = 0x80;
awc->ext_pulse_width_mask = 0xfffff; /* TODO */
awc->reset_ctrl_reg = AST2600_SCU_RESET_CONTROL1;
awc->reset_pulse = aspeed_2500_wdt_reset_pulse;
diff --git a/include/hw/watchdog/wdt_aspeed.h b/include/hw/watchdog/wdt_aspeed.h
index dfa5dfa424..db91ee6b51 100644
--- a/include/hw/watchdog/wdt_aspeed.h
+++ b/include/hw/watchdog/wdt_aspeed.h
@@ -40,7 +40,7 @@ struct AspeedWDTState {
struct AspeedWDTClass {
SysBusDeviceClass parent_class;
- uint32_t offset;
+ uint32_t iosize;
uint32_t ext_pulse_width_mask;
uint32_t reset_ctrl_reg;
void (*reset_pulse)(AspeedWDTState *s, uint32_t property);
--
2.38.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 2/9] hw/arm/aspeed: Use the IEC binary prefix definitions
2022-12-29 15:23 [PATCH 0/9] hw/arm/aspeed_ast10x0: Map more peripherals & few more fixes Philippe Mathieu-Daudé
2022-12-29 15:23 ` [PATCH 1/9] hw/watchdog/wdt_aspeed: Map the whole MMIO range Philippe Mathieu-Daudé
@ 2022-12-29 15:23 ` Philippe Mathieu-Daudé
2022-12-29 20:42 ` Peter Delevoryas
2022-12-29 15:23 ` [PATCH 3/9] hw/arm/aspeed_ast10x0: Add various unimplemented peripherals Philippe Mathieu-Daudé
` (8 subsequent siblings)
10 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-29 15:23 UTC (permalink / raw)
To: qemu-devel
Cc: Steven Lee, Peter Delevoryas, Peter Delevoryas, qemu-arm,
Cleber Rosa, Peter Maydell, Beraldo Leal,
Wainer dos Santos Moschetta, Troy Lee, Andrew Jeffery,
Joel Stanley, Cédric Le Goater, Peter Delevoryas,
Philippe Mathieu-Daudé,
Jamin Lin
IEC binary prefixes ease code review: the unit is explicit.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/arm/aspeed_ast10x0.c | 3 ++-
hw/arm/aspeed_ast2600.c | 3 ++-
hw/arm/aspeed_soc.c | 4 ++--
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index 122b3fd3f3..3500294df7 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -10,6 +10,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/units.h"
#include "qapi/error.h"
#include "exec/address-spaces.h"
#include "sysemu/sysemu.h"
@@ -348,7 +349,7 @@ static void aspeed_soc_ast1030_class_init(ObjectClass *klass, void *data)
sc->name = "ast1030-a1";
sc->cpu_type = ARM_CPU_TYPE_NAME("cortex-m4");
sc->silicon_rev = AST1030_A1_SILICON_REV;
- sc->sram_size = 0xc0000;
+ sc->sram_size = 768 * KiB;
sc->spis_num = 2;
sc->ehcis_num = 0;
sc->wdts_num = 4;
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index a79e05ddbd..72df72a540 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -8,6 +8,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/units.h"
#include "qapi/error.h"
#include "hw/misc/unimp.h"
#include "hw/arm/aspeed_soc.h"
@@ -619,7 +620,7 @@ static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
sc->name = "ast2600-a3";
sc->cpu_type = ARM_CPU_TYPE_NAME("cortex-a7");
sc->silicon_rev = AST2600_A3_SILICON_REV;
- sc->sram_size = 0x16400;
+ sc->sram_size = 89 * KiB;
sc->spis_num = 2;
sc->ehcis_num = 2;
sc->wdts_num = 4;
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 2c0924d311..677342c9ed 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -517,7 +517,7 @@ static void aspeed_soc_ast2400_class_init(ObjectClass *oc, void *data)
sc->name = "ast2400-a1";
sc->cpu_type = ARM_CPU_TYPE_NAME("arm926");
sc->silicon_rev = AST2400_A1_SILICON_REV;
- sc->sram_size = 0x8000;
+ sc->sram_size = 32 * KiB;
sc->spis_num = 1;
sc->ehcis_num = 1;
sc->wdts_num = 2;
@@ -544,7 +544,7 @@ static void aspeed_soc_ast2500_class_init(ObjectClass *oc, void *data)
sc->name = "ast2500-a1";
sc->cpu_type = ARM_CPU_TYPE_NAME("arm1176");
sc->silicon_rev = AST2500_A1_SILICON_REV;
- sc->sram_size = 0x9000;
+ sc->sram_size = 36 * KiB;
sc->spis_num = 2;
sc->ehcis_num = 2;
sc->wdts_num = 3;
--
2.38.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 3/9] hw/arm/aspeed_ast10x0: Add various unimplemented peripherals
2022-12-29 15:23 [PATCH 0/9] hw/arm/aspeed_ast10x0: Map more peripherals & few more fixes Philippe Mathieu-Daudé
2022-12-29 15:23 ` [PATCH 1/9] hw/watchdog/wdt_aspeed: Map the whole MMIO range Philippe Mathieu-Daudé
2022-12-29 15:23 ` [PATCH 2/9] hw/arm/aspeed: Use the IEC binary prefix definitions Philippe Mathieu-Daudé
@ 2022-12-29 15:23 ` Philippe Mathieu-Daudé
2022-12-29 20:44 ` Peter Delevoryas
2022-12-29 15:23 ` [PATCH 4/9] hw/arm/aspeed_ast10x0: Map I3C peripheral Philippe Mathieu-Daudé
` (7 subsequent siblings)
10 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-29 15:23 UTC (permalink / raw)
To: qemu-devel
Cc: Steven Lee, Peter Delevoryas, Peter Delevoryas, qemu-arm,
Cleber Rosa, Peter Maydell, Beraldo Leal,
Wainer dos Santos Moschetta, Troy Lee, Andrew Jeffery,
Joel Stanley, Cédric Le Goater, Peter Delevoryas,
Philippe Mathieu-Daudé,
Jamin Lin
Based on booting Zephyr demo from [1] running QEMU with
'-d unimp' and checking missing devices in [2].
[1] https://github.com/AspeedTech-BMC/zephyr/releases/tag/v00.01.07
[2] https://github.com/AspeedTech-BMC/zephyr/blob/v00.01.08/dts/arm/aspeed/ast10x0.dtsi
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/arm/aspeed_ast10x0.c | 35 +++++++++++++++++++++++++++++++++++
include/hw/arm/aspeed_soc.h | 11 +++++++++++
2 files changed, 46 insertions(+)
diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index 3500294df7..d7dbc1a801 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -28,10 +28,15 @@ static const hwaddr aspeed_soc_ast1030_memmap[] = {
[ASPEED_DEV_FMC] = 0x7E620000,
[ASPEED_DEV_SPI1] = 0x7E630000,
[ASPEED_DEV_SPI2] = 0x7E640000,
+ [ASPEED_DEV_UDC] = 0x7E6A2000,
[ASPEED_DEV_SCU] = 0x7E6E2000,
+ [ASPEED_DEV_JTAG0] = 0x7E6E4000,
+ [ASPEED_DEV_JTAG1] = 0x7E6E4100,
[ASPEED_DEV_ADC] = 0x7E6E9000,
+ [ASPEED_DEV_ESPI] = 0x7E6EE000,
[ASPEED_DEV_SBC] = 0x7E6F2000,
[ASPEED_DEV_GPIO] = 0x7E780000,
+ [ASPEED_DEV_SGPIOM] = 0x7E780500,
[ASPEED_DEV_TIMER1] = 0x7E782000,
[ASPEED_DEV_UART1] = 0x7E783000,
[ASPEED_DEV_UART2] = 0x7E78D000,
@@ -79,12 +84,17 @@ static const int aspeed_soc_ast1030_irqmap[] = {
[ASPEED_DEV_LPC] = 35,
[ASPEED_DEV_PECI] = 38,
[ASPEED_DEV_FMC] = 39,
+ [ASPEED_DEV_ESPI] = 42,
[ASPEED_DEV_PWM] = 44,
[ASPEED_DEV_ADC] = 46,
[ASPEED_DEV_SPI1] = 65,
[ASPEED_DEV_SPI2] = 66,
[ASPEED_DEV_I2C] = 110, /* 110 ~ 123 */
[ASPEED_DEV_KCS] = 138, /* 138 -> 142 */
+ [ASPEED_DEV_UDC] = 9,
+ [ASPEED_DEV_SGPIOM] = 51,
+ [ASPEED_DEV_JTAG0] = 27,
+ [ASPEED_DEV_JTAG1] = 53,
};
static qemu_irq aspeed_soc_ast1030_get_irq(AspeedSoCState *s, int dev)
@@ -155,6 +165,15 @@ static void aspeed_soc_ast1030_init(Object *obj)
object_initialize_child(obj, "iomem", &s->iomem, TYPE_UNIMPLEMENTED_DEVICE);
object_initialize_child(obj, "sbc-unimplemented", &s->sbc_unimplemented,
TYPE_UNIMPLEMENTED_DEVICE);
+ object_initialize_child(obj, "pwm", &s->pwm, TYPE_UNIMPLEMENTED_DEVICE);
+ object_initialize_child(obj, "espi", &s->espi, TYPE_UNIMPLEMENTED_DEVICE);
+ object_initialize_child(obj, "udc", &s->udc, TYPE_UNIMPLEMENTED_DEVICE);
+ object_initialize_child(obj, "sgpiom", &s->sgpiom,
+ TYPE_UNIMPLEMENTED_DEVICE);
+ object_initialize_child(obj, "jtag[0]", &s->jtag[0],
+ TYPE_UNIMPLEMENTED_DEVICE);
+ object_initialize_child(obj, "jtag[1]", &s->jtag[1],
+ TYPE_UNIMPLEMENTED_DEVICE);
}
static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
@@ -337,6 +356,22 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
sc->memmap[ASPEED_DEV_GPIO]);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio), 0,
aspeed_soc_get_irq(s, ASPEED_DEV_GPIO));
+
+ aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->pwm), "aspeed.pwm",
+ sc->memmap[ASPEED_DEV_PWM], 0x100);
+
+ aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->espi), "aspeed.espi",
+ sc->memmap[ASPEED_DEV_ESPI], 0x800);
+
+ aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->udc), "aspeed.udc",
+ sc->memmap[ASPEED_DEV_UDC], 0x1000);
+ aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->sgpiom), "aspeed.sgpiom",
+ sc->memmap[ASPEED_DEV_SGPIOM], 0x100);
+
+ aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->jtag[0]), "aspeed.jtag",
+ sc->memmap[ASPEED_DEV_JTAG0], 0x20);
+ aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->jtag[1]), "aspeed.jtag",
+ sc->memmap[ASPEED_DEV_JTAG1], 0x20);
}
static void aspeed_soc_ast1030_class_init(ObjectClass *klass, void *data)
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 8389200b2d..9a5e3c0bac 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -44,6 +44,7 @@
#define ASPEED_CPUS_NUM 2
#define ASPEED_MACS_NUM 4
#define ASPEED_UARTS_NUM 13
+#define ASPEED_JTAG_NUM 2
struct AspeedSoCState {
/*< private >*/
@@ -87,6 +88,11 @@ struct AspeedSoCState {
UnimplementedDeviceState video;
UnimplementedDeviceState emmc_boot_controller;
UnimplementedDeviceState dpmcu;
+ UnimplementedDeviceState pwm;
+ UnimplementedDeviceState espi;
+ UnimplementedDeviceState udc;
+ UnimplementedDeviceState sgpiom;
+ UnimplementedDeviceState jtag[ASPEED_JTAG_NUM];
};
#define TYPE_ASPEED_SOC "aspeed-soc"
@@ -174,6 +180,11 @@ enum {
ASPEED_DEV_DPMCU,
ASPEED_DEV_DP,
ASPEED_DEV_I3C,
+ ASPEED_DEV_ESPI,
+ ASPEED_DEV_UDC,
+ ASPEED_DEV_SGPIOM,
+ ASPEED_DEV_JTAG0,
+ ASPEED_DEV_JTAG1,
};
qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int dev);
--
2.38.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 4/9] hw/arm/aspeed_ast10x0: Map I3C peripheral
2022-12-29 15:23 [PATCH 0/9] hw/arm/aspeed_ast10x0: Map more peripherals & few more fixes Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2022-12-29 15:23 ` [PATCH 3/9] hw/arm/aspeed_ast10x0: Add various unimplemented peripherals Philippe Mathieu-Daudé
@ 2022-12-29 15:23 ` Philippe Mathieu-Daudé
2022-12-29 20:46 ` Peter Delevoryas
2022-12-29 15:23 ` [PATCH 5/9] hw/arm/aspeed_ast10x0: Map the secure SRAM Philippe Mathieu-Daudé
` (6 subsequent siblings)
10 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-29 15:23 UTC (permalink / raw)
To: qemu-devel
Cc: Steven Lee, Peter Delevoryas, Peter Delevoryas, qemu-arm,
Cleber Rosa, Peter Maydell, Beraldo Leal,
Wainer dos Santos Moschetta, Troy Lee, Andrew Jeffery,
Joel Stanley, Cédric Le Goater, Peter Delevoryas,
Philippe Mathieu-Daudé,
Jamin Lin
Since I don't have access to the datasheet, the relevant
values were found in:
https://github.com/AspeedTech-BMC/zephyr/blob/v00.01.08/dts/arm/aspeed/ast10x0.dtsi
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/arm/aspeed_ast10x0.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index d7dbc1a801..53ea6d471f 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -54,6 +54,7 @@ static const hwaddr aspeed_soc_ast1030_memmap[] = {
[ASPEED_DEV_WDT] = 0x7E785000,
[ASPEED_DEV_LPC] = 0x7E789000,
[ASPEED_DEV_PECI] = 0x7E78B000,
+ [ASPEED_DEV_I3C] = 0x7E7A0000,
[ASPEED_DEV_I2C] = 0x7E7B0000,
};
@@ -89,6 +90,7 @@ static const int aspeed_soc_ast1030_irqmap[] = {
[ASPEED_DEV_ADC] = 46,
[ASPEED_DEV_SPI1] = 65,
[ASPEED_DEV_SPI2] = 66,
+ [ASPEED_DEV_I3C] = 102, /* 102 -> 105 */
[ASPEED_DEV_I2C] = 110, /* 110 ~ 123 */
[ASPEED_DEV_KCS] = 138, /* 138 -> 142 */
[ASPEED_DEV_UDC] = 9,
@@ -130,6 +132,8 @@ static void aspeed_soc_ast1030_init(Object *obj)
snprintf(typename, sizeof(typename), "aspeed.i2c-%s", socname);
object_initialize_child(obj, "i2c", &s->i2c, typename);
+ object_initialize_child(obj, "i3c", &s->i3c, TYPE_ASPEED_I3C);
+
snprintf(typename, sizeof(typename), "aspeed.timer-%s", socname);
object_initialize_child(obj, "timerctrl", &s->timerctrl, typename);
@@ -240,6 +244,18 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c.busses[i]), 0, irq);
}
+ /* I3C */
+ if (!sysbus_realize(SYS_BUS_DEVICE(&s->i3c), errp)) {
+ return;
+ }
+ aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->i3c), 0, sc->memmap[ASPEED_DEV_I3C]);
+ for (i = 0; i < ASPEED_I3C_NR_DEVICES; i++) {
+ qemu_irq irq = qdev_get_gpio_in(DEVICE(&s->armv7m),
+ sc->irqmap[ASPEED_DEV_I3C] + i);
+ /* The AST1030 I2C controller has one IRQ per bus. */
+ sysbus_connect_irq(SYS_BUS_DEVICE(&s->i3c.devices[i]), 0, irq);
+ }
+
/* PECI */
if (!sysbus_realize(SYS_BUS_DEVICE(&s->peci), errp)) {
return;
--
2.38.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 5/9] hw/arm/aspeed_ast10x0: Map the secure SRAM
2022-12-29 15:23 [PATCH 0/9] hw/arm/aspeed_ast10x0: Map more peripherals & few more fixes Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2022-12-29 15:23 ` [PATCH 4/9] hw/arm/aspeed_ast10x0: Map I3C peripheral Philippe Mathieu-Daudé
@ 2022-12-29 15:23 ` Philippe Mathieu-Daudé
2022-12-29 20:50 ` Peter Delevoryas
2022-12-29 15:23 ` [PATCH 6/9] hw/arm/aspeed_ast10x0: Map HACE peripheral Philippe Mathieu-Daudé
` (5 subsequent siblings)
10 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-29 15:23 UTC (permalink / raw)
To: qemu-devel
Cc: Steven Lee, Peter Delevoryas, Peter Delevoryas, qemu-arm,
Cleber Rosa, Peter Maydell, Beraldo Leal,
Wainer dos Santos Moschetta, Troy Lee, Andrew Jeffery,
Joel Stanley, Cédric Le Goater, Peter Delevoryas,
Philippe Mathieu-Daudé,
Jamin Lin
Some SRAM appears to be used by the Secure Boot unit and
crypto accelerators. Name it 'secure sram'.
Note, the SRAM base address was already present but unused
(the 'SBC' index is used for the MMIO peripheral).
Interestingly using CFLAGS=-Winitializer-overrides reports:
../hw/arm/aspeed_ast10x0.c:32:30: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
[ASPEED_DEV_SBC] = 0x7E6F2000,
^~~~~~~~~~
../hw/arm/aspeed_ast10x0.c:24:30: note: previous initialization is here
[ASPEED_DEV_SBC] = 0x79000000,
^~~~~~~~~~
This fixes with Zephyr:
uart:~$ rsa test
rsa test vector[0]:
[00:00:26.156,000] <err> os: ***** BUS FAULT *****
[00:00:26.157,000] <err> os: Precise data bus error
[00:00:26.157,000] <err> os: BFAR Address: 0x79000000
[00:00:26.158,000] <err> os: r0/a1: 0x79000000 r1/a2: 0x00000000 r2/a3: 0x00001800
[00:00:26.158,000] <err> os: r3/a4: 0x79001800 r12/ip: 0x00000800 r14/lr: 0x0001098d
[00:00:26.158,000] <err> os: xpsr: 0x81000000
[00:00:26.158,000] <err> os: Faulting instruction address (r15/pc): 0x0001e1bc
[00:00:26.158,000] <err> os: >>> ZEPHYR FATAL ERROR 0: CPU exception on CPU 0
[00:00:26.158,000] <err> os: Current thread: 0x38248 (shell_uart)
[00:00:26.165,000] <err> os: Halting system
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/arm/aspeed_ast10x0.c | 11 ++++++++++-
include/hw/arm/aspeed_soc.h | 3 +++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index 53ea6d471f..21a2e62345 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -22,7 +22,7 @@
static const hwaddr aspeed_soc_ast1030_memmap[] = {
[ASPEED_DEV_SRAM] = 0x00000000,
- [ASPEED_DEV_SBC] = 0x79000000,
+ [ASPEED_DEV_SECSRAM] = 0x79000000,
[ASPEED_DEV_IOMEM] = 0x7E600000,
[ASPEED_DEV_PWM] = 0x7E610000,
[ASPEED_DEV_FMC] = 0x7E620000,
@@ -222,6 +222,14 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
memory_region_add_subregion(s->memory,
sc->memmap[ASPEED_DEV_SRAM],
&s->sram);
+ memory_region_init_ram(&s->secsram, OBJECT(s), "sec.sram",
+ sc->secsram_size, &err);
+ if (err != NULL) {
+ error_propagate(errp, err);
+ return;
+ }
+ memory_region_add_subregion(s->memory, sc->memmap[ASPEED_DEV_SECSRAM],
+ &s->secsram);
/* SCU */
if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
@@ -401,6 +409,7 @@ static void aspeed_soc_ast1030_class_init(ObjectClass *klass, void *data)
sc->cpu_type = ARM_CPU_TYPE_NAME("cortex-m4");
sc->silicon_rev = AST1030_A1_SILICON_REV;
sc->sram_size = 768 * KiB;
+ sc->secsram_size = 9 * KiB;
sc->spis_num = 2;
sc->ehcis_num = 0;
sc->wdts_num = 4;
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 9a5e3c0bac..bd1e03e78a 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -71,6 +71,7 @@ struct AspeedSoCState {
AspeedSMCState spi[ASPEED_SPIS_NUM];
EHCISysBusState ehci[ASPEED_EHCIS_NUM];
AspeedSBCState sbc;
+ MemoryRegion secsram;
UnimplementedDeviceState sbc_unimplemented;
AspeedSDMCState sdmc;
AspeedWDTState wdt[ASPEED_WDTS_NUM];
@@ -105,6 +106,7 @@ struct AspeedSoCClass {
const char *cpu_type;
uint32_t silicon_rev;
uint64_t sram_size;
+ uint64_t secsram_size;
int spis_num;
int ehcis_num;
int wdts_num;
@@ -143,6 +145,7 @@ enum {
ASPEED_DEV_SCU,
ASPEED_DEV_ADC,
ASPEED_DEV_SBC,
+ ASPEED_DEV_SECSRAM,
ASPEED_DEV_EMMC_BC,
ASPEED_DEV_VIDEO,
ASPEED_DEV_SRAM,
--
2.38.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 6/9] hw/arm/aspeed_ast10x0: Map HACE peripheral
2022-12-29 15:23 [PATCH 0/9] hw/arm/aspeed_ast10x0: Map more peripherals & few more fixes Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2022-12-29 15:23 ` [PATCH 5/9] hw/arm/aspeed_ast10x0: Map the secure SRAM Philippe Mathieu-Daudé
@ 2022-12-29 15:23 ` Philippe Mathieu-Daudé
2022-12-29 20:52 ` Peter Delevoryas
2022-12-29 15:23 ` [PATCH 7/9] hw/misc/aspeed_hace: Do not crash if address_space_map() failed Philippe Mathieu-Daudé
` (4 subsequent siblings)
10 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-29 15:23 UTC (permalink / raw)
To: qemu-devel
Cc: Steven Lee, Peter Delevoryas, Peter Delevoryas, qemu-arm,
Cleber Rosa, Peter Maydell, Beraldo Leal,
Wainer dos Santos Moschetta, Troy Lee, Andrew Jeffery,
Joel Stanley, Cédric Le Goater, Peter Delevoryas,
Philippe Mathieu-Daudé,
Jamin Lin
Since I don't have access to the datasheet, the relevant
values were found in:
https://github.com/AspeedTech-BMC/zephyr/blob/v00.01.08/dts/arm/aspeed/ast10x0.dtsi
Before on Zephyr:
uart:~$ crypto aes256_cbc_vault
aes256_cbc vault key 1
[00:00:06.699,000] <inf> hace_global: aspeed_crypto_session_setup
[00:00:06.699,000] <inf> hace_global: data->cmd: 1c2098
[00:00:06.699,000] <inf> hace_global: crypto_data_src: 93340
[00:00:06.699,000] <inf> hace_global: crypto_data_dst: 93348
[00:00:06.699,000] <inf> hace_global: crypto_ctx_base: 93300
[00:00:06.699,000] <inf> hace_global: crypto_data_len: 80000040
[00:00:06.699,000] <inf> hace_global: crypto_cmd_reg: 11c2098
[00:00:09.743,000] <inf> hace_global: HACE_STS: 0
[00:00:09.743,000] <err> hace_global: HACE poll timeout
[00:00:09.743,000] <err> crypto: CBC mode ENCRYPT - Failed
[00:00:09.743,000] <inf> hace_global: aspeed_crypto_session_free
uart:~$
After:
uart:~$ crypto aes256_cbc_vault
aes256_cbc vault key 1
Was waiting for:
6b c1 be e2 2e 40 9f 96 e9 3d 7e 11 73 93 17 2a
ae 2d 8a 57 1e 03 ac 9c 9e b7 6f ac 45 af 8e 51
30 c8 1c 46 a3 5c e4 11 e5 fb c1 19 1a 0a 52 ef
f6 9f 24 45 df 4f 9b 17 ad 2b 41 7b e6 6c 37 10
But got:
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[00:00:05.771,000] <inf> hace_global: aspeed_crypto_session_setup
[00:00:05.772,000] <inf> hace_global: data->cmd: 1c2098
[00:00:05.772,000] <inf> hace_global: crypto_data_src: 93340
[00:00:05.772,000] <inf> hace_global: crypto_data_dst: 93348
[00:00:05.772,000] <inf> hace_global: crypto_ctx_base: 93300
[00:00:05.772,000] <inf> hace_global: crypto_data_len: 80000040
[00:00:05.772,000] <inf> hace_global: crypto_cmd_reg: 11c2098
[00:00:05.772,000] <inf> hace_global: HACE_STS: 1000
[00:00:05.772,000] <inf> crypto: Output length (encryption): 80
[00:00:05.772,000] <inf> hace_global: aspeed_crypto_session_free
[00:00:05.772,000] <inf> hace_global: aspeed_crypto_session_setup
[00:00:05.772,000] <inf> hace_global: data->cmd: 1c2018
[00:00:05.772,000] <inf> hace_global: crypto_data_src: 93340
[00:00:05.772,000] <inf> hace_global: crypto_data_dst: 93348
[00:00:05.772,000] <inf> hace_global: crypto_ctx_base: 93300
[00:00:05.772,000] <inf> hace_global: crypto_data_len: 80000040
[00:00:05.772,000] <inf> hace_global: crypto_cmd_reg: 11c2018
[00:00:05.772,000] <inf> hace_global: HACE_STS: 1000
[00:00:05.772,000] <inf> crypto: Output length (decryption): 64
[00:00:05.772,000] <err> crypto: CBC mode DECRYPT - Mismatch between plaintext and decrypted cipher text
[00:00:05.774,000] <inf> hace_global: aspeed_crypto_session_free
uart:~$
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
Should we rename HACE 'dram' as 'secram' / 'secure-ram'?
---
hw/arm/aspeed_ast10x0.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index 21a2e62345..02636705b6 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -29,6 +29,7 @@ static const hwaddr aspeed_soc_ast1030_memmap[] = {
[ASPEED_DEV_SPI1] = 0x7E630000,
[ASPEED_DEV_SPI2] = 0x7E640000,
[ASPEED_DEV_UDC] = 0x7E6A2000,
+ [ASPEED_DEV_HACE] = 0x7E6D0000,
[ASPEED_DEV_SCU] = 0x7E6E2000,
[ASPEED_DEV_JTAG0] = 0x7E6E4000,
[ASPEED_DEV_JTAG1] = 0x7E6E4100,
@@ -166,6 +167,9 @@ static void aspeed_soc_ast1030_init(Object *obj)
snprintf(typename, sizeof(typename), "aspeed.gpio-%s", socname);
object_initialize_child(obj, "gpio", &s->gpio, typename);
+ snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
+ object_initialize_child(obj, "hace", &s->hace, typename);
+
object_initialize_child(obj, "iomem", &s->iomem, TYPE_UNIMPLEMENTED_DEVICE);
object_initialize_child(obj, "sbc-unimplemented", &s->sbc_unimplemented,
TYPE_UNIMPLEMENTED_DEVICE);
@@ -359,6 +363,17 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
}
aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->sbc), 0, sc->memmap[ASPEED_DEV_SBC]);
+ /* HACE */
+ object_property_set_link(OBJECT(&s->hace), "dram", OBJECT(&s->secsram),
+ &error_abort);
+ if (!sysbus_realize(SYS_BUS_DEVICE(&s->hace), errp)) {
+ return;
+ }
+ aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->hace), 0,
+ sc->memmap[ASPEED_DEV_HACE]);
+ sysbus_connect_irq(SYS_BUS_DEVICE(&s->hace), 0,
+ aspeed_soc_get_irq(s, ASPEED_DEV_HACE));
+
/* Watch dog */
for (i = 0; i < sc->wdts_num; i++) {
AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(&s->wdt[i]);
--
2.38.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 7/9] hw/misc/aspeed_hace: Do not crash if address_space_map() failed
2022-12-29 15:23 [PATCH 0/9] hw/arm/aspeed_ast10x0: Map more peripherals & few more fixes Philippe Mathieu-Daudé
` (5 preceding siblings ...)
2022-12-29 15:23 ` [PATCH 6/9] hw/arm/aspeed_ast10x0: Map HACE peripheral Philippe Mathieu-Daudé
@ 2022-12-29 15:23 ` Philippe Mathieu-Daudé
2022-12-29 20:54 ` Peter Delevoryas
2022-12-29 15:23 ` [PATCH 8/9] hw/arm/aspeed_ast10x0: Add TODO comment to use Cortex-M4F Philippe Mathieu-Daudé
` (3 subsequent siblings)
10 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-29 15:23 UTC (permalink / raw)
To: qemu-devel
Cc: Steven Lee, Peter Delevoryas, Peter Delevoryas, qemu-arm,
Cleber Rosa, Peter Maydell, Beraldo Leal,
Wainer dos Santos Moschetta, Troy Lee, Andrew Jeffery,
Joel Stanley, Cédric Le Goater, Peter Delevoryas,
Philippe Mathieu-Daudé,
Jamin Lin
address_space_map() can fail:
uart:~$ hash test
sha256_test
tv[0]:
Segmentation fault: 11
Thread 3 "qemu-system-arm" received signal SIGSEGV, Segmentation fault.
gen_acc_mode_iov (req_len=0x7ffff18b7778, id=<optimized out>, iov=0x7ffff18b7780, s=0x555556ce0bd0)
at ../hw/misc/aspeed_hace.c:171
171 if (has_padding(s, &iov[id], *req_len, &total_msg_len, &pad_offset)) {
(gdb) bt
#0 gen_acc_mode_iov (req_len=0x7ffff18b7778, id=<optimized out>, iov=0x7ffff18b7780, s=0x555556ce0bd0)
at ../hw/misc/aspeed_hace.c:171
#1 do_hash_operation (s=s@entry=0x555556ce0bd0, algo=3, sg_mode=sg_mode@entry=true, acc_mode=acc_mode@entry=true)
at ../hw/misc/aspeed_hace.c:224
#2 0x00005555559bdbb8 in aspeed_hace_write (opaque=<optimized out>, addr=12, data=262488, size=<optimized out>)
at ../hw/misc/aspeed_hace.c:358
This change doesn't fix much, but at least the guest
can't crash QEMU anymore. Instead it is still usable:
uart:~$ hash test
sha256_test
tv[0]:hash_final error
sha384_test
tv[0]:hash_final error
sha512_test
tv[0]:hash_final error
[00:00:06.278,000] <err> hace_global: HACE poll timeout
[00:00:09.324,000] <err> hace_global: HACE poll timeout
[00:00:12.261,000] <err> hace_global: HACE poll timeout
uart:~$
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/misc/aspeed_hace.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index ac21be306c..12a761f1f5 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -193,6 +193,7 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
size_t digest_len = 0;
int niov = 0;
int i;
+ void *haddr;
if (sg_mode) {
uint32_t len = 0;
@@ -217,9 +218,13 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
addr &= SG_LIST_ADDR_MASK;
plen = len & SG_LIST_LEN_MASK;
- iov[i].iov_base = address_space_map(&s->dram_as, addr, &plen, false,
- MEMTXATTRS_UNSPECIFIED);
-
+ haddr = address_space_map(&s->dram_as, addr, &plen, false,
+ MEMTXATTRS_UNSPECIFIED);
+ if (haddr == NULL) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: qcrypto failed\n", __func__);
+ return;
+ }
+ iov[i].iov_base = haddr;
if (acc_mode) {
niov = gen_acc_mode_iov(s, iov, i, &plen);
@@ -230,10 +235,14 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
} else {
hwaddr len = s->regs[R_HASH_SRC_LEN];
+ haddr = address_space_map(&s->dram_as, s->regs[R_HASH_SRC],
+ &len, false, MEMTXATTRS_UNSPECIFIED);
+ if (haddr == NULL) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: qcrypto failed\n", __func__);
+ return;
+ }
+ iov[0].iov_base = haddr;
iov[0].iov_len = len;
- iov[0].iov_base = address_space_map(&s->dram_as, s->regs[R_HASH_SRC],
- &len, false,
- MEMTXATTRS_UNSPECIFIED);
i = 1;
if (s->iov_count) {
--
2.38.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 8/9] hw/arm/aspeed_ast10x0: Add TODO comment to use Cortex-M4F
2022-12-29 15:23 [PATCH 0/9] hw/arm/aspeed_ast10x0: Map more peripherals & few more fixes Philippe Mathieu-Daudé
` (6 preceding siblings ...)
2022-12-29 15:23 ` [PATCH 7/9] hw/misc/aspeed_hace: Do not crash if address_space_map() failed Philippe Mathieu-Daudé
@ 2022-12-29 15:23 ` Philippe Mathieu-Daudé
2022-12-29 20:55 ` Peter Delevoryas
2022-12-29 15:23 ` [PATCH 9/9] tests/avocado: Test Aspeed Zephyr SDK v00.01.08 on AST1030 board Philippe Mathieu-Daudé
` (2 subsequent siblings)
10 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-29 15:23 UTC (permalink / raw)
To: qemu-devel
Cc: Steven Lee, Peter Delevoryas, Peter Delevoryas, qemu-arm,
Cleber Rosa, Peter Maydell, Beraldo Leal,
Wainer dos Santos Moschetta, Troy Lee, Andrew Jeffery,
Joel Stanley, Cédric Le Goater, Peter Delevoryas,
Philippe Mathieu-Daudé,
Jamin Lin
This SoC uses a Cortex-M4F. QEMU only implements a M4,
which is good enough. Add a TODO note in case the M4F
is added.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/arm/aspeed_ast10x0.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index 02636705b6..788827ca9d 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -421,7 +421,7 @@ static void aspeed_soc_ast1030_class_init(ObjectClass *klass, void *data)
dc->realize = aspeed_soc_ast1030_realize;
sc->name = "ast1030-a1";
- sc->cpu_type = ARM_CPU_TYPE_NAME("cortex-m4");
+ sc->cpu_type = ARM_CPU_TYPE_NAME("cortex-m4"); /* TODO cortex-m4f */
sc->silicon_rev = AST1030_A1_SILICON_REV;
sc->sram_size = 768 * KiB;
sc->secsram_size = 9 * KiB;
--
2.38.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 9/9] tests/avocado: Test Aspeed Zephyr SDK v00.01.08 on AST1030 board
2022-12-29 15:23 [PATCH 0/9] hw/arm/aspeed_ast10x0: Map more peripherals & few more fixes Philippe Mathieu-Daudé
` (7 preceding siblings ...)
2022-12-29 15:23 ` [PATCH 8/9] hw/arm/aspeed_ast10x0: Add TODO comment to use Cortex-M4F Philippe Mathieu-Daudé
@ 2022-12-29 15:23 ` Philippe Mathieu-Daudé
2022-12-29 20:56 ` Peter Delevoryas
2022-12-29 20:37 ` [PATCH 0/9] hw/arm/aspeed_ast10x0: Map more peripherals & few more fixes Peter Delevoryas
2023-01-02 10:55 ` Cédric Le Goater
10 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-29 15:23 UTC (permalink / raw)
To: qemu-devel
Cc: Steven Lee, Peter Delevoryas, Peter Delevoryas, qemu-arm,
Cleber Rosa, Peter Maydell, Beraldo Leal,
Wainer dos Santos Moschetta, Troy Lee, Andrew Jeffery,
Joel Stanley, Cédric Le Goater, Peter Delevoryas,
Philippe Mathieu-Daudé,
Jamin Lin
Add a very quick test that runs some commands in a Zephyr shell:
$ tests/venv/bin/avocado --show=app,console run -t os:zephyr tests/avocado
(2/2) tests/avocado/machine_aspeed.py:AST1030Machine.test_ast1030_zephyros_1_07:
console: *** Booting Zephyr OS build v00.01.07 ***
console: ast1030_evb demo
console: SOC: AST1030-A1
console: uart:~$ kernel stacks
console: 0x36910 wdt_background (real size 1024): unused 988 usage 36 / 1024 (3 %)
console: 0x36ad8 shell_uart (real size 4096): unused 3084 usage 1012 / 4096 (24 %)
console: 0x2edb8 ADC0 (real size 400): unused 260 usage 140 / 400 (35 %)
console: 0x2f0f0 ADC1 (real size 400): unused 260 usage 140 / 400 (35 %)
console: 0x3b098 sysworkq (real size 1024): unused 860 usage 164 / 1024 (16 %)
console: 0x36cc0 usbdworkq (real size 1024): unused 860 usage 164 / 1024 (16 %)
console: 0x36bd8 usbworkq (real size 1024): unused 860 usage 164 / 1024 (16 %)
console: 0x36a10 logging (real size 768): unused 548 usage 220 / 768 (28 %)
console: 0x36ef8 idle 00 (real size 320): unused 268 usage 52 / 320 (16 %)
console: 0x47800 IRQ 00 (real size 2048): unused 1504 usage 544 / 2048 (26 %)
console: uart:~$ otp info scu
console: SCU BIT reg_protect Description
console: ____________________________________________________________________
console: 0x500 0x0 0x0 Disable ARM CM4 CPU boot (TXD5)
console: 0x500 0x1 0x0 /Reserved
console: 0x500 0x2 0x0 \ "
console: 0x500 0x3 0x0 Address offset of single chip ABR mode
console: 0x500 0x4 0x0 /Reserved
console: 0x500 0x5 0x0 | "
console: 0x500 0x6 0x0 | "
console: 0x500 0x7 0x0 | "
console: 0x500 0x8 0x0 | "
console: 0x500 0x9 0x0 | "
console: 0x500 0xA 0x0 | "
console: 0x500 0xB 0x0 | "
console: 0x500 0xC 0x0 | "
console: 0x500 0xD 0x0 | "
console: 0x500 0xE 0x0 | "
console: 0x500 0xF 0x0 | "
console: 0x500 0x10 0x0 \ "
console: 0x500 0x11 0x0 Disabl3 ARM JTAG debug
console: 0x500 0x12 0x0 /Reserved
console: 0x500 0x13 0x0 | "
console: 0x500 0x14 0x0 | "
console: 0x500 0x15 0x0 | "
console: 0x500 0x16 0x0 | "
console: 0x500 0x17 0x0 | "
console: 0x500 0x18 0x0 | "
console: 0x500 0x19 0x0 | "
console: 0x500 0x1A 0x0 | "
console: 0x500 0x1B 0x0 | "
console: 0x500 0x1C 0x0 | "
console: 0x500 0x1D 0x0 | "
console: 0x500 0x1E 0x0 | "
console: 0x500 0x1F 0x0 \ "
console: 0x510 0x0 0x0 /Reserved
console: 0x510 0x1 0x0 | "
console: 0x510 0x2 0x0 | "
console: 0x510 0x3 0x0 \ "
console: 0x510 0x4 0x0 Disable debug interfaces
console: 0x510 0x5 0x0 /Reserved
console: 0x510 0x6 0x0 | "
console: 0x510 0x7 0x0 \ "
console: 0x510 0x8 0x0 Enable boot from Uart5 by Pin Strap
console: 0x510 0x9 0x0 /Reserved
console: 0x510 0xA 0x0 \ "
console: 0x510 0xB 0x0 Enable boot SPI ABR
console: 0x510 0xC 0x0 Boot SPI ABR Mode
console: 0x510 0xD 0x0 /Boot SPI flash size
console: 0x510 0xE 0x0 | "
console: 0x510 0xF 0x0 \ "
console: 0x510 0x10 0x0 /Reserved
console: 0x510 0x11 0x0 | "
console: 0x510 0x12 0x0 | "
console: 0x510 0x13 0x0 | "
console: 0x510 0x14 0x0 | "
console: 0x510 0x15 0x0 \ "
console: 0x510 0x16 0x0 Enable boot SPI auxiliary control pins
console: 0x510 0x19 0x0 /Reserved
console: 0x510 0x1A 0x0 | "
console: 0x510 0x1B 0x0 | "
console: 0x510 0x1C 0x0 | "
console: 0x510 0x1D 0x0 | "
console: 0x510 0x1E 0x0 | "
console: 0x510 0x1F 0x0 \ "
console: 0x510 0x1E 0x0 Enable dedicate GPIO strap pins
console: 0x510 0x1F 0x0 Enable Secure Boot by Pin Strap
console: uart:~$ hwinfo devid
console: Length: 8
console: ID: 0x0000018000000180
console: uart:~$ crypto aes256_cbc_vault
console: aes256_cbc vault key 1
console: Was waiting for:
console: 6b c1 be e2 2e 40 9f 96 e9 3d 7e 11 73 93 17 2a
console: ae 2d 8a 57 1e 03 ac 9c 9e b7 6f ac 45 af 8e 51
console: 30 c8 1c 46 a3 5c e4 11 e5 fb c1 19 1a 0a 52 ef
console: f6 9f 24 45 df 4f 9b 17 ad 2b 41 7b e6 6c 37 10
console: But got:
console: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
console: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
console: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
console: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
console: uart:~$ random get
console: 0x862460d
console: uart:~$ i2c scan I2C_0
console: 0 1 2 3 4 5 6 7 8 9 a b c d e f
console: 00: -- -- -- -- -- -- -- -- -- -- -- --
console: 10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
console: 20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
console: 30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
console: 40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
console: 50: 50 -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
console: 60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
console: 70: -- -- -- -- -- -- -- --
console: 1 devices found on I2C_0
console: uart:~$ kernel uptime
console: Uptime: 9897 ms
console: uart:~$ kernel reboot warm
console: *** Booting Zephyr OS build v00.01.07 ***
PASS (1.08 s)
Ref: https://github.com/AspeedTech-BMC/zephyr/releases/download/v00.01.07/Aspeed_Zephy_SDK_User_Guide_v00.01.07.pdf
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
tests/avocado/machine_aspeed.py | 41 ++++++++++++++++++++++++++++++++-
1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/tests/avocado/machine_aspeed.py b/tests/avocado/machine_aspeed.py
index 1fc385e1c8..11f5b17eb9 100644
--- a/tests/avocado/machine_aspeed.py
+++ b/tests/avocado/machine_aspeed.py
@@ -22,10 +22,11 @@ class AST1030Machine(QemuSystemTest):
timeout = 10
- def test_ast1030_zephyros(self):
+ def test_ast1030_zephyros_1_04(self):
"""
:avocado: tags=arch:arm
:avocado: tags=machine:ast1030-evb
+ :avocado: tags=os:zephyr
"""
tar_url = ('https://github.com/AspeedTech-BMC'
'/zephyr/releases/download/v00.01.04/ast1030-evb-demo.zip')
@@ -41,6 +42,44 @@ def test_ast1030_zephyros(self):
exec_command_and_wait_for_pattern(self, "help",
"Available commands")
+ def test_ast1030_zephyros_1_07(self):
+ """
+ :avocado: tags=arch:arm
+ :avocado: tags=machine:ast1030-evb
+ :avocado: tags=os:zephyr
+ """
+ tar_url = ('https://github.com/AspeedTech-BMC'
+ '/zephyr/releases/download/v00.01.07/ast1030-evb-demo.zip')
+ tar_hash = '40ac87eabdcd3b3454ce5aad11fedc72a33ecda2'
+ tar_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
+ archive.extract(tar_path, self.workdir)
+ kernel_file = self.workdir + "/ast1030-evb-demo/zephyr.bin"
+ self.vm.set_console()
+ self.vm.add_args('-kernel', kernel_file,
+ '-nographic')
+ self.vm.launch()
+ wait_for_console_pattern(self, "Booting Zephyr OS")
+ for shell_cmd in [
+ 'kernel stacks',
+ 'otp info conf',
+ 'otp info scu',
+ 'hwinfo devid',
+ 'crypto aes256_cbc_vault',
+ 'random get',
+ 'jtag JTAG1 sw_xfer high TMS',
+ 'adc ADC0 resolution 12',
+ 'adc ADC0 read 42',
+ 'adc ADC1 read 69',
+ 'i2c scan I2C_0',
+ 'i3c attach I3C_0',
+ 'hash test',
+ 'kernel uptime',
+ 'kernel reboot warm',
+ 'kernel uptime',
+ 'kernel reboot cold',
+ 'kernel uptime',
+ ]: exec_command_and_wait_for_pattern(self, shell_cmd, "uart:~$")
+
class AST2x00Machine(QemuSystemTest):
timeout = 90
--
2.38.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 0/9] hw/arm/aspeed_ast10x0: Map more peripherals & few more fixes
2022-12-29 15:23 [PATCH 0/9] hw/arm/aspeed_ast10x0: Map more peripherals & few more fixes Philippe Mathieu-Daudé
` (8 preceding siblings ...)
2022-12-29 15:23 ` [PATCH 9/9] tests/avocado: Test Aspeed Zephyr SDK v00.01.08 on AST1030 board Philippe Mathieu-Daudé
@ 2022-12-29 20:37 ` Peter Delevoryas
2023-01-02 10:55 ` Cédric Le Goater
10 siblings, 0 replies; 27+ messages in thread
From: Peter Delevoryas @ 2022-12-29 20:37 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Steven Lee, Peter Delevoryas, qemu-arm, Cleber Rosa,
Peter Maydell, Beraldo Leal, Wainer dos Santos Moschetta,
Troy Lee, Andrew Jeffery, Joel Stanley, Cédric Le Goater,
Peter Delevoryas, Jamin Lin
On Thu, Dec 29, 2022 at 04:23:16PM +0100, Philippe Mathieu-Daudé wrote:
> Trying to fix some bugs triggered running Zephyr.
Yay!
>
> Still 2 bugs:
>
> 1/
> uart:~$ sensor get SYSCLK
> [00:00:23.592,000] <err> os: ***** USAGE FAULT *****
> [00:00:23.593,000] <err> os: Illegal use of the EPSR
> [00:00:23.593,000] <err> os: r0/a1: 0x00033448 r1/a2: 0x00000000 r2/a3: 0x00047f50
> [00:00:23.593,000] <err> os: r3/a4: 0x00000000 r12/ip: 0x00000000 r14/lr: 0x00000fbd
> [00:00:23.593,000] <err> os: xpsr: 0x60000000
> [00:00:23.593,000] <err> os: Faulting instruction address (r15/pc): 0x00000000
> [00:00:23.593,000] <err> os: >>> ZEPHYR FATAL ERROR 0: CPU exception on CPU 0
> [00:00:23.594,000] <err> os: Current thread: 0x38248 (shell_uart)
> [00:00:23.601,000] <err> os: Halting system
>
> 2/
> uart:~$ mcuboot
> [00:01:04.990,000] <err> os: ***** BUS FAULT *****
> [00:01:04.990,000] <err> os: Instruction bus error
> [00:01:04.991,000] <err> os: r0/a1: 0x00000000 r1/a2: 0x000ffff0 r2/a3: 0x00047ef0
> [00:01:04.991,000] <err> os: r3/a4: 0x00000010 r12/ip: 0x6df7ecb5 r14/lr: 0x000188ed
> [00:01:04.991,000] <err> os: xpsr: 0x61000000
> [00:01:04.991,000] <err> os: Faulting instruction address (r15/pc): 0x6df7ecb4
> [00:01:04.991,000] <err> os: >>> ZEPHYR FATAL ERROR 0: CPU exception on CPU 0
> [00:01:04.991,000] <err> os: Current thread: 0x38248 (shell_uart)
> [00:01:04.994,000] <err> os: Halting system
>
> ----------------
> IN:
> PMSA MPU lookup for reading at 0x0001d400 mmu_idx 65 -> Hit (prot rwx)
> 0x0001d5a2: 6869 ldr r1, [r5, #4]
> 0x0001d5a4: 4421 add r1, r4
> 0x0001d5a6: 6883 ldr r3, [r0, #8]
> 0x0001d5a8: 681c ldr r4, [r3]
> 0x0001d5aa: 463a mov r2, r7
> 0x0001d5ac: 4633 mov r3, r6
> 0x0001d5ae: 46a4 mov ip, r4
> 0x0001d5b0: e8bd 41f0 pop.w {r4, r5, r6, r7, r8, lr}
> 0x0001d5b4: 4760 bx ip
>
> PMSA MPU lookup for reading at 0x00000008 mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for execute at 0x6df7ecb4 mmu_idx 65 -> Hit (prot rwx)
> Taking exception 3 [Prefetch Abort] on CPU 0
> ...at fault address 0x6df7ecb4
> ...with CFSR.IBUSERR
> PMSA MPU lookup for writing at 0x00047ec8 mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for writing at 0x00047ecc mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for writing at 0x00047ed0 mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for writing at 0x00047ed4 mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for writing at 0x00047ed8 mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for writing at 0x00047edc mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for writing at 0x00047ee0 mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for writing at 0x00047ee4 mmu_idx 65 -> Hit (prot rwx)
> ...taking pending nonsecure exception 5
> ...loading from element 5 of non-secure vector table at 0x14
> ...loaded new PC 0xa0cd
> ----------------
>
> HACE isn't really functional there. I probably screwed smth while wiring
> the peripheral. Not obvious without access to the datasheet.
Hmmmmm well I have the datasheet, but I don't see what the issue could be. The
MMIO address for the HACE is correct (0x7E6D_0000), what else could be wrong?
>
> Philippe Mathieu-Daudé (9):
> hw/watchdog/wdt_aspeed: Map the whole MMIO range
> hw/arm/aspeed: Use the IEC binary prefix definitions
> hw/arm/aspeed_ast10x0: Add various unimplemented peripherals
> hw/arm/aspeed_ast10x0: Map I3C peripheral
> hw/arm/aspeed_ast10x0: Map the secure SRAM
> hw/arm/aspeed_ast10x0: Map HACE peripheral
> hw/misc/aspeed_hace: Do not crash if address_space_map() failed
> hw/arm/aspeed_ast10x0: Add TODO comment to use Cortex-M4F
> tests/avocado: Test Aspeed Zephyr SDK v00.01.08 on AST1030 board
>
> hw/arm/aspeed_ast10x0.c | 84 ++++++++++++++++++++++++++++++--
> hw/arm/aspeed_ast2600.c | 5 +-
> hw/arm/aspeed_soc.c | 6 +--
> hw/misc/aspeed_hace.c | 21 +++++---
> hw/watchdog/wdt_aspeed.c | 12 +++--
> include/hw/arm/aspeed_soc.h | 14 ++++++
> include/hw/watchdog/wdt_aspeed.h | 2 +-
> tests/avocado/machine_aspeed.py | 41 +++++++++++++++-
> 8 files changed, 163 insertions(+), 22 deletions(-)
>
> --
> 2.38.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/9] hw/watchdog/wdt_aspeed: Map the whole MMIO range
2022-12-29 15:23 ` [PATCH 1/9] hw/watchdog/wdt_aspeed: Map the whole MMIO range Philippe Mathieu-Daudé
@ 2022-12-29 20:42 ` Peter Delevoryas
2022-12-30 7:32 ` Philippe Mathieu-Daudé
2023-01-02 10:21 ` Cédric Le Goater
1 sibling, 1 reply; 27+ messages in thread
From: Peter Delevoryas @ 2022-12-29 20:42 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Steven Lee, Peter Delevoryas, qemu-arm, Cleber Rosa,
Peter Maydell, Beraldo Leal, Wainer dos Santos Moschetta,
Troy Lee, Andrew Jeffery, Joel Stanley, Cédric Le Goater,
Peter Delevoryas, Jamin Lin
On Thu, Dec 29, 2022 at 04:23:17PM +0100, Philippe Mathieu-Daudé wrote:
> Avoid confusing two different things:
> - the WDT I/O region size ('iosize')
> - at which offset the SoC map the WDT ('offset')
> While it is often the same, we can map smaller region sizes at
> larger offsets.
>
> Here we are interested in the I/O region size. Rename as 'iosize'
> and map the whole range, not only the first implemented registers.
> Unimplemented registers accesses are already logged as guest-errors.
>
> Otherwise when booting the demo in [*] we get:
>
> aspeed.io: unimplemented device write (size 4, offset 0x1851a8, value 0x030f1ff1)
> aspeed.io: unimplemented device write (size 4, offset 0x1851ac, value 0x03fffff1)
> aspeed.io: unimplemented device write (size 4, offset 0x185128, value 0x030f1ff1)
> aspeed.io: unimplemented device write (size 4, offset 0x18512c, value 0x03fffff1)
>
> [*] https://github.com/AspeedTech-BMC/zephyr/releases/tag/v00.01.07
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/arm/aspeed_ast10x0.c | 2 +-
> hw/arm/aspeed_ast2600.c | 2 +-
> hw/arm/aspeed_soc.c | 2 +-
> hw/watchdog/wdt_aspeed.c | 12 +++++++-----
> include/hw/watchdog/wdt_aspeed.h | 2 +-
> 5 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
> index 4d0b9b115f..122b3fd3f3 100644
> --- a/hw/arm/aspeed_ast10x0.c
> +++ b/hw/arm/aspeed_ast10x0.c
> @@ -325,7 +325,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
> return;
> }
> aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->wdt[i]), 0,
> - sc->memmap[ASPEED_DEV_WDT] + i * awc->offset);
> + sc->memmap[ASPEED_DEV_WDT] + i * awc->iosize);
> }
>
> /* GPIO */
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index cd75465c2b..a79e05ddbd 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -472,7 +472,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
> return;
> }
> aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->wdt[i]), 0,
> - sc->memmap[ASPEED_DEV_WDT] + i * awc->offset);
> + sc->memmap[ASPEED_DEV_WDT] + i * awc->iosize);
> }
>
> /* RAM */
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index b05b9dd416..2c0924d311 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -393,7 +393,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> return;
> }
> aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->wdt[i]), 0,
> - sc->memmap[ASPEED_DEV_WDT] + i * awc->offset);
> + sc->memmap[ASPEED_DEV_WDT] + i * awc->iosize);
> }
>
> /* RAM */
> diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
> index d753693a2e..c1311ce74c 100644
> --- a/hw/watchdog/wdt_aspeed.c
> +++ b/hw/watchdog/wdt_aspeed.c
> @@ -260,6 +260,7 @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
> {
> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> AspeedWDTState *s = ASPEED_WDT(dev);
> + AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(dev);
>
> assert(s->scu);
>
> @@ -270,8 +271,9 @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
> */
> s->pclk_freq = PCLK_HZ;
>
> + assert(awc->iosize >= ASPEED_WDT_REGS_MAX * 4);
> memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_wdt_ops, s,
> - TYPE_ASPEED_WDT, ASPEED_WDT_REGS_MAX * 4);
> + TYPE_ASPEED_WDT, awc->iosize);
Does this fix the unimplemented thing you referred to in the commit message?
I'm not sure which part of this diff actually removes the unimplemented traces.
> sysbus_init_mmio(sbd, &s->iomem);
> }
>
> @@ -309,7 +311,7 @@ static void aspeed_2400_wdt_class_init(ObjectClass *klass, void *data)
> AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass);
>
> dc->desc = "ASPEED 2400 Watchdog Controller";
> - awc->offset = 0x20;
> + awc->iosize = 0x20;
> awc->ext_pulse_width_mask = 0xff;
> awc->reset_ctrl_reg = SCU_RESET_CONTROL1;
> awc->wdt_reload = aspeed_wdt_reload;
> @@ -346,7 +348,7 @@ static void aspeed_2500_wdt_class_init(ObjectClass *klass, void *data)
> AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass);
>
> dc->desc = "ASPEED 2500 Watchdog Controller";
> - awc->offset = 0x20;
> + awc->iosize = 0x20;
> awc->ext_pulse_width_mask = 0xfffff;
> awc->reset_ctrl_reg = SCU_RESET_CONTROL1;
> awc->reset_pulse = aspeed_2500_wdt_reset_pulse;
> @@ -369,7 +371,7 @@ static void aspeed_2600_wdt_class_init(ObjectClass *klass, void *data)
> AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass);
>
> dc->desc = "ASPEED 2600 Watchdog Controller";
> - awc->offset = 0x40;
> + awc->iosize = 0x40;
> awc->ext_pulse_width_mask = 0xfffff; /* TODO */
> awc->reset_ctrl_reg = AST2600_SCU_RESET_CONTROL1;
> awc->reset_pulse = aspeed_2500_wdt_reset_pulse;
> @@ -392,7 +394,7 @@ static void aspeed_1030_wdt_class_init(ObjectClass *klass, void *data)
> AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass);
>
> dc->desc = "ASPEED 1030 Watchdog Controller";
> - awc->offset = 0x80;
> + awc->iosize = 0x80;
> awc->ext_pulse_width_mask = 0xfffff; /* TODO */
> awc->reset_ctrl_reg = AST2600_SCU_RESET_CONTROL1;
> awc->reset_pulse = aspeed_2500_wdt_reset_pulse;
> diff --git a/include/hw/watchdog/wdt_aspeed.h b/include/hw/watchdog/wdt_aspeed.h
> index dfa5dfa424..db91ee6b51 100644
> --- a/include/hw/watchdog/wdt_aspeed.h
> +++ b/include/hw/watchdog/wdt_aspeed.h
> @@ -40,7 +40,7 @@ struct AspeedWDTState {
> struct AspeedWDTClass {
> SysBusDeviceClass parent_class;
>
> - uint32_t offset;
> + uint32_t iosize;
Oh yeah, iosize is a way better name for this. +1. But to be honest, I don't
understand how this is changing the unimplemented traces?
Reviewed-by: Peter Delevoryas <peter@pjd.dev>
> uint32_t ext_pulse_width_mask;
> uint32_t reset_ctrl_reg;
> void (*reset_pulse)(AspeedWDTState *s, uint32_t property);
> --
> 2.38.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/9] hw/arm/aspeed: Use the IEC binary prefix definitions
2022-12-29 15:23 ` [PATCH 2/9] hw/arm/aspeed: Use the IEC binary prefix definitions Philippe Mathieu-Daudé
@ 2022-12-29 20:42 ` Peter Delevoryas
0 siblings, 0 replies; 27+ messages in thread
From: Peter Delevoryas @ 2022-12-29 20:42 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Steven Lee, Peter Delevoryas, qemu-arm, Cleber Rosa,
Peter Maydell, Beraldo Leal, Wainer dos Santos Moschetta,
Troy Lee, Andrew Jeffery, Joel Stanley, Cédric Le Goater,
Peter Delevoryas, Jamin Lin
On Thu, Dec 29, 2022 at 04:23:18PM +0100, Philippe Mathieu-Daudé wrote:
> IEC binary prefixes ease code review: the unit is explicit.
Oh, yeah, another good idea.
Reviewed-by: Peter Delevoryas <peter@pjd.dev>
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/arm/aspeed_ast10x0.c | 3 ++-
> hw/arm/aspeed_ast2600.c | 3 ++-
> hw/arm/aspeed_soc.c | 4 ++--
> 3 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
> index 122b3fd3f3..3500294df7 100644
> --- a/hw/arm/aspeed_ast10x0.c
> +++ b/hw/arm/aspeed_ast10x0.c
> @@ -10,6 +10,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/units.h"
> #include "qapi/error.h"
> #include "exec/address-spaces.h"
> #include "sysemu/sysemu.h"
> @@ -348,7 +349,7 @@ static void aspeed_soc_ast1030_class_init(ObjectClass *klass, void *data)
> sc->name = "ast1030-a1";
> sc->cpu_type = ARM_CPU_TYPE_NAME("cortex-m4");
> sc->silicon_rev = AST1030_A1_SILICON_REV;
> - sc->sram_size = 0xc0000;
> + sc->sram_size = 768 * KiB;
> sc->spis_num = 2;
> sc->ehcis_num = 0;
> sc->wdts_num = 4;
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index a79e05ddbd..72df72a540 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -8,6 +8,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/units.h"
> #include "qapi/error.h"
> #include "hw/misc/unimp.h"
> #include "hw/arm/aspeed_soc.h"
> @@ -619,7 +620,7 @@ static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
> sc->name = "ast2600-a3";
> sc->cpu_type = ARM_CPU_TYPE_NAME("cortex-a7");
> sc->silicon_rev = AST2600_A3_SILICON_REV;
> - sc->sram_size = 0x16400;
> + sc->sram_size = 89 * KiB;
> sc->spis_num = 2;
> sc->ehcis_num = 2;
> sc->wdts_num = 4;
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index 2c0924d311..677342c9ed 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -517,7 +517,7 @@ static void aspeed_soc_ast2400_class_init(ObjectClass *oc, void *data)
> sc->name = "ast2400-a1";
> sc->cpu_type = ARM_CPU_TYPE_NAME("arm926");
> sc->silicon_rev = AST2400_A1_SILICON_REV;
> - sc->sram_size = 0x8000;
> + sc->sram_size = 32 * KiB;
> sc->spis_num = 1;
> sc->ehcis_num = 1;
> sc->wdts_num = 2;
> @@ -544,7 +544,7 @@ static void aspeed_soc_ast2500_class_init(ObjectClass *oc, void *data)
> sc->name = "ast2500-a1";
> sc->cpu_type = ARM_CPU_TYPE_NAME("arm1176");
> sc->silicon_rev = AST2500_A1_SILICON_REV;
> - sc->sram_size = 0x9000;
> + sc->sram_size = 36 * KiB;
> sc->spis_num = 2;
> sc->ehcis_num = 2;
> sc->wdts_num = 3;
> --
> 2.38.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/9] hw/arm/aspeed_ast10x0: Add various unimplemented peripherals
2022-12-29 15:23 ` [PATCH 3/9] hw/arm/aspeed_ast10x0: Add various unimplemented peripherals Philippe Mathieu-Daudé
@ 2022-12-29 20:44 ` Peter Delevoryas
0 siblings, 0 replies; 27+ messages in thread
From: Peter Delevoryas @ 2022-12-29 20:44 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Steven Lee, Peter Delevoryas, qemu-arm, Cleber Rosa,
Peter Maydell, Beraldo Leal, Wainer dos Santos Moschetta,
Troy Lee, Andrew Jeffery, Joel Stanley, Cédric Le Goater,
Peter Delevoryas, Jamin Lin
:n Thu, Dec 29, 2022 at 04:23:19PM +0100, Philippe Mathieu-Daudé wrote:
> Based on booting Zephyr demo from [1] running QEMU with
> '-d unimp' and checking missing devices in [2].
>
> [1] https://github.com/AspeedTech-BMC/zephyr/releases/tag/v00.01.07
> [2] https://github.com/AspeedTech-BMC/zephyr/blob/v00.01.08/dts/arm/aspeed/ast10x0.dtsi
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Oh cool, yeah looks good.
Reviewed-by: Peter Delevoryas <peter@pjd.dev>
> ---
> hw/arm/aspeed_ast10x0.c | 35 +++++++++++++++++++++++++++++++++++
> include/hw/arm/aspeed_soc.h | 11 +++++++++++
> 2 files changed, 46 insertions(+)
>
> diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
> index 3500294df7..d7dbc1a801 100644
> --- a/hw/arm/aspeed_ast10x0.c
> +++ b/hw/arm/aspeed_ast10x0.c
> @@ -28,10 +28,15 @@ static const hwaddr aspeed_soc_ast1030_memmap[] = {
> [ASPEED_DEV_FMC] = 0x7E620000,
> [ASPEED_DEV_SPI1] = 0x7E630000,
> [ASPEED_DEV_SPI2] = 0x7E640000,
> + [ASPEED_DEV_UDC] = 0x7E6A2000,
> [ASPEED_DEV_SCU] = 0x7E6E2000,
> + [ASPEED_DEV_JTAG0] = 0x7E6E4000,
> + [ASPEED_DEV_JTAG1] = 0x7E6E4100,
> [ASPEED_DEV_ADC] = 0x7E6E9000,
> + [ASPEED_DEV_ESPI] = 0x7E6EE000,
> [ASPEED_DEV_SBC] = 0x7E6F2000,
> [ASPEED_DEV_GPIO] = 0x7E780000,
> + [ASPEED_DEV_SGPIOM] = 0x7E780500,
> [ASPEED_DEV_TIMER1] = 0x7E782000,
> [ASPEED_DEV_UART1] = 0x7E783000,
> [ASPEED_DEV_UART2] = 0x7E78D000,
> @@ -79,12 +84,17 @@ static const int aspeed_soc_ast1030_irqmap[] = {
> [ASPEED_DEV_LPC] = 35,
> [ASPEED_DEV_PECI] = 38,
> [ASPEED_DEV_FMC] = 39,
> + [ASPEED_DEV_ESPI] = 42,
> [ASPEED_DEV_PWM] = 44,
> [ASPEED_DEV_ADC] = 46,
> [ASPEED_DEV_SPI1] = 65,
> [ASPEED_DEV_SPI2] = 66,
> [ASPEED_DEV_I2C] = 110, /* 110 ~ 123 */
> [ASPEED_DEV_KCS] = 138, /* 138 -> 142 */
> + [ASPEED_DEV_UDC] = 9,
> + [ASPEED_DEV_SGPIOM] = 51,
> + [ASPEED_DEV_JTAG0] = 27,
> + [ASPEED_DEV_JTAG1] = 53,
> };
>
> static qemu_irq aspeed_soc_ast1030_get_irq(AspeedSoCState *s, int dev)
> @@ -155,6 +165,15 @@ static void aspeed_soc_ast1030_init(Object *obj)
> object_initialize_child(obj, "iomem", &s->iomem, TYPE_UNIMPLEMENTED_DEVICE);
> object_initialize_child(obj, "sbc-unimplemented", &s->sbc_unimplemented,
> TYPE_UNIMPLEMENTED_DEVICE);
> + object_initialize_child(obj, "pwm", &s->pwm, TYPE_UNIMPLEMENTED_DEVICE);
> + object_initialize_child(obj, "espi", &s->espi, TYPE_UNIMPLEMENTED_DEVICE);
> + object_initialize_child(obj, "udc", &s->udc, TYPE_UNIMPLEMENTED_DEVICE);
> + object_initialize_child(obj, "sgpiom", &s->sgpiom,
> + TYPE_UNIMPLEMENTED_DEVICE);
> + object_initialize_child(obj, "jtag[0]", &s->jtag[0],
> + TYPE_UNIMPLEMENTED_DEVICE);
> + object_initialize_child(obj, "jtag[1]", &s->jtag[1],
> + TYPE_UNIMPLEMENTED_DEVICE);
> }
>
> static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
> @@ -337,6 +356,22 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
> sc->memmap[ASPEED_DEV_GPIO]);
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio), 0,
> aspeed_soc_get_irq(s, ASPEED_DEV_GPIO));
> +
> + aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->pwm), "aspeed.pwm",
> + sc->memmap[ASPEED_DEV_PWM], 0x100);
> +
> + aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->espi), "aspeed.espi",
> + sc->memmap[ASPEED_DEV_ESPI], 0x800);
> +
> + aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->udc), "aspeed.udc",
> + sc->memmap[ASPEED_DEV_UDC], 0x1000);
> + aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->sgpiom), "aspeed.sgpiom",
> + sc->memmap[ASPEED_DEV_SGPIOM], 0x100);
> +
> + aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->jtag[0]), "aspeed.jtag",
> + sc->memmap[ASPEED_DEV_JTAG0], 0x20);
> + aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->jtag[1]), "aspeed.jtag",
> + sc->memmap[ASPEED_DEV_JTAG1], 0x20);
> }
>
> static void aspeed_soc_ast1030_class_init(ObjectClass *klass, void *data)
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 8389200b2d..9a5e3c0bac 100644
> --- a/include/hw/arm/aspeed_soc.h
r +++ b/include/hw/arm/aspeed_soc.h
> @@ -44,6 +44,7 @@
> #define ASPEED_CPUS_NUM 2
> #define ASPEED_MACS_NUM 4
> #define ASPEED_UARTS_NUM 13
> +#define ASPEED_JTAG_NUM 2
>
> struct AspeedSoCState {
> /*< private >*/
> @@ -87,6 +88,11 @@ struct AspeedSoCState {
> UnimplementedDeviceState video;
> UnimplementedDeviceState emmc_boot_controller;
> UnimplementedDeviceState dpmcu;
> + UnimplementedDeviceState pwm;
> + UnimplementedDeviceState espi;
> + UnimplementedDeviceState udc;
> + UnimplementedDeviceState sgpiom;
> + UnimplementedDeviceState jtag[ASPEED_JTAG_NUM];
> };
>
> #define TYPE_ASPEED_SOC "aspeed-soc"
> @@ -174,6 +180,11 @@ enum {
> ASPEED_DEV_DPMCU,
> ASPEED_DEV_DP,
> ASPEED_DEV_I3C,
> + ASPEED_DEV_ESPI,
> + ASPEED_DEV_UDC,
> + ASPEED_DEV_SGPIOM,
> + ASPEED_DEV_JTAG0,
> + ASPEED_DEV_JTAG1,
> };
>
> qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int dev);
> --
> 2.38.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/9] hw/arm/aspeed_ast10x0: Map I3C peripheral
2022-12-29 15:23 ` [PATCH 4/9] hw/arm/aspeed_ast10x0: Map I3C peripheral Philippe Mathieu-Daudé
@ 2022-12-29 20:46 ` Peter Delevoryas
2022-12-30 7:07 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 27+ messages in thread
From: Peter Delevoryas @ 2022-12-29 20:46 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Steven Lee, Peter Delevoryas, qemu-arm, Cleber Rosa,
Peter Maydell, Beraldo Leal, Wainer dos Santos Moschetta,
Troy Lee, Andrew Jeffery, Joel Stanley, Cédric Le Goater,
Peter Delevoryas, Jamin Lin
On Thu, Dec 29, 2022 at 04:23:20PM +0100, Philippe Mathieu-Daudé wrote:
> Since I don't have access to the datasheet, the relevant
> values were found in:
> https://github.com/AspeedTech-BMC/zephyr/blob/v00.01.08/dts/arm/aspeed/ast10x0.dtsi
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/arm/aspeed_ast10x0.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
> index d7dbc1a801..53ea6d471f 100644
> --- a/hw/arm/aspeed_ast10x0.c
> +++ b/hw/arm/aspeed_ast10x0.c
> @@ -54,6 +54,7 @@ static const hwaddr aspeed_soc_ast1030_memmap[] = {
> [ASPEED_DEV_WDT] = 0x7E785000,
> [ASPEED_DEV_LPC] = 0x7E789000,
> [ASPEED_DEV_PECI] = 0x7E78B000,
> + [ASPEED_DEV_I3C] = 0x7E7A0000,
> [ASPEED_DEV_I2C] = 0x7E7B0000,
> };
>
> @@ -89,6 +90,7 @@ static const int aspeed_soc_ast1030_irqmap[] = {
> [ASPEED_DEV_ADC] = 46,
> [ASPEED_DEV_SPI1] = 65,
> [ASPEED_DEV_SPI2] = 66,
> + [ASPEED_DEV_I3C] = 102, /* 102 -> 105 */
> [ASPEED_DEV_I2C] = 110, /* 110 ~ 123 */
> [ASPEED_DEV_KCS] = 138, /* 138 -> 142 */
> [ASPEED_DEV_UDC] = 9,
> @@ -130,6 +132,8 @@ static void aspeed_soc_ast1030_init(Object *obj)
> snprintf(typename, sizeof(typename), "aspeed.i2c-%s", socname);
> object_initialize_child(obj, "i2c", &s->i2c, typename);
>
> + object_initialize_child(obj, "i3c", &s->i3c, TYPE_ASPEED_I3C);
> +
> snprintf(typename, sizeof(typename), "aspeed.timer-%s", socname);
> object_initialize_child(obj, "timerctrl", &s->timerctrl, typename);
>
> @@ -240,6 +244,18 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c.busses[i]), 0, irq);
> }
>
> + /* I3C */
> + if (!sysbus_realize(SYS_BUS_DEVICE(&s->i3c), errp)) {
> + return;
> + }
> + aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->i3c), 0, sc->memmap[ASPEED_DEV_I3C]);
> + for (i = 0; i < ASPEED_I3C_NR_DEVICES; i++) {
> + qemu_irq irq = qdev_get_gpio_in(DEVICE(&s->armv7m),
> + sc->irqmap[ASPEED_DEV_I3C] + i);
> + /* The AST1030 I2C controller has one IRQ per bus. */
Should this comment be I2C or I3C?
Otherwise looks good!
Reviewed-by: Peter Delevoryas <peter@pjd.dev>
> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->i3c.devices[i]), 0, irq);
> + }
> +
> /* PECI */
> if (!sysbus_realize(SYS_BUS_DEVICE(&s->peci), errp)) {
> return;
> --
> 2.38.1
k
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 5/9] hw/arm/aspeed_ast10x0: Map the secure SRAM
2022-12-29 15:23 ` [PATCH 5/9] hw/arm/aspeed_ast10x0: Map the secure SRAM Philippe Mathieu-Daudé
@ 2022-12-29 20:50 ` Peter Delevoryas
0 siblings, 0 replies; 27+ messages in thread
From: Peter Delevoryas @ 2022-12-29 20:50 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Steven Lee, Peter Delevoryas, qemu-arm, Cleber Rosa,
Peter Maydell, Beraldo Leal, Wainer dos Santos Moschetta,
Troy Lee, Andrew Jeffery, Joel Stanley, Cédric Le Goater,
Peter Delevoryas, Jamin Lin
On Thu, Dec 29, 2022 at 04:23:21PM +0100, Philippe Mathieu-Daudé wrote:
> Some SRAM appears to be used by the Secure Boot unit and
> crypto accelerators. Name it 'secure sram'.
>
> Note, the SRAM base address was already present but unused
> (the 'SBC' index is used for the MMIO peripheral).
>
> Interestingly using CFLAGS=-Winitializer-overrides reports:
>
> ../hw/arm/aspeed_ast10x0.c:32:30: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
> [ASPEED_DEV_SBC] = 0x7E6F2000,
> ^~~~~~~~~~
> ../hw/arm/aspeed_ast10x0.c:24:30: note: previous initialization is here
> [ASPEED_DEV_SBC] = 0x79000000,
> ^~~~~~~~~~
Ohhhh! Oh no, yeah I think Zephyr has this warning enabled by default, right? I
guess it's not enabled in QEMU? Hmmmm.
Reviewed-by: Peter Delevoryas <peter@pjd.dev>
Also maybe include this tag?
Fixes: 356b230ed1 ("aspeed/soc : Add AST1030 support")
> This fixes with Zephyr:
>
> uart:~$ rsa test
> rsa test vector[0]:
> [00:00:26.156,000] <err> os: ***** BUS FAULT *****
> [00:00:26.157,000] <err> os: Precise data bus error
> [00:00:26.157,000] <err> os: BFAR Address: 0x79000000
> [00:00:26.158,000] <err> os: r0/a1: 0x79000000 r1/a2: 0x00000000 r2/a3: 0x00001800
> [00:00:26.158,000] <err> os: r3/a4: 0x79001800 r12/ip: 0x00000800 r14/lr: 0x0001098d
> [00:00:26.158,000] <err> os: xpsr: 0x81000000
> [00:00:26.158,000] <err> os: Faulting instruction address (r15/pc): 0x0001e1bc
> [00:00:26.158,000] <err> os: >>> ZEPHYR FATAL ERROR 0: CPU exception on CPU 0
> [00:00:26.158,000] <err> os: Current thread: 0x38248 (shell_uart)
> [00:00:26.165,000] <err> os: Halting system
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/arm/aspeed_ast10x0.c | 11 ++++++++++-
> include/hw/arm/aspeed_soc.h | 3 +++
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
> index 53ea6d471f..21a2e62345 100644
> --- a/hw/arm/aspeed_ast10x0.c
> +++ b/hw/arm/aspeed_ast10x0.c
> @@ -22,7 +22,7 @@
>
> static const hwaddr aspeed_soc_ast1030_memmap[] = {
> [ASPEED_DEV_SRAM] = 0x00000000,
> - [ASPEED_DEV_SBC] = 0x79000000,
> + [ASPEED_DEV_SECSRAM] = 0x79000000,
> [ASPEED_DEV_IOMEM] = 0x7E600000,
> [ASPEED_DEV_PWM] = 0x7E610000,
> [ASPEED_DEV_FMC] = 0x7E620000,
> @@ -222,6 +222,14 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
> memory_region_add_subregion(s->memory,
> sc->memmap[ASPEED_DEV_SRAM],
> &s->sram);
> + memory_region_init_ram(&s->secsram, OBJECT(s), "sec.sram",
> + sc->secsram_size, &err);
> + if (err != NULL) {
> + error_propagate(errp, err);
> + return;
> + }
> + memory_region_add_subregion(s->memory, sc->memmap[ASPEED_DEV_SECSRAM],
> + &s->secsram);
>
> /* SCU */
> if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
> @@ -401,6 +409,7 @@ static void aspeed_soc_ast1030_class_init(ObjectClass *klass, void *data)
> sc->cpu_type = ARM_CPU_TYPE_NAME("cortex-m4");
> sc->silicon_rev = AST1030_A1_SILICON_REV;
> sc->sram_size = 768 * KiB;
> + sc->secsram_size = 9 * KiB;
> sc->spis_num = 2;
> sc->ehcis_num = 0;
> sc->wdts_num = 4;
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 9a5e3c0bac..bd1e03e78a 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -71,6 +71,7 @@ struct AspeedSoCState {
> AspeedSMCState spi[ASPEED_SPIS_NUM];
> EHCISysBusState ehci[ASPEED_EHCIS_NUM];
> AspeedSBCState sbc;
> + MemoryRegion secsram;
> UnimplementedDeviceState sbc_unimplemented;
> AspeedSDMCState sdmc;
> AspeedWDTState wdt[ASPEED_WDTS_NUM];
> @@ -105,6 +106,7 @@ struct AspeedSoCClass {
> const char *cpu_type;
> uint32_t silicon_rev;
> uint64_t sram_size;
> + uint64_t secsram_size;
> int spis_num;
> int ehcis_num;
> int wdts_num;
> @@ -143,6 +145,7 @@ enum {
> ASPEED_DEV_SCU,
> ASPEED_DEV_ADC,
> ASPEED_DEV_SBC,
> + ASPEED_DEV_SECSRAM,
> ASPEED_DEV_EMMC_BC,
> ASPEED_DEV_VIDEO,
> ASPEED_DEV_SRAM,
> --
> 2.38.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 6/9] hw/arm/aspeed_ast10x0: Map HACE peripheral
2022-12-29 15:23 ` [PATCH 6/9] hw/arm/aspeed_ast10x0: Map HACE peripheral Philippe Mathieu-Daudé
@ 2022-12-29 20:52 ` Peter Delevoryas
2022-12-30 8:13 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 27+ messages in thread
From: Peter Delevoryas @ 2022-12-29 20:52 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Steven Lee, Peter Delevoryas, qemu-arm, Cleber Rosa,
Peter Maydell, Beraldo Leal, Wainer dos Santos Moschetta,
Troy Lee, Andrew Jeffery, Joel Stanley, Cédric Le Goater,
Peter Delevoryas, Jamin Lin
On Thu, Dec 29, 2022 at 04:23:22PM +0100, Philippe Mathieu-Daudé wrote:
> Since I don't have access to the datasheet, the relevant
> values were found in:
> https://github.com/AspeedTech-BMC/zephyr/blob/v00.01.08/dts/arm/aspeed/ast10x0.dtsi
>
> Before on Zephyr:
>
> uart:~$ crypto aes256_cbc_vault
> aes256_cbc vault key 1
> [00:00:06.699,000] <inf> hace_global: aspeed_crypto_session_setup
> [00:00:06.699,000] <inf> hace_global: data->cmd: 1c2098
> [00:00:06.699,000] <inf> hace_global: crypto_data_src: 93340
> [00:00:06.699,000] <inf> hace_global: crypto_data_dst: 93348
> [00:00:06.699,000] <inf> hace_global: crypto_ctx_base: 93300
> [00:00:06.699,000] <inf> hace_global: crypto_data_len: 80000040
> [00:00:06.699,000] <inf> hace_global: crypto_cmd_reg: 11c2098
> [00:00:09.743,000] <inf> hace_global: HACE_STS: 0
> [00:00:09.743,000] <err> hace_global: HACE poll timeout
> [00:00:09.743,000] <err> crypto: CBC mode ENCRYPT - Failed
> [00:00:09.743,000] <inf> hace_global: aspeed_crypto_session_free
> uart:~$
>
> After:
>
> uart:~$ crypto aes256_cbc_vault
> aes256_cbc vault key 1
> Was waiting for:
> 6b c1 be e2 2e 40 9f 96 e9 3d 7e 11 73 93 17 2a
> ae 2d 8a 57 1e 03 ac 9c 9e b7 6f ac 45 af 8e 51
> 30 c8 1c 46 a3 5c e4 11 e5 fb c1 19 1a 0a 52 ef
> f6 9f 24 45 df 4f 9b 17 ad 2b 41 7b e6 6c 37 10
>
> But got:
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>
> [00:00:05.771,000] <inf> hace_global: aspeed_crypto_session_setup
> [00:00:05.772,000] <inf> hace_global: data->cmd: 1c2098
> [00:00:05.772,000] <inf> hace_global: crypto_data_src: 93340
> [00:00:05.772,000] <inf> hace_global: crypto_data_dst: 93348
> [00:00:05.772,000] <inf> hace_global: crypto_ctx_base: 93300
> [00:00:05.772,000] <inf> hace_global: crypto_data_len: 80000040
> [00:00:05.772,000] <inf> hace_global: crypto_cmd_reg: 11c2098
> [00:00:05.772,000] <inf> hace_global: HACE_STS: 1000
> [00:00:05.772,000] <inf> crypto: Output length (encryption): 80
> [00:00:05.772,000] <inf> hace_global: aspeed_crypto_session_free
> [00:00:05.772,000] <inf> hace_global: aspeed_crypto_session_setup
> [00:00:05.772,000] <inf> hace_global: data->cmd: 1c2018
> [00:00:05.772,000] <inf> hace_global: crypto_data_src: 93340
> [00:00:05.772,000] <inf> hace_global: crypto_data_dst: 93348
> [00:00:05.772,000] <inf> hace_global: crypto_ctx_base: 93300
> [00:00:05.772,000] <inf> hace_global: crypto_data_len: 80000040
> [00:00:05.772,000] <inf> hace_global: crypto_cmd_reg: 11c2018
> [00:00:05.772,000] <inf> hace_global: HACE_STS: 1000
> [00:00:05.772,000] <inf> crypto: Output length (decryption): 64
> [00:00:05.772,000] <err> crypto: CBC mode DECRYPT - Mismatch between plaintext and decrypted cipher text
> [00:00:05.774,000] <inf> hace_global: aspeed_crypto_session_free
> uart:~$
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Awesome!
Reviewed-by: Peter Delevoryas <peter@pjd.dev>
> ---
> Should we rename HACE 'dram' as 'secram' / 'secure-ram'?
Sure, sounds good to me.
> ---
> hw/arm/aspeed_ast10x0.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
> index 21a2e62345..02636705b6 100644
> --- a/hw/arm/aspeed_ast10x0.c
> +++ b/hw/arm/aspeed_ast10x0.c
> @@ -29,6 +29,7 @@ static const hwaddr aspeed_soc_ast1030_memmap[] = {
> [ASPEED_DEV_SPI1] = 0x7E630000,
> [ASPEED_DEV_SPI2] = 0x7E640000,
> [ASPEED_DEV_UDC] = 0x7E6A2000,
> + [ASPEED_DEV_HACE] = 0x7E6D0000,
> [ASPEED_DEV_SCU] = 0x7E6E2000,
> [ASPEED_DEV_JTAG0] = 0x7E6E4000,
> [ASPEED_DEV_JTAG1] = 0x7E6E4100,
> @@ -166,6 +167,9 @@ static void aspeed_soc_ast1030_init(Object *obj)
> snprintf(typename, sizeof(typename), "aspeed.gpio-%s", socname);
> object_initialize_child(obj, "gpio", &s->gpio, typename);
>
> + snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
> + object_initialize_child(obj, "hace", &s->hace, typename);
> +
> object_initialize_child(obj, "iomem", &s->iomem, TYPE_UNIMPLEMENTED_DEVICE);
> object_initialize_child(obj, "sbc-unimplemented", &s->sbc_unimplemented,
> TYPE_UNIMPLEMENTED_DEVICE);
> @@ -359,6 +363,17 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
> }
> aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->sbc), 0, sc->memmap[ASPEED_DEV_SBC]);
>
> + /* HACE */
> + object_property_set_link(OBJECT(&s->hace), "dram", OBJECT(&s->secsram),
> + &error_abort);
> + if (!sysbus_realize(SYS_BUS_DEVICE(&s->hace), errp)) {
> + return;
> + }
> + aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->hace), 0,
> + sc->memmap[ASPEED_DEV_HACE]);
> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->hace), 0,
> + aspeed_soc_get_irq(s, ASPEED_DEV_HACE));
> +
> /* Watch dog */
> for (i = 0; i < sc->wdts_num; i++) {
> AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(&s->wdt[i]);
> --
> 2.38.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 7/9] hw/misc/aspeed_hace: Do not crash if address_space_map() failed
2022-12-29 15:23 ` [PATCH 7/9] hw/misc/aspeed_hace: Do not crash if address_space_map() failed Philippe Mathieu-Daudé
@ 2022-12-29 20:54 ` Peter Delevoryas
0 siblings, 0 replies; 27+ messages in thread
From: Peter Delevoryas @ 2022-12-29 20:54 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Steven Lee, Peter Delevoryas, qemu-arm, Cleber Rosa,
Peter Maydell, Beraldo Leal, Wainer dos Santos Moschetta,
Troy Lee, Andrew Jeffery, Joel Stanley, Cédric Le Goater,
Peter Delevoryas, Jamin Lin
On Thu, Dec 29, 2022 at 04:23:23PM +0100, Philippe Mathieu-Daudé wrote:
> address_space_map() can fail:
>
> uart:~$ hash test
> sha256_test
> tv[0]:
> Segmentation fault: 11
> Thread 3 "qemu-system-arm" received signal SIGSEGV, Segmentation fault.
> gen_acc_mode_iov (req_len=0x7ffff18b7778, id=<optimized out>, iov=0x7ffff18b7780, s=0x555556ce0bd0)
> at ../hw/misc/aspeed_hace.c:171
> 171 if (has_padding(s, &iov[id], *req_len, &total_msg_len, &pad_offset)) {
> (gdb) bt
> #0 gen_acc_mode_iov (req_len=0x7ffff18b7778, id=<optimized out>, iov=0x7ffff18b7780, s=0x555556ce0bd0)
> at ../hw/misc/aspeed_hace.c:171
> #1 do_hash_operation (s=s@entry=0x555556ce0bd0, algo=3, sg_mode=sg_mode@entry=true, acc_mode=acc_mode@entry=true)
> at ../hw/misc/aspeed_hace.c:224
> #2 0x00005555559bdbb8 in aspeed_hace_write (opaque=<optimized out>, addr=12, data=262488, size=<optimized out>)
> at ../hw/misc/aspeed_hace.c:358
>
> This change doesn't fix much, but at least the guest
> can't crash QEMU anymore. Instead it is still usable:
>
> uart:~$ hash test
> sha256_test
> tv[0]:hash_final error
> sha384_test
> tv[0]:hash_final error
> sha512_test
> tv[0]:hash_final error
> [00:00:06.278,000] <err> hace_global: HACE poll timeout
> [00:00:09.324,000] <err> hace_global: HACE poll timeout
> [00:00:12.261,000] <err> hace_global: HACE poll timeout
> uart:~$
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Good catch,
Reviewed-by: Peter Delevoryas <peter@pjd.dev>
Maybe include this tag?
Fixes: c5475b3f9a ("hw: Model ASPEED's Hash and Crypto Engine")
> ---
> hw/misc/aspeed_hace.c | 21 +++++++++++++++------
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
> index ac21be306c..12a761f1f5 100644
> --- a/hw/misc/aspeed_hace.c
> +++ b/hw/misc/aspeed_hace.c
> @@ -193,6 +193,7 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
> size_t digest_len = 0;
> int niov = 0;
> int i;
> + void *haddr;
>
> if (sg_mode) {
> uint32_t len = 0;
> @@ -217,9 +218,13 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
> addr &= SG_LIST_ADDR_MASK;
>
> plen = len & SG_LIST_LEN_MASK;
> - iov[i].iov_base = address_space_map(&s->dram_as, addr, &plen, false,
> - MEMTXATTRS_UNSPECIFIED);
> -
> + haddr = address_space_map(&s->dram_as, addr, &plen, false,
> + MEMTXATTRS_UNSPECIFIED);
> + if (haddr == NULL) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: qcrypto failed\n", __func__);
> + return;
> + }
> + iov[i].iov_base = haddr;
> if (acc_mode) {
> niov = gen_acc_mode_iov(s, iov, i, &plen);
>
> @@ -230,10 +235,14 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
> } else {
> hwaddr len = s->regs[R_HASH_SRC_LEN];
>
> + haddr = address_space_map(&s->dram_as, s->regs[R_HASH_SRC],
> + &len, false, MEMTXATTRS_UNSPECIFIED);
> + if (haddr == NULL) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: qcrypto failed\n", __func__);
> + return;
> + }
> + iov[0].iov_base = haddr;
> iov[0].iov_len = len;
> - iov[0].iov_base = address_space_map(&s->dram_as, s->regs[R_HASH_SRC],
> - &len, false,
> - MEMTXATTRS_UNSPECIFIED);
> i = 1;
>
> if (s->iov_count) {
> --
> 2.38.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 8/9] hw/arm/aspeed_ast10x0: Add TODO comment to use Cortex-M4F
2022-12-29 15:23 ` [PATCH 8/9] hw/arm/aspeed_ast10x0: Add TODO comment to use Cortex-M4F Philippe Mathieu-Daudé
@ 2022-12-29 20:55 ` Peter Delevoryas
0 siblings, 0 replies; 27+ messages in thread
From: Peter Delevoryas @ 2022-12-29 20:55 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Steven Lee, Peter Delevoryas, qemu-arm, Cleber Rosa,
Peter Maydell, Beraldo Leal, Wainer dos Santos Moschetta,
Troy Lee, Andrew Jeffery, Joel Stanley, Cédric Le Goater,
Peter Delevoryas, Jamin Lin
On Thu, Dec 29, 2022 at 04:23:24PM +0100, Philippe Mathieu-Daudé wrote:
> This SoC uses a Cortex-M4F. QEMU only implements a M4,
> which is good enough. Add a TODO note in case the M4F
> is added.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Oh, yeah good to have a note of this somewhere.
Reviewed-by: Peter Delevoryas <peter@pjd.dev>
> ---
> hw/arm/aspeed_ast10x0.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
> index 02636705b6..788827ca9d 100644
> --- a/hw/arm/aspeed_ast10x0.c
> +++ b/hw/arm/aspeed_ast10x0.c
> @@ -421,7 +421,7 @@ static void aspeed_soc_ast1030_class_init(ObjectClass *klass, void *data)
> dc->realize = aspeed_soc_ast1030_realize;
>
> sc->name = "ast1030-a1";
> - sc->cpu_type = ARM_CPU_TYPE_NAME("cortex-m4");
> + sc->cpu_type = ARM_CPU_TYPE_NAME("cortex-m4"); /* TODO cortex-m4f */
> sc->silicon_rev = AST1030_A1_SILICON_REV;
> sc->sram_size = 768 * KiB;
> sc->secsram_size = 9 * KiB;
> --
> 2.38.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 9/9] tests/avocado: Test Aspeed Zephyr SDK v00.01.08 on AST1030 board
2022-12-29 15:23 ` [PATCH 9/9] tests/avocado: Test Aspeed Zephyr SDK v00.01.08 on AST1030 board Philippe Mathieu-Daudé
@ 2022-12-29 20:56 ` Peter Delevoryas
0 siblings, 0 replies; 27+ messages in thread
From: Peter Delevoryas @ 2022-12-29 20:56 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Steven Lee, Peter Delevoryas, qemu-arm, Cleber Rosa,
Peter Maydell, Beraldo Leal, Wainer dos Santos Moschetta,
Troy Lee, Andrew Jeffery, Joel Stanley, Cédric Le Goater,
Peter Delevoryas, Jamin Lin
On Thu, Dec 29, 2022 at 04:23:25PM +0100, Philippe Mathieu-Daudé wrote:
> Add a very quick test that runs some commands in a Zephyr shell:
>
> $ tests/venv/bin/avocado --show=app,console run -t os:zephyr tests/avocado
> (2/2) tests/avocado/machine_aspeed.py:AST1030Machine.test_ast1030_zephyros_1_07:
> console: *** Booting Zephyr OS build v00.01.07 ***
> console: ast1030_evb demo
> console: SOC: AST1030-A1
> console: uart:~$ kernel stacks
> console: 0x36910 wdt_background (real size 1024): unused 988 usage 36 / 1024 (3 %)
> console: 0x36ad8 shell_uart (real size 4096): unused 3084 usage 1012 / 4096 (24 %)
> console: 0x2edb8 ADC0 (real size 400): unused 260 usage 140 / 400 (35 %)
> console: 0x2f0f0 ADC1 (real size 400): unused 260 usage 140 / 400 (35 %)
> console: 0x3b098 sysworkq (real size 1024): unused 860 usage 164 / 1024 (16 %)
> console: 0x36cc0 usbdworkq (real size 1024): unused 860 usage 164 / 1024 (16 %)
> console: 0x36bd8 usbworkq (real size 1024): unused 860 usage 164 / 1024 (16 %)
> console: 0x36a10 logging (real size 768): unused 548 usage 220 / 768 (28 %)
> console: 0x36ef8 idle 00 (real size 320): unused 268 usage 52 / 320 (16 %)
> console: 0x47800 IRQ 00 (real size 2048): unused 1504 usage 544 / 2048 (26 %)
> console: uart:~$ otp info scu
> console: SCU BIT reg_protect Description
> console: ____________________________________________________________________
> console: 0x500 0x0 0x0 Disable ARM CM4 CPU boot (TXD5)
> console: 0x500 0x1 0x0 /Reserved
> console: 0x500 0x2 0x0 \ "
> console: 0x500 0x3 0x0 Address offset of single chip ABR mode
> console: 0x500 0x4 0x0 /Reserved
> console: 0x500 0x5 0x0 | "
> console: 0x500 0x6 0x0 | "
> console: 0x500 0x7 0x0 | "
> console: 0x500 0x8 0x0 | "
> console: 0x500 0x9 0x0 | "
> console: 0x500 0xA 0x0 | "
> console: 0x500 0xB 0x0 | "
> console: 0x500 0xC 0x0 | "
> console: 0x500 0xD 0x0 | "
> console: 0x500 0xE 0x0 | "
> console: 0x500 0xF 0x0 | "
> console: 0x500 0x10 0x0 \ "
> console: 0x500 0x11 0x0 Disabl3 ARM JTAG debug
> console: 0x500 0x12 0x0 /Reserved
> console: 0x500 0x13 0x0 | "
> console: 0x500 0x14 0x0 | "
> console: 0x500 0x15 0x0 | "
> console: 0x500 0x16 0x0 | "
> console: 0x500 0x17 0x0 | "
> console: 0x500 0x18 0x0 | "
> console: 0x500 0x19 0x0 | "
> console: 0x500 0x1A 0x0 | "
> console: 0x500 0x1B 0x0 | "
> console: 0x500 0x1C 0x0 | "
> console: 0x500 0x1D 0x0 | "
> console: 0x500 0x1E 0x0 | "
> console: 0x500 0x1F 0x0 \ "
> console: 0x510 0x0 0x0 /Reserved
> console: 0x510 0x1 0x0 | "
> console: 0x510 0x2 0x0 | "
> console: 0x510 0x3 0x0 \ "
> console: 0x510 0x4 0x0 Disable debug interfaces
> console: 0x510 0x5 0x0 /Reserved
> console: 0x510 0x6 0x0 | "
> console: 0x510 0x7 0x0 \ "
> console: 0x510 0x8 0x0 Enable boot from Uart5 by Pin Strap
> console: 0x510 0x9 0x0 /Reserved
> console: 0x510 0xA 0x0 \ "
> console: 0x510 0xB 0x0 Enable boot SPI ABR
> console: 0x510 0xC 0x0 Boot SPI ABR Mode
> console: 0x510 0xD 0x0 /Boot SPI flash size
> console: 0x510 0xE 0x0 | "
> console: 0x510 0xF 0x0 \ "
> console: 0x510 0x10 0x0 /Reserved
> console: 0x510 0x11 0x0 | "
> console: 0x510 0x12 0x0 | "
> console: 0x510 0x13 0x0 | "
> console: 0x510 0x14 0x0 | "
> console: 0x510 0x15 0x0 \ "
> console: 0x510 0x16 0x0 Enable boot SPI auxiliary control pins
> console: 0x510 0x19 0x0 /Reserved
> console: 0x510 0x1A 0x0 | "
> console: 0x510 0x1B 0x0 | "
> console: 0x510 0x1C 0x0 | "
> console: 0x510 0x1D 0x0 | "
> console: 0x510 0x1E 0x0 | "
> console: 0x510 0x1F 0x0 \ "
> console: 0x510 0x1E 0x0 Enable dedicate GPIO strap pins
> console: 0x510 0x1F 0x0 Enable Secure Boot by Pin Strap
> console: uart:~$ hwinfo devid
> console: Length: 8
> console: ID: 0x0000018000000180
> console: uart:~$ crypto aes256_cbc_vault
> console: aes256_cbc vault key 1
> console: Was waiting for:
> console: 6b c1 be e2 2e 40 9f 96 e9 3d 7e 11 73 93 17 2a
> console: ae 2d 8a 57 1e 03 ac 9c 9e b7 6f ac 45 af 8e 51
> console: 30 c8 1c 46 a3 5c e4 11 e5 fb c1 19 1a 0a 52 ef
> console: f6 9f 24 45 df 4f 9b 17 ad 2b 41 7b e6 6c 37 10
> console: But got:
> console: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> console: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> console: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> console: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> console: uart:~$ random get
> console: 0x862460d
> console: uart:~$ i2c scan I2C_0
> console: 0 1 2 3 4 5 6 7 8 9 a b c d e f
> console: 00: -- -- -- -- -- -- -- -- -- -- -- --
> console: 10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
> console: 20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
> console: 30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
> console: 40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
> console: 50: 50 -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
> console: 60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
> console: 70: -- -- -- -- -- -- -- --
> console: 1 devices found on I2C_0
> console: uart:~$ kernel uptime
> console: Uptime: 9897 ms
> console: uart:~$ kernel reboot warm
> console: *** Booting Zephyr OS build v00.01.07 ***
> PASS (1.08 s)
>
> Ref: https://github.com/AspeedTech-BMC/zephyr/releases/download/v00.01.07/Aspeed_Zephy_SDK_User_Guide_v00.01.07.pdf
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Nice, very cool!
Reviewed-by: Peter Delevoryas <peter@pjd.dev>
> ---
> tests/avocado/machine_aspeed.py | 41 ++++++++++++++++++++++++++++++++-
> 1 file changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/tests/avocado/machine_aspeed.py b/tests/avocado/machine_aspeed.py
> index 1fc385e1c8..11f5b17eb9 100644
> --- a/tests/avocado/machine_aspeed.py
> +++ b/tests/avocado/machine_aspeed.py
> @@ -22,10 +22,11 @@ class AST1030Machine(QemuSystemTest):
>
> timeout = 10
>
> - def test_ast1030_zephyros(self):
> + def test_ast1030_zephyros_1_04(self):
> """
> :avocado: tags=arch:arm
> :avocado: tags=machine:ast1030-evb
> + :avocado: tags=os:zephyr
> """
> tar_url = ('https://github.com/AspeedTech-BMC'
> '/zephyr/releases/download/v00.01.04/ast1030-evb-demo.zip')
> @@ -41,6 +42,44 @@ def test_ast1030_zephyros(self):
> exec_command_and_wait_for_pattern(self, "help",
> "Available commands")
>
> + def test_ast1030_zephyros_1_07(self):
> + """
> + :avocado: tags=arch:arm
> + :avocado: tags=machine:ast1030-evb
> + :avocado: tags=os:zephyr
> + """
> + tar_url = ('https://github.com/AspeedTech-BMC'
> + '/zephyr/releases/download/v00.01.07/ast1030-evb-demo.zip')
> + tar_hash = '40ac87eabdcd3b3454ce5aad11fedc72a33ecda2'
> + tar_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
> + archive.extract(tar_path, self.workdir)
> + kernel_file = self.workdir + "/ast1030-evb-demo/zephyr.bin"
> + self.vm.set_console()
> + self.vm.add_args('-kernel', kernel_file,
> + '-nographic')
> + self.vm.launch()
> + wait_for_console_pattern(self, "Booting Zephyr OS")
> + for shell_cmd in [
> + 'kernel stacks',
> + 'otp info conf',
> + 'otp info scu',
> + 'hwinfo devid',
> + 'crypto aes256_cbc_vault',
> + 'random get',
> + 'jtag JTAG1 sw_xfer high TMS',
> + 'adc ADC0 resolution 12',
> + 'adc ADC0 read 42',
> + 'adc ADC1 read 69',
> + 'i2c scan I2C_0',
> + 'i3c attach I3C_0',
> + 'hash test',
> + 'kernel uptime',
> + 'kernel reboot warm',
> + 'kernel uptime',
> + 'kernel reboot cold',
> + 'kernel uptime',
> + ]: exec_command_and_wait_for_pattern(self, shell_cmd, "uart:~$")
> +
> class AST2x00Machine(QemuSystemTest):
>
> timeout = 90
> --
> 2.38.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/9] hw/arm/aspeed_ast10x0: Map I3C peripheral
2022-12-29 20:46 ` Peter Delevoryas
@ 2022-12-30 7:07 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-30 7:07 UTC (permalink / raw)
To: Peter Delevoryas
Cc: qemu-devel, Steven Lee, Peter Delevoryas, qemu-arm, Cleber Rosa,
Peter Maydell, Beraldo Leal, Wainer dos Santos Moschetta,
Troy Lee, Andrew Jeffery, Joel Stanley, Cédric Le Goater,
Peter Delevoryas, Jamin Lin
On 29/12/22 21:46, Peter Delevoryas wrote:
> On Thu, Dec 29, 2022 at 04:23:20PM +0100, Philippe Mathieu-Daudé wrote:
>> Since I don't have access to the datasheet, the relevant
>> values were found in:
>> https://github.com/AspeedTech-BMC/zephyr/blob/v00.01.08/dts/arm/aspeed/ast10x0.dtsi
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> hw/arm/aspeed_ast10x0.c | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>> @@ -240,6 +244,18 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
>> sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c.busses[i]), 0, irq);
>> }
>>
>> + /* I3C */
>> + if (!sysbus_realize(SYS_BUS_DEVICE(&s->i3c), errp)) {
>> + return;
>> + }
>> + aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->i3c), 0, sc->memmap[ASPEED_DEV_I3C]);
>> + for (i = 0; i < ASPEED_I3C_NR_DEVICES; i++) {
>> + qemu_irq irq = qdev_get_gpio_in(DEVICE(&s->armv7m),
>> + sc->irqmap[ASPEED_DEV_I3C] + i);
>> + /* The AST1030 I2C controller has one IRQ per bus. */
>
> Should this comment be I2C or I3C?
Oops indeed, copy/paste leftover :)
> Reviewed-by: Peter Delevoryas <peter@pjd.dev>
Thanks!
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/9] hw/watchdog/wdt_aspeed: Map the whole MMIO range
2022-12-29 20:42 ` Peter Delevoryas
@ 2022-12-30 7:32 ` Philippe Mathieu-Daudé
2023-01-02 10:34 ` Cédric Le Goater
0 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-30 7:32 UTC (permalink / raw)
To: Peter Delevoryas
Cc: qemu-devel, Steven Lee, Peter Delevoryas, qemu-arm, Cleber Rosa,
Peter Maydell, Beraldo Leal, Wainer dos Santos Moschetta,
Troy Lee, Andrew Jeffery, Joel Stanley, Cédric Le Goater,
Peter Delevoryas, Jamin Lin, Chin-Ting Kuo
On 29/12/22 21:42, Peter Delevoryas wrote:
> On Thu, Dec 29, 2022 at 04:23:17PM +0100, Philippe Mathieu-Daudé wrote:
>> Avoid confusing two different things:
>> - the WDT I/O region size ('iosize')
>> - at which offset the SoC map the WDT ('offset')
>> While it is often the same, we can map smaller region sizes at
>> larger offsets.
>>
>> Here we are interested in the I/O region size. Rename as 'iosize'
>> and map the whole range, not only the first implemented registers.
>> Unimplemented registers accesses are already logged as guest-errors.
>>
>> Otherwise when booting the demo in [*] we get:
>>
>> aspeed.io: unimplemented device write (size 4, offset 0x1851a8, value 0x030f1ff1)
>> aspeed.io: unimplemented device write (size 4, offset 0x1851ac, value 0x03fffff1)
>> aspeed.io: unimplemented device write (size 4, offset 0x185128, value 0x030f1ff1)
>> aspeed.io: unimplemented device write (size 4, offset 0x18512c, value 0x03fffff1)
>>
>> [*] https://github.com/AspeedTech-BMC/zephyr/releases/tag/v00.01.07
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> hw/arm/aspeed_ast10x0.c | 2 +-
>> hw/arm/aspeed_ast2600.c | 2 +-
>> hw/arm/aspeed_soc.c | 2 +-
>> hw/watchdog/wdt_aspeed.c | 12 +++++++-----
>> include/hw/watchdog/wdt_aspeed.h | 2 +-
>> 5 files changed, 11 insertions(+), 9 deletions(-)
>> diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
>> index d753693a2e..c1311ce74c 100644
>> --- a/hw/watchdog/wdt_aspeed.c
>> +++ b/hw/watchdog/wdt_aspeed.c
>> @@ -260,6 +260,7 @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
>> {
>> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> AspeedWDTState *s = ASPEED_WDT(dev);
>> + AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(dev);
>>
>> assert(s->scu);
>>
>> @@ -270,8 +271,9 @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
>> */
>> s->pclk_freq = PCLK_HZ;
>>
>> + assert(awc->iosize >= ASPEED_WDT_REGS_MAX * 4);
>> memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_wdt_ops, s,
>> - TYPE_ASPEED_WDT, ASPEED_WDT_REGS_MAX * 4);
>> + TYPE_ASPEED_WDT, awc->iosize);
>
> Does this fix the unimplemented thing you referred to in the commit message?
Yes, I'll reword the description to be clearer.
> I'm not sure which part of this diff actually removes the unimplemented traces.
Having:
#define ASPEED_WDT_REGS_MAX (0x20 / 4)
Before this patch, we map regions of 0x20 ...
>> @@ -392,7 +394,7 @@ static void aspeed_1030_wdt_class_init(ObjectClass *klass, void *data)
>> AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass);
>>
>> dc->desc = "ASPEED 1030 Watchdog Controller";
>> - awc->offset = 0x80;
>> + awc->iosize = 0x80;
... every span of 0x80, so there is a gap of 0x60, apparently accessed
by the Zephyr WDT driver (address 0x28 - register #10 - is accessed in
the traces).
From the driver source we can see [1] added in [2]:
#define WDT_RELOAD_VAL_REG 0x0004
#define WDT_RESTART_REG 0x0008
#define WDT_CTRL_REG 0x000C
#define WDT_TIMEOUT_STATUS_REG 0x0010
#define WDT_TIMEOUT_STATUS_CLR_REG 0x0014
#define WDT_RESET_MASK1_REG 0x001C
#define WDT_RESET_MASK2_REG 0x0020
#define WDT_SW_RESET_MASK1_REG 0x0028 <------
#define WDT_SW_RESET_MASK2_REG 0x002C
#define WDT_SW_RESET_CTRL_REG 0x0024
So the traces likely correspond to this code:
static int aspeed_wdt_init(const struct device *dev)
{
const struct aspeed_wdt_config *config = dev->config;
struct aspeed_wdt_data *const data = dev->data;
uint32_t reg_val;
/* disable WDT by default */
reg_val = sys_read32(config->ctrl_base + WDT_CTRL_REG);
reg_val &= ~WDT_CTRL_ENABLE;
sys_write32(reg_val, config->ctrl_base + WDT_CTRL_REG);
sys_write32(data->rst_mask1,
config->ctrl_base + WDT_SW_RESET_MASK1_REG);
sys_write32(data->rst_mask2,
config->ctrl_base + WDT_SW_RESET_MASK2_REG);
return 0;
}
After this patch, we map (in this case) a MMIO region of 0x80.
Accesses to address 0x28 hits this device model but is handled
as 'WDT register not covered'.
Better would be to extend the Aspeed WDT model in QEMU, or at
least report the valid-but-unimplemented registers as UNIMP instead
of GUEST_ERRORS.
[1]
https://github.com/AspeedTech-BMC/zephyr/blob/v00.01.08/drivers/watchdog/wdt_aspeed.c#L31
[2] https://github.com/AspeedTech-BMC/zephyr/commit/2e99f10ac27b
>> diff --git a/include/hw/watchdog/wdt_aspeed.h b/include/hw/watchdog/wdt_aspeed.h
>> index dfa5dfa424..db91ee6b51 100644
>> --- a/include/hw/watchdog/wdt_aspeed.h
>> +++ b/include/hw/watchdog/wdt_aspeed.h
>> @@ -40,7 +40,7 @@ struct AspeedWDTState {
>> struct AspeedWDTClass {
>> SysBusDeviceClass parent_class;
>>
>> - uint32_t offset;
>> + uint32_t iosize;
>
> Oh yeah, iosize is a way better name for this. +1. But to be honest, I don't
> understand how this is changing the unimplemented traces?
>
> Reviewed-by: Peter Delevoryas <peter@pjd.dev>
Thanks!
Phil.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 6/9] hw/arm/aspeed_ast10x0: Map HACE peripheral
2022-12-29 20:52 ` Peter Delevoryas
@ 2022-12-30 8:13 ` Philippe Mathieu-Daudé
2022-12-30 20:23 ` Peter Delevoryas
0 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-30 8:13 UTC (permalink / raw)
To: Peter Delevoryas
Cc: qemu-devel, Steven Lee, Peter Delevoryas, qemu-arm, Cleber Rosa,
Peter Maydell, Beraldo Leal, Wainer dos Santos Moschetta,
Troy Lee, Andrew Jeffery, Joel Stanley, Cédric Le Goater,
Peter Delevoryas, Jamin Lin
On 29/12/22 21:52, Peter Delevoryas wrote:
> On Thu, Dec 29, 2022 at 04:23:22PM +0100, Philippe Mathieu-Daudé wrote:
>> Since I don't have access to the datasheet, the relevant
>> values were found in:
>> https://github.com/AspeedTech-BMC/zephyr/blob/v00.01.08/dts/arm/aspeed/ast10x0.dtsi
>>
>> Before on Zephyr:
>>
>> uart:~$ crypto aes256_cbc_vault
>> aes256_cbc vault key 1
>> [00:00:06.699,000] <inf> hace_global: aspeed_crypto_session_setup
>> [00:00:06.699,000] <inf> hace_global: data->cmd: 1c2098
>> [00:00:06.699,000] <inf> hace_global: crypto_data_src: 93340
>> [00:00:06.699,000] <inf> hace_global: crypto_data_dst: 93348
>> [00:00:06.699,000] <inf> hace_global: crypto_ctx_base: 93300
>> [00:00:06.699,000] <inf> hace_global: crypto_data_len: 80000040
>> [00:00:06.699,000] <inf> hace_global: crypto_cmd_reg: 11c2098
>> [00:00:09.743,000] <inf> hace_global: HACE_STS: 0
>> [00:00:09.743,000] <err> hace_global: HACE poll timeout
>> [00:00:09.743,000] <err> crypto: CBC mode ENCRYPT - Failed
>> [00:00:09.743,000] <inf> hace_global: aspeed_crypto_session_free
>> uart:~$
>>
>> After:
>>
>> uart:~$ crypto aes256_cbc_vault
>> aes256_cbc vault key 1
>> Was waiting for:
>> 6b c1 be e2 2e 40 9f 96 e9 3d 7e 11 73 93 17 2a
>> ae 2d 8a 57 1e 03 ac 9c 9e b7 6f ac 45 af 8e 51
>> 30 c8 1c 46 a3 5c e4 11 e5 fb c1 19 1a 0a 52 ef
>> f6 9f 24 45 df 4f 9b 17 ad 2b 41 7b e6 6c 37 10
>>
>> But got:
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>
>> [00:00:05.771,000] <inf> hace_global: aspeed_crypto_session_setup
>> [00:00:05.772,000] <inf> hace_global: data->cmd: 1c2098
>> [00:00:05.772,000] <inf> hace_global: crypto_data_src: 93340
>> [00:00:05.772,000] <inf> hace_global: crypto_data_dst: 93348
>> [00:00:05.772,000] <inf> hace_global: crypto_ctx_base: 93300
>> [00:00:05.772,000] <inf> hace_global: crypto_data_len: 80000040
>> [00:00:05.772,000] <inf> hace_global: crypto_cmd_reg: 11c2098
>> [00:00:05.772,000] <inf> hace_global: HACE_STS: 1000
>> [00:00:05.772,000] <inf> crypto: Output length (encryption): 80
>> [00:00:05.772,000] <inf> hace_global: aspeed_crypto_session_free
>> [00:00:05.772,000] <inf> hace_global: aspeed_crypto_session_setup
>> [00:00:05.772,000] <inf> hace_global: data->cmd: 1c2018
>> [00:00:05.772,000] <inf> hace_global: crypto_data_src: 93340
>> [00:00:05.772,000] <inf> hace_global: crypto_data_dst: 93348
>> [00:00:05.772,000] <inf> hace_global: crypto_ctx_base: 93300
>> [00:00:05.772,000] <inf> hace_global: crypto_data_len: 80000040
>> [00:00:05.772,000] <inf> hace_global: crypto_cmd_reg: 11c2018
>> [00:00:05.772,000] <inf> hace_global: HACE_STS: 1000
>> [00:00:05.772,000] <inf> crypto: Output length (decryption): 64
>> [00:00:05.772,000] <err> crypto: CBC mode DECRYPT - Mismatch between plaintext and decrypted cipher text
>> [00:00:05.774,000] <inf> hace_global: aspeed_crypto_session_free
>> uart:~$
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> Awesome!
>
> Reviewed-by: Peter Delevoryas <peter@pjd.dev>
>
>> ---
>> Should we rename HACE 'dram' as 'secram' / 'secure-ram'?
>
> Sure, sounds good to me.
>
>> ---
>> hw/arm/aspeed_ast10x0.c | 15 +++++++++++++++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
>> index 21a2e62345..02636705b6 100644
>> --- a/hw/arm/aspeed_ast10x0.c
>> +++ b/hw/arm/aspeed_ast10x0.c
>> @@ -29,6 +29,7 @@ static const hwaddr aspeed_soc_ast1030_memmap[] = {
>> [ASPEED_DEV_SPI1] = 0x7E630000,
>> [ASPEED_DEV_SPI2] = 0x7E640000,
>> [ASPEED_DEV_UDC] = 0x7E6A2000,
>> + [ASPEED_DEV_HACE] = 0x7E6D0000,
>> [ASPEED_DEV_SCU] = 0x7E6E2000,
>> [ASPEED_DEV_JTAG0] = 0x7E6E4000,
>> [ASPEED_DEV_JTAG1] = 0x7E6E4100,
>> @@ -166,6 +167,9 @@ static void aspeed_soc_ast1030_init(Object *obj)
>> snprintf(typename, sizeof(typename), "aspeed.gpio-%s", socname);
>> object_initialize_child(obj, "gpio", &s->gpio, typename);
>>
>> + snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
>> + object_initialize_child(obj, "hace", &s->hace, typename);
>> +
>> object_initialize_child(obj, "iomem", &s->iomem, TYPE_UNIMPLEMENTED_DEVICE);
>> object_initialize_child(obj, "sbc-unimplemented", &s->sbc_unimplemented,
>> TYPE_UNIMPLEMENTED_DEVICE);
>> @@ -359,6 +363,17 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
>> }
>> aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->sbc), 0, sc->memmap[ASPEED_DEV_SBC]);
>>
>> + /* HACE */
>> + object_property_set_link(OBJECT(&s->hace), "dram", OBJECT(&s->secsram),
We need to link the SRAM here, not the sec-SRAM.
Doing so the hash test works:
uart:~$ hash test
sha256_test
tv[0]:PASS
tv[1]:PASS
tv[2]:PASS
tv[3]:PASS
tv[4]:PASS
sha384_test
tv[0]:PASS
tv[1]:PASS
tv[2]:PASS
tv[3]:PASS
tv[4]:PASS
tv[5]:PASS
sha512_test
tv[0]:PASS
tv[1]:PASS
tv[2]:PASS
tv[3]:PASS
tv[4]:PASS
tv[5]:PASS
uart:~$
rsa / aes256_cbc tests still fail.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 6/9] hw/arm/aspeed_ast10x0: Map HACE peripheral
2022-12-30 8:13 ` Philippe Mathieu-Daudé
@ 2022-12-30 20:23 ` Peter Delevoryas
0 siblings, 0 replies; 27+ messages in thread
From: Peter Delevoryas @ 2022-12-30 20:23 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Steven Lee, Peter Delevoryas, qemu-arm, Cleber Rosa,
Peter Maydell, Beraldo Leal, Wainer dos Santos Moschetta,
Troy Lee, Andrew Jeffery, Joel Stanley, Cédric Le Goater,
Peter Delevoryas, Jamin Lin
On Fri, Dec 30, 2022 at 09:13:29AM +0100, Philippe Mathieu-Daudé wrote:
> On 29/12/22 21:52, Peter Delevoryas wrote:
> > On Thu, Dec 29, 2022 at 04:23:22PM +0100, Philippe Mathieu-Daudé wrote:
> > > Since I don't have access to the datasheet, the relevant
> > > values were found in:
> > > https://github.com/AspeedTech-BMC/zephyr/blob/v00.01.08/dts/arm/aspeed/ast10x0.dtsi
> > >
> > > Before on Zephyr:
> > >
> > > uart:~$ crypto aes256_cbc_vault
> > > aes256_cbc vault key 1
> > > [00:00:06.699,000] <inf> hace_global: aspeed_crypto_session_setup
> > > [00:00:06.699,000] <inf> hace_global: data->cmd: 1c2098
> > > [00:00:06.699,000] <inf> hace_global: crypto_data_src: 93340
> > > [00:00:06.699,000] <inf> hace_global: crypto_data_dst: 93348
> > > [00:00:06.699,000] <inf> hace_global: crypto_ctx_base: 93300
> > > [00:00:06.699,000] <inf> hace_global: crypto_data_len: 80000040
> > > [00:00:06.699,000] <inf> hace_global: crypto_cmd_reg: 11c2098
> > > [00:00:09.743,000] <inf> hace_global: HACE_STS: 0
> > > [00:00:09.743,000] <err> hace_global: HACE poll timeout
> > > [00:00:09.743,000] <err> crypto: CBC mode ENCRYPT - Failed
> > > [00:00:09.743,000] <inf> hace_global: aspeed_crypto_session_free
> > > uart:~$
> > >
> > > After:
> > >
> > > uart:~$ crypto aes256_cbc_vault
> > > aes256_cbc vault key 1
> > > Was waiting for:
> > > 6b c1 be e2 2e 40 9f 96 e9 3d 7e 11 73 93 17 2a
> > > ae 2d 8a 57 1e 03 ac 9c 9e b7 6f ac 45 af 8e 51
> > > 30 c8 1c 46 a3 5c e4 11 e5 fb c1 19 1a 0a 52 ef
> > > f6 9f 24 45 df 4f 9b 17 ad 2b 41 7b e6 6c 37 10
> > >
> > > But got:
> > > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >
> > > [00:00:05.771,000] <inf> hace_global: aspeed_crypto_session_setup
> > > [00:00:05.772,000] <inf> hace_global: data->cmd: 1c2098
> > > [00:00:05.772,000] <inf> hace_global: crypto_data_src: 93340
> > > [00:00:05.772,000] <inf> hace_global: crypto_data_dst: 93348
> > > [00:00:05.772,000] <inf> hace_global: crypto_ctx_base: 93300
> > > [00:00:05.772,000] <inf> hace_global: crypto_data_len: 80000040
> > > [00:00:05.772,000] <inf> hace_global: crypto_cmd_reg: 11c2098
> > > [00:00:05.772,000] <inf> hace_global: HACE_STS: 1000
> > > [00:00:05.772,000] <inf> crypto: Output length (encryption): 80
> > > [00:00:05.772,000] <inf> hace_global: aspeed_crypto_session_free
> > > [00:00:05.772,000] <inf> hace_global: aspeed_crypto_session_setup
> > > [00:00:05.772,000] <inf> hace_global: data->cmd: 1c2018
> > > [00:00:05.772,000] <inf> hace_global: crypto_data_src: 93340
> > > [00:00:05.772,000] <inf> hace_global: crypto_data_dst: 93348
> > > [00:00:05.772,000] <inf> hace_global: crypto_ctx_base: 93300
> > > [00:00:05.772,000] <inf> hace_global: crypto_data_len: 80000040
> > > [00:00:05.772,000] <inf> hace_global: crypto_cmd_reg: 11c2018
> > > [00:00:05.772,000] <inf> hace_global: HACE_STS: 1000
> > > [00:00:05.772,000] <inf> crypto: Output length (decryption): 64
> > > [00:00:05.772,000] <err> crypto: CBC mode DECRYPT - Mismatch between plaintext and decrypted cipher text
> > > [00:00:05.774,000] <inf> hace_global: aspeed_crypto_session_free
> > > uart:~$
> > >
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >
> > Awesome!
> >
> > Reviewed-by: Peter Delevoryas <peter@pjd.dev>
> >
> > > ---
> > > Should we rename HACE 'dram' as 'secram' / 'secure-ram'?
> >
> > Sure, sounds good to me.
> >
> > > ---
> > > hw/arm/aspeed_ast10x0.c | 15 +++++++++++++++
> > > 1 file changed, 15 insertions(+)
> > >
> > > diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
> > > index 21a2e62345..02636705b6 100644
> > > --- a/hw/arm/aspeed_ast10x0.c
> > > +++ b/hw/arm/aspeed_ast10x0.c
> > > @@ -29,6 +29,7 @@ static const hwaddr aspeed_soc_ast1030_memmap[] = {
> > > [ASPEED_DEV_SPI1] = 0x7E630000,
> > > [ASPEED_DEV_SPI2] = 0x7E640000,
> > > [ASPEED_DEV_UDC] = 0x7E6A2000,
> > > + [ASPEED_DEV_HACE] = 0x7E6D0000,
> > > [ASPEED_DEV_SCU] = 0x7E6E2000,
> > > [ASPEED_DEV_JTAG0] = 0x7E6E4000,
> > > [ASPEED_DEV_JTAG1] = 0x7E6E4100,
> > > @@ -166,6 +167,9 @@ static void aspeed_soc_ast1030_init(Object *obj)
> > > snprintf(typename, sizeof(typename), "aspeed.gpio-%s", socname);
> > > object_initialize_child(obj, "gpio", &s->gpio, typename);
> > > + snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
> > > + object_initialize_child(obj, "hace", &s->hace, typename);
> > > +
> > > object_initialize_child(obj, "iomem", &s->iomem, TYPE_UNIMPLEMENTED_DEVICE);
> > > object_initialize_child(obj, "sbc-unimplemented", &s->sbc_unimplemented,
> > > TYPE_UNIMPLEMENTED_DEVICE);
> > > @@ -359,6 +363,17 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
> > > }
> > > aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->sbc), 0, sc->memmap[ASPEED_DEV_SBC]);
> > > + /* HACE */
> > > + object_property_set_link(OBJECT(&s->hace), "dram", OBJECT(&s->secsram),
>
> We need to link the SRAM here, not the sec-SRAM.
Nice catch 😅
>
> Doing so the hash test works:
>
> uart:~$ hash test
> sha256_test
> tv[0]:PASS
> tv[1]:PASS
> tv[2]:PASS
> tv[3]:PASS
> tv[4]:PASS
> sha384_test
> tv[0]:PASS
> tv[1]:PASS
> tv[2]:PASS
> tv[3]:PASS
> tv[4]:PASS
> tv[5]:PASS
> sha512_test
> tv[0]:PASS
> tv[1]:PASS
> tv[2]:PASS
> tv[3]:PASS
> tv[4]:PASS
> tv[5]:PASS
> uart:~$
>
> rsa / aes256_cbc tests still fail.
>
Oh nice!!
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/9] hw/watchdog/wdt_aspeed: Map the whole MMIO range
2022-12-29 15:23 ` [PATCH 1/9] hw/watchdog/wdt_aspeed: Map the whole MMIO range Philippe Mathieu-Daudé
2022-12-29 20:42 ` Peter Delevoryas
@ 2023-01-02 10:21 ` Cédric Le Goater
1 sibling, 0 replies; 27+ messages in thread
From: Cédric Le Goater @ 2023-01-02 10:21 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Steven Lee, Peter Delevoryas, Peter Delevoryas, qemu-arm,
Cleber Rosa, Peter Maydell, Beraldo Leal,
Wainer dos Santos Moschetta, Troy Lee, Andrew Jeffery,
Joel Stanley, Peter Delevoryas, Jamin Lin
On 12/29/22 16:23, Philippe Mathieu-Daudé wrote:
> Avoid confusing two different things:
> - the WDT I/O region size ('iosize')
> - at which offset the SoC map the WDT ('offset')
> While it is often the same, we can map smaller region sizes at
> larger offsets.
>
> Here we are interested in the I/O region size. Rename as 'iosize'
> and map the whole range, not only the first implemented registers.
> Unimplemented registers accesses are already logged as guest-errors.
>
> Otherwise when booting the demo in [*] we get:
>
> aspeed.io: unimplemented device write (size 4, offset 0x1851a8, value 0x030f1ff1)
> aspeed.io: unimplemented device write (size 4, offset 0x1851ac, value 0x03fffff1)
> aspeed.io: unimplemented device write (size 4, offset 0x185128, value 0x030f1ff1)
> aspeed.io: unimplemented device write (size 4, offset 0x18512c, value 0x03fffff1)
These are unimplemented registers related to software mode reset, which is a new
feature of the AST2600 SoC. The AST10x0 SoCs add a few more regs, hence the larger
MMIO width for the WDT registers. Until now, they have been harmless.
>
> [*] https://github.com/AspeedTech-BMC/zephyr/releases/tag/v00.01.07
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/arm/aspeed_ast10x0.c | 2 +-
> hw/arm/aspeed_ast2600.c | 2 +-
> hw/arm/aspeed_soc.c | 2 +-
> hw/watchdog/wdt_aspeed.c | 12 +++++++-----
> include/hw/watchdog/wdt_aspeed.h | 2 +-
> 5 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
> index 4d0b9b115f..122b3fd3f3 100644
> --- a/hw/arm/aspeed_ast10x0.c
> +++ b/hw/arm/aspeed_ast10x0.c
> @@ -325,7 +325,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
> return;
> }
> aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->wdt[i]), 0,
> - sc->memmap[ASPEED_DEV_WDT] + i * awc->offset);
> + sc->memmap[ASPEED_DEV_WDT] + i * awc->iosize);
> }
>
> /* GPIO */
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index cd75465c2b..a79e05ddbd 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -472,7 +472,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
> return;
> }
> aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->wdt[i]), 0,
> - sc->memmap[ASPEED_DEV_WDT] + i * awc->offset);
> + sc->memmap[ASPEED_DEV_WDT] + i * awc->iosize);
> }
>
> /* RAM */
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index b05b9dd416..2c0924d311 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -393,7 +393,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> return;
> }
> aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->wdt[i]), 0,
> - sc->memmap[ASPEED_DEV_WDT] + i * awc->offset);
> + sc->memmap[ASPEED_DEV_WDT] + i * awc->iosize);
> }
>
> /* RAM */
> diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
> index d753693a2e..c1311ce74c 100644
> --- a/hw/watchdog/wdt_aspeed.c
> +++ b/hw/watchdog/wdt_aspeed.c
> @@ -260,6 +260,7 @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
> {
> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> AspeedWDTState *s = ASPEED_WDT(dev);
> + AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(dev);
>
> assert(s->scu);
>
> @@ -270,8 +271,9 @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
> */
> s->pclk_freq = PCLK_HZ;
>
> + assert(awc->iosize >= ASPEED_WDT_REGS_MAX * 4);
> memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_wdt_ops, s,
> - TYPE_ASPEED_WDT, ASPEED_WDT_REGS_MAX * 4);
> + TYPE_ASPEED_WDT, awc->iosize);
> sysbus_init_mmio(sbd, &s->iomem);
> }
>
> @@ -309,7 +311,7 @@ static void aspeed_2400_wdt_class_init(ObjectClass *klass, void *data)
> AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass);
>
> dc->desc = "ASPEED 2400 Watchdog Controller";
> - awc->offset = 0x20;
> + awc->iosize = 0x20;
> awc->ext_pulse_width_mask = 0xff;
> awc->reset_ctrl_reg = SCU_RESET_CONTROL1;
> awc->wdt_reload = aspeed_wdt_reload;
> @@ -346,7 +348,7 @@ static void aspeed_2500_wdt_class_init(ObjectClass *klass, void *data)
> AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass);
>
> dc->desc = "ASPEED 2500 Watchdog Controller";
> - awc->offset = 0x20;
> + awc->iosize = 0x20;
> awc->ext_pulse_width_mask = 0xfffff;
> awc->reset_ctrl_reg = SCU_RESET_CONTROL1;
> awc->reset_pulse = aspeed_2500_wdt_reset_pulse;
> @@ -369,7 +371,7 @@ static void aspeed_2600_wdt_class_init(ObjectClass *klass, void *data)
> AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass);
>
> dc->desc = "ASPEED 2600 Watchdog Controller";
> - awc->offset = 0x40;
> + awc->iosize = 0x40;
> awc->ext_pulse_width_mask = 0xfffff; /* TODO */
> awc->reset_ctrl_reg = AST2600_SCU_RESET_CONTROL1;
> awc->reset_pulse = aspeed_2500_wdt_reset_pulse;
> @@ -392,7 +394,7 @@ static void aspeed_1030_wdt_class_init(ObjectClass *klass, void *data)
> AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass);
>
> dc->desc = "ASPEED 1030 Watchdog Controller";
> - awc->offset = 0x80;
> + awc->iosize = 0x80;
> awc->ext_pulse_width_mask = 0xfffff; /* TODO */
> awc->reset_ctrl_reg = AST2600_SCU_RESET_CONTROL1;
> awc->reset_pulse = aspeed_2500_wdt_reset_pulse;
> diff --git a/include/hw/watchdog/wdt_aspeed.h b/include/hw/watchdog/wdt_aspeed.h
> index dfa5dfa424..db91ee6b51 100644
> --- a/include/hw/watchdog/wdt_aspeed.h
> +++ b/include/hw/watchdog/wdt_aspeed.h
> @@ -40,7 +40,7 @@ struct AspeedWDTState {
> struct AspeedWDTClass {
> SysBusDeviceClass parent_class;
>
> - uint32_t offset;
> + uint32_t iosize;
> uint32_t ext_pulse_width_mask;
> uint32_t reset_ctrl_reg;
> void (*reset_pulse)(AspeedWDTState *s, uint32_t property);
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/9] hw/watchdog/wdt_aspeed: Map the whole MMIO range
2022-12-30 7:32 ` Philippe Mathieu-Daudé
@ 2023-01-02 10:34 ` Cédric Le Goater
0 siblings, 0 replies; 27+ messages in thread
From: Cédric Le Goater @ 2023-01-02 10:34 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Peter Delevoryas
Cc: qemu-devel, Steven Lee, Peter Delevoryas, qemu-arm, Cleber Rosa,
Peter Maydell, Beraldo Leal, Wainer dos Santos Moschetta,
Troy Lee, Andrew Jeffery, Joel Stanley, Peter Delevoryas,
Jamin Lin, Chin-Ting Kuo
On 12/30/22 08:32, Philippe Mathieu-Daudé wrote:
> On 29/12/22 21:42, Peter Delevoryas wrote:
>> On Thu, Dec 29, 2022 at 04:23:17PM +0100, Philippe Mathieu-Daudé wrote:
>>> Avoid confusing two different things:
>>> - the WDT I/O region size ('iosize')
>>> - at which offset the SoC map the WDT ('offset')
>>> While it is often the same, we can map smaller region sizes at
>>> larger offsets.
>>>
>>> Here we are interested in the I/O region size. Rename as 'iosize'
>>> and map the whole range, not only the first implemented registers.
>>> Unimplemented registers accesses are already logged as guest-errors.
>>>
>>> Otherwise when booting the demo in [*] we get:
>>>
>>> aspeed.io: unimplemented device write (size 4, offset 0x1851a8, value 0x030f1ff1)
>>> aspeed.io: unimplemented device write (size 4, offset 0x1851ac, value 0x03fffff1)
>>> aspeed.io: unimplemented device write (size 4, offset 0x185128, value 0x030f1ff1)
>>> aspeed.io: unimplemented device write (size 4, offset 0x18512c, value 0x03fffff1)
>>>
>>> [*] https://github.com/AspeedTech-BMC/zephyr/releases/tag/v00.01.07
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>> hw/arm/aspeed_ast10x0.c | 2 +-
>>> hw/arm/aspeed_ast2600.c | 2 +-
>>> hw/arm/aspeed_soc.c | 2 +-
>>> hw/watchdog/wdt_aspeed.c | 12 +++++++-----
>>> include/hw/watchdog/wdt_aspeed.h | 2 +-
>>> 5 files changed, 11 insertions(+), 9 deletions(-)
>
>
>>> diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
>>> index d753693a2e..c1311ce74c 100644
>>> --- a/hw/watchdog/wdt_aspeed.c
>>> +++ b/hw/watchdog/wdt_aspeed.c
>>> @@ -260,6 +260,7 @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
>>> {
>>> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>> AspeedWDTState *s = ASPEED_WDT(dev);
>>> + AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(dev);
>>> assert(s->scu);
>>> @@ -270,8 +271,9 @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
>>> */
>>> s->pclk_freq = PCLK_HZ;
>>> + assert(awc->iosize >= ASPEED_WDT_REGS_MAX * 4);
>>> memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_wdt_ops, s,
>>> - TYPE_ASPEED_WDT, ASPEED_WDT_REGS_MAX * 4);
>>> + TYPE_ASPEED_WDT, awc->iosize);
>>
>> Does this fix the unimplemented thing you referred to in the commit message?
>
> Yes, I'll reword the description to be clearer.
>
>> I'm not sure which part of this diff actually removes the unimplemented traces.
>
> Having:
>
> #define ASPEED_WDT_REGS_MAX (0x20 / 4)
>
> Before this patch, we map regions of 0x20 ...
>
>>> @@ -392,7 +394,7 @@ static void aspeed_1030_wdt_class_init(ObjectClass *klass, void *data)
>>> AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass);
>>> dc->desc = "ASPEED 1030 Watchdog Controller";
>>> - awc->offset = 0x80;
>>> + awc->iosize = 0x80;
>
> ... every span of 0x80, so there is a gap of 0x60, apparently accessed
> by the Zephyr WDT driver (address 0x28 - register #10 - is accessed in
> the traces).
>
> From the driver source we can see [1] added in [2]:
>
> #define WDT_RELOAD_VAL_REG 0x0004
> #define WDT_RESTART_REG 0x0008
> #define WDT_CTRL_REG 0x000C
> #define WDT_TIMEOUT_STATUS_REG 0x0010
> #define WDT_TIMEOUT_STATUS_CLR_REG 0x0014
> #define WDT_RESET_MASK1_REG 0x001C
> #define WDT_RESET_MASK2_REG 0x0020
> #define WDT_SW_RESET_MASK1_REG 0x0028 <------
> #define WDT_SW_RESET_MASK2_REG 0x002C
> #define WDT_SW_RESET_CTRL_REG 0x0024
>
> So the traces likely correspond to this code:
>
> static int aspeed_wdt_init(const struct device *dev)
> {
> const struct aspeed_wdt_config *config = dev->config;
> struct aspeed_wdt_data *const data = dev->data;
> uint32_t reg_val;
>
> /* disable WDT by default */
> reg_val = sys_read32(config->ctrl_base + WDT_CTRL_REG);
> reg_val &= ~WDT_CTRL_ENABLE;
> sys_write32(reg_val, config->ctrl_base + WDT_CTRL_REG);
>
> sys_write32(data->rst_mask1,
> config->ctrl_base + WDT_SW_RESET_MASK1_REG);
> sys_write32(data->rst_mask2,
> config->ctrl_base + WDT_SW_RESET_MASK2_REG);
>
> return 0;
> }
Yes. These registers have been on the TODO list for 3/4 years :/
> After this patch, we map (in this case) a MMIO region of 0x80.
> Accesses to address 0x28 hits this device model but is handled
> as 'WDT register not covered'.
If we are to change things, adding the definitions of the software mode
reset regs in the WDT model would be cleaner. All accesses to these regs
could generate an UNIMP log saying that "support for software mode reset"
is not implemented.
Thanks,
C.
>
> Better would be to extend the Aspeed WDT model in QEMU, or at
> least report the valid-but-unimplemented registers as UNIMP instead
> of GUEST_ERRORS.
>
> [1] https://github.com/AspeedTech-BMC/zephyr/blob/v00.01.08/drivers/watchdog/wdt_aspeed.c#L31
> [2] https://github.com/AspeedTech-BMC/zephyr/commit/2e99f10ac27b
>
>>> diff --git a/include/hw/watchdog/wdt_aspeed.h b/include/hw/watchdog/wdt_aspeed.h
>>> index dfa5dfa424..db91ee6b51 100644
>>> --- a/include/hw/watchdog/wdt_aspeed.h
>>> +++ b/include/hw/watchdog/wdt_aspeed.h
>>> @@ -40,7 +40,7 @@ struct AspeedWDTState {
>>> struct AspeedWDTClass {
>>> SysBusDeviceClass parent_class;
>>> - uint32_t offset;
>>> + uint32_t iosize;
>>
>> Oh yeah, iosize is a way better name for this. +1. But to be honest, I don't
>> understand how this is changing the unimplemented traces?
>>
>> Reviewed-by: Peter Delevoryas <peter@pjd.dev>
>
> Thanks!
>
> Phil.
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/9] hw/arm/aspeed_ast10x0: Map more peripherals & few more fixes
2022-12-29 15:23 [PATCH 0/9] hw/arm/aspeed_ast10x0: Map more peripherals & few more fixes Philippe Mathieu-Daudé
` (9 preceding siblings ...)
2022-12-29 20:37 ` [PATCH 0/9] hw/arm/aspeed_ast10x0: Map more peripherals & few more fixes Peter Delevoryas
@ 2023-01-02 10:55 ` Cédric Le Goater
10 siblings, 0 replies; 27+ messages in thread
From: Cédric Le Goater @ 2023-01-02 10:55 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Steven Lee, Peter Delevoryas, Peter Delevoryas, qemu-arm,
Cleber Rosa, Peter Maydell, Beraldo Leal,
Wainer dos Santos Moschetta, Troy Lee, Andrew Jeffery,
Joel Stanley, Peter Delevoryas, Jamin Lin
On 12/29/22 16:23, Philippe Mathieu-Daudé wrote:
> Trying to fix some bugs triggered running Zephyr.
>
> Still 2 bugs:
>
> 1/
> uart:~$ sensor get SYSCLK
> [00:00:23.592,000] <err> os: ***** USAGE FAULT *****
> [00:00:23.593,000] <err> os: Illegal use of the EPSR
> [00:00:23.593,000] <err> os: r0/a1: 0x00033448 r1/a2: 0x00000000 r2/a3: 0x00047f50
> [00:00:23.593,000] <err> os: r3/a4: 0x00000000 r12/ip: 0x00000000 r14/lr: 0x00000fbd
> [00:00:23.593,000] <err> os: xpsr: 0x60000000
> [00:00:23.593,000] <err> os: Faulting instruction address (r15/pc): 0x00000000
> [00:00:23.593,000] <err> os: >>> ZEPHYR FATAL ERROR 0: CPU exception on CPU 0
> [00:00:23.594,000] <err> os: Current thread: 0x38248 (shell_uart)
> [00:00:23.601,000] <err> os: Halting system
>
> 2/
> uart:~$ mcuboot
> [00:01:04.990,000] <err> os: ***** BUS FAULT *****
> [00:01:04.990,000] <err> os: Instruction bus error
> [00:01:04.991,000] <err> os: r0/a1: 0x00000000 r1/a2: 0x000ffff0 r2/a3: 0x00047ef0
> [00:01:04.991,000] <err> os: r3/a4: 0x00000010 r12/ip: 0x6df7ecb5 r14/lr: 0x000188ed
> [00:01:04.991,000] <err> os: xpsr: 0x61000000
> [00:01:04.991,000] <err> os: Faulting instruction address (r15/pc): 0x6df7ecb4
> [00:01:04.991,000] <err> os: >>> ZEPHYR FATAL ERROR 0: CPU exception on CPU 0
> [00:01:04.991,000] <err> os: Current thread: 0x38248 (shell_uart)
> [00:01:04.994,000] <err> os: Halting system
>
> ----------------
> IN:
> PMSA MPU lookup for reading at 0x0001d400 mmu_idx 65 -> Hit (prot rwx)
> 0x0001d5a2: 6869 ldr r1, [r5, #4]
> 0x0001d5a4: 4421 add r1, r4
> 0x0001d5a6: 6883 ldr r3, [r0, #8]
> 0x0001d5a8: 681c ldr r4, [r3]
> 0x0001d5aa: 463a mov r2, r7
> 0x0001d5ac: 4633 mov r3, r6
> 0x0001d5ae: 46a4 mov ip, r4
> 0x0001d5b0: e8bd 41f0 pop.w {r4, r5, r6, r7, r8, lr}
> 0x0001d5b4: 4760 bx ip
>
> PMSA MPU lookup for reading at 0x00000008 mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for execute at 0x6df7ecb4 mmu_idx 65 -> Hit (prot rwx)
> Taking exception 3 [Prefetch Abort] on CPU 0
> ...at fault address 0x6df7ecb4
> ...with CFSR.IBUSERR
> PMSA MPU lookup for writing at 0x00047ec8 mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for writing at 0x00047ecc mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for writing at 0x00047ed0 mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for writing at 0x00047ed4 mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for writing at 0x00047ed8 mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for writing at 0x00047edc mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for writing at 0x00047ee0 mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for writing at 0x00047ee4 mmu_idx 65 -> Hit (prot rwx)
> ...taking pending nonsecure exception 5
> ...loading from element 5 of non-secure vector table at 0x14
> ...loaded new PC 0xa0cd
> ----------------
>
> HACE isn't really functional there. I probably screwed smth while wiring
> the peripheral. Not obvious without access to the datasheet.
The HACE logic is quite complex and the model might be a bit fragile for some
modes. I think accumulation still has some problems.
Here are drivers for it :
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/crypto/aspeed
https://github.com/openbmc/u-boot/blob/v2019.04-aspeed-openbmc/drivers/crypto/aspeed_hace_v1.c
https://github.com/openbmc/u-boot/blob/v2019.04-aspeed-openbmc/drivers/crypto/aspeed_hace.c
Thanks,
C.
>
> Philippe Mathieu-Daudé (9):
> hw/watchdog/wdt_aspeed: Map the whole MMIO range
> hw/arm/aspeed: Use the IEC binary prefix definitions
> hw/arm/aspeed_ast10x0: Add various unimplemented peripherals
> hw/arm/aspeed_ast10x0: Map I3C peripheral
> hw/arm/aspeed_ast10x0: Map the secure SRAM
> hw/arm/aspeed_ast10x0: Map HACE peripheral
> hw/misc/aspeed_hace: Do not crash if address_space_map() failed
> hw/arm/aspeed_ast10x0: Add TODO comment to use Cortex-M4F
> tests/avocado: Test Aspeed Zephyr SDK v00.01.08 on AST1030 board
>
> hw/arm/aspeed_ast10x0.c | 84 ++++++++++++++++++++++++++++++--
> hw/arm/aspeed_ast2600.c | 5 +-
> hw/arm/aspeed_soc.c | 6 +--
> hw/misc/aspeed_hace.c | 21 +++++---
> hw/watchdog/wdt_aspeed.c | 12 +++--
> include/hw/arm/aspeed_soc.h | 14 ++++++
> include/hw/watchdog/wdt_aspeed.h | 2 +-
> tests/avocado/machine_aspeed.py | 41 +++++++++++++++-
> 8 files changed, 163 insertions(+), 22 deletions(-)
>
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2023-01-02 10:56 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-29 15:23 [PATCH 0/9] hw/arm/aspeed_ast10x0: Map more peripherals & few more fixes Philippe Mathieu-Daudé
2022-12-29 15:23 ` [PATCH 1/9] hw/watchdog/wdt_aspeed: Map the whole MMIO range Philippe Mathieu-Daudé
2022-12-29 20:42 ` Peter Delevoryas
2022-12-30 7:32 ` Philippe Mathieu-Daudé
2023-01-02 10:34 ` Cédric Le Goater
2023-01-02 10:21 ` Cédric Le Goater
2022-12-29 15:23 ` [PATCH 2/9] hw/arm/aspeed: Use the IEC binary prefix definitions Philippe Mathieu-Daudé
2022-12-29 20:42 ` Peter Delevoryas
2022-12-29 15:23 ` [PATCH 3/9] hw/arm/aspeed_ast10x0: Add various unimplemented peripherals Philippe Mathieu-Daudé
2022-12-29 20:44 ` Peter Delevoryas
2022-12-29 15:23 ` [PATCH 4/9] hw/arm/aspeed_ast10x0: Map I3C peripheral Philippe Mathieu-Daudé
2022-12-29 20:46 ` Peter Delevoryas
2022-12-30 7:07 ` Philippe Mathieu-Daudé
2022-12-29 15:23 ` [PATCH 5/9] hw/arm/aspeed_ast10x0: Map the secure SRAM Philippe Mathieu-Daudé
2022-12-29 20:50 ` Peter Delevoryas
2022-12-29 15:23 ` [PATCH 6/9] hw/arm/aspeed_ast10x0: Map HACE peripheral Philippe Mathieu-Daudé
2022-12-29 20:52 ` Peter Delevoryas
2022-12-30 8:13 ` Philippe Mathieu-Daudé
2022-12-30 20:23 ` Peter Delevoryas
2022-12-29 15:23 ` [PATCH 7/9] hw/misc/aspeed_hace: Do not crash if address_space_map() failed Philippe Mathieu-Daudé
2022-12-29 20:54 ` Peter Delevoryas
2022-12-29 15:23 ` [PATCH 8/9] hw/arm/aspeed_ast10x0: Add TODO comment to use Cortex-M4F Philippe Mathieu-Daudé
2022-12-29 20:55 ` Peter Delevoryas
2022-12-29 15:23 ` [PATCH 9/9] tests/avocado: Test Aspeed Zephyr SDK v00.01.08 on AST1030 board Philippe Mathieu-Daudé
2022-12-29 20:56 ` Peter Delevoryas
2022-12-29 20:37 ` [PATCH 0/9] hw/arm/aspeed_ast10x0: Map more peripherals & few more fixes Peter Delevoryas
2023-01-02 10:55 ` Cédric Le Goater
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.