[PATCH v2 15/17] hw/arm/aspeed: Model AST1700 I3C block as unimplemented device

Kane Chen via posted 17 patches 1 week, 2 days ago
Maintainers: "Cédric Le Goater" <clg@kaod.org>, Peter Maydell <peter.maydell@linaro.org>, Steven Lee <steven_lee@aspeedtech.com>, Troy Lee <leetroy@gmail.com>, Jamin Lin <jamin_lin@aspeedtech.com>, Andrew Jeffery <andrew@codeconstruct.com.au>, Joel Stanley <joel@jms.id.au>
[PATCH v2 15/17] hw/arm/aspeed: Model AST1700 I3C block as unimplemented device
Posted by Kane Chen via 1 week, 2 days ago
From: Kane-Chen-AS <kane_chen@aspeedtech.com>

AST1700 exposes more I3C buses than the current dummy I3C model
provides. When Linux probes the I3C devices on AST1700 this mismatch
can trigger a kernel panic. Model the I3C block as an unimplemented
device to make the missing functionality explicit and avoid unexpected
side effects.

This wires up the I3C interrupt lines for the IO expanders and adds the
corresponding device entries for the AST1700 model.

No functional I3C emulation is provided yet; this only prevents crashes and
documents the missing piece.

Signed-off-by: Kane-Chen-AS <kane_chen@aspeedtech.com>
---
 include/hw/arm/aspeed_soc.h      |  2 ++
 include/hw/misc/aspeed_ast1700.h |  2 ++
 hw/arm/aspeed_ast27x0.c          | 19 +++++++++++++++++--
 hw/misc/aspeed_ast1700.c         | 17 +++++++++++++++++
 4 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 464ef2d755..c58c861841 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -287,6 +287,8 @@ enum {
     ASPEED_DEV_IOEXP1_I2C,
     ASPEED_DEV_IOEXP0_INTCIO,
     ASPEED_DEV_IOEXP1_INTCIO,
+    ASPEED_DEV_IOEXP0_I3C,
+    ASPEED_DEV_IOEXP1_I3C,
 };
 
 const char *aspeed_soc_cpu_type(const char * const *valid_cpu_types);
diff --git a/include/hw/misc/aspeed_ast1700.h b/include/hw/misc/aspeed_ast1700.h
index f89de44539..4048d31154 100644
--- a/include/hw/misc/aspeed_ast1700.h
+++ b/include/hw/misc/aspeed_ast1700.h
@@ -42,6 +42,8 @@ struct AspeedAST1700SoCState {
     AspeedGPIOState gpio;
     AspeedI2CState i2c;
     AspeedWDTState wdt[AST1700_WDT_NUM];
+
+    UnimplementedDeviceState i3c;
 };
 
 #endif /* ASPEED_AST1700_H */
diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index a5d98f541b..f8dd4f6a3a 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -200,7 +200,9 @@ static const int aspeed_soc_ast2700a1_irqmap[] = {
     [ASPEED_DEV_PECI]      = 197,
     [ASPEED_DEV_SDHCI]     = 197,
     [ASPEED_DEV_IOEXP0_I2C] = 198,
+    [ASPEED_DEV_IOEXP0_I3C] = 199,
     [ASPEED_DEV_IOEXP1_I2C] = 200,
+    [ASPEED_DEV_IOEXP1_I3C] = 201,
 };
 
 /* GICINT 128 */
@@ -267,11 +269,24 @@ static const int ast2700_gic198_intcmap[] = {
     [ASPEED_DEV_IOEXP0_I2C]       = 0, /* 0 - 15 */
 };
 
+/* Primary AST1700 Interrupts */
+/* A1: GINTC 199 */
+static const int ast2700_gic199_intcmap[] = {
+    [ASPEED_DEV_IOEXP0_I3C]       = 0, /* 0 - 15 */
+};
+
 /* Secondary AST1700 Interrupts */
 /* A1: GINTC 200 */
 static const int ast2700_gic200_intcmap[] = {
     [ASPEED_DEV_IOEXP1_I2C]       = 0, /* 0 - 15 */
 };
