Re: [PATCH v2 5/5] hw/misc: Model KCS devices in the Aspeed LPC controller

2021-03-01 Thread Cédric Le Goater
On 3/1/21 2:06 AM, Andrew Jeffery wrote:
> Keyboard-Controller-Style devices for IPMI purposes are exposed via LPC
> IO cycles from the BMC to the host.
> 
> Expose support on the BMC side by implementing the usual MMIO
> behaviours, and expose the ability to inspect the KCS registers in
> "host" style by accessing QOM properties associated with each register.
> 
> The model caters to the IRQ style of both the AST2600 and the earlier
> SoCs (AST2400 and AST2500). The AST2600 allocates an IRQ for each LPC
> sub-device, while there is a single IRQ shared across all subdevices on
> the AST2400 and AST2500.
> 
> Signed-off-by: Andrew Jeffery 

Reviewed-by: Cédric Le Goater 


> ---
>  hw/arm/aspeed_ast2600.c  |  28 ++-
>  hw/arm/aspeed_soc.c  |  24 ++-
>  hw/misc/aspeed_lpc.c | 359 ++-
>  include/hw/arm/aspeed_soc.h  |   1 +
>  include/hw/misc/aspeed_lpc.h |  17 +-
>  5 files changed, 424 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index 60152de001e6..fd463775d281 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -104,7 +104,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
>  [ASPEED_DEV_ETH2]  = 3,
>  [ASPEED_DEV_ETH3]  = 32,
>  [ASPEED_DEV_ETH4]  = 33,
> -
> +[ASPEED_DEV_KCS]   = 138,   /* 138 -> 142 */
>  };
>  
>  static qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int ctrl)
> @@ -477,8 +477,34 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
> Error **errp)
>  return;
>  }
>  sysbus_mmio_map(SYS_BUS_DEVICE(&s->lpc), 0, sc->memmap[ASPEED_DEV_LPC]);
> +
> +/* Connect the LPC IRQ to the GIC. It is otherwise unused. */
>  sysbus_connect_irq(SYS_BUS_DEVICE(&s->emmc), 0,
> aspeed_soc_get_irq(s, ASPEED_DEV_LPC));
> +
> +/*
> + * On the AST2600 LPC subdevice IRQs are connected straight to the GIC.
> + *
> + * LPC subdevice IRQ sources are offset from 1 because the LPC model 
> caters
> + * to the AST2400 and AST2500. SoCs before the AST2600 have one LPC IRQ
> + * shared across the subdevices, and the shared IRQ output to the VIC is 
> at
> + * offset 0.
> + */
> +sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_kcs_1,
> +   qdev_get_gpio_in(DEVICE(&s->a7mpcore),
> +sc->irqmap[ASPEED_DEV_KCS] + 
> aspeed_lpc_kcs_1));
> +
> +sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_kcs_2,
> +   qdev_get_gpio_in(DEVICE(&s->a7mpcore),
> +sc->irqmap[ASPEED_DEV_KCS] + 
> aspeed_lpc_kcs_2));
> +
> +sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_kcs_3,
> +   qdev_get_gpio_in(DEVICE(&s->a7mpcore),
> +sc->irqmap[ASPEED_DEV_KCS] + 
> aspeed_lpc_kcs_3));
> +
> +sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_kcs_4,
> +   qdev_get_gpio_in(DEVICE(&s->a7mpcore),
> +sc->irqmap[ASPEED_DEV_KCS] + 
> aspeed_lpc_kcs_4));
>  }
>  
>  static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index 4f098da437ac..057d053c8478 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -112,7 +112,6 @@ static const int aspeed_soc_ast2400_irqmap[] = {
>  [ASPEED_DEV_WDT]= 27,
>  [ASPEED_DEV_PWM]= 28,
>  [ASPEED_DEV_LPC]= 8,
> -[ASPEED_DEV_IBT]= 8, /* LPC */
>  [ASPEED_DEV_I2C]= 12,
>  [ASPEED_DEV_ETH1]   = 2,
>  [ASPEED_DEV_ETH2]   = 3,
> @@ -401,8 +400,31 @@ static void aspeed_soc_realize(DeviceState *dev, Error 
> **errp)
>  return;
>  }
>  sysbus_mmio_map(SYS_BUS_DEVICE(&s->lpc), 0, sc->memmap[ASPEED_DEV_LPC]);
> +
> +/* Connect the LPC IRQ to the VIC */
>  sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 0,
> aspeed_soc_get_irq(s, ASPEED_DEV_LPC));
> +
> +/*
> + * On the AST2400 and AST2500 the one LPC IRQ is shared between all of 
> the
> + * subdevices. Connect the LPC subdevice IRQs to the LPC controller IRQ 
> (by
> + * contrast, on the AST2600, the subdevice IRQs are connected straight to
> + * the GIC).
> + *
> + * LPC subdevice IRQ sources are offset from 1 because the shared IRQ 
> output
> + * to the VIC is at offset 0.
> + */
> +sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_kcs_1,
> +   qdev_get_gpio_in(DEVICE(&s->lpc), aspeed_lpc_kcs_1));
> +
> +sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_kcs_2,
> +   qdev_get_gpio_in(DEVICE(&s->lpc), aspeed_lpc_kcs_2));
> +
> +sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_kcs_3,
> +   qdev_get_gpio_in(DEVICE(&s->lpc), aspeed_lpc_kcs_3));
> +
> +sysbus_connect

