[PATCH 2/2] hw: aspeed: Init all UART's with serial devices
Cédric Le Goater
clg at kaod.org
Fri May 13 15:31:38 AEST 2022
On 5/13/22 06:02, Peter Delevoryas wrote:
> Usually, QEMU users just provide one serial device on the command line,
> either through "-nographic" or "-serial stdio -display none", or just using
> VNC and popping up a window. We try to match what the user expects, which is
> to connect the first (and usually only) serial device to the UART a board is
> using as serial0.
>
> Most Aspeed machines in hw/arm/aspeed.c use UART5 for serial0 in their
> device tree, so we connect UART5 to the first serial device. Some machines
> use UART1 though, or UART3, so the uart_default property lets us specify
> that in a board definition.
>
> In order to specify a nonstandard serial0 UART, a user basically *must* add
> a new board definition in hw/arm/aspeed.c. There's no way to do this without
> recompiling QEMU, besides constructing the machine completely from scratch
> on the command line.
>
> To provide more flexibility, we can also support the user specifying more
> serial devices, and connect them to the UART memory regions if possible.
> Even if a user doesn't specify any extra serial devices, it's useful to
> initialize these memory regions as UART's, so that they respond to the guest
> OS more naturally. At the moment, they will just always return zero's for
> everything, and some UART registers have a default non-zero state.
>
> With this change, if a new OpenBMC image uses UART3 or some other
> nonstandard UART for serial0, you can still use it with the EVB without
> recompiling QEMU, even though uart-default=UART5 for the EVB.
>
> For example, Facebook's Wedge100 BMC uses UART3: you can fetch an image from
> Github[1] and get the serial console output even while running the palmetto
> machine type, because we explicitly specify that we want UART3 to be
> connected to stdio.
>
> qemu-system-arm -machine palmetto-bmc \
> -drive file=wedge100.mtd,format=raw,if=mtd \
> -serial null -serial null -serial null -serial stdio -display none
>
> Similarly, you can boot a Fuji BMC image[2], which uses UART1, using the
> AST2600 EVB machine:
>
> qemu-system-arm -machine ast2600-evb \
> -drive file=fuji.mtd,format=raw,if=mtd \
> -serial null -serial stdio -display none
>
> This is kind of complicated, of course: it might be more natural to get rid
> of the uart_default attribute completely, and initialize UART's
> sequentially. But, keeping backward compatibility and the way most users
> know how to use QEMU in mind, this seems to make the most sense.
>
> [1] https://github.com/facebook/openbmc/releases/download/v2021.49.0/wedge100.mtd
> [2] https://github.com/facebook/openbmc/releases/download/v2021.49.0/fuji.mtd
>
> Signed-off-by: Peter Delevoryas <pdel at fb.com>
> ---
> hw/arm/aspeed_ast10x0.c | 14 +++++++++++---
> hw/arm/aspeed_ast2600.c | 10 +++++++++-
> hw/arm/aspeed_soc.c | 10 +++++++++-
> 3 files changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
> index f65dc139da..5e6f3a8fed 100644
> --- a/hw/arm/aspeed_ast10x0.c
> +++ b/hw/arm/aspeed_ast10x0.c
> @@ -215,10 +215,18 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
> qdev_get_gpio_in(DEVICE(&s->armv7m),
> sc->irqmap[ASPEED_DEV_KCS] + aspeed_lpc_kcs_4));
>
> - /* UART5 - attach an 8250 to the IO space as our UART */
> - serial_mm_init(get_system_memory(), sc->memmap[ASPEED_DEV_UART5], 2,
> - aspeed_soc_get_irq(s, ASPEED_DEV_UART5),
> + /* UART - attach 8250's to the IO space for each UART */
> + serial_mm_init(get_system_memory(), sc->memmap[s->uart_default], 2,
> + aspeed_soc_get_irq(s, s->uart_default),
That's a fix for aspeed_ast10x0 that should come first.
> 38400, serial_hd(0), DEVICE_LITTLE_ENDIAN);
> + for (int i = 1, uart = ASPEED_DEV_UART1; i < 13; i++, uart++) {
'13' should be a AspeecSoCClass attribute. The number of uarts varies
depending on the SoC model and we might want to model that one day.
> + if (uart == s->uart_default) {
> + uart++;
> + }
Shouldn't we test serial_hd(i) validity ?
Thanks,
C.
> + serial_mm_init(get_system_memory(), sc->memmap[uart], 2,
> + aspeed_soc_get_irq(s, uart), 38400, serial_hd(i),
> + DEVICE_LITTLE_ENDIAN);
> + }
>
> /* Timer */
> object_property_set_link(OBJECT(&s->timerctrl), "scu", OBJECT(&s->scu),
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index 1b72800682..cbeca7f655 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -372,10 +372,18 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->adc), 0,
> aspeed_soc_get_irq(s, ASPEED_DEV_ADC));
>
> - /* UART - attach an 8250 to the IO space as our UART */
> + /* UART - attach 8250's to the IO space for each UART */
> serial_mm_init(get_system_memory(), sc->memmap[s->uart_default], 2,
> aspeed_soc_get_irq(s, s->uart_default), 38400,
> serial_hd(0), DEVICE_LITTLE_ENDIAN);
> + for (int i = 1, uart = ASPEED_DEV_UART1; i < 13; i++, uart++) {
> + if (uart == s->uart_default) {
> + uart++;
> + }
> + serial_mm_init(get_system_memory(), sc->memmap[uart], 2,
> + aspeed_soc_get_irq(s, uart), 38400, serial_hd(i),
> + DEVICE_LITTLE_ENDIAN);
> + }
>
> /* I2C */
> object_property_set_link(OBJECT(&s->i2c), "dram", OBJECT(s->dram_mr),
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index 2cd03d49da..1fc1ed808d 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -303,10 +303,18 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->adc), 0,
> aspeed_soc_get_irq(s, ASPEED_DEV_ADC));
>
> - /* UART - attach an 8250 to the IO space as our UART */
> + /* UART - attach 8250's to the IO space for each UART */
> serial_mm_init(get_system_memory(), sc->memmap[s->uart_default], 2,
> aspeed_soc_get_irq(s, s->uart_default), 38400,
> serial_hd(0), DEVICE_LITTLE_ENDIAN);
> + for (int i = 1, uart = ASPEED_DEV_UART1; i < 5; i++, uart++) {
> + if (uart == s->uart_default) {
> + uart++;
> + }
> + serial_mm_init(get_system_memory(), sc->memmap[uart], 2,
> + aspeed_soc_get_irq(s, uart), 38400, serial_hd(i),
> + DEVICE_LITTLE_ENDIAN);
> + }
>
> /* I2C */
> object_property_set_link(OBJECT(&s->i2c), "dram", OBJECT(s->dram_mr),
More information about the openbmc
mailing list