qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/2] hw: aspeed: Init all UART's with serial devices


From: Cédric Le Goater
Subject: Re: [PATCH 2/2] hw: aspeed: Init all UART's with serial devices
Date: Fri, 13 May 2022 07:31:38 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.0

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@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),




reply via email to

[Prev in Thread] Current Thread [Next in Thread]