[PATCH v2 5/5] hw/misc: Model KCS devices in the Aspeed LPC controller

2021-02-28 Thread Andrew Jeffery
Keyboard-Controller-Style devices for IPMI purposes are exposed via LPC
IO cycles from the BMC to the host.

Expose support on the BMC side by implementing the usual MMIO
behaviours, and expose the ability to inspect the KCS registers in
"host" style by accessing QOM properties associated with each register.

The model caters to the IRQ style of both the AST2600 and the earlier
SoCs (AST2400 and AST2500). The AST2600 allocates an IRQ for each LPC
sub-device, while there is a single IRQ shared across all subdevices on
the AST2400 and AST2500.

Signed-off-by: Andrew Jeffery 
---
 hw/arm/aspeed_ast2600.c  |  28 ++-
 hw/arm/aspeed_soc.c  |  24 ++-
 hw/misc/aspeed_lpc.c | 359 ++-
 include/hw/arm/aspeed_soc.h  |   1 +
 include/hw/misc/aspeed_lpc.h |  17 +-
 5 files changed, 424 insertions(+), 5 deletions(-)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 60152de001e6..fd463775d281 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -104,7 +104,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
 [ASPEED_DEV_ETH2]  = 3,
 [ASPEED_DEV_ETH3]  = 32,
 [ASPEED_DEV_ETH4]  = 33,
-
+[ASPEED_DEV_KCS]   = 138,   /* 138 -> 142 */
 };
 
 static qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int ctrl)
@@ -477,8 +477,34 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
 return;
 }
 sysbus_mmio_map(SYS_BUS_DEVICE(&s->lpc), 0, sc->memmap[ASPEED_DEV_LPC]);
+
+/* Connect the LPC IRQ to the GIC. It is otherwise unused. */
 sysbus_connect_irq(SYS_BUS_DEVICE(&s->emmc), 0,
aspeed_soc_get_irq(s, ASPEED_DEV_LPC));
+
+/*
+ * On the AST2600 LPC subdevice IRQs are connected straight to the GIC.
+ *
+ * LPC subdevice IRQ sources are offset from 1 because the LPC model caters
+ * to the AST2400 and AST2500. SoCs before the AST2600 have one LPC IRQ
+ * shared across the subdevices, and the shared IRQ output to the VIC is at
+ * offset 0.
+ */
+sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_kcs_1,
+   qdev_get_gpio_in(DEVICE(&s->a7mpcore),
+sc->irqmap[ASPEED_DEV_KCS] + 
aspeed_lpc_kcs_1));
+
+sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_kcs_2,
+   qdev_get_gpio_in(DEVICE(&s->a7mpcore),
+sc->irqmap[ASPEED_DEV_KCS] + 
aspeed_lpc_kcs_2));
+
+sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_kcs_3,
+   qdev_get_gpio_in(DEVICE(&s->a7mpcore),
+sc->irqmap[ASPEED_DEV_KCS] + 
aspeed_lpc_kcs_3));
+
+sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_kcs_4,
+   qdev_get_gpio_in(DEVICE(&s->a7mpcore),
+sc->irqmap[ASPEED_DEV_KCS] + 
aspeed_lpc_kcs_4));
 }
 
 static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 4f098da437ac..057d053c8478 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -112,7 +112,6 @@ static const int aspeed_soc_ast2400_irqmap[] = {
 [ASPEED_DEV_WDT]= 27,
 [ASPEED_DEV_PWM]= 28,
 [ASPEED_DEV_LPC]= 8,
-[ASPEED_DEV_IBT]= 8, /* LPC */
 [ASPEED_DEV_I2C]= 12,
 [ASPEED_DEV_ETH1]   = 2,
 [ASPEED_DEV_ETH2]   = 3,
@@ -401,8 +400,31 @@ static void aspeed_soc_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 sysbus_mmio_map(SYS_BUS_DEVICE(&s->lpc), 0, sc->memmap[ASPEED_DEV_LPC]);
+
+/* Connect the LPC IRQ to the VIC */
 sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 0,
aspeed_soc_get_irq(s, ASPEED_DEV_LPC));
+
+/*
+ * On the AST2400 and AST2500 the one LPC IRQ is shared between all of the
+ * subdevices. Connect the LPC subdevice IRQs to the LPC controller IRQ (by
+ * contrast, on the AST2600, the subdevice IRQs are connected straight to
+ * the GIC).
+ *
+ * LPC subdevice IRQ sources are offset from 1 because the shared IRQ 
output
+ * to the VIC is at offset 0.
+ */
+sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_kcs_1,
+   qdev_get_gpio_in(DEVICE(&s->lpc), aspeed_lpc_kcs_1));
+
+sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_kcs_2,
+   qdev_get_gpio_in(DEVICE(&s->lpc), aspeed_lpc_kcs_2));
+
+sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_kcs_3,
+   qdev_get_gpio_in(DEVICE(&s->lpc), aspeed_lpc_kcs_3));
+
+sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_kcs_4,
+   qdev_get_gpio_in(DEVICE(&s->lpc), aspeed_lpc_kcs_4));
 }
 static Property aspeed_soc_properties[] = {
 DEFINE_PROP_LINK("dram", AspeedSoCState, dram_mr, TYPE_MEMORY_REGION,
diff --git a/hw/misc/aspeed_lpc.c b/hw/misc/aspeed_lpc.c
index e66