RE: Possible race condition in aspeed ast2600 smp boot on TCG QEMU

2024-01-15 Thread Troy Lee
Hi Stephen and Cedric,

This issue haven't been found in real platform but sometime happens in
emulator, e.g. Simic. 

> Adding Aspeed Engineers. This reminds me of a discussion a while ago.
> 
> On 1/11/24 18:38, Stephen Longfield wrote:
> > We’ve noticed inconsistent behavior when running a large number of aspeed
> ast2600 executions, that seems to be tied to a race condition in the smp boot
> when executing on TCG-QEMU, and were wondering what a good mediation
> strategy might be.
> >
> > The problem first shows up as part of SMP boot. On a run that’s likely to
> later run into issues, we’ll see something like:
> >
> > ```
> > [    0.008350] smp: Bringing up secondary CPUs ...
> > [    1.168584] CPU1: failed to come online [    1.187277] smp: Brought
> > up 1 node, 1 CPU ```
> >
> > Compared to the more likely to succeed:
> >
> > ```
> > [    0.080313] smp: Bringing up secondary CPUs ...
> > [    0.093166] smp: Brought up 1 node, 2 CPUs [    0.093345] SMP:
> > Total of 2 processors activated (4800.00 BogoMIPS).
> > ```
> >
> > It’s somewhat reliably reproducible by running the ast2600-evb with an
> OpenBMC image, using ‘-icount auto’ to slow execution and make the race
> condition more frequent (it happens without this, just easier to debug if we
> can reproduce):
> >
> >
> > ```
> > ./aarch64-softmmu/qemu-system-aarch64 -machine ast2600-evb -
> nographic
> > -drive
> > file=~/bmc-bin/image-obmc-ast2600,if=mtd,bus=0,unit=0,snapshot=on -nic
> > user -icount auto ```

Have you try to run qemu with "-smp 2"?

> >
> > Our current hypothesis is that the problem comes up in the platform
> uboot.  As part of the boot, the secondary core waits for the smp mailbox to
> get a magic number written by the primary core:
> >
> > https://github.com/AspeedTech-BMC/u-boot/blob/aspeed-master-
> v2019.04/a
> > rch/arm/mach-aspeed/ast2600/platform.S#L168
> > <https://github.com/AspeedTech-BMC/u-boot/blob/aspeed-master-
> v2019.04/
> > arch/arm/mach-aspeed/ast2600/platform.S#L168>
> >
> > However, this memory address is cleared on boot:
> >
> > https://github.com/AspeedTech-BMC/u-boot/blob/aspeed-master-
> v2019.04/a
> > rch/arm/mach-aspeed/ast2600/platform.S#L146
> > <https://github.com/AspeedTech-BMC/u-boot/blob/aspeed-master-
> v2019.04/
> > arch/arm/mach-aspeed/ast2600/platform.S#L146>
> >
> > The race condition occurs if the primary core runs far ahead of the 
> > secondary
> core: if the primary core gets to the point where it signals the secondary 
> core’s
> mailbox before the secondary core gets past the point where it does the 
> initial
> reset and starts waiting, the reset will clear the signal, and then the 
> secondary
> core will never get past the point where it’s looping in
> `poll_smp_mbox_ready`.
> >
> > We’ve observed this race happening by dumping all SCU reads and writes,
> and validated that this is the problem by using a modified `platform.S` that
> doesn’t clear the =SCU_SMP_READY mailbox on reset, but would rather not
> have to use a modified version of SMP boot just for QEMU-TCG execution.

To prevent the race condition described, SCU188 zeroization is conducted
as early as possible by both CPU#0 and CPU#1. After that, there are at 
least 100 instructions for CPU#0 to execute before it get the chance to
set SCU188 to 0xbabecafe. For real, parallel HW, it is unusual that CPU#1
will be slower than CPU#0 by 100 instruction cycles.

> 
> you could use '-trace aspeed_scu*' to collect the MMIO accesses on the SCU
> unit. A TCG plugin also.
> 
> > Is there a way to have QEMU insert a barrier synchronization at some point
> in the bootloader?  I think getting both cores past the =SCU_SMP_READY reset
> would get rid of this race, but I’m not aware of a way to do that kind of 
> thing
> in QEMU-TCG.
> >
> > Thanks for any insights!
> 
> Could we change the default value to registers 0x180 ... 0x18C in
> hw/misc/aspeed_scu.c to make sure the SMP regs are immune to the race ?
> 
> Thanks,
> 
> C.

Thanks,
Troy Lee


RE: [PATCH v2 03/10] contrib/gitdm: Add ASPEED Technology to the domain map

2023-03-13 Thread Troy Lee
Hi Alex,

> -Original Message-
> From: Alex Bennée 
> Sent: Saturday, March 11, 2023 2:03 AM
> To: qemu-devel@nongnu.org
> Cc: Alex Bennée ; Steven Lee
> ; Troy Lee ;
> Howard Chiu ; Jamin Lin
> 
> Subject: [PATCH v2 03/10] contrib/gitdm: Add ASPEED Technology to the
> domain map
> 
> We have a number of contributors from this domain which looks like it is a
> corporate endeavour.
> 
> Signed-off-by: Alex Bennée 
> Cc: Steven Lee 
> Cc: Troy Lee 
> Cc: Howard Chiu 
> Cc: Jamin Lin 

Reviewed-by: Troy Lee 

> ---
>  contrib/gitdm/domain-map | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/contrib/gitdm/domain-map b/contrib/gitdm/domain-map index
> 7a8077e241..bd989d065c 100644
> --- a/contrib/gitdm/domain-map
> +++ b/contrib/gitdm/domain-map
> @@ -5,6 +5,7 @@
>  #
> 
>  amd.com AMD
> +aspeedtech.com  ASPEED Technology Inc.
>  baidu.com   Baidu
>  bytedance.com   ByteDance
>  cmss.chinamobile.com China Mobile
> --
> 2.39.2



RE: [PATCH v5 7/9] aspeed/soc : Add AST1030 support

2023-02-23 Thread Troy Lee
Hi Philippe,