+
+/* Secondary AST1700 Interrupts */
+/* A1: GINTC 201 */
+static const int ast2700_gic201_intcmap[] = {
+    [ASPEED_DEV_IOEXP1_I3C]       = 0, /* 0 - 15 */
+};
+
 /* GICINT 128 ~ 136 */
 /* GICINT 192 ~ 201 */
 struct gic_intc_irq_info {
@@ -289,9 +304,9 @@ static const struct gic_intc_irq_info ast2700_gic_intcmap[] = {
     {196, 1, 4, ast2700_gic132_gic196_intcmap},
     {197, 1, 5, ast2700_gic133_gic197_intcmap},
     {198, 2, 0, ast2700_gic198_intcmap},
-    {199, 1, 7, NULL},
+    {199, 2, 1, ast2700_gic199_intcmap},
     {200, 3, 0, ast2700_gic200_intcmap},
-    {201, 1, 9, NULL},
+    {201, 3, 1, ast2700_gic201_intcmap},
     {128, 0, 1, ast2700_gic128_gic192_intcmap},
     {129, 0, 2, NULL},
     {130, 0, 3, ast2700_gic130_gic194_intcmap},
diff --git a/hw/misc/aspeed_ast1700.c b/hw/misc/aspeed_ast1700.c
index c2dc834b4f..37b2946fc0 100644
--- a/hw/misc/aspeed_ast1700.c
+++ b/hw/misc/aspeed_ast1700.c
@@ -18,6 +18,7 @@
 #define AST1700_BOARD1_MEM_ADDR      0x30000000
 #define AST2700_SOC_LTPI_SIZE        0x01000000
 #define AST1700_SOC_SRAM_SIZE        0x00040000
+#define AST1700_SOC_I3C_SIZE         0x00010000
 
 enum {
     ASPEED_AST1700_DEV_SPI0,
@@ -26,6 +27,7 @@ enum {
     ASPEED_AST1700_DEV_SCU,
     ASPEED_AST1700_DEV_GPIO,
     ASPEED_AST1700_DEV_I2C,
+    ASPEED_AST1700_DEV_I3C,
     ASPEED_AST1700_DEV_UART12,
     ASPEED_AST1700_DEV_LTPI_CTRL,
     ASPEED_AST1700_DEV_WDT,
@@ -39,6 +41,7 @@ static const hwaddr aspeed_ast1700_io_memmap[] = {
     [ASPEED_AST1700_DEV_SCU]       =  0x00C02000,
     [ASPEED_AST1700_DEV_GPIO]      =  0x00C0B000,
     [ASPEED_AST1700_DEV_I2C]       =  0x00C0F000,
+    [ASPEED_AST1700_DEV_I3C]       =  0x00C20000,
     [ASPEED_AST1700_DEV_UART12]    =  0x00C33B00,
     [ASPEED_AST1700_DEV_LTPI_CTRL] =  0x00C34000,
     [ASPEED_AST1700_DEV_WDT]       =  0x00C37000,
@@ -142,6 +145,15 @@ static void aspeed_ast1700_realize(DeviceState *dev, Error **errp)
                         aspeed_ast1700_io_memmap[ASPEED_AST1700_DEV_I2C],
                         sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->i2c), 0));
 
+    /* I3C */
+    qdev_prop_set_string(DEVICE(&s->i3c), "name", "ioexp-i3c");
+    qdev_prop_set_uint64(DEVICE(&s->i3c), "size", AST1700_SOC_I3C_SIZE);
+    sysbus_realize(SYS_BUS_DEVICE(&s->i3c), errp);
+    memory_region_add_subregion_overlap(&s->iomem,
+                        aspeed_ast1700_io_memmap[ASPEED_AST1700_DEV_I3C],
+                        sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->i3c), 0),
+                        -1000);
+
     /* LTPI controller */
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->ltpi), errp)) {
         return;
@@ -204,6 +216,11 @@ static void aspeed_ast1700_instance_init(Object *obj)
     snprintf(typename, sizeof(typename), "aspeed.i2c-%s", socname);
     object_initialize_child(obj, "ioexp-i2c[*]", &s->i2c,
                             typename);
+
+    /* I3C */
+    object_initialize_child(obj, "ioexp-i3c[*]", &s->i3c,
+                            TYPE_UNIMPLEMENTED_DEVICE);
+
     /* LTPI controller */
     object_initialize_child(obj, "ltpi-ctrl",
                             &s->ltpi, TYPE_ASPEED_LTPI);
-- 
2.43.0
Re: [PATCH v2 15/17] hw/arm/aspeed: Model AST1700 I3C block as unimplemented device
Posted by Cédric Le Goater 1 week ago
Hello,

+ Joe, Patrick

On 11/5/25 04:58, Kane Chen wrote:
> From: Kane-Chen-AS <kane_chen@aspeedtech.com>
> 
> AST1700 exposes more I3C buses than the current dummy I3C model
> provides. When Linux probes the I3C devices on AST1700 this mismatch
> can trigger a kernel panic. Model the I3C block as an unimplemented
> device to make the missing functionality explicit and avoid unexpected
> side effects.
> 
> This wires up the I3C interrupt lines for the IO expanders and adds the
> corresponding device entries for the AST1700 model.
> 
> No functional I3C emulation is provided yet; this only prevents crashes and
> documents the missing piece.
> 
> Signed-off-by: Kane-Chen-AS <kane_chen@aspeedtech.com>
> ---
>   include/hw/arm/aspeed_soc.h      |  2 ++
>   include/hw/misc/aspeed_ast1700.h |  2 ++
>   hw/arm/aspeed_ast27x0.c          | 19 +++++++++++++++++--
>   hw/misc/aspeed_ast1700.c         | 17 +++++++++++++++++
>   4 files changed, 38 insertions(+), 2 deletions(-)

Joe sent (twice) changes adding I3C support [1].

I’ve been maintaining it in my branch, and from both a code and testing
perspective, it looks solid. I believe it’s ready to be merged. We now
just need maintainers and reviewers to step in.

Would it be useful for this model ? If so, that would be an additional
reason.

Thanks,

C.

[1] https://lore.kernel.org/qemu-devel/20250613000411.1516521-1-komlodi@google.com/

RE: [PATCH v2 15/17] hw/arm/aspeed: Model AST1700 I3C block as unimplemented device
Posted by Kane Chen 1 week ago
Hi Cédric,

If I use the current I3C model, it causes a kernel crash because the value of
ASPEED_I3C_NR_DEVICES is smaller than what AST1700 supports:
https://github.com/qemu/qemu/blob/master/include/hw/misc/aspeed_i3c.h#L21

The AST1700 can support up to 16 I3C buses. In the current I3C model (and in Joe's patches),
the value of ASPEED_I3C_NR_DEVICES remains 6, so I would encounter the same issue if
I used them directly.

I plan to either increase the value of ASPEED_I3C_NR_DEVICES or add a configurable method
to set the number of I3C buses after Joe's patches are merged.
After that, I'll update the AST1700 I3C model to use the new I3C framework.

If you have any comments or suggestions, please let me know.

Best Regards,
Kane
> -----Original Message-----
> From: Cédric Le Goater <clg@kaod.org>
> Sent: Friday, November 7, 2025 4:06 PM
> To: Kane Chen <kane_chen@aspeedtech.com>; Peter Maydell
> <peter.maydell@linaro.org>; Steven Lee <steven_lee@aspeedtech.com>; Troy
> Lee <leetroy@gmail.com>; Jamin Lin <jamin_lin@aspeedtech.com>; Andrew
> Jeffery <andrew@codeconstruct.com.au>; Joel Stanley <joel@jms.id.au>;
> open list:ASPEED BMCs <qemu-arm@nongnu.org>; open list:All patches CC
> here <qemu-devel@nongnu.org>; Joe Komlodi <komlodi@google.com>;
> Patrick Leis <venture@google.com>
> Cc: Troy Lee <troy_lee@aspeedtech.com>
> Subject: Re: [PATCH v2 15/17] hw/arm/aspeed: Model AST1700 I3C block as
> unimplemented device
> 
> Hello,
> 
> + Joe, Patrick
> 
> On 11/5/25 04:58, Kane Chen wrote:
> > From: Kane-Chen-AS <kane_chen@aspeedtech.com>
> >
> > AST1700 exposes more I3C buses than the current dummy I3C model
> > provides. When Linux probes the I3C devices on AST1700 this mismatch
> > can trigger a kernel panic. Model the I3C block as an unimplemented
> > device to make the missing functionality explicit and avoid unexpected
> > side effects.
> >
> > This wires up the I3C interrupt lines for the IO expanders and adds
> > the corresponding device entries for the AST1700 model.
> >
> > No functional I3C emulation is provided yet; this only prevents
> > crashes and documents the missing piece.
> >
> > Signed-off-by: Kane-Chen-AS <kane_chen@aspeedtech.com>
> > ---
> >   include/hw/arm/aspeed_soc.h      |  2 ++
> >   include/hw/misc/aspeed_ast1700.h |  2 ++
> >   hw/arm/aspeed_ast27x0.c          | 19 +++++++++++++++++--
> >   hw/misc/aspeed_ast1700.c         | 17 +++++++++++++++++
> >   4 files changed, 38 insertions(+), 2 deletions(-)
> 
> Joe sent (twice) changes adding I3C support [1].
> 
> I’ve been maintaining it in my branch, and from both a code and testing
> perspective, it looks solid. I believe it’s ready to be merged. We now just need
> maintainers and reviewers to step in.
> 
> Would it be useful for this model ? If so, that would be an additional reason.
> 
> Thanks,
> 
> C.
> 
> [1]
> https://lore.kernel.org/qemu-devel/20250613000411.1516521-1-komlodi@g
> oogle.com/