> -Original Message-
> From: Philippe Mathieu-Daudé 
> Sent: Thursday, February 23, 2023 6:44 PM
> To: Jamin Lin ; Cédric Le Goater ;
> Peter Maydell ; Andrew Jeffery
> ; Joel Stanley ; Alistair Francis
> ; Cleber Rosa ; Wainer dos
> Santos Moschetta ; Beraldo Leal
> ; open list:ASPEED BMCs ;
> open list:All patches CC here 
> Cc: Steven Lee ; Troy Lee
> 
> Subject: Re: [PATCH v5 7/9] aspeed/soc : Add AST1030 support
> 
> ping
> 
> On 29/12/22 12:16, Philippe Mathieu-Daudé wrote:
> > Hi,
> >
> > On 1/4/22 10:38, Jamin Lin wrote:
> >> From: Steven Lee 
> >>
> >> The embedded core of AST1030 SoC is ARM Coretex M4.
> >> It is hard to be integrated in the common Aspeed Soc framework.
> >> We introduce a new ast1030 class with instance_init and realize
> >> handlers.
> >>
> >> Signed-off-by: Troy Lee 
> >> Signed-off-by: Jamin Lin 
> >> Signed-off-by: Steven Lee 
> >> Reviewed-by: Cédric Le Goater 
> >> ---
> >>   hw/arm/aspeed_ast10xx.c | 299
> >> 
> >>   hw/arm/meson.build  |   6 +-
> >>   include/hw/arm/aspeed_soc.h |   3 +
> >>   3 files changed, 307 insertions(+), 1 deletion(-)
> >>   create mode 100644 hw/arm/aspeed_ast10xx.c
> >
> >
> >> +static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error
> >> **errp)
> >> +{
> >> +    AspeedSoCState *s = ASPEED_SOC(dev_soc);
> >> +    AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
> >> +    MemoryRegion *system_memory = get_system_memory();
> >> +    DeviceState *armv7m;
> >> +    Error *err = NULL;
> >> +    int i;
> >> +
> >> +    if (!clock_has_source(s->sysclk)) {
> >> +    error_setg(errp, "sysclk clock must be wired up by the board
> >> code");
> >> +    return;
> >> +    }
> >> +
> >> +    /* General I/O memory space to catch all unimplemented device */
> >> +    create_unimplemented_device("aspeed.sbc",
> >> +    sc->memmap[ASPEED_DEV_SBC],
> >> +    0x4);
> >> +    create_unimplemented_device("aspeed.io",
> >> +    sc->memmap[ASPEED_DEV_IOMEM],
> >> +    ASPEED_SOC_IOMEM_SIZE);
> >> +
> >> +    /* AST1030 CPU Core */
> >> +    armv7m = DEVICE(>armv7m);
> >> +    qdev_prop_set_uint32(armv7m, "num-irq", 256);
> >
> > Can you confirm this SoC has 256 and not 240 IRQs?
> >
The interrupt number start from 0 to 148, and some of them are reserved. 
Should we send an update to seth num-irq to 149?

Thanks,
Troy Lee

> >> +    qdev_prop_set_string(armv7m, "cpu-type", sc->cpu_type);
> >> +    qdev_connect_clock_in(armv7m, "cpuclk", s->sysclk);
> >> +    object_property_set_link(OBJECT(>armv7m), "memory",
> >> + OBJECT(system_memory), _abort);
> >> +    sysbus_realize(SYS_BUS_DEVICE(>armv7m), _abort);
> >



RE: [RFC 0/1] i2c/aspeed: Add slave device handling in new register mode

2022-06-01 Thread Troy Lee
Hi Cedric,

> -Original Message-
> From: Cédric Le Goater 
> Sent: Wednesday, June 1, 2022 3:10 PM
> To: Peter Delevoryas 
> Cc: qemu-...@nongnu.org; qemu-devel@nongnu.org; zhdan...@fb.com; Troy
> Lee ; Jamin Lin ;
> Steven Lee ; k.jen...@samsung.com; Joe
> Komlodi ; Joel Stanley ; Andrew
> Jeffery 
> Subject: Re: [RFC 0/1] i2c/aspeed: Add slave device handling in new register
> mode
> 
> [ Adding Joe ]
> 
> On 5/25/22 22:50, Peter Delevoryas wrote:
> > The AST2600/AST1030 new register mode patches[1] and the I2C slave
> > device patches[2] will be really useful, but we still need DMA slave
> > device handling in the new register mode too for the use-cases I'm
> > thinking of (OpenBIC Zephyr kernel using Aspeed SDK drivers[3]).
> >
> > My test images are on Github[4]. They can be used with the
> > ast1030-evb, or the oby35-cl and oby35-bb machines in the fb qemu
> branch[5].
> >
> > I'm submitting this as an RFC cause I just want to see how other
> > people expect these changes to be made based on the previously
> > submitted "new register mode" and "old register mode slave device" patches.
> 
> 
> Currently, my preferred approach would be to start with Joe's patchset because
> the registerfields conversion is a huge effort and it's adding new mode 
> support
> which should cover the needs for the AST1030 SoC [1].
> 
> Troy, could you please confirm this is OK with you ? I have pushed the patches
> on :
> 
>https://github.com/legoater/qemu/commits/aspeed-7.1
> 

Yes, I'm ok with this. We have tested Joe's patch set as well.

> Then, adding slave support for old [2] and new mode (this patch) shouldn't be
> too much of a problem since they are small.
> 

I'm looking forward to have slave device support, with that we could have more 
firmware test case in QEMU.

Thanks,
Troy Lee

> we lack a test case for this controller and writing a I2C Aspeed bus driver 
> for
> qtest is not an easy task.
> 
> It might be easier to start an ast2600-evb machine with a lightweight 
> userspace
> (buildroot, I can host that somewhere on GH) and run some I2C get/set/detect
> commands from a python/expect framework, like avocado.
> I2C devices can be added on the command line for the purpose.
> 
> 
> Thanks,
> 
> C.
> 
> 
> > Thanks,
> > Peter
> >
> > [1]
> > https://patchwork.kernel.org/project/qemu-devel/list/?series=626028
> > chive=both [2]
> > https://patchwork.kernel.org/project/qemu-devel/list/?series=627914
> > chive=both [3]
> > https://github.com/AspeedTech-
> BMC/zephyr/blob/db3dbcc9c52e67a47180890a
> > c938ed380b33f91c/drivers/i2c/i2c_aspeed.c#L1362-L1368
> > [4]
> > https://github.com/peterdelevoryas/OpenBIC/releases/tag/oby35-cl-2022.
> > 13.01 [5] https://github.com/facebook/openbmc-qemu
> >
> > Peter Delevoryas (1):
> >i2c/aspeed: Add slave device handling in new register mode
> >
> >   hw/i2c/aspeed_i2c.c | 118 ++-
> -
> >   include/hw/i2c/aspeed_i2c.h |  14 +++--
> >   2 files changed, 124 insertions(+), 8 deletions(-)
> >



[PATCH v1 1/2] aspeed/i2c: Add new register mode for ast2600/1030

2022-03-24 Thread Troy Lee
AST2600/1030 provides a new register mode which is controlled by
I2CG0C[2]. If the I2CG0C[2] = 1, then I2C will switch to a new set of
register.

This commit supports new register mode with packet operation and DMA
enabled. Byte/buffer mode is not implemented.

Signed-off-by: Troy Lee 
Signed-off-by: Jamin Lin 
Signed-off-by: Steven Lee 
---
 hw/i2c/aspeed_i2c.c | 490 ++--
 hw/i2c/trace-events |   7 +-
 include/hw/i2c/aspeed_i2c.h |   8 +
 3 files changed, 487 insertions(+), 18 deletions(-)

diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
index 03a4f5a910..f571c8bbca 100644
--- a/hw/i2c/aspeed_i2c.c
+++ b/hw/i2c/aspeed_i2c.c
@@ -37,6 +37,8 @@
Assignment */
 #define I2C_CTRL_GLOBAL 0x0C/* Global Control Register */
 #define   I2C_CTRL_SRAM_EN BIT(0)
+#define   I2C_CTRL_NEW_REG_MODEBIT(2)
+#define I2C_NEW_DIV_GLOBAL  0x10
 
 /* I2C Device (Bus) Register */
 
@@ -145,6 +147,102 @@
 #define I2CD_DMA_ADDR   0x24   /* DMA Buffer Address */
 #define I2CD_DMA_LEN0x28   /* DMA Transfer Length < 4KB */
 
+/* New register mode */
+#define I2CC_M_S_FUNC_CTRL_REG  0x00
+#define   I2CC_SLAVE_ADDR_RX_EN BIT(20)
+#define   I2CC_MASTER_RETRY_MASK(0x3 << 18)
+#define   I2CC_MASTER_RETRY(x)  ((x & 0x3) << 18)
+#define   I2CC_BUS_AUTO_RELEASE BIT(17)
+#define   I2CC_M_SDA_LOCK_ENBIT(16)
+#define   I2CC_MULTI_MASTER_DIS BIT(15)
+#define   I2CC_M_SCL_DRIVE_EN   BIT(14)
+#define   I2CC_MSB_STS  BIT(9)
+#define   I2CC_SDA_DRIVE_1T_EN  BIT(8)
+#define   I2CC_M_SDA_DRIVE_1T_ENBIT(7)
+#define   I2CC_M_HIGH_SPEED_EN  BIT(6)
+#define   I2CC_SLAVE_EN BIT(1)
+#define   I2CC_MASTER_ENBIT(0)
+
+#define I2CC_M_S_CLK_AC_TIMING_REG 0x04
+#define I2CC_M_S_TX_RX_BUF_REG  0x08
+#define I2CC_M_X_POOL_BUF_CTRL_REG 0x0c
+#define I2CM_INT_CTRL_REG 0x10
+#define I2CM_INT_STS_REG  0x14
+#define   I2CM_PKT_OP_SM_SHIFT  28
+#define   I2CM_PKT_OP_SM_IDLE   0x0
+#define   I2CM_PKT_OP_SM_STARTH 0x1
+#define   I2CM_PKT_OP_SM_STARTW 0x2
+#define   I2CM_PKT_OP_SM_STARTR 0x3
+#define   I2CM_PKT_OP_SM_TXMCODE0x4
+#define   I2CM_PKT_OP_SM_TXAW   0x5
+#define   I2CM_PKT_OP_SM_INIT   0x8
+#define   I2CM_PKT_OP_SM_TXD0x9
+#define   I2CM_PKT_OP_SM_RXD0xa
+#define   I2CM_PKT_OP_SM_STOP   0xb
+#define   I2CM_PKT_OP_SM_RETRY  0xc
+#define   I2CM_PKT_OP_SM_FAIL   0xd
+#define   I2CM_PKT_OP_SM_WAIT   0xe
+#define   I2CM_PKT_OP_SM_PASS   0xf
+#define   I2CM_PKT_TIMEOUT  BIT(18)
+#define   I2CM_PKT_ERRORBIT(17)
+#define   I2CM_PKT_DONE BIT(16)
+#define   I2CM_BUS_RECOVER_FAIL BIT(15)
+#define   I2CM_SDA_DL_TOBIT(14)
+#define   I2CM_BUS_RECOVER  BIT(13)
+#define   I2CM_SMBUS_ALTBIT(12)
+#define   I2CM_SCL_LOW_TO   BIT(6)
+#define   I2CM_ABNORMAL BIT(5)
+#define   I2CM_NORMAL_STOP  BIT(4)
+#define   I2CM_ARBIT_LOSS   BIT(3)
+#define   I2CM_RX_DONE  BIT(2)
+#define   I2CM_TX_NAK   BIT(1)
+#define   I2CM_TX_ACK   BIT(0)
+#define I2CM_CMD_STS_REG  0x18
+#define   I2CM_CMD_PKT_MODE   (1 << 16)
+
+#define   I2CM_PKT_EN   BIT(16)
+#define   I2CM_SDA_OE_OUT_DIR   BIT(15)
+#define   I2CM_SDA_O_OUT_DIRBIT(14)
+#define   I2CM_SCL_OE_OUT_DIR   BIT(13)
+#define   I2CM_SCL_O_OUT_DIRBIT(12)
+#define   I2CM_RECOVER_CMD_EN   BIT(11)
+#define   I2CM_RX_DMA_ENBIT(9)
+#define   I2CM_TX_DMA_ENBIT(8)
+/* Command Bit */
+#define   I2CM_RX_BUFF_EN   BIT(7)
+#define   I2CM_TX_BUFF_EN   BIT(6)
+#define   I2CM_STOP_CMD BIT(5)
+#define   I2CM_RX_CMD_LAST  BIT(4)
+#define   I2CM_RX_CMD   BIT(3)
+#define   I2CM_TX_CMD   BIT(1)
+#define   I2CM_START_CMDBIT(0)
+#define   I2CM_PKT_ADDR(x)  ((x & 0x7f) << 24)
+
+#define I2CM_DMA_LEN  0x1c
+#define I2CS_INT_CTRL_REG 0x20
+#define I2CS_INT_STS_REG  0x24
+#define I2CS_CMD_STS_REG  0x28
+#define I2CS_DMA_LEN  0x2c
+#define I2CM_DMA_TX_BUF   0x30
+#define I2CM_DMA_RX_BUF   0x34
+#define I2CS_DMA_TX_BUF   0x38
+#define I2CS_DMA_RX_BUF   0x3c
+#define I2CS_SA_REG   0x40
+#define I2CM_DMA_LEN_STS_REG  0x48
+#define I2CS_DMA_LEN_STS_REG  0x4c
+#define I2CC_DMA_OP_ADDR_REG  0x50
+#define I2CC_DMA_OP_LEN_REG   0x54
+
+static inline bool aspeed_i2c_ctrl_is_new_mode(AspeedI2CState *controller)
+{
+return (controller->ctrl_global & I2C_CTRL_NEW_REG_MODE) == 
I2C_CTRL_NEW_REG_MODE;
+}
+
+static inline bool aspeed_i2c_bus_is_new_mode(AspeedI2CBus *bus)
+{
+return aspeed_i2c_ctrl_is_new_mode(bus->controller);
+}
+
 static inline bool aspeed_i2c_bu

[PATCH v1 2/2] aspeed: Add I2C buses to AST1030 model

2022-03-24 Thread Troy Lee
Instanitate the I2C buses in AST1030 model and create two slave device
for ast1030-evb.

Signed-off-by: Troy Lee 
Signed-off-by: Jamin Lin 
Signed-off-by: Steven Lee 
---
 hw/arm/aspeed_ast1030.c | 17 +
 hw/arm/aspeed_minibmc.c | 13 +
 2 files changed, 30 insertions(+)

diff --git a/hw/arm/aspeed_ast1030.c b/hw/arm/aspeed_ast1030.c
index fe700d922f..c16bcba7c9 100644
--- a/hw/arm/aspeed_ast1030.c
+++ b/hw/arm/aspeed_ast1030.c
@@ -92,6 +92,9 @@ static void aspeed_soc_ast1030_init(Object *obj)
 object_property_add_alias(obj, "hw-strap1", OBJECT(>scu), "hw-strap1");
 object_property_add_alias(obj, "hw-strap2", OBJECT(>scu), "hw-strap2");
 
+snprintf(typename, sizeof(typename), "aspeed.i2c-%s", socname);
+object_initialize_child(obj, "i2c", >i2c, typename);
+
 snprintf(typename, sizeof(typename), "aspeed.timer-%s", socname);
 object_initialize_child(obj, "timerctrl", >timerctrl, typename);
 
@@ -163,6 +166,20 @@ static void aspeed_soc_ast1030_realize(DeviceState 
*dev_soc, Error **errp)
 }
 sysbus_mmio_map(SYS_BUS_DEVICE(>scu), 0, sc->memmap[ASPEED_DEV_SCU]);
 
+/* I2C */
+object_property_set_link(OBJECT(>i2c), "dram", OBJECT(>sram),
+ _abort);
+if (!sysbus_realize(SYS_BUS_DEVICE(>i2c), errp)) {
+return;
+}
+sysbus_mmio_map(SYS_BUS_DEVICE(>i2c), 0, sc->memmap[ASPEED_DEV_I2C]);
+for (i = 0; i < ASPEED_I2C_GET_CLASS(>i2c)->num_busses; i++) {
+qemu_irq irq = qdev_get_gpio_in(DEVICE(>armv7m),
+sc->irqmap[ASPEED_DEV_I2C] + i);
+/* The AST2600 I2C controller has one IRQ per bus. */
+sysbus_connect_irq(SYS_BUS_DEVICE(>i2c.busses[i]), 0, irq);
+}
+
 /* LPC */
 if (!sysbus_realize(SYS_BUS_DEVICE(>lpc), errp)) {
 return;
diff --git a/hw/arm/aspeed_minibmc.c b/hw/arm/aspeed_minibmc.c
index 6a29475919..764df92f65 100644
--- a/hw/arm/aspeed_minibmc.c
+++ b/hw/arm/aspeed_minibmc.c
@@ -37,6 +37,18 @@ struct AspeedMiniBmcMachineState {
 /* Main SYSCLK frequency in Hz (200MHz) */
 #define SYSCLK_FRQ 2ULL
 
+static void ast1030_evb_i2c_init(AspeedMiniBmcMachineState *bmc)
+{
+AspeedSoCState *soc = >soc;
+
+/* U10 24C08 connects to SDA/SCL Groupt 1 by default */
+uint8_t *eeprom_buf = g_malloc0(32 * 1024);
+smbus_eeprom_init_one(aspeed_i2c_get_bus(>i2c, 0), 0x50, eeprom_buf);
+
+/* U11 LM75 connects to SDA/SCL Group 2 by default */
+i2c_slave_create_simple(aspeed_i2c_get_bus(>i2c, 1), "tmp105", 0x4d);
+}
+
 static void aspeed_minibmc_machine_ast1030_evb_class_init(ObjectClass *oc,
   void *data)
 {
@@ -47,6 +59,7 @@ static void 
aspeed_minibmc_machine_ast1030_evb_class_init(ObjectClass *oc,
 amc->soc_name = "ast1030-a1";
 amc->hw_strap1 = 0;
 amc->hw_strap2 = 0;
+amc->i2c_init = ast1030_evb_i2c_init;
 mc->default_ram_size = 0;
 mc->default_cpus = mc->min_cpus = mc->max_cpus = 1;
 amc->fmc_model = "sst25vf032b";
-- 
2.25.1




Re: [PATCH v2 1/2] hw/misc: Supporting AST2600 HACE accumulative mode

2022-02-09 Thread Troy Lee
Hi Joel,

On Tue, Feb 8, 2022 at 6:46 PM Joel Stanley  wrote:
>
> Hello Troy,
>
> On Wed, 12 Jan 2022 at 08:10, Troy Lee  wrote:
> >
> > Accumulative mode will supply a initial state and append padding bit at
> > the end of hash stream.  However, the crypto library will padding those
> > bit automatically, so ripped it off from iov array.
> >
> > The aspeed ast2600 acculumative mode is described in datasheet
> > ast2600v10.pdf section 25.6.4:
> >  1. Allocationg and initiating accumulative hash digest write buffer
> > with initial state.
> > * Since QEMU crypto/hash api doesn't provide the API to set initial
> >   state of hash library, and the initial state is already setted by
> >   crypto library (gcrypt/glib/...), so skip this step.
> >  2. Calculating accumulative hash digest.
> > (a) When receiving the last accumulative data, software need to add
> > padding message at the end of the accumulative data. Padding
> > message described in specific of MD5, SHA-1, SHA224, SHA256,
> > SHA512, SHA512/224, SHA512/256.
> > * Since the crypto library (gcrypt/glib) already pad the
> >   padding message internally.
> > * This patch is to remove the padding message which fed byguest
> >   machine driver.
>
>
> I tested the latest aspeed SDK u-boot, loaded form mmc (with our mmc
> model that lives in Cedric's tree) and qemu crashed:
>
> #0  0x7fe867d44932 in ?? () from 
> /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
> #1  0x557aba2b6e22 in qcrypto_glib_hash_bytesv (alg= out>, iov=0x7fe8662ee0b0, niov=1, result=0x7fe8662ee0a8,
> resultlen=0x7fe8662ee0a0, errp=0x0) at ../crypto/hash-glib.c:68
> #2  0x557ab9f549ea in do_hash_operation (s=s@entry=0x7fe866e1b3b0,
> algo=5, sg_mode=sg_mode@entry=true, acc_mode=acc_mode@entry=true) at
> ../hw/misc/aspeed_hace.c:161
> #3  0x557ab9f54dd1 in aspeed_hace_write (opaque=,
> addr=12, data=262504, size=) at
> ../hw/misc/aspeed_hace.c:260
>
> WIthout your patch applied the HACE operation fails, as we do not have
> support for accumulative mode, but we do not crash.

I'll double check on this issue.

> >
> > Changes in v2:
> > - Coding style
> > - Add accumulative mode description in comment
> >
> > Signed-off-by: Troy Lee 
> > ---
> >  hw/misc/aspeed_hace.c | 43 ---
> >  include/hw/misc/aspeed_hace.h |  1 +
> >  2 files changed, 36 insertions(+), 8 deletions(-)
> >
> > diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
> > index 10f00e65f4..0710f44621 100644
> > --- a/hw/misc/aspeed_hace.c
> > +++ b/hw/misc/aspeed_hace.c
> > @@ -11,6 +11,7 @@
> >  #include "qemu/osdep.h"
> >  #include "qemu/log.h"
> >  #include "qemu/error-report.h"
> > +#include "qemu/bswap.h"
> >  #include "hw/misc/aspeed_hace.h"
> >  #include "qapi/error.h"
> >  #include "migration/vmstate.h"
> > @@ -27,6 +28,7 @@
> >
> >  #define R_HASH_SRC  (0x20 / 4)
> >  #define R_HASH_DEST (0x24 / 4)
> > +#define R_HASH_KEY_BUFF (0x28 / 4)
> >  #define R_HASH_SRC_LEN  (0x2c / 4)
> >
> >  #define R_HASH_CMD  (0x30 / 4)
> > @@ -94,7 +96,8 @@ static int hash_algo_lookup(uint32_t reg)
> >  return -1;
> >  }
> >
> > -static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode)
> > +static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
> > +  bool acc_mode)
> >  {
> >  struct iovec iov[ASPEED_HACE_MAX_SG];
> >  g_autofree uint8_t *digest_buf;
> > @@ -103,6 +106,7 @@ static void do_hash_operation(AspeedHACEState *s, int 
> > algo, bool sg_mode)
> >
> >  if (sg_mode) {
> >  uint32_t len = 0;
> > +uint32_t total_len = 0;
> >
> >  for (i = 0; !(len & SG_LIST_LEN_LAST); i++) {
> >  uint32_t addr, src;
> > @@ -123,10 +127,26 @@ static void do_hash_operation(AspeedHACEState *s, int 
> > algo, bool sg_mode)
> >  MEMTXATTRS_UNSPECIFIED, NULL);
> >  addr &= SG_LIST_ADDR_MASK;
> >
> > -iov[i].iov_len = len & SG_LIST_LEN_MASK;
> > -plen = iov[i].iov_len;
> > +plen = len & SG_LIST_LEN_MASK;
> >  iov[i].iov_base = address_space_map(>dram_as, addr, , 
> > false,
> >  MEMTXATTRS_UNSPECIFIED);
> 

Re: [PATCH v3 0/2] Aspeed I3C device model

2022-01-12 Thread Troy Lee
On Wed, Jan 12, 2022 at 6:57 PM Graeme Gregory
 wrote:
>
> On Tue, Jan 11, 2022 at 04:45:44PM +0800, Troy Lee wrote:
> > This series of patch introduce a dummy implemenation of aspeed i3c
> > model, and it provide just enough information for guest machine.
> > However, the driver probing is still failed, but it will not cause
> > kernel panic.
> >
>
> These patches arrived just in time for our i3c testing. This stops
> our CI halting due to kernel panic on i3c probing.
>
> Reviewed-by: Graeme Gregory 
> Tested-by: Graeme Gregory 
>

I'm glad it was able to help.

Thanks,
Troy Lee

> > v3:
> > - Remove unused AspeedI3CClass
> > - Refine memory region
> > - Refine register reset
> > - Remove unrelated changes to SPI2 address
> > - Remove i3c controller irq line
> >
> > v2:
> > - Split i3c model into i3c and i3c_device
> > - Create 6x i3c devices
> > - Using register fields macro
> > - Rebase to mainline QEMU
> >
> > Troy Lee (2):
> >   Introduce a dummy AST2600 I3C model.
> >   This patch includes i3c instance in ast2600 soc.
> >
> >  hw/arm/aspeed_ast2600.c  |  16 ++
> >  hw/misc/aspeed_i3c.c | 381 +++
> >  hw/misc/meson.build  |   1 +
> >  hw/misc/trace-events |   6 +
> >  include/hw/arm/aspeed_soc.h  |   3 +
> >  include/hw/misc/aspeed_i3c.h |  48 +
> >  6 files changed, 455 insertions(+)
> >  create mode 100644 hw/misc/aspeed_i3c.c
> >  create mode 100644 include/hw/misc/aspeed_i3c.h
> >
> > --
> > 2.25.1
> >
> >



[PATCH v2 1/2] hw/misc: Supporting AST2600 HACE accumulative mode

2022-01-12 Thread Troy Lee
Accumulative mode will supply a initial state and append padding bit at
the end of hash stream.  However, the crypto library will padding those
bit automatically, so ripped it off from iov array.

The aspeed ast2600 acculumative mode is described in datasheet
ast2600v10.pdf section 25.6.4:
 1. Allocationg and initiating accumulative hash digest write buffer
with initial state.
* Since QEMU crypto/hash api doesn't provide the API to set initial
  state of hash library, and the initial state is already setted by
  crypto library (gcrypt/glib/...), so skip this step.
 2. Calculating accumulative hash digest.
(a) When receiving the last accumulative data, software need to add
padding message at the end of the accumulative data. Padding
message described in specific of MD5, SHA-1, SHA224, SHA256,
SHA512, SHA512/224, SHA512/256.
* Since the crypto library (gcrypt/glib) already pad the
  padding message internally.
* This patch is to remove the padding message which fed byguest
  machine driver.

Changes in v2:
- Coding style
- Add accumulative mode description in comment

Signed-off-by: Troy Lee 
---
 hw/misc/aspeed_hace.c | 43 ---
 include/hw/misc/aspeed_hace.h |  1 +
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 10f00e65f4..0710f44621 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -11,6 +11,7 @@
 #include "qemu/osdep.h"
 #include "qemu/log.h"
 #include "qemu/error-report.h"
+#include "qemu/bswap.h"
 #include "hw/misc/aspeed_hace.h"
 #include "qapi/error.h"
 #include "migration/vmstate.h"
@@ -27,6 +28,7 @@
 
 #define R_HASH_SRC  (0x20 / 4)
 #define R_HASH_DEST (0x24 / 4)
+#define R_HASH_KEY_BUFF (0x28 / 4)
 #define R_HASH_SRC_LEN  (0x2c / 4)
 
 #define R_HASH_CMD  (0x30 / 4)
@@ -94,7 +96,8 @@ static int hash_algo_lookup(uint32_t reg)
 return -1;
 }
 
-static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode)
+static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
+  bool acc_mode)
 {
 struct iovec iov[ASPEED_HACE_MAX_SG];
 g_autofree uint8_t *digest_buf;
@@ -103,6 +106,7 @@ static void do_hash_operation(AspeedHACEState *s, int algo, 
bool sg_mode)
 
 if (sg_mode) {
 uint32_t len = 0;
+uint32_t total_len = 0;
 
 for (i = 0; !(len & SG_LIST_LEN_LAST); i++) {
 uint32_t addr, src;
@@ -123,10 +127,26 @@ static void do_hash_operation(AspeedHACEState *s, int 
algo, bool sg_mode)
 MEMTXATTRS_UNSPECIFIED, NULL);
 addr &= SG_LIST_ADDR_MASK;
 
-iov[i].iov_len = len & SG_LIST_LEN_MASK;
-plen = iov[i].iov_len;
+plen = len & SG_LIST_LEN_MASK;
 iov[i].iov_base = address_space_map(>dram_as, addr, , 
false,
 MEMTXATTRS_UNSPECIFIED);
+
+if (acc_mode) {
+total_len += plen;
+
+if (len & SG_LIST_LEN_LAST) {
+/*
+ * In the padding message, the last 64/128 bit represents
+ * the total length of bitstream in big endian.
+ * SHA-224, SHA-256 are 64 bit
+ * SHA-384, SHA-512, SHA-512/224, SHA-512/256 are 128 bit
+ * However, we would not process such a huge bit stream.
+ */
+plen -= total_len - (ldq_be_p(iov[i].iov_base + plen - 8) 
/ 8);
+}
+}
+
+iov[i].iov_len = plen;
 }
 } else {
 hwaddr len = s->regs[R_HASH_SRC_LEN];
@@ -210,6 +230,9 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, 
uint64_t data,
 case R_HASH_DEST:
 data &= ahc->dest_mask;
 break;
+case R_HASH_KEY_BUFF:
+data &= ahc->key_mask;
+break;
 case R_HASH_SRC_LEN:
 data &= 0x0FFF;
 break;
@@ -229,12 +252,13 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, 
uint64_t data,
 }
 algo = hash_algo_lookup(data);
 if (algo < 0) {
-qemu_log_mask(LOG_GUEST_ERROR,
-"%s: Invalid hash algorithm selection 0x%"PRIx64"\n",
-__func__, data & ahc->hash_mask);
-break;
+qemu_log_mask(LOG_GUEST_ERROR,
+"%s: Invalid hash algorithm selection 0x%"PRIx64"\n",
+__func__, data & ahc->hash_mask);
+break;
 }
-do_hash_operation(s, algo, data & HASH_SG_EN);
+do_hash_operation(s, algo, data & HASH

[PATCH v2 0/2] Supporting AST2600 HACE engine accumulative mode

2022-01-12 Thread Troy Lee
This patch series implements ast2600 hace engine with accumulative mode
and unit test against to it.

Changes in v2:
- Coding style
- Add accumulative mode description in comment
- Add unit test cases

Troy Lee (2):
  hw/misc: Supporting AST2600 HACE accumulative mode
  tests/qtest: Add test for Aspeed HACE accumulative mode

 hw/misc/aspeed_hace.c  |  43 --
 include/hw/misc/aspeed_hace.h  |   1 +
 tests/qtest/aspeed_hace-test.c | 145 +
 3 files changed, 181 insertions(+), 8 deletions(-)

-- 
2.25.1




[PATCH v2 2/2] tests/qtest: Add test for Aspeed HACE accumulative mode

2022-01-12 Thread Troy Lee
This add two addition test cases for accumulative mode under sg enabled.

The input vector was manually craft with "abc" + bit 1 + padding zeros + L.
The padding length depends on algorithm, i.e. SHA512 (1024 bit),
SHA256 (512 bit).

The result was calculated by command line sha512sum/sha256sum utilities
without padding, i.e. only "abc" ascii text.

Signed-off-by: Troy Lee 
---
 tests/qtest/aspeed_hace-test.c | 145 +
 1 file changed, 145 insertions(+)

diff --git a/tests/qtest/aspeed_hace-test.c b/tests/qtest/aspeed_hace-test.c
index 09ee31545e..6a2f404b93 100644
--- a/tests/qtest/aspeed_hace-test.c
+++ b/tests/qtest/aspeed_hace-test.c
@@ -21,6 +21,7 @@
 #define  HACE_ALGO_SHA512(BIT(5) | BIT(6))
 #define  HACE_ALGO_SHA384(BIT(5) | BIT(6) | BIT(10))
 #define  HACE_SG_EN  BIT(18)
+#define  HACE_ACCUM_EN   BIT(8)
 
 #define HACE_STS 0x1c
 #define  HACE_RSA_ISRBIT(13)
@@ -96,6 +97,57 @@ static const uint8_t test_result_sg_sha256[] = {
 0x55, 0x1e, 0x1e, 0xc5, 0x80, 0xdd, 0x6d, 0x5a, 0x6e, 0xcd, 0xe9, 0xf3,
 0xd3, 0x5e, 0x6e, 0x4a, 0x71, 0x7f, 0xbd, 0xe4};
 
+/*
+ * The accumulative mode requires firmware to provide internal initial state
+ * and message padding (including length L at the end of padding).
+ *
+ * This test vector is a ascii text "abc" with padding message.
+ *
+ * Expected results were generated using command line utitiles:
+ *
+ *  echo -n -e 'abc' | dd of=/tmp/test
+ *  for hash in sha512sum sha256sum; do $hash /tmp/test; done
+ */
+static const uint8_t test_vector_accum_512[] = {
+0x61, 0x62, 0x63, 0x80, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18};
+
+static const uint8_t test_vector_accum_256[] = {
+0x61, 0x62, 0x63, 0x80, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18};
+
+static const uint8_t test_result_accum_sha512[] = {
+0xdd, 0xaf, 0x35, 0xa1, 0x93, 0x61, 0x7a, 0xba, 0xcc, 0x41, 0x73, 0x49,
+0xae, 0x20, 0x41, 0x31, 0x12, 0xe6, 0xfa, 0x4e, 0x89, 0xa9, 0x7e, 0xa2,
+0x0a, 0x9e, 0xee, 0xe6, 0x4b, 0x55, 0xd3, 0x9a, 0x21, 0x92, 0x99, 0x2a,
+0x27, 0x4f, 0xc1, 0xa8, 0x36, 0xba, 0x3c, 0x23, 0xa3, 0xfe, 0xeb, 0xbd,
+0x45, 0x4d, 0x44, 0x23, 0x64, 0x3c, 0xe8, 0x0e, 0x2a, 0x9a, 0xc9, 0x4f,
+0xa5, 0x4c, 0xa4, 0x9f};
+
+static const uint8_t test_result_accum_sha256[] = {
+0xba, 0x78, 0x16, 0xbf, 0x8f, 0x01, 0xcf, 0xea, 0x41, 0x41, 0x40, 0xde,
+0x5d, 0xae, 0x22, 0x23, 0xb0, 0x03, 0x61, 0xa3, 0x96, 0x17, 0x7a, 0x9c,
+0xb4, 0x10, 0xff, 0x61, 0xf2, 0x00, 0x15, 0xad};
 
 static void write_regs(QTestState *s, uint32_t base, uint32_t src,
uint32_t length, uint32_t out, uint32_t method)
@@ -308,6 +360,86 @@ static void test_sha512_sg(const char *machine, const 
uint32_t base,
 qtest_quit(s);
 }
 
+static void test_sha256_accum(const char *machine, const uint32_t base,
+const uint32_t src_addr)
+{
+QTestState *s = qtest_init(machine);
+
+const uint32_t buffer_addr = src_addr + 0x100;
+const uint32_t digest_addr = src_addr + 0x400;
+uint8_t digest[32] = {0};
+struct AspeedSgList array[] = {
+{  cpu_to_le32(sizeof(test_vector_accum_256) | SG_LIST_LEN_LAST),
+   cpu_to_le32(buffer_addr) },
+};
+
+/* Check engine is idle, no busy or irq bits set */
+g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+/* Write test vector into memory */
+qtest_memwrite(s, buffer_addr, test_vector_accum_256, 
sizeof(test_vector_accum_256));
+qtest_memwrite(s, src_addr, array, sizeof(array));
+
+write_regs(s, base, src_addr, sizeof(test_vector_accum_256),
+   digest_addr, HACE_ALGO_SHA256 | HACE_SG_EN | HACE_ACCUM_EN);
+
+/* Check hash IRQ status is asserted */
+g_assert_cmphex(qtest_readl(s, ba

Re: [PATCH v2 1/2] hw/misc: Supporting AST2600 HACE accumulative mode

2022-01-12 Thread Troy Lee
[ Adding Klaus ]

Sorry I forgot to add Klaus to the CC list.

On Wed, Jan 12, 2022 at 4:10 PM Troy Lee  wrote:
>
> Accumulative mode will supply a initial state and append padding bit at
> the end of hash stream.  However, the crypto library will padding those
> bit automatically, so ripped it off from iov array.
>
> The aspeed ast2600 acculumative mode is described in datasheet
> ast2600v10.pdf section 25.6.4:
>  1. Allocationg and initiating accumulative hash digest write buffer
> with initial state.
> * Since QEMU crypto/hash api doesn't provide the API to set initial
>   state of hash library, and the initial state is already setted by
>   crypto library (gcrypt/glib/...), so skip this step.
>  2. Calculating accumulative hash digest.
> (a) When receiving the last accumulative data, software need to add
> padding message at the end of the accumulative data. Padding
> message described in specific of MD5, SHA-1, SHA224, SHA256,
> SHA512, SHA512/224, SHA512/256.
> * Since the crypto library (gcrypt/glib) already pad the
>   padding message internally.
> * This patch is to remove the padding message which fed byguest
>   machine driver.
>
> Changes in v2:
> - Coding style
> - Add accumulative mode description in comment
>
> Signed-off-by: Troy Lee 
> ---
>  hw/misc/aspeed_hace.c | 43 ---
>  include/hw/misc/aspeed_hace.h |  1 +
>  2 files changed, 36 insertions(+), 8 deletions(-)
>
> diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
> index 10f00e65f4..0710f44621 100644
> --- a/hw/misc/aspeed_hace.c
> +++ b/hw/misc/aspeed_hace.c
> @@ -11,6 +11,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu/log.h"
>  #include "qemu/error-report.h"
> +#include "qemu/bswap.h"
>  #include "hw/misc/aspeed_hace.h"
>  #include "qapi/error.h"
>  #include "migration/vmstate.h"
> @@ -27,6 +28,7 @@
>
>  #define R_HASH_SRC  (0x20 / 4)
>  #define R_HASH_DEST (0x24 / 4)
> +#define R_HASH_KEY_BUFF (0x28 / 4)
>  #define R_HASH_SRC_LEN  (0x2c / 4)
>
>  #define R_HASH_CMD  (0x30 / 4)
> @@ -94,7 +96,8 @@ static int hash_algo_lookup(uint32_t reg)
>  return -1;
>  }
>
> -static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode)
> +static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
> +  bool acc_mode)
>  {
>  struct iovec iov[ASPEED_HACE_MAX_SG];
>  g_autofree uint8_t *digest_buf;
> @@ -103,6 +106,7 @@ static void do_hash_operation(AspeedHACEState *s, int 
> algo, bool sg_mode)
>
>  if (sg_mode) {
>  uint32_t len = 0;
> +uint32_t total_len = 0;
>
>  for (i = 0; !(len & SG_LIST_LEN_LAST); i++) {
>  uint32_t addr, src;
> @@ -123,10 +127,26 @@ static void do_hash_operation(AspeedHACEState *s, int 
> algo, bool sg_mode)
>  MEMTXATTRS_UNSPECIFIED, NULL);
>  addr &= SG_LIST_ADDR_MASK;
>
> -iov[i].iov_len = len & SG_LIST_LEN_MASK;
> -plen = iov[i].iov_len;
> +plen = len & SG_LIST_LEN_MASK;
>  iov[i].iov_base = address_space_map(>dram_as, addr, , 
> false,
>  MEMTXATTRS_UNSPECIFIED);
> +
> +if (acc_mode) {
> +total_len += plen;
> +
> +if (len & SG_LIST_LEN_LAST) {
> +/*
> + * In the padding message, the last 64/128 bit represents
> + * the total length of bitstream in big endian.
> + * SHA-224, SHA-256 are 64 bit
> + * SHA-384, SHA-512, SHA-512/224, SHA-512/256 are 128 bit
> + * However, we would not process such a huge bit stream.
> + */
> +plen -= total_len - (ldq_be_p(iov[i].iov_base + plen - 
> 8) / 8);
> +}
> +}
> +
> +iov[i].iov_len = plen;
>  }
>  } else {
>  hwaddr len = s->regs[R_HASH_SRC_LEN];
> @@ -210,6 +230,9 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, 
> uint64_t data,
>  case R_HASH_DEST:
>  data &= ahc->dest_mask;
>  break;
> +case R_HASH_KEY_BUFF:
> +data &= ahc->key_mask;
> +break;
>  case R_HASH_SRC_LEN:
>  data &= 0x0FFF;
>  break;
> @@ -229,12 +252,13 @@ static void aspeed_hace_write(void *opaque, hwaddr 
> addr, uint64_t data,
>

Re: [PATCH v2 2/2] tests/qtest: Add test for Aspeed HACE accumulative mode

2022-01-12 Thread Troy Lee
[ Adding Klaus ]

On Wed, Jan 12, 2022 at 4:09 PM Troy Lee  wrote:
>
> This add two addition test cases for accumulative mode under sg enabled.
>
> The input vector was manually craft with "abc" + bit 1 + padding zeros + L.
> The padding length depends on algorithm, i.e. SHA512 (1024 bit),
> SHA256 (512 bit).
>
> The result was calculated by command line sha512sum/sha256sum utilities
> without padding, i.e. only "abc" ascii text.
>
> Signed-off-by: Troy Lee 
> ---
>  tests/qtest/aspeed_hace-test.c | 145 +
>  1 file changed, 145 insertions(+)
>
> diff --git a/tests/qtest/aspeed_hace-test.c b/tests/qtest/aspeed_hace-test.c
> index 09ee31545e..6a2f404b93 100644
> --- a/tests/qtest/aspeed_hace-test.c
> +++ b/tests/qtest/aspeed_hace-test.c
> @@ -21,6 +21,7 @@
>  #define  HACE_ALGO_SHA512(BIT(5) | BIT(6))
>  #define  HACE_ALGO_SHA384(BIT(5) | BIT(6) | BIT(10))
>  #define  HACE_SG_EN  BIT(18)
> +#define  HACE_ACCUM_EN   BIT(8)
>
>  #define HACE_STS 0x1c
>  #define  HACE_RSA_ISRBIT(13)
> @@ -96,6 +97,57 @@ static const uint8_t test_result_sg_sha256[] = {
>  0x55, 0x1e, 0x1e, 0xc5, 0x80, 0xdd, 0x6d, 0x5a, 0x6e, 0xcd, 0xe9, 0xf3,
>  0xd3, 0x5e, 0x6e, 0x4a, 0x71, 0x7f, 0xbd, 0xe4};
>
> +/*
> + * The accumulative mode requires firmware to provide internal initial state
> + * and message padding (including length L at the end of padding).
> + *
> + * This test vector is a ascii text "abc" with padding message.
> + *
> + * Expected results were generated using command line utitiles:
> + *
> + *  echo -n -e 'abc' | dd of=/tmp/test
> + *  for hash in sha512sum sha256sum; do $hash /tmp/test; done
> + */
> +static const uint8_t test_vector_accum_512[] = {
> +0x61, 0x62, 0x63, 0x80, 0x00, 0x00, 0x00, 0x00,
> +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18};
> +
> +static const uint8_t test_vector_accum_256[] = {
> +0x61, 0x62, 0x63, 0x80, 0x00, 0x00, 0x00, 0x00,
> +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18};
> +
> +static const uint8_t test_result_accum_sha512[] = {
> +0xdd, 0xaf, 0x35, 0xa1, 0x93, 0x61, 0x7a, 0xba, 0xcc, 0x41, 0x73, 0x49,
> +0xae, 0x20, 0x41, 0x31, 0x12, 0xe6, 0xfa, 0x4e, 0x89, 0xa9, 0x7e, 0xa2,
> +0x0a, 0x9e, 0xee, 0xe6, 0x4b, 0x55, 0xd3, 0x9a, 0x21, 0x92, 0x99, 0x2a,
> +0x27, 0x4f, 0xc1, 0xa8, 0x36, 0xba, 0x3c, 0x23, 0xa3, 0xfe, 0xeb, 0xbd,
> +0x45, 0x4d, 0x44, 0x23, 0x64, 0x3c, 0xe8, 0x0e, 0x2a, 0x9a, 0xc9, 0x4f,
> +0xa5, 0x4c, 0xa4, 0x9f};
> +
> +static const uint8_t test_result_accum_sha256[] = {
> +0xba, 0x78, 0x16, 0xbf, 0x8f, 0x01, 0xcf, 0xea, 0x41, 0x41, 0x40, 0xde,
> +0x5d, 0xae, 0x22, 0x23, 0xb0, 0x03, 0x61, 0xa3, 0x96, 0x17, 0x7a, 0x9c,
> +0xb4, 0x10, 0xff, 0x61, 0xf2, 0x00, 0x15, 0xad};
>
>  static void write_regs(QTestState *s, uint32_t base, uint32_t src,
> uint32_t length, uint32_t out, uint32_t method)
> @@ -308,6 +360,86 @@ static void test_sha512_sg(const char *machine, const 
> uint32_t base,
>  qtest_quit(s);
>  }
>
> +static void test_sha256_accum(const char *machine, const uint32_t base,
> +const uint32_t src_addr)
> +{
> +QTestState *s = qtest_init(machine);
> +
> +const uint32_t buffer_addr = src_addr + 0x100;
> +const uint32_t digest_addr = src_addr + 0x400;
> +uint8_t digest[32] = {0};
> +struct AspeedSgList array[] = {
> +{  cpu_to_le32(sizeof(test_vector_accum_256) | SG_LIST_LEN_LAST),
> +   cpu_to_le32(buffer_addr) },
&

[PATCH v3 2/2] This patch includes i3c instance in ast2600 soc.

2022-01-11 Thread Troy Lee
v3:
- Remove unrelated changes to SPI2 address
- Remove controller irq line

v2: Rebase to mainline QEMU

Signed-off-by: Troy Lee 
---
 hw/arm/aspeed_ast2600.c | 16 
 include/hw/arm/aspeed_soc.h |  3 +++
 2 files changed, 19 insertions(+)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index e33483fb5d..8f37bdb1d8 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -61,6 +61,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
 [ASPEED_DEV_UART1] = 0x1E783000,
 [ASPEED_DEV_UART5] = 0x1E784000,
 [ASPEED_DEV_VUART] = 0x1E787000,
+[ASPEED_DEV_I3C]   = 0x1E7A,
 [ASPEED_DEV_SDRAM] = 0x8000,
 };
 
@@ -108,6 +109,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
 [ASPEED_DEV_ETH4]  = 33,
 [ASPEED_DEV_KCS]   = 138,   /* 138 -> 142 */
 [ASPEED_DEV_DP]= 62,
+[ASPEED_DEV_I3C]   = 102,   /* 102 -> 107 */
 };
 
 static qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int ctrl)
@@ -223,6 +225,8 @@ static void aspeed_soc_ast2600_init(Object *obj)
 
 snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
 object_initialize_child(obj, "hace", >hace, typename);
+
+object_initialize_child(obj, "i3c", >i3c, TYPE_ASPEED_I3C);
 }
 
 /*
@@ -523,6 +527,18 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
 sysbus_mmio_map(SYS_BUS_DEVICE(>hace), 0, sc->memmap[ASPEED_DEV_HACE]);
 sysbus_connect_irq(SYS_BUS_DEVICE(>hace), 0,
aspeed_soc_get_irq(s, ASPEED_DEV_HACE));
+
+/* I3C */
+if (!sysbus_realize(SYS_BUS_DEVICE(>i3c), errp)) {
+return;
+}
+sysbus_mmio_map(SYS_BUS_DEVICE(>i3c), 0, sc->memmap[ASPEED_DEV_I3C]);
+for (i = 0; i < ASPEED_I3C_NR_DEVICES; i++) {
+qemu_irq irq = qdev_get_gpio_in(DEVICE(>a7mpcore),
+sc->irqmap[ASPEED_DEV_I3C] + i);
+/* The AST2600 I3C controller has one IRQ per bus. */
+sysbus_connect_irq(SYS_BUS_DEVICE(>i3c.devices[i]), 0, irq);
+}
 }
 
 static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 18fb7eed46..cae9906684 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -21,6 +21,7 @@
 #include "hw/timer/aspeed_timer.h"
 #include "hw/rtc/aspeed_rtc.h"
 #include "hw/i2c/aspeed_i2c.h"
+#include "hw/misc/aspeed_i3c.h"
 #include "hw/ssi/aspeed_smc.h"
 #include "hw/misc/aspeed_hace.h"
 #include "hw/watchdog/wdt_aspeed.h"
@@ -51,6 +52,7 @@ struct AspeedSoCState {
 AspeedRtcState rtc;
 AspeedTimerCtrlState timerctrl;
 AspeedI2CState i2c;
+AspeedI3CState i3c;
 AspeedSCUState scu;
 AspeedHACEState hace;
 AspeedXDMAState xdma;
@@ -141,6 +143,7 @@ enum {
 ASPEED_DEV_HACE,
 ASPEED_DEV_DPMCU,
 ASPEED_DEV_DP,
+ASPEED_DEV_I3C,
 };
 
 #endif /* ASPEED_SOC_H */
-- 
2.25.1




[PATCH v3 1/2] Introduce a dummy AST2600 I3C model.

2022-01-11 Thread Troy Lee
Aspeed 2600 SDK enables I3C support by default.  The I3C driver will try
to reset the device controller and setup through device address table
register.  This dummy model response these register with default value
listed on ast2600v10 datasheet chapter 54.2.  If the device address
table register doesn't set correctly, it will cause guest machine kernel
panic due to reference to invalid address.

v3:
- Remove unused AspeedI3CClass
- Refine memory region
- Refine register reset

v2:
- Split i3c model into i3c and i3c_device
- Create 6x i3c devices
- Using register fields macro

Signed-off-by: Troy Lee 
---
 hw/misc/aspeed_i3c.c | 381 +++
 hw/misc/meson.build  |   1 +
 hw/misc/trace-events |   6 +
 include/hw/misc/aspeed_i3c.h |  48 +
 4 files changed, 436 insertions(+)
 create mode 100644 hw/misc/aspeed_i3c.c
 create mode 100644 include/hw/misc/aspeed_i3c.h

diff --git a/hw/misc/aspeed_i3c.c b/hw/misc/aspeed_i3c.c
new file mode 100644
index 00..43771d768a
--- /dev/null
+++ b/hw/misc/aspeed_i3c.c
@@ -0,0 +1,381 @@
+/*
+ * ASPEED I3C Controller
+ *
+ * Copyright (C) 2021 ASPEED Technology Inc.
+ *
+ * This code is licensed under the GPL version 2 or later.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/error-report.h"
+#include "hw/misc/aspeed_i3c.h"
+#include "hw/registerfields.h"
+#include "hw/qdev-properties.h"
+#include "qapi/error.h"
+#include "migration/vmstate.h"
+#include "trace.h"
+
+/* I3C Controller Registers */
+REG32(I3C1_REG0, 0x10)
+REG32(I3C1_REG1, 0x14)
+FIELD(I3C1_REG1, I2C_MODE,  0,  1)
+FIELD(I3C1_REG1, SA_EN, 15, 1)
+REG32(I3C2_REG0, 0x20)
+REG32(I3C2_REG1, 0x24)
+FIELD(I3C2_REG1, I2C_MODE,  0,  1)
+FIELD(I3C2_REG1, SA_EN, 15, 1)
+REG32(I3C3_REG0, 0x30)
+REG32(I3C3_REG1, 0x34)
+FIELD(I3C3_REG1, I2C_MODE,  0,  1)
+FIELD(I3C3_REG1, SA_EN, 15, 1)
+REG32(I3C4_REG0, 0x40)
+REG32(I3C4_REG1, 0x44)
+FIELD(I3C4_REG1, I2C_MODE,  0,  1)
+FIELD(I3C4_REG1, SA_EN, 15, 1)
+REG32(I3C5_REG0, 0x50)
+REG32(I3C5_REG1, 0x54)
+FIELD(I3C5_REG1, I2C_MODE,  0,  1)
+FIELD(I3C5_REG1, SA_EN, 15, 1)
+REG32(I3C6_REG0, 0x60)
+REG32(I3C6_REG1, 0x64)
+FIELD(I3C6_REG1, I2C_MODE,  0,  1)
+FIELD(I3C6_REG1, SA_EN, 15, 1)
+
+/* I3C Device Registers */
+REG32(DEVICE_CTRL,  0x00)
+REG32(DEVICE_ADDR,  0x04)
+REG32(HW_CAPABILITY,0x08)
+REG32(COMMAND_QUEUE_PORT,   0x0c)
+REG32(RESPONSE_QUEUE_PORT,  0x10)
+REG32(RX_TX_DATA_PORT,  0x14)
+REG32(IBI_QUEUE_STATUS, 0x18)
+REG32(IBI_QUEUE_DATA,   0x18)
+REG32(QUEUE_THLD_CTRL,  0x1c)
+REG32(DATA_BUFFER_THLD_CTRL,0x20)
+REG32(IBI_QUEUE_CTRL,   0x24)
+REG32(IBI_MR_REQ_REJECT,0x2c)
+REG32(IBI_SIR_REQ_REJECT,   0x30)
+REG32(RESET_CTRL,   0x34)
+REG32(SLV_EVENT_CTRL,   0x38)
+REG32(INTR_STATUS,  0x3c)
+REG32(INTR_STATUS_EN,   0x40)
+REG32(INTR_SIGNAL_EN,   0x44)
+REG32(INTR_FORCE,   0x48)
+REG32(QUEUE_STATUS_LEVEL,   0x4c)
+REG32(DATA_BUFFER_STATUS_LEVEL, 0x50)
+REG32(PRESENT_STATE,0x54)
+REG32(CCC_DEVICE_STATUS,0x58)
+REG32(DEVICE_ADDR_TABLE_POINTER,0x5c)
+FIELD(DEVICE_ADDR_TABLE_POINTER, DEPTH, 16, 16)
+FIELD(DEVICE_ADDR_TABLE_POINTER, ADDR,  0,  16)
+REG32(DEV_CHAR_TABLE_POINTER,   0x60)
+REG32(VENDOR_SPECIFIC_REG_POINTER,  0x6c)
+REG32(SLV_MIPI_PID_VALUE,   0x70)
+REG32(SLV_PID_VALUE,0x74)
+REG32(SLV_CHAR_CTRL,0x78)
+REG32(SLV_MAX_LEN,  0x7c)
+REG32(MAX_READ_TURNAROUND,  0x80)
+REG32(MAX_DATA_SPEED,   0x84)
+REG32(SLV_DEBUG_STATUS, 0x88)
+REG32(SLV_INTR_REQ, 0x8c)
+REG32(DEVICE_CTRL_EXTENDED, 0xb0)
+REG32(SCL_I3C_OD_TIMING,0xb4)
+REG32(SCL_I3C_PP_TIMING,0xb8)
+REG32(SCL_I2C_FM_TIMING,0xbc)
+REG32(SCL_I2C_FMP_TIMING,   0xc0)
+REG32(SCL_EXT_LCNT_TIMING,  0xc8)
+REG32(SCL_EXT_TERMN_LCNT_TIMING,0xcc)
+REG32(BUS_FREE_TIMING,  0xd4)
+REG32(BUS_IDLE_TIMING,  0xd8)
+REG32(I3C_VER_ID,   0xe0)
+REG32(I3C_VER_TYPE, 0xe4)
+REG32(EXTENDED_CAPABILITY,  0xe8)
+REG32(SLAVE_CONFIG, 0xec)
+
+static const uint32_t ast2600_i3c_device_resets[ASPEED_I3C_DEVICE_NR_REGS] = {
+[R_HW_CAPABILITY]   = 0x000e00bf,
+[R_QUEUE_THLD_CTRL] = 0x01000101,
+[R_I3C_VER_ID]  = 0x3130302a,
+[R_I3C_VER_TYPE]= 0x6c633033,
+[R_DEVICE_ADDR_TABLE_POINTER]   = 0x00080280,
+[R_DEV_CHAR_TABLE_POINTER]

[PATCH v3 0/2] Aspeed I3C device model

2022-01-11 Thread Troy Lee
This series of patch introduce a dummy implemenation of aspeed i3c
model, and it provide just enough information for guest machine.
However, the driver probing is still failed, but it will not cause
kernel panic.

v3:
- Remove unused AspeedI3CClass
- Refine memory region
- Refine register reset
- Remove unrelated changes to SPI2 address
- Remove i3c controller irq line

v2:
- Split i3c model into i3c and i3c_device
- Create 6x i3c devices
- Using register fields macro
- Rebase to mainline QEMU

Troy Lee (2):
  Introduce a dummy AST2600 I3C model.
  This patch includes i3c instance in ast2600 soc.

 hw/arm/aspeed_ast2600.c  |  16 ++
 hw/misc/aspeed_i3c.c | 381 +++
 hw/misc/meson.build  |   1 +
 hw/misc/trace-events |   6 +
 include/hw/arm/aspeed_soc.h  |   3 +
 include/hw/misc/aspeed_i3c.h |  48 +
 6 files changed, 455 insertions(+)
 create mode 100644 hw/misc/aspeed_i3c.c
 create mode 100644 include/hw/misc/aspeed_i3c.h

-- 
2.25.1




Re: [PATCH v2 2/2] hw/arm/aspeed_ast2600: create i3c instance

2022-01-10 Thread Troy Lee
On Mon, Jan 10, 2022 at 10:31 PM Cédric Le Goater  wrote:
>
> On 1/10/22 08:21, Troy Lee wrote:
> > This patch includes i3c instance in ast2600 soc.
> >
> > v2: Rebase to mainline QEMU
> >
> > Signed-off-by: Troy Lee 
> > ---
> >   hw/arm/aspeed_ast2600.c | 19 ++-
> >   include/hw/arm/aspeed_soc.h |  3 +++
> >   2 files changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> > index e33483fb5d..36aa31601a 100644
> > --- a/hw/arm/aspeed_ast2600.c
> > +++ b/hw/arm/aspeed_ast2600.c
> > @@ -29,7 +29,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
> >   [ASPEED_DEV_PWM]   = 0x1E61,
> >   [ASPEED_DEV_FMC]   = 0x1E62,
> >   [ASPEED_DEV_SPI1]  = 0x1E63,
> > -[ASPEED_DEV_SPI2]  = 0x1E641000,
> > +[ASPEED_DEV_SPI2]  = 0x1E631000,
>
> Indeed ! But this belongs to another patch fixing the value.
>

Oops, that should be in a different branch, I might accidentally pick
that into my working branch. dkodihal will send it separately.


> >   [ASPEED_DEV_EHCI1] = 0x1E6A1000,
> >   [ASPEED_DEV_EHCI2] = 0x1E6A3000,
> >   [ASPEED_DEV_MII1]  = 0x1E65,
> > @@ -61,6 +61,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
> >   [ASPEED_DEV_UART1] = 0x1E783000,
> >   [ASPEED_DEV_UART5] = 0x1E784000,
> >   [ASPEED_DEV_VUART] = 0x1E787000,
> > +[ASPEED_DEV_I3C]   = 0x1E7A,
> >   [ASPEED_DEV_SDRAM] = 0x8000,
> >   };
> >
> > @@ -108,6 +109,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
> >   [ASPEED_DEV_ETH4]  = 33,
> >   [ASPEED_DEV_KCS]   = 138,   /* 138 -> 142 */
> >   [ASPEED_DEV_DP]= 62,
> > +[ASPEED_DEV_I3C]   = 102,   /* 102 -> 107 */
> >   };
> >
> >   static qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int ctrl)
> > @@ -223,6 +225,8 @@ static void aspeed_soc_ast2600_init(Object *obj)
> >
> >   snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
> >   object_initialize_child(obj, "hace", >hace, typename);
> > +
> > +object_initialize_child(obj, "i3c", >i3c, TYPE_ASPEED_I3C);
> >   }
> >
> >   /*
> > @@ -523,6 +527,19 @@ static void aspeed_soc_ast2600_realize(DeviceState 
> > *dev, Error **errp)
> >   sysbus_mmio_map(SYS_BUS_DEVICE(>hace), 0, 
> > sc->memmap[ASPEED_DEV_HACE]);
> >   sysbus_connect_irq(SYS_BUS_DEVICE(>hace), 0,
> >  aspeed_soc_get_irq(s, ASPEED_DEV_HACE));
> > +/* I3C */
> > +if (!sysbus_realize(SYS_BUS_DEVICE(>i3c), errp)) {
> > +return;
> > +}
> > +sysbus_mmio_map(SYS_BUS_DEVICE(>i3c), 0, 
> > sc->memmap[ASPEED_DEV_I3C]);
> > +sysbus_connect_irq(SYS_BUS_DEVICE(>i3c), 0,
> > +   aspeed_soc_get_irq(s, ASPEED_DEV_I3C));
>
> The controller device does not have an IRQ line.
>

Removed in v3.
Thanks for the review,
Troy Lee

> Thanks,
>
> C.
>
>
>
> > +for (i = 0; i < ASPEED_I3C_NR_DEVICES; i++) {
> > +qemu_irq irq = qdev_get_gpio_in(DEVICE(>a7mpcore),
> > +sc->irqmap[ASPEED_DEV_I3C] + i);
> > +/* The AST2600 I3C controller has one IRQ per bus. */
> > +sysbus_connect_irq(SYS_BUS_DEVICE(>i3c.devices[i]), 0, irq);
> > +}
> >   }
> >
> >   static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
> > diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> > index 18fb7eed46..cae9906684 100644
> > --- a/include/hw/arm/aspeed_soc.h
> > +++ b/include/hw/arm/aspeed_soc.h
> > @@ -21,6 +21,7 @@
> >   #include "hw/timer/aspeed_timer.h"
> >   #include "hw/rtc/aspeed_rtc.h"
> >   #include "hw/i2c/aspeed_i2c.h"
> > +#include "hw/misc/aspeed_i3c.h"
> >   #include "hw/ssi/aspeed_smc.h"
> >   #include "hw/misc/aspeed_hace.h"
> >   #include "hw/watchdog/wdt_aspeed.h"
> > @@ -51,6 +52,7 @@ struct AspeedSoCState {
> >   AspeedRtcState rtc;
> >   AspeedTimerCtrlState timerctrl;
> >   AspeedI2CState i2c;
> > +AspeedI3CState i3c;
> >   AspeedSCUState scu;
> >   AspeedHACEState hace;
> >   AspeedXDMAState xdma;
> > @@ -141,6 +143,7 @@ enum {
> >   ASPEED_DEV_HACE,
> >   ASPEED_DEV_DPMCU,
> >   ASPEED_DEV_DP,
> > +ASPEED_DEV_I3C,
> >   };
> >
> >   #endif /* ASPEED_SOC_H */
> >
>



Re: [PATCH v2 1/2] hw/misc: Implementating dummy AST2600 I3C model

2022-01-10 Thread Troy Lee
Hi Cedric,
On Mon, Jan 10, 2022 at 10:25 PM Cédric Le Goater  wrote:
>
> Hello Troy,
>
> On 1/10/22 08:21, Troy Lee wrote:
> > Introduce a dummy AST2600 I3C model.
> >
> > Aspeed 2600 SDK enables I3C support by default.  The I3C driver will try
> > to reset the device controller and setup through device address table
> > register.  This dummy model response these register with default value
> > listed on ast2600v10 datasheet chapter 54.2.  If the device address
> > table register doesn't set correctly, it will cause guest machine kernel
> > panic due to reference to invalid address.
> >
> > v2:
> > - Split i3c model into i3c and i3c_device
> > - Create 6x i3c devices
> > - Using register fields macro
> >
> > Signed-off-by: Troy Lee 
> > ---
> >   hw/misc/aspeed_i3c.c | 410 +++
> >   hw/misc/meson.build  |   1 +
> >   hw/misc/trace-events |   6 +
> >   include/hw/misc/aspeed_i3c.h |  57 +
> >   4 files changed, 474 insertions(+)
> >   create mode 100644 hw/misc/aspeed_i3c.c
> >   create mode 100644 include/hw/misc/aspeed_i3c.h
> >
> > diff --git a/hw/misc/aspeed_i3c.c b/hw/misc/aspeed_i3c.c
> > new file mode 100644
> > index 00..16a4f2d4e4
> > --- /dev/null
> > +++ b/hw/misc/aspeed_i3c.c
> > @@ -0,0 +1,410 @@
> > +/*
> > + * ASPEED I3C Controller
> > + *
> > + * Copyright (C) 2021 ASPEED Technology Inc.
> > + *
> > + * This code is licensed under the GPL version 2 or later.  See
> > + * the COPYING file in the top-level directory.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/log.h"
> > +#include "qemu/error-report.h"
> > +#include "hw/misc/aspeed_i3c.h"
> > +#include "hw/registerfields.h"
> > +#include "hw/qdev-properties.h"
> > +#include "qapi/error.h"
> > +#include "migration/vmstate.h"
> > +#include "trace.h"
> > +
> > +/* I3C Controller Registers */
> > +REG32(I3C1_REG0, 0x10)
> > +REG32(I3C1_REG1, 0x14)
> > +FIELD(I3C1_REG1, I2C_MODE,  0,  1)
> > +FIELD(I3C1_REG1, SA_EN, 15, 1)
> > +REG32(I3C2_REG0, 0x20)
> > +REG32(I3C2_REG1, 0x24)
> > +FIELD(I3C2_REG1, I2C_MODE,  0,  1)
> > +FIELD(I3C2_REG1, SA_EN, 15, 1)
> > +REG32(I3C3_REG0, 0x30)
> > +REG32(I3C3_REG1, 0x34)
> > +FIELD(I3C3_REG1, I2C_MODE,  0,  1)
> > +FIELD(I3C3_REG1, SA_EN, 15, 1)
> > +REG32(I3C4_REG0, 0x40)
> > +REG32(I3C4_REG1, 0x44)
> > +FIELD(I3C4_REG1, I2C_MODE,  0,  1)
> > +FIELD(I3C4_REG1, SA_EN, 15, 1)
> > +REG32(I3C5_REG0, 0x50)
> > +REG32(I3C5_REG1, 0x54)
> > +FIELD(I3C5_REG1, I2C_MODE,  0,  1)
> > +FIELD(I3C5_REG1, SA_EN, 15, 1)
> > +REG32(I3C6_REG0, 0x60)
> > +REG32(I3C6_REG1, 0x64)
> > +FIELD(I3C6_REG1, I2C_MODE,  0,  1)
> > +FIELD(I3C6_REG1, SA_EN, 15, 1)
> > +
> > +/* I3C Device Registers */
> > +REG32(DEVICE_CTRL,  0x00)
> > +REG32(DEVICE_ADDR,  0x04)
> > +REG32(HW_CAPABILITY,0x08)
> > +REG32(COMMAND_QUEUE_PORT,   0x0c)
> > +REG32(RESPONSE_QUEUE_PORT,  0x10)
> > +REG32(RX_TX_DATA_PORT,  0x14)
> > +REG32(IBI_QUEUE_STATUS, 0x18)
> > +REG32(IBI_QUEUE_DATA,   0x18)
> > +REG32(QUEUE_THLD_CTRL,  0x1c)
> > +REG32(DATA_BUFFER_THLD_CTRL,0x20)
> > +REG32(IBI_QUEUE_CTRL,   0x24)
> > +REG32(IBI_MR_REQ_REJECT,0x2c)
> > +REG32(IBI_SIR_REQ_REJECT,   0x30)
> > +REG32(RESET_CTRL,   0x34)
> > +REG32(SLV_EVENT_CTRL,   0x38)
> > +REG32(INTR_STATUS,  0x3c)
> > +REG32(INTR_STATUS_EN,   0x40)
> > +REG32(INTR_SIGNAL_EN,   0x44)
> > +REG32(INTR_FORCE,   0x48)
> > +REG32(QUEUE_STATUS_LEVEL,   0x4c)
> > +REG32(DATA_BUFFER_STATUS_LEVEL, 0x50)
> > +REG32(PRESENT_STATE,0x54)
> > +REG32(CCC_DEVICE_STATUS,0x58)
> > +REG32(DEVICE_ADDR_TABLE_POINTER,0x5c)
> > +FIELD(DEVICE_ADDR_TABLE_POINTER, DEPTH, 16, 16)
> > +FIELD(DEVICE_ADDR_TABLE_POINTER, ADDR,  0,  16)
> > +REG32(DEV_CHAR_TABLE_POINTER,   0x60)
> > +REG32(VENDOR_SPECIFIC_REG_POINTER,  0x6c)
> > +REG32(SLV_MIPI_PID_VALUE,   0x70)
> > +REG32(SLV_PID_VALUE,0x74)
> > +REG32(SLV_CHAR_CTRL,  

[PATCH v2 1/2] hw/misc: Implementating dummy AST2600 I3C model

2022-01-09 Thread Troy Lee
Introduce a dummy AST2600 I3C model.

Aspeed 2600 SDK enables I3C support by default.  The I3C driver will try
to reset the device controller and setup through device address table
register.  This dummy model response these register with default value
listed on ast2600v10 datasheet chapter 54.2.  If the device address
table register doesn't set correctly, it will cause guest machine kernel
panic due to reference to invalid address.

v2:
- Split i3c model into i3c and i3c_device
- Create 6x i3c devices
- Using register fields macro

Signed-off-by: Troy Lee 
---
 hw/misc/aspeed_i3c.c | 410 +++
 hw/misc/meson.build  |   1 +
 hw/misc/trace-events |   6 +
 include/hw/misc/aspeed_i3c.h |  57 +
 4 files changed, 474 insertions(+)
 create mode 100644 hw/misc/aspeed_i3c.c
 create mode 100644 include/hw/misc/aspeed_i3c.h

diff --git a/hw/misc/aspeed_i3c.c b/hw/misc/aspeed_i3c.c
new file mode 100644
index 00..16a4f2d4e4
--- /dev/null
+++ b/hw/misc/aspeed_i3c.c
@@ -0,0 +1,410 @@
+/*
+ * ASPEED I3C Controller
+ *
+ * Copyright (C) 2021 ASPEED Technology Inc.
+ *
+ * This code is licensed under the GPL version 2 or later.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/error-report.h"
+#include "hw/misc/aspeed_i3c.h"
+#include "hw/registerfields.h"
+#include "hw/qdev-properties.h"
+#include "qapi/error.h"
+#include "migration/vmstate.h"
+#include "trace.h"
+
+/* I3C Controller Registers */
+REG32(I3C1_REG0, 0x10)
+REG32(I3C1_REG1, 0x14)
+FIELD(I3C1_REG1, I2C_MODE,  0,  1)
+FIELD(I3C1_REG1, SA_EN, 15, 1)
+REG32(I3C2_REG0, 0x20)
+REG32(I3C2_REG1, 0x24)
+FIELD(I3C2_REG1, I2C_MODE,  0,  1)
+FIELD(I3C2_REG1, SA_EN, 15, 1)
+REG32(I3C3_REG0, 0x30)
+REG32(I3C3_REG1, 0x34)
+FIELD(I3C3_REG1, I2C_MODE,  0,  1)
+FIELD(I3C3_REG1, SA_EN, 15, 1)
+REG32(I3C4_REG0, 0x40)
+REG32(I3C4_REG1, 0x44)
+FIELD(I3C4_REG1, I2C_MODE,  0,  1)
+FIELD(I3C4_REG1, SA_EN, 15, 1)
+REG32(I3C5_REG0, 0x50)
+REG32(I3C5_REG1, 0x54)
+FIELD(I3C5_REG1, I2C_MODE,  0,  1)
+FIELD(I3C5_REG1, SA_EN, 15, 1)
+REG32(I3C6_REG0, 0x60)
+REG32(I3C6_REG1, 0x64)
+FIELD(I3C6_REG1, I2C_MODE,  0,  1)
+FIELD(I3C6_REG1, SA_EN, 15, 1)
+
+/* I3C Device Registers */
+REG32(DEVICE_CTRL,  0x00)
+REG32(DEVICE_ADDR,  0x04)
+REG32(HW_CAPABILITY,0x08)
+REG32(COMMAND_QUEUE_PORT,   0x0c)
+REG32(RESPONSE_QUEUE_PORT,  0x10)
+REG32(RX_TX_DATA_PORT,  0x14)
+REG32(IBI_QUEUE_STATUS, 0x18)
+REG32(IBI_QUEUE_DATA,   0x18)
+REG32(QUEUE_THLD_CTRL,  0x1c)
+REG32(DATA_BUFFER_THLD_CTRL,0x20)
+REG32(IBI_QUEUE_CTRL,   0x24)
+REG32(IBI_MR_REQ_REJECT,0x2c)
+REG32(IBI_SIR_REQ_REJECT,   0x30)
+REG32(RESET_CTRL,   0x34)
+REG32(SLV_EVENT_CTRL,   0x38)
+REG32(INTR_STATUS,  0x3c)
+REG32(INTR_STATUS_EN,   0x40)
+REG32(INTR_SIGNAL_EN,   0x44)
+REG32(INTR_FORCE,   0x48)
+REG32(QUEUE_STATUS_LEVEL,   0x4c)
+REG32(DATA_BUFFER_STATUS_LEVEL, 0x50)
+REG32(PRESENT_STATE,0x54)
+REG32(CCC_DEVICE_STATUS,0x58)
+REG32(DEVICE_ADDR_TABLE_POINTER,0x5c)
+FIELD(DEVICE_ADDR_TABLE_POINTER, DEPTH, 16, 16)
+FIELD(DEVICE_ADDR_TABLE_POINTER, ADDR,  0,  16)
+REG32(DEV_CHAR_TABLE_POINTER,   0x60)
+REG32(VENDOR_SPECIFIC_REG_POINTER,  0x6c)
+REG32(SLV_MIPI_PID_VALUE,   0x70)
+REG32(SLV_PID_VALUE,0x74)
+REG32(SLV_CHAR_CTRL,0x78)
+REG32(SLV_MAX_LEN,  0x7c)
+REG32(MAX_READ_TURNAROUND,  0x80)
+REG32(MAX_DATA_SPEED,   0x84)
+REG32(SLV_DEBUG_STATUS, 0x88)
+REG32(SLV_INTR_REQ, 0x8c)
+REG32(DEVICE_CTRL_EXTENDED, 0xb0)
+REG32(SCL_I3C_OD_TIMING,0xb4)
+REG32(SCL_I3C_PP_TIMING,0xb8)
+REG32(SCL_I2C_FM_TIMING,0xbc)
+REG32(SCL_I2C_FMP_TIMING,   0xc0)
+REG32(SCL_EXT_LCNT_TIMING,  0xc8)
+REG32(SCL_EXT_TERMN_LCNT_TIMING,0xcc)
+REG32(BUS_FREE_TIMING,  0xd4)
+REG32(BUS_IDLE_TIMING,  0xd8)
+REG32(I3C_VER_ID,   0xe0)
+REG32(I3C_VER_TYPE, 0xe4)
+REG32(EXTENDED_CAPABILITY,  0xe8)
+REG32(SLAVE_CONFIG, 0xec)
+
+static uint64_t aspeed_i3c_device_read(void *opaque, hwaddr offset,
+   unsigned size)
+{
+AspeedI3CDevice *s = ASPEED_I3C_DEVICE(opaque);
+uint32_t addr = offset >> 2;
+uint64_t value;
+
+switch (addr) {
+case R_COMMAND_QUEUE_PORT:
+value = 0;
+break;
+default:
+value = s->regs[addr];
+break;
+}
+
+trace_aspeed_i3c_dev

[PATCH v2 2/2] hw/arm/aspeed_ast2600: create i3c instance

2022-01-09 Thread Troy Lee
This patch includes i3c instance in ast2600 soc.

v2: Rebase to mainline QEMU

Signed-off-by: Troy Lee 
---
 hw/arm/aspeed_ast2600.c | 19 ++-
 include/hw/arm/aspeed_soc.h |  3 +++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index e33483fb5d..36aa31601a 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -29,7 +29,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
 [ASPEED_DEV_PWM]   = 0x1E61,
 [ASPEED_DEV_FMC]   = 0x1E62,
 [ASPEED_DEV_SPI1]  = 0x1E63,
-[ASPEED_DEV_SPI2]  = 0x1E641000,
+[ASPEED_DEV_SPI2]  = 0x1E631000,
 [ASPEED_DEV_EHCI1] = 0x1E6A1000,
 [ASPEED_DEV_EHCI2] = 0x1E6A3000,
 [ASPEED_DEV_MII1]  = 0x1E65,
@@ -61,6 +61,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
 [ASPEED_DEV_UART1] = 0x1E783000,
 [ASPEED_DEV_UART5] = 0x1E784000,
 [ASPEED_DEV_VUART] = 0x1E787000,
+[ASPEED_DEV_I3C]   = 0x1E7A,
 [ASPEED_DEV_SDRAM] = 0x8000,
 };
 
@@ -108,6 +109,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
 [ASPEED_DEV_ETH4]  = 33,
 [ASPEED_DEV_KCS]   = 138,   /* 138 -> 142 */
 [ASPEED_DEV_DP]= 62,
+[ASPEED_DEV_I3C]   = 102,   /* 102 -> 107 */
 };
 
 static qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int ctrl)
@@ -223,6 +225,8 @@ static void aspeed_soc_ast2600_init(Object *obj)
 
 snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
 object_initialize_child(obj, "hace", >hace, typename);
+
+object_initialize_child(obj, "i3c", >i3c, TYPE_ASPEED_I3C);
 }
 
 /*
@@ -523,6 +527,19 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
 sysbus_mmio_map(SYS_BUS_DEVICE(>hace), 0, sc->memmap[ASPEED_DEV_HACE]);
 sysbus_connect_irq(SYS_BUS_DEVICE(>hace), 0,
aspeed_soc_get_irq(s, ASPEED_DEV_HACE));
+/* I3C */
+if (!sysbus_realize(SYS_BUS_DEVICE(>i3c), errp)) {
+return;
+}
+sysbus_mmio_map(SYS_BUS_DEVICE(>i3c), 0, sc->memmap[ASPEED_DEV_I3C]);
+sysbus_connect_irq(SYS_BUS_DEVICE(>i3c), 0,
+   aspeed_soc_get_irq(s, ASPEED_DEV_I3C));
+for (i = 0; i < ASPEED_I3C_NR_DEVICES; i++) {
+qemu_irq irq = qdev_get_gpio_in(DEVICE(>a7mpcore),
+sc->irqmap[ASPEED_DEV_I3C] + i);
+/* The AST2600 I3C controller has one IRQ per bus. */
+sysbus_connect_irq(SYS_BUS_DEVICE(>i3c.devices[i]), 0, irq);
+}
 }
 
 static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 18fb7eed46..cae9906684 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -21,6 +21,7 @@
 #include "hw/timer/aspeed_timer.h"
 #include "hw/rtc/aspeed_rtc.h"
 #include "hw/i2c/aspeed_i2c.h"
+#include "hw/misc/aspeed_i3c.h"
 #include "hw/ssi/aspeed_smc.h"
 #include "hw/misc/aspeed_hace.h"
 #include "hw/watchdog/wdt_aspeed.h"
@@ -51,6 +52,7 @@ struct AspeedSoCState {
 AspeedRtcState rtc;
 AspeedTimerCtrlState timerctrl;
 AspeedI2CState i2c;
+AspeedI3CState i3c;
 AspeedSCUState scu;
 AspeedHACEState hace;
 AspeedXDMAState xdma;
@@ -141,6 +143,7 @@ enum {
 ASPEED_DEV_HACE,
 ASPEED_DEV_DPMCU,
 ASPEED_DEV_DP,
+ASPEED_DEV_I3C,
 };
 
 #endif /* ASPEED_SOC_H */
-- 
2.25.1




[PATCH v2 0/2] Aspeed I3C device model

2022-01-09 Thread Troy Lee
This series of patch introduce a dummy implemenation of aspeed i3c
model, and it provide just enough information for guest machine.
However, the driver probing is still failed, but it will not cause
kernel panic.

v2:
- Split i3c model into i3c and i3c_device
- Create 6x i3c devices
- Using register fields macro
- Rebase to mainline QEMU

Troy Lee (2):
  hw/misc: Implementating dummy AST2600 I3C model
  hw/arm/aspeed_ast2600: create i3c instance

 hw/arm/aspeed_ast2600.c  |  19 +-
 hw/misc/aspeed_i3c.c | 410 +++
 hw/misc/meson.build  |   1 +
 hw/misc/trace-events |   6 +
 include/hw/arm/aspeed_soc.h  |   3 +
 include/hw/misc/aspeed_i3c.h |  57 +
 6 files changed, 495 insertions(+), 1 deletion(-)
 create mode 100644 hw/misc/aspeed_i3c.c
 create mode 100644 include/hw/misc/aspeed_i3c.h

-- 
2.25.1




Re: [PATCH] Supporting AST2600 HACE engine accumulative mode

2022-01-06 Thread Troy Lee
On Thu, Jan 6, 2022 at 11:27 PM Peter Maydell  wrote:
>
> On Tue, 28 Dec 2021 at 03:34, Troy Lee  wrote:
> >
> > Hi Klaus,
> >
> > On Thu, Dec 23, 2021 at 11:57 PM Klaus Heinrich Kiwi
> >  wrote:
> > >
> > > Em qui., 23 de dez. de 2021 às 09:54, Cédric Le Goater  
> > > escreveu:
> > > >
> > > > [ Adding Klaus ]
> > >
> > > Thanks Cedric. It's been a while since I've looked at this but I'll do my 
> > > best..
> > >
> > > >
> > > > On 12/22/21 03:22, Troy Lee wrote:
>
>
> > > > > +/*
> > > > > + * Read the message length in bit from last 64/128 
> > > > > bits
> > > > > + * and tear the padding bits from iov
> > > > > + */
> > > > > +uint64_t stream_len;
> > > > > +
> > > > > +memcpy(_len, iov[i].iov_base + iov[i].iov_len 
> > > > > - 8, 8);
> > > > > +stream_len = __bswap_64(stream_len) / 8;
> > > > > +
> > >
> > > I no longer have access to the aspeed workbook I guess, but what is
> > > the actual specification here? is the message length 64 or 128 bits?
> > > and bswap seems arch-dependent - you probably want to be explicit if
> > > this is big or little-endian and use the appropriate conversion
> > > function.
> > The message length is described in RFC4634:
> > - SHA224 or SHA256 should be 64-bit.
> > - SHA384 or SHA512 should be 128-bit.
> > And it should be in big-endian.
>
> You can read a 64-bit BE value with
>  uint64_t val = ldq_be_p(iov[i].iov_base + iov[i].iov_len - 8);
> or similar. (We don't have a similar function for 128 bits because
> there's no fully-portable native C data type for that.)
>
> -- PMM

Thanks for the suggestion!
Troy Lee



Re: [PATCH v1 2/2] hw/arm/aspeed_ast2600: create i3c instance

2021-12-28 Thread Troy Lee
On Thu, Dec 23, 2021 at 9:54 PM Cédric Le Goater  wrote:
>
> On 12/22/21 10:23, Troy Lee wrote:
> > This patch includes i3c instance in ast2600 soc.
> >
> > Signed-off-by: Troy Lee 
>
> Looks good but it is based on the QEMU aspeed branch for OpenBMC.
> You should rebase on upstream.
>
> Thanks,
>
> C.
>
Will do.

Thanks,
Troy Lee

> > ---
> >   hw/arm/aspeed_ast2600.c | 12 
> >   include/hw/arm/aspeed_soc.h |  3 +++
> >   2 files changed, 15 insertions(+)
> >
> > diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> > index f2fef9d706..219b025bc2 100644
> > --- a/hw/arm/aspeed_ast2600.c
> > +++ b/hw/arm/aspeed_ast2600.c
> > @@ -63,6 +63,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
> >   [ASPEED_DEV_VUART] = 0x1E787000,
> >   [ASPEED_DEV_FSI1]  = 0x1E79B000,
> >   [ASPEED_DEV_FSI2]  = 0x1E79B100,
> > +[ASPEED_DEV_I3C]   = 0x1E7A,
> >   [ASPEED_DEV_SDRAM] = 0x8000,
> >   };
> >
> > @@ -112,6 +113,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
> >   [ASPEED_DEV_FSI1]  = 100,
> >   [ASPEED_DEV_FSI2]  = 101,
> >   [ASPEED_DEV_DP]= 62,
> > +[ASPEED_DEV_I3C]   = 102,   /* 102 -> 107 */
> >   };
> >
> >   static qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int ctrl)
> > @@ -230,6 +232,8 @@ static void aspeed_soc_ast2600_init(Object *obj)
> >
> >   object_initialize_child(obj, "pwm", >pwm, TYPE_ASPEED_PWM);
> >
> > +object_initialize_child(obj, "i3c", >i3c, TYPE_ASPEED_I3C);
> > +
> >   object_initialize_child(obj, "fsi[*]", >fsi[0], 
> > TYPE_ASPEED_APB2OPB);
> >   }
> >
> > @@ -542,6 +546,14 @@ static void aspeed_soc_ast2600_realize(DeviceState 
> > *dev, Error **errp)
> >   sysbus_connect_irq(SYS_BUS_DEVICE(>pwm), 0,
> >  aspeed_soc_get_irq(s, ASPEED_DEV_PWM));
> >
> > +/* I3C */
> > +if (!sysbus_realize(SYS_BUS_DEVICE(>i3c), errp)) {
> > +return;
> > +}
> > +sysbus_mmio_map(SYS_BUS_DEVICE(>i3c), 0, 
> > sc->memmap[ASPEED_DEV_I3C]);
> > +sysbus_connect_irq(SYS_BUS_DEVICE(>i3c), 0,
> > +   aspeed_soc_get_irq(s, ASPEED_DEV_I3C));
> > +
> >   /* FSI */
> >   if (!sysbus_realize(SYS_BUS_DEVICE(>fsi[0]), errp)) {
> >   return;
> > diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> > index 0db200d813..0c950fab3c 100644
> > --- a/include/hw/arm/aspeed_soc.h
> > +++ b/include/hw/arm/aspeed_soc.h
> > @@ -21,6 +21,7 @@
> >   #include "hw/timer/aspeed_timer.h"
> >   #include "hw/rtc/aspeed_rtc.h"
> >   #include "hw/i2c/aspeed_i2c.h"
> > +#include "hw/misc/aspeed_i3c.h"
> >   #include "hw/ssi/aspeed_smc.h"
> >   #include "hw/misc/aspeed_hace.h"
> >   #include "hw/watchdog/wdt_aspeed.h"
> > @@ -53,6 +54,7 @@ struct AspeedSoCState {
> >   AspeedRtcState rtc;
> >   AspeedTimerCtrlState timerctrl;
> >   AspeedI2CState i2c;
> > +AspeedI3CState i3c;
> >   AspeedSCUState scu;
> >   AspeedHACEState hace;
> >   AspeedXDMAState xdma;
> > @@ -148,6 +150,7 @@ enum {
> >   ASPEED_DEV_FSI2,
> >   ASPEED_DEV_DPMCU,
> >   ASPEED_DEV_DP,
> > +ASPEED_DEV_I3C,
> >   };
> >
> >   #endif /* ASPEED_SOC_H */
> >
>



Re: [PATCH v1 1/2] hw/misc: Implementating dummy AST2600 I3C model

2021-12-28 Thread Troy Lee
Hi,
On Thu, Dec 23, 2021 at 9:48 PM Cédric Le Goater  wrote:
>
>
> Hello,
>
> On 12/22/21 10:23, Troy Lee wrote:
> > Introduce a dummy AST2600 I3C model.
> >
> > Aspeed 2600 SDK enables I3C support by default.  The I3C driver will try
> > to reset the device controller and setup through device address table
> > register.  This dummy model response these register with default value
> > listed on ast2600v10 datasheet chapter 54.2.  If the device address
> > table register doesn't set correctly, it will cause guest machine kernel
> > panic due to reference to invalid address.
>
> Overall looks good. Some comments,
>
> >
> > Signed-off-by: Troy Lee 
> > ---
> >   hw/misc/aspeed_i3c.c | 258 +++
> >   hw/misc/meson.build  |   1 +
> >   include/hw/misc/aspeed_i3c.h |  30 
> >   3 files changed, 289 insertions(+)
> >   create mode 100644 hw/misc/aspeed_i3c.c
> >   create mode 100644 include/hw/misc/aspeed_i3c.h
> >
> > diff --git a/hw/misc/aspeed_i3c.c b/hw/misc/aspeed_i3c.c
> > new file mode 100644
> > index 00..9d2bda203e
> > --- /dev/null
> > +++ b/hw/misc/aspeed_i3c.c
> > @@ -0,0 +1,258 @@
> > +/*
> > + * ASPEED I3C Controller
> > + *
> > + * Copyright (C) 2021 ASPEED Technology Inc.
> > + *
> > + * This code is licensed under the GPL version 2 or later.  See
> > + * the COPYING file in the top-level directory.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/log.h"
> > +#include "qemu/error-report.h"
> > +#include "hw/misc/aspeed_i3c.h"
> > +#include "qapi/error.h"
> > +#include "migration/vmstate.h"
> > +
> > +/* I3C Controller Registers */
> > +#define R_I3CG_REG0(x)  (((x * 0x10) + 0x10) / 4)
> > +#define  I3CG_REG0_SDA_PULLUP_EN_MASK   GENMASK(29, 28)
>
> GENMASK() is a macro defined in the FSI model which is not upstream.
> There are other ways to define bitfield masks in QEMU. Please take a
> look at include/hw/registerfields.h.
Got it. I found some reference code with registerfields, I'll improve
in next patch.

> > +#define  I3CG_REG0_SDA_PULLUP_EN_2K BIT(28)
> > +#define  I3CG_REG0_SDA_PULLUP_EN_750BIT(29)
> > +#define  I3CG_REG0_SDA_PULLUP_EN_545(BIT(29) | BIT(28))
> > +
> > +#define R_I3CG_REG1(x)  (((x * 0x10) + 0x14) / 4)
> > +#define  I3CG_REG1_I2C_MODE BIT(0)
> > +#define  I3CG_REG1_TEST_MODEBIT(1)
> > +#define  I3CG_REG1_ACT_MODE_MASKGENMASK(3, 2)
> > +#define  I3CG_REG1_ACT_MODE(x)  (((x) << 2) & 
> > I3CG_REG1_ACT_MODE_MASK)
> > +#define  I3CG_REG1_PENDING_INT_MASK GENMASK(7, 4)
> > +#define  I3CG_REG1_PENDING_INT(x)   (((x) << 4) & 
> > I3CG_REG1_PENDING_INT_MASK)
> > +#define  I3CG_REG1_SA_MASK  GENMASK(14, 8)
> > +#define  I3CG_REG1_SA(x)(((x) << 8) & I3CG_REG1_SA_MASK)
> > +#define  I3CG_REG1_SA_ENBIT(15)
> > +#define  I3CG_REG1_INST_ID_MASK GENMASK(19, 16)
> > +#define  I3CG_REG1_INST_ID(x)   (((x) << 16) & 
> > I3CG_REG1_INST_ID_MASK)
> > +
> > +/* I3C Device Registers */
> > +#define R_DEVICE_CTRL   (0x00 / 4)
> > +#define R_DEVICE_ADDR   (0x04 / 4)
> > +#define R_HW_CAPABILITY (0x08 / 4)
> > +#define R_COMMAND_QUEUE_PORT(0x0c / 4)
> > +#define R_RESPONSE_QUEUE_PORT   (0x10 / 4)
> > +#define R_RX_TX_DATA_PORT   (0x14 / 4)
> > +#define R_IBI_QUEUE_STATUS  (0x18 / 4)
> > +#define R_IBI_QUEUE_DATA(0x18 / 4)
> > +#define R_QUEUE_THLD_CTRL   (0x1c / 4)
> > +#define R_DATA_BUFFER_THLD_CTRL (0x20 / 4)
> > +#define R_IBI_QUEUE_CTRL(0x24 / 4)
> > +#define R_IBI_MR_REQ_REJECT (0x2c / 4)
> > +#define R_IBI_SIR_REQ_REJECT(0x30 / 4)
> > +#define R_RESET_CTRL(0x34 / 4)
> > +#define R_SLV_EVENT_CTRL(0x38 / 4)
> > +#define R_INTR_STATUS   (0x3c / 4)
> > +#define R_INTR_STATUS_EN(0x40 / 4)
> > +#define R_INTR_SIGNAL_EN(0x44 / 4)
> > +#define R_INTR_FORCE(0x48 / 4)
> > +#define R_QUEUE_STATUS_LEVEL(0x4c / 4)
> > +#define R_DATA_BUFFER_STATUS_LEVEL  (0x50 / 4)
> > +#define R_PRESENT_STATE (0x54 / 4)
> > +#define R_CCC_DEVICE_STATUS

Re: [PATCH] Supporting AST2600 HACE engine accumulative mode

2021-12-27 Thread Troy Lee
Hi Klaus,

On Thu, Dec 23, 2021 at 11:57 PM Klaus Heinrich Kiwi
 wrote:
>
> Em qui., 23 de dez. de 2021 às 09:54, Cédric Le Goater  
> escreveu:
> >
> > [ Adding Klaus ]
>
> Thanks Cedric. It's been a while since I've looked at this but I'll do my 
> best..
>
> >
> > On 12/22/21 03:22, Troy Lee wrote:
> > > Accumulative mode will supply a initial state and append padding bit at
> > > the end of hash stream.  However, the crypto library will padding those
> > > bit automatically, so ripped it off from iov array.
> > >
> > > Signed-off-by: Troy Lee 
> > > ---
> > >   hw/misc/aspeed_hace.c | 30 --
> > >   include/hw/misc/aspeed_hace.h |  1 +
> > >   2 files changed, 29 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
> > > index 10f00e65f4..7c1794d6d0 100644
> > > --- a/hw/misc/aspeed_hace.c
> > > +++ b/hw/misc/aspeed_hace.c
> > > @@ -27,6 +27,7 @@
> > >
> > >   #define R_HASH_SRC  (0x20 / 4)
> > >   #define R_HASH_DEST (0x24 / 4)
> > > +#define R_HASH_KEY_BUFF (0x28 / 4)
> > >   #define R_HASH_SRC_LEN  (0x2c / 4)
> > >
> > >   #define R_HASH_CMD  (0x30 / 4)
> > > @@ -94,7 +95,10 @@ static int hash_algo_lookup(uint32_t reg)
> > >   return -1;
> > >   }
> > >
> > > -static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode)
> > > +static void do_hash_operation(AspeedHACEState *s,
> > > +  int algo,
> > > +  bool sg_mode,
> > > +  bool acc_mode)
> > >   {
> > >   struct iovec iov[ASPEED_HACE_MAX_SG];
> > >   g_autofree uint8_t *digest_buf;
> > > @@ -103,6 +107,7 @@ static void do_hash_operation(AspeedHACEState *s, int 
> > > algo, bool sg_mode)
> > >
> > >   if (sg_mode) {
> > >   uint32_t len = 0;
> > > +uint32_t total_len = 0;
> > >
> > >   for (i = 0; !(len & SG_LIST_LEN_LAST); i++) {
> > >   uint32_t addr, src;
> > > @@ -127,6 +132,21 @@ static void do_hash_operation(AspeedHACEState *s, 
> > > int algo, bool sg_mode)
> > >   plen = iov[i].iov_len;
> > >   iov[i].iov_base = address_space_map(>dram_as, addr, 
> > > , false,
> > >   MEMTXATTRS_UNSPECIFIED);
> > > +
> > > +total_len += plen;
> > > +if (acc_mode && len & SG_LIST_LEN_LAST) {
> I think we should make the precedence here explicit (with the use of
> ()) instead of implicit. Also, isn't (len * SGL_LIST_LEN_LAST) always
> true inside this for loop?
I'll change it to have explicit precedence.
No, the *len* will be assigned after goes into the loop, and the
SG_LAST_LEN_LAST will be raise at the last of scatter-gather list.

> > > +/*
> > > + * Read the message length in bit from last 64/128 bits
> > > + * and tear the padding bits from iov
> > > + */
> > > +uint64_t stream_len;
> > > +
> > > +memcpy(_len, iov[i].iov_base + iov[i].iov_len - 
> > > 8, 8);
> > > +stream_len = __bswap_64(stream_len) / 8;
> > > +
>
> I no longer have access to the aspeed workbook I guess, but what is
> the actual specification here? is the message length 64 or 128 bits?
> and bswap seems arch-dependent - you probably want to be explicit if
> this is big or little-endian and use the appropriate conversion
> function.
The message length is described in RFC4634:
- SHA224 or SHA256 should be 64-bit.
- SHA384 or SHA512 should be 128-bit.
And it should be in big-endian.

The aspeed ast2600 acculumative mode is described in datasheet
ast2600v10.pdf section 25.6.4:
1. Allocationg and initiating accumulative hash digest write buffer
with initial state.
   * Since QEMU crypto/hash api doesn't provide the API to set initial
state of hash library,
 and the initial state is already setted by crypto library
(gcrypt/glib/...), so skip this step.
2. Calculating accumulative hash digest.
   (a) When receiving the last accumulative data, software need to add
padding message
   at the end of the accumulative data. Padding message described
in specific of MD5,
   SHA-1, SHA224, SHA256, SHA512, SHA512/224, SHA512/256.
   * Since the crypto library (gcrypt/glib) already pad the
padding message

[PATCH qemu master] hw/misc/aspeed_pwm: fix typo

2021-12-22 Thread Troy Lee
Typo found during developing.

Fixes: 70b3f1a34d3c ("hw/misc: Add basic Aspeed PWM model")
Signed-off-by: Troy Lee 
---
 hw/misc/aspeed_pwm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/misc/aspeed_pwm.c b/hw/misc/aspeed_pwm.c
index 8ebab5dcef..dbf9634da3 100644
--- a/hw/misc/aspeed_pwm.c
+++ b/hw/misc/aspeed_pwm.c
@@ -96,7 +96,7 @@ static void aspeed_pwm_class_init(ObjectClass *klass, void 
*data)
 
 dc->realize = aspeed_pwm_realize;
 dc->reset = aspeed_pwm_reset;
-dc->desc = "Aspeed PWM Controller",
+dc->desc = "Aspeed PWM Controller";
 dc->vmsd = _aspeed_pwm;
 }
 
-- 
2.25.1




[PATCH v1 1/2] hw/misc: Implementating dummy AST2600 I3C model

2021-12-22 Thread Troy Lee
Introduce a dummy AST2600 I3C model.

Aspeed 2600 SDK enables I3C support by default.  The I3C driver will try
to reset the device controller and setup through device address table
register.  This dummy model response these register with default value
listed on ast2600v10 datasheet chapter 54.2.  If the device address
table register doesn't set correctly, it will cause guest machine kernel
panic due to reference to invalid address.

Signed-off-by: Troy Lee 
---
 hw/misc/aspeed_i3c.c | 258 +++
 hw/misc/meson.build  |   1 +
 include/hw/misc/aspeed_i3c.h |  30 
 3 files changed, 289 insertions(+)
 create mode 100644 hw/misc/aspeed_i3c.c
 create mode 100644 include/hw/misc/aspeed_i3c.h

diff --git a/hw/misc/aspeed_i3c.c b/hw/misc/aspeed_i3c.c
new file mode 100644
index 00..9d2bda203e
--- /dev/null
+++ b/hw/misc/aspeed_i3c.c
@@ -0,0 +1,258 @@
+/*
+ * ASPEED I3C Controller
+ *
+ * Copyright (C) 2021 ASPEED Technology Inc.
+ *
+ * This code is licensed under the GPL version 2 or later.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/error-report.h"
+#include "hw/misc/aspeed_i3c.h"
+#include "qapi/error.h"
+#include "migration/vmstate.h"
+
+/* I3C Controller Registers */
+#define R_I3CG_REG0(x)  (((x * 0x10) + 0x10) / 4)
+#define  I3CG_REG0_SDA_PULLUP_EN_MASK   GENMASK(29, 28)
+#define  I3CG_REG0_SDA_PULLUP_EN_2K BIT(28)
+#define  I3CG_REG0_SDA_PULLUP_EN_750BIT(29)
+#define  I3CG_REG0_SDA_PULLUP_EN_545(BIT(29) | BIT(28))
+
+#define R_I3CG_REG1(x)  (((x * 0x10) + 0x14) / 4)
+#define  I3CG_REG1_I2C_MODE BIT(0)
+#define  I3CG_REG1_TEST_MODEBIT(1)
+#define  I3CG_REG1_ACT_MODE_MASKGENMASK(3, 2)
+#define  I3CG_REG1_ACT_MODE(x)  (((x) << 2) & I3CG_REG1_ACT_MODE_MASK)
+#define  I3CG_REG1_PENDING_INT_MASK GENMASK(7, 4)
+#define  I3CG_REG1_PENDING_INT(x)   (((x) << 4) & I3CG_REG1_PENDING_INT_MASK)
+#define  I3CG_REG1_SA_MASK  GENMASK(14, 8)
+#define  I3CG_REG1_SA(x)(((x) << 8) & I3CG_REG1_SA_MASK)
+#define  I3CG_REG1_SA_ENBIT(15)
+#define  I3CG_REG1_INST_ID_MASK GENMASK(19, 16)
+#define  I3CG_REG1_INST_ID(x)   (((x) << 16) & I3CG_REG1_INST_ID_MASK)
+
+/* I3C Device Registers */
+#define R_DEVICE_CTRL   (0x00 / 4)
+#define R_DEVICE_ADDR   (0x04 / 4)
+#define R_HW_CAPABILITY (0x08 / 4)
+#define R_COMMAND_QUEUE_PORT(0x0c / 4)
+#define R_RESPONSE_QUEUE_PORT   (0x10 / 4)
+#define R_RX_TX_DATA_PORT   (0x14 / 4)
+#define R_IBI_QUEUE_STATUS  (0x18 / 4)
+#define R_IBI_QUEUE_DATA(0x18 / 4)
+#define R_QUEUE_THLD_CTRL   (0x1c / 4)
+#define R_DATA_BUFFER_THLD_CTRL (0x20 / 4)
+#define R_IBI_QUEUE_CTRL(0x24 / 4)
+#define R_IBI_MR_REQ_REJECT (0x2c / 4)
+#define R_IBI_SIR_REQ_REJECT(0x30 / 4)
+#define R_RESET_CTRL(0x34 / 4)
+#define R_SLV_EVENT_CTRL(0x38 / 4)
+#define R_INTR_STATUS   (0x3c / 4)
+#define R_INTR_STATUS_EN(0x40 / 4)
+#define R_INTR_SIGNAL_EN(0x44 / 4)
+#define R_INTR_FORCE(0x48 / 4)
+#define R_QUEUE_STATUS_LEVEL(0x4c / 4)
+#define R_DATA_BUFFER_STATUS_LEVEL  (0x50 / 4)
+#define R_PRESENT_STATE (0x54 / 4)
+#define R_CCC_DEVICE_STATUS (0x58 / 4)
+#define R_DEVICE_ADDR_TABLE_POINTER (0x5c / 4)
+#define  DEVICE_ADDR_TABLE_DEPTH(x) (((x) & GENMASK(31, 16)) >> 16)
+#define  DEVICE_ADDR_TABLE_ADDR(x)  ((x) & GENMASK(7, 0))
+#define R_DEV_CHAR_TABLE_POINTER(0x60 / 4)
+#define R_VENDOR_SPECIFIC_REG_POINTER   (0x6c / 4)
+#define R_SLV_MIPI_PID_VALUE(0x70 / 4)
+#define R_SLV_PID_VALUE (0x74 / 4)
+#define R_SLV_CHAR_CTRL (0x78 / 4)
+#define R_SLV_MAX_LEN   (0x7c / 4)
+#define R_MAX_READ_TURNAROUND   (0x80 / 4)
+#define R_MAX_DATA_SPEED(0x84 / 4)
+#define R_SLV_DEBUG_STATUS  (0x88 / 4)
+#define R_SLV_INTR_REQ  (0x8c / 4)
+#define R_DEVICE_CTRL_EXTENDED  (0xb0 / 4)
+#define R_SCL_I3C_OD_TIMING (0xb4 / 4)
+#define R_SCL_I3C_PP_TIMING (0xb8 / 4)
+#define R_SCL_I2C_FM_TIMING (0xbc / 4)
+#define R_SCL_I2C_FMP_TIMING(0xc0 / 4)
+#define R_SCL_EXT_LCNT_TIMING   (0xc8 / 4)
+#define R_SCL_EXT_TERMN_LCNT_TIMING (0xcc / 4)
+#define R_BUS_FREE_TIMING   (0xd4 / 4)
+#define R_BUS_IDLE_TIMING   (0xd8 / 4)
+#define R_I3C_VER_ID(0xe0 / 4)
+#define R_I3C_VER_TYPE  (0xe4 / 4)
+#define R_EXTENDED_CAPA

[PATCH v1 2/2] hw/arm/aspeed_ast2600: create i3c instance

2021-12-22 Thread Troy Lee
This patch includes i3c instance in ast2600 soc.

Signed-off-by: Troy Lee 
---
 hw/arm/aspeed_ast2600.c | 12 
 include/hw/arm/aspeed_soc.h |  3 +++
 2 files changed, 15 insertions(+)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index f2fef9d706..219b025bc2 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -63,6 +63,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
 [ASPEED_DEV_VUART] = 0x1E787000,
 [ASPEED_DEV_FSI1]  = 0x1E79B000,
 [ASPEED_DEV_FSI2]  = 0x1E79B100,
+[ASPEED_DEV_I3C]   = 0x1E7A,
 [ASPEED_DEV_SDRAM] = 0x8000,
 };
 
@@ -112,6 +113,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
 [ASPEED_DEV_FSI1]  = 100,
 [ASPEED_DEV_FSI2]  = 101,
 [ASPEED_DEV_DP]= 62,
+[ASPEED_DEV_I3C]   = 102,   /* 102 -> 107 */
 };
 
 static qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int ctrl)
@@ -230,6 +232,8 @@ static void aspeed_soc_ast2600_init(Object *obj)
 
 object_initialize_child(obj, "pwm", >pwm, TYPE_ASPEED_PWM);
 
+object_initialize_child(obj, "i3c", >i3c, TYPE_ASPEED_I3C);
+
 object_initialize_child(obj, "fsi[*]", >fsi[0], TYPE_ASPEED_APB2OPB);
 }
 
@@ -542,6 +546,14 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
 sysbus_connect_irq(SYS_BUS_DEVICE(>pwm), 0,
aspeed_soc_get_irq(s, ASPEED_DEV_PWM));
 
+/* I3C */
+if (!sysbus_realize(SYS_BUS_DEVICE(>i3c), errp)) {
+return;
+}
+sysbus_mmio_map(SYS_BUS_DEVICE(>i3c), 0, sc->memmap[ASPEED_DEV_I3C]);
+sysbus_connect_irq(SYS_BUS_DEVICE(>i3c), 0,
+   aspeed_soc_get_irq(s, ASPEED_DEV_I3C));
+
 /* FSI */
 if (!sysbus_realize(SYS_BUS_DEVICE(>fsi[0]), errp)) {
 return;
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 0db200d813..0c950fab3c 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -21,6 +21,7 @@
 #include "hw/timer/aspeed_timer.h"
 #include "hw/rtc/aspeed_rtc.h"
 #include "hw/i2c/aspeed_i2c.h"
+#include "hw/misc/aspeed_i3c.h"
 #include "hw/ssi/aspeed_smc.h"
 #include "hw/misc/aspeed_hace.h"
 #include "hw/watchdog/wdt_aspeed.h"
@@ -53,6 +54,7 @@ struct AspeedSoCState {
 AspeedRtcState rtc;
 AspeedTimerCtrlState timerctrl;
 AspeedI2CState i2c;
+AspeedI3CState i3c;
 AspeedSCUState scu;
 AspeedHACEState hace;
 AspeedXDMAState xdma;
@@ -148,6 +150,7 @@ enum {
 ASPEED_DEV_FSI2,
 ASPEED_DEV_DPMCU,
 ASPEED_DEV_DP,
+ASPEED_DEV_I3C,
 };
 
 #endif /* ASPEED_SOC_H */
-- 
2.25.1




[PATCH v1 0/2] Aspeed I3C device model

2021-12-22 Thread Troy Lee
This series of patch introduce a dummy implemenation of aspeed i3c
model, and it provide just enough information for guest machine.
However, the driver probing is still failed, but it will not cause
kernel panic.

Troy Lee (2):
  hw/misc: Implementating dummy AST2600 I3C model
  hw/arm/aspeed_ast2600: create i3c instance

 hw/arm/aspeed_ast2600.c  |  12 ++
 hw/misc/aspeed_i3c.c | 258 +++
 hw/misc/meson.build  |   1 +
 include/hw/arm/aspeed_soc.h  |   3 +
 include/hw/misc/aspeed_i3c.h |  30 
 5 files changed, 304 insertions(+)
 create mode 100644 hw/misc/aspeed_i3c.c
 create mode 100644 include/hw/misc/aspeed_i3c.h

-- 
2.25.1




[PATCH] Supporting AST2600 HACE engine accumulative mode

2021-12-21 Thread Troy Lee
Accumulative mode will supply a initial state and append padding bit at
the end of hash stream.  However, the crypto library will padding those
bit automatically, so ripped it off from iov array.

Signed-off-by: Troy Lee 
---
 hw/misc/aspeed_hace.c | 30 --
 include/hw/misc/aspeed_hace.h |  1 +
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 10f00e65f4..7c1794d6d0 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -27,6 +27,7 @@
 
 #define R_HASH_SRC  (0x20 / 4)
 #define R_HASH_DEST (0x24 / 4)
+#define R_HASH_KEY_BUFF (0x28 / 4)
 #define R_HASH_SRC_LEN  (0x2c / 4)
 
 #define R_HASH_CMD  (0x30 / 4)
@@ -94,7 +95,10 @@ static int hash_algo_lookup(uint32_t reg)
 return -1;
 }
 
-static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode)
+static void do_hash_operation(AspeedHACEState *s,
+  int algo,
+  bool sg_mode,
+  bool acc_mode)
 {
 struct iovec iov[ASPEED_HACE_MAX_SG];
 g_autofree uint8_t *digest_buf;
@@ -103,6 +107,7 @@ static void do_hash_operation(AspeedHACEState *s, int algo, 
bool sg_mode)
 
 if (sg_mode) {
 uint32_t len = 0;
+uint32_t total_len = 0;
 
 for (i = 0; !(len & SG_LIST_LEN_LAST); i++) {
 uint32_t addr, src;
@@ -127,6 +132,21 @@ static void do_hash_operation(AspeedHACEState *s, int 
algo, bool sg_mode)
 plen = iov[i].iov_len;
 iov[i].iov_base = address_space_map(>dram_as, addr, , 
false,
 MEMTXATTRS_UNSPECIFIED);
+
+total_len += plen;
+if (acc_mode && len & SG_LIST_LEN_LAST) {
+/*
+ * Read the message length in bit from last 64/128 bits
+ * and tear the padding bits from iov
+ */
+uint64_t stream_len;
+
+memcpy(_len, iov[i].iov_base + iov[i].iov_len - 8, 8);
+stream_len = __bswap_64(stream_len) / 8;
+
+if (total_len > stream_len)
+iov[i].iov_len -= total_len - stream_len;
+}
 }
 } else {
 hwaddr len = s->regs[R_HASH_SRC_LEN];
@@ -210,6 +230,9 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, 
uint64_t data,
 case R_HASH_DEST:
 data &= ahc->dest_mask;
 break;
+case R_HASH_KEY_BUFF:
+data &= ahc->key_mask;
+break;
 case R_HASH_SRC_LEN:
 data &= 0x0FFF;
 break;
@@ -234,7 +257,7 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, 
uint64_t data,
 __func__, data & ahc->hash_mask);
 break;
 }
-do_hash_operation(s, algo, data & HASH_SG_EN);
+do_hash_operation(s, algo, data & HASH_SG_EN, data & 
HASH_DIGEST_ACCUM);
 
 if (data & HASH_IRQ_EN) {
 qemu_irq_raise(s->irq);
@@ -333,6 +356,7 @@ static void aspeed_ast2400_hace_class_init(ObjectClass 
*klass, void *data)
 
 ahc->src_mask = 0x0FFF;
 ahc->dest_mask = 0x0FF8;
+ahc->key_mask = 0x0FC0;
 ahc->hash_mask = 0x03ff; /* No SG or SHA512 modes */
 }
 
@@ -351,6 +375,7 @@ static void aspeed_ast2500_hace_class_init(ObjectClass 
*klass, void *data)
 
 ahc->src_mask = 0x3fff;
 ahc->dest_mask = 0x3ff8;
+ahc->key_mask = 0x3FC0;
 ahc->hash_mask = 0x03ff; /* No SG or SHA512 modes */
 }
 
@@ -369,6 +394,7 @@ static void aspeed_ast2600_hace_class_init(ObjectClass 
*klass, void *data)
 
 ahc->src_mask = 0x7FFF;
 ahc->dest_mask = 0x7FF8;
+ahc->key_mask = 0x7FF8;
 ahc->hash_mask = 0x00147FFF;
 }
 
diff --git a/include/hw/misc/aspeed_hace.h b/include/hw/misc/aspeed_hace.h
index 94d5ada95f..2242945eb4 100644
--- a/include/hw/misc/aspeed_hace.h
+++ b/include/hw/misc/aspeed_hace.h
@@ -37,6 +37,7 @@ struct AspeedHACEClass {
 
 uint32_t src_mask;
 uint32_t dest_mask;
+uint32_t key_mask;
 uint32_t hash_mask;
 };
 
-- 
2.25.1




Re: [PATCH v1] Add dummy Aspeed AST2600 Display Port MCU (DPMCU)

2021-12-13 Thread Troy Lee
On Fri, Dec 10, 2021 at 11:13 PM Cédric Le Goater  wrote:
>
> On 12/10/21 15:33, Troy Lee wrote:
> > On Fri, Dec 10, 2021 at 10:05 PM Cédric Le Goater  wrote:
> >>
> >> On 12/10/21 09:30, Troy Lee wrote:
> >>> AST2600 Display Port MCU introduces 0x1800~0x1803 as it's memory
> >>> and io address. If guest machine try to access DPMCU memory, it will
> >>> cause a fatal error.
> >>
> >> The Aspeed SoCs have an "aspeed_soc.io" region for unimplemented devices
> >> but it's too small. Anyhow, it is better to have per logic unit. We should
> >> change that one day.
> >>
> > Good idea!
> >
> >> For my information, which FW image are you using ?
> >>
> >
> > We're using Aspeed's SDK image, I tested with ast2600-default machine.
> > Prebuilt image can be download from:
> > https://github.com/AspeedTech-BMC/openbmc/releases/tag/v07.02
>
> Excellent ! Is there one I could try in particular ?
You could use ast2600-default-obmc.tgz, but we will send another patch
for HACE engine.

>
>
> Once correctly supported, we should include one of these SDK images in :
>
>tests/avocado/boot_linux_console.py
>
> to complete our tests of the device models.
Sure, once the image successfully boots into rootfs, I'll add a test
case for it.

> QEMU is not making much difference between the revision. You might need
> to improve that.
>
> > Without declaring the DPMCU memory, the image will hangs in u-boot.
>
> yeah. You can use -d guest_errors,unimp to catch accesses done on AHB
> windows not covered by the QEMU models. There are plenty of ways to
> move past U-Boot when models are not implemented yet. Don't waste
> too much time, just ask.
>
> eMMC is only on these branches :
>
>https://github.com/openbmc/qemu/
>https://github.com/legoater/qemu/
These two branches are very useful!

> Same for SBC and support is primitive.
>
> > We're still working on I3C and SPI issue to be resolved to get into rootfs.
>
> I3C has not much support in Linux and none in QEMU. You will have to
> add dummy models.
Can I submit a dummy model that only responds to the RESET control register?
Or it has to be well implemented like i2c/core.c and i2c/aspeed_i2c.c?


> SPI as a non-SPI flash driver ? The SPI flash controller models should
> be quite well covered today. What's the issue ?
I need some more investigation, because we're using a different spi
driver for fmc-spi.

>
> Thanks,
>
> C.

Thanks,
Troy Lee



Re: [PATCH v1] Add dummy Aspeed AST2600 Display Port MCU (DPMCU)

2021-12-10 Thread Troy Lee
On Fri, Dec 10, 2021 at 10:05 PM Cédric Le Goater  wrote:
>
> On 12/10/21 09:30, Troy Lee wrote:
> > AST2600 Display Port MCU introduces 0x1800~0x1803 as it's memory
> > and io address. If guest machine try to access DPMCU memory, it will
> > cause a fatal error.
>
> The Aspeed SoCs have an "aspeed_soc.io" region for unimplemented devices
> but it's too small. Anyhow, it is better to have per logic unit. We should
> change that one day.
>
Good idea!

> For my information, which FW image are you using ?
>

We're using Aspeed's SDK image, I tested with ast2600-default machine.
Prebuilt image can be download from:
https://github.com/AspeedTech-BMC/openbmc/releases/tag/v07.02

Without declaring the DPMCU memory, the image will hangs in u-boot.
We're still working on I3C and SPI issue to be resolved to get into rootfs.

Thanks for your review.
Troy Lee

> > Signed-off-by: Troy Lee 
>
> Reviewed-by: Cédric Le Goater 
>
> Thanks,
>
> C.
>
> > ---
> >   hw/arm/aspeed_ast2600.c | 8 
> >   include/hw/arm/aspeed_soc.h | 2 ++
> >   2 files changed, 10 insertions(+)
> >
> > diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> > index 0384357a95..e33483fb5d 100644
> > --- a/hw/arm/aspeed_ast2600.c
> > +++ b/hw/arm/aspeed_ast2600.c
> > @@ -19,9 +19,11 @@
> >   #include "sysemu/sysemu.h"
> >
> >   #define ASPEED_SOC_IOMEM_SIZE   0x0020
> > +#define ASPEED_SOC_DPMCU_SIZE   0x0004
> >
> >   static const hwaddr aspeed_soc_ast2600_memmap[] = {
> >   [ASPEED_DEV_SRAM]  = 0x1000,
> > +[ASPEED_DEV_DPMCU] = 0x1800,
> >   /* 0x1600 0x17FF : AHB BUS do LPC Bus bridge */
> >   [ASPEED_DEV_IOMEM] = 0x1E60,
> >   [ASPEED_DEV_PWM]   = 0x1E61,
> > @@ -44,6 +46,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
> >   [ASPEED_DEV_SCU]   = 0x1E6E2000,
> >   [ASPEED_DEV_XDMA]  = 0x1E6E7000,
> >   [ASPEED_DEV_ADC]   = 0x1E6E9000,
> > +[ASPEED_DEV_DP]= 0x1E6EB000,
> >   [ASPEED_DEV_VIDEO] = 0x1E70,
> >   [ASPEED_DEV_SDHCI] = 0x1E74,
> >   [ASPEED_DEV_EMMC]  = 0x1E75,
> > @@ -104,6 +107,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
> >   [ASPEED_DEV_ETH3]  = 32,
> >   [ASPEED_DEV_ETH4]  = 33,
> >   [ASPEED_DEV_KCS]   = 138,   /* 138 -> 142 */
> > +[ASPEED_DEV_DP]= 62,
> >   };
> >
> >   static qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int ctrl)
> > @@ -298,6 +302,10 @@ static void aspeed_soc_ast2600_realize(DeviceState 
> > *dev, Error **errp)
> >   memory_region_add_subregion(get_system_memory(),
> >   sc->memmap[ASPEED_DEV_SRAM], >sram);
> >
> > +/* DPMCU */
> > +create_unimplemented_device("aspeed.dpmcu", 
> > sc->memmap[ASPEED_DEV_DPMCU],
> > +ASPEED_SOC_DPMCU_SIZE);
> > +
> >   /* SCU */
> >   if (!sysbus_realize(SYS_BUS_DEVICE(>scu), errp)) {
> >   return;
> > diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> > index 8139358549..18fb7eed46 100644
> > --- a/include/hw/arm/aspeed_soc.h
> > +++ b/include/hw/arm/aspeed_soc.h
> > @@ -139,6 +139,8 @@ enum {
> >   ASPEED_DEV_EMMC,
> >   ASPEED_DEV_KCS,
> >   ASPEED_DEV_HACE,
> > +ASPEED_DEV_DPMCU,
> > +ASPEED_DEV_DP,
> >   };
> >
> >   #endif /* ASPEED_SOC_H */
> >
>



[PATCH v1] Add dummy Aspeed AST2600 Display Port MCU (DPMCU)

2021-12-10 Thread Troy Lee
AST2600 Display Port MCU introduces 0x1800~0x1803 as it's memory
and io address. If guest machine try to access DPMCU memory, it will
cause a fatal error.

Signed-off-by: Troy Lee 
---
 hw/arm/aspeed_ast2600.c | 8 
 include/hw/arm/aspeed_soc.h | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 0384357a95..e33483fb5d 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -19,9 +19,11 @@
 #include "sysemu/sysemu.h"
 
 #define ASPEED_SOC_IOMEM_SIZE   0x0020
+#define ASPEED_SOC_DPMCU_SIZE   0x0004
 
 static const hwaddr aspeed_soc_ast2600_memmap[] = {
 [ASPEED_DEV_SRAM]  = 0x1000,
+[ASPEED_DEV_DPMCU] = 0x1800,
 /* 0x1600 0x17FF : AHB BUS do LPC Bus bridge */
 [ASPEED_DEV_IOMEM] = 0x1E60,
 [ASPEED_DEV_PWM]   = 0x1E61,
@@ -44,6 +46,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
 [ASPEED_DEV_SCU]   = 0x1E6E2000,
 [ASPEED_DEV_XDMA]  = 0x1E6E7000,
 [ASPEED_DEV_ADC]   = 0x1E6E9000,
+[ASPEED_DEV_DP]= 0x1E6EB000,
 [ASPEED_DEV_VIDEO] = 0x1E70,
 [ASPEED_DEV_SDHCI] = 0x1E74,
 [ASPEED_DEV_EMMC]  = 0x1E75,
@@ -104,6 +107,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
 [ASPEED_DEV_ETH3]  = 32,
 [ASPEED_DEV_ETH4]  = 33,
 [ASPEED_DEV_KCS]   = 138,   /* 138 -> 142 */
+[ASPEED_DEV_DP]= 62,
 };
 
 static qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int ctrl)
@@ -298,6 +302,10 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
 memory_region_add_subregion(get_system_memory(),
 sc->memmap[ASPEED_DEV_SRAM], >sram);
 
+/* DPMCU */
+create_unimplemented_device("aspeed.dpmcu", sc->memmap[ASPEED_DEV_DPMCU],
+ASPEED_SOC_DPMCU_SIZE);
+
 /* SCU */
 if (!sysbus_realize(SYS_BUS_DEVICE(>scu), errp)) {
 return;
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 8139358549..18fb7eed46 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -139,6 +139,8 @@ enum {
 ASPEED_DEV_EMMC,
 ASPEED_DEV_KCS,
 ASPEED_DEV_HACE,
+ASPEED_DEV_DPMCU,
+ASPEED_DEV_DP,
 };
 
 #endif /* ASPEED_SOC_H */
-- 
2.25.1




[PATCH v1] Add dummy Aspeed AST2600 Display Port MCU (DPMCU)

2021-12-10 Thread Troy Lee
AST2600 Display Port MCU introduces 0x1800~0x1803 as it's memory
and io address. If guest machine try to access DPMCU memory, it will
cause a fatal error.

Signed-off-by: Troy Lee 
---
 hw/arm/aspeed_ast2600.c | 8 
 include/hw/arm/aspeed_soc.h | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 0384357a95..e33483fb5d 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -19,9 +19,11 @@
 #include "sysemu/sysemu.h"
 
 #define ASPEED_SOC_IOMEM_SIZE   0x0020
+#define ASPEED_SOC_DPMCU_SIZE   0x0004
 
 static const hwaddr aspeed_soc_ast2600_memmap[] = {
 [ASPEED_DEV_SRAM]  = 0x1000,
+[ASPEED_DEV_DPMCU] = 0x1800,
 /* 0x1600 0x17FF : AHB BUS do LPC Bus bridge */
 [ASPEED_DEV_IOMEM] = 0x1E60,
 [ASPEED_DEV_PWM]   = 0x1E61,
@@ -44,6 +46,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
 [ASPEED_DEV_SCU]   = 0x1E6E2000,
 [ASPEED_DEV_XDMA]  = 0x1E6E7000,
 [ASPEED_DEV_ADC]   = 0x1E6E9000,
+[ASPEED_DEV_DP]= 0x1E6EB000,
 [ASPEED_DEV_VIDEO] = 0x1E70,
 [ASPEED_DEV_SDHCI] = 0x1E74,
 [ASPEED_DEV_EMMC]  = 0x1E75,
@@ -104,6 +107,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
 [ASPEED_DEV_ETH3]  = 32,
 [ASPEED_DEV_ETH4]  = 33,
 [ASPEED_DEV_KCS]   = 138,   /* 138 -> 142 */
+[ASPEED_DEV_DP]= 62,
 };
 
 static qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int ctrl)
@@ -298,6 +302,10 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
 memory_region_add_subregion(get_system_memory(),
 sc->memmap[ASPEED_DEV_SRAM], >sram);
 
+/* DPMCU */
+create_unimplemented_device("aspeed.dpmcu", sc->memmap[ASPEED_DEV_DPMCU],
+ASPEED_SOC_DPMCU_SIZE);
+
 /* SCU */
 if (!sysbus_realize(SYS_BUS_DEVICE(>scu), errp)) {
 return;
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 8139358549..18fb7eed46 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -139,6 +139,8 @@ enum {
 ASPEED_DEV_EMMC,
 ASPEED_DEV_KCS,
 ASPEED_DEV_HACE,
+ASPEED_DEV_DPMCU,
+ASPEED_DEV_DP,
 };
 
 #endif /* ASPEED_SOC_H */
-- 
2.25.1