diff mbox series

[v2] hw/loongarch: virt: support up to 4 serial ports

Message ID 20240906143146.2553953-1-Jason@zx2c4.com (mailing list archive)
State New, archived
Headers show
Series [v2] hw/loongarch: virt: support up to 4 serial ports | expand

Commit Message

Jason A. Donenfeld Sept. 6, 2024, 2:31 p.m. UTC 
In order to support additional channels of communication using
`-serial`, add several serial ports, up to the standard 4 generally
supported by the 8250 driver.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
As I don't use ACPI, I haven't tested the ACPI part of this, which
Maobibo wrote.

 hw/loongarch/acpi-build.c  | 23 +++++++++++++++--------
 hw/loongarch/virt.c        | 24 ++++++++++++++----------
 include/hw/pci-host/ls7a.h |  9 +++++----
 3 files changed, 34 insertions(+), 22 deletions(-)

Comments

bibo mao Sept. 7, 2024, 3:37 a.m. UTC | #1 
Hi Jason,

It works well with ELF kernel, however it fails to boot with UEFI BIOS. 
Maybe it is problem of UEFI BIOS, can we create UART in reverse order? 
so that it can work well on both ELF kernel and UEFI BIOS.

Also for develops they usually use as earlycon with command line 
-serial stdio --append "... earlycon=uart,mmio,0x1fe001e0", this 
requires to uart with address 0x1fe001e0 as the first serial port.

Just small code base your patch like this:

-    for (i = 0; i < VIRT_UART_COUNT; ++i) {
+    for (i = VIRT_UART_COUNT - 1; i >= 0; i--) {
          hwaddr base = VIRT_UART_BASE + i * VIRT_UART_SIZE;
          int irq = VIRT_UART_IRQ + i - VIRT_GSI_BASE;
          serial_mm_init(get_system_memory(), base, 0,
                         qdev_get_gpio_in(pch_pic, irq),
-                       115200, serial_hd(VIRT_UART_COUNT - 1 - i),
+                       115200, serial_hd(i),
                         DEVICE_LITTLE_ENDIAN);
-        fdt_add_uart_node(lvms, pch_pic_phandle, base, irq, i == 
VIRT_UART_COUNT - 1 - i);
+        fdt_add_uart_node(lvms, pch_pic_phandle, base, irq, i == 0);
      }

Regards
Bibo Mao

On 2024/9/6 下午10:31, Jason A. Donenfeld wrote:
> In order to support additional channels of communication using
> `-serial`, add several serial ports, up to the standard 4 generally
> supported by the 8250 driver.
> 
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
> As I don't use ACPI, I haven't tested the ACPI part of this, which
> Maobibo wrote.
> 
>   hw/loongarch/acpi-build.c  | 23 +++++++++++++++--------
>   hw/loongarch/virt.c        | 24 ++++++++++++++----------
>   include/hw/pci-host/ls7a.h |  9 +++++----
>   3 files changed, 34 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
> index 2638f87434..2750c6e858 100644
> --- a/hw/loongarch/acpi-build.c
> +++ b/hw/loongarch/acpi-build.c
> @@ -31,6 +31,7 @@
>   
>   #include "hw/acpi/generic_event_device.h"
>   #include "hw/pci-host/gpex.h"
> +#include "sysemu/sysemu.h"
>   #include "sysemu/tpm.h"
>   #include "hw/platform-bus.h"
>   #include "hw/acpi/aml-build.h"
> @@ -252,23 +253,27 @@ struct AcpiBuildState {
>       MemoryRegion *linker_mr;
>   } AcpiBuildState;
>   
> -static void build_uart_device_aml(Aml *table)
> +static void build_uart_device_aml(Aml *table, int index)
>   {
>       Aml *dev;
>       Aml *crs;
>       Aml *pkg0, *pkg1, *pkg2;
> -    uint32_t uart_irq = VIRT_UART_IRQ;
> -
> -    Aml *scope = aml_scope("_SB");
> -    dev = aml_device("COMA");
> +    Aml *scope;
> +    uint32_t uart_irq;
> +    uint64_t base;
> +
> +    uart_irq = VIRT_UART_IRQ + index;
> +    base = VIRT_UART_BASE + index * VIRT_UART_SIZE;
> +    scope = aml_scope("_SB");
> +    dev = aml_device("COM%d", VIRT_UART_COUNT - 1 - index);
>       aml_append(dev, aml_name_decl("_HID", aml_string("PNP0501")));
> -    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> +    aml_append(dev, aml_name_decl("_UID", aml_int(VIRT_UART_COUNT - 1 - index)));
>       aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
>       crs = aml_resource_template();
>       aml_append(crs,
>           aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
>                            AML_NON_CACHEABLE, AML_READ_WRITE,
> -                         0, VIRT_UART_BASE, VIRT_UART_BASE + VIRT_UART_SIZE - 1,
> +                         0, base, base + VIRT_UART_SIZE - 1,
>                            0, VIRT_UART_SIZE));
>       aml_append(crs, aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
>                                     AML_SHARED, &uart_irq, 1));
> @@ -401,6 +406,7 @@ static void acpi_dsdt_add_tpm(Aml *scope, LoongArchVirtMachineState *vms)
>   static void
>   build_dsdt(GArray *table_data, BIOSLinker *linker, MachineState *machine)
>   {
> +    int i;
>       Aml *dsdt, *scope, *pkg;
>       LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(machine);
>       AcpiTable table = { .sig = "DSDT", .rev = 1, .oem_id = lvms->oem_id,
> @@ -408,7 +414,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, MachineState *machine)
>   
>       acpi_table_begin(&table, table_data);
>       dsdt = init_aml_allocator();
> -    build_uart_device_aml(dsdt);
> +    for (i = 0; i < VIRT_UART_COUNT; ++i)
> +        build_uart_device_aml(dsdt, i);
>       build_pci_device_aml(dsdt, lvms);
>       build_la_ged_aml(dsdt, machine);
>       build_flash_aml(dsdt, lvms);
> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> index 4151fc5e0c..6e8608874c 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -319,10 +319,10 @@ static void fdt_add_ged_reset(LoongArchVirtMachineState *lvms)
>   }
>   
>   static void fdt_add_uart_node(LoongArchVirtMachineState *lvms,
> -                              uint32_t *pch_pic_phandle)
> +                              uint32_t *pch_pic_phandle, hwaddr base,
> +                              int irq, bool chosen)
>   {
>       char *nodename;
> -    hwaddr base = VIRT_UART_BASE;
>       hwaddr size = VIRT_UART_SIZE;
>       MachineState *ms = MACHINE(lvms);
>   
> @@ -331,9 +331,9 @@ static void fdt_add_uart_node(LoongArchVirtMachineState *lvms,
>       qemu_fdt_setprop_string(ms->fdt, nodename, "compatible", "ns16550a");
>       qemu_fdt_setprop_cells(ms->fdt, nodename, "reg", 0x0, base, 0x0, size);
>       qemu_fdt_setprop_cell(ms->fdt, nodename, "clock-frequency", 100000000);
> -    qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
> -    qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
> -                           VIRT_UART_IRQ - VIRT_GSI_BASE, 0x4);
> +    if (chosen)
> +        qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
> +    qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts", irq, 0x4);
>       qemu_fdt_setprop_cell(ms->fdt, nodename, "interrupt-parent",
>                             *pch_pic_phandle);
>       g_free(nodename);
> @@ -750,11 +750,15 @@ static void virt_devices_init(DeviceState *pch_pic,
>       /* Add pcie node */
>       fdt_add_pcie_node(lvms, pch_pic_phandle, pch_msi_phandle);
>   
> -    serial_mm_init(get_system_memory(), VIRT_UART_BASE, 0,
> -                   qdev_get_gpio_in(pch_pic,
> -                                    VIRT_UART_IRQ - VIRT_GSI_BASE),
> -                   115200, serial_hd(0), DEVICE_LITTLE_ENDIAN);
> -    fdt_add_uart_node(lvms, pch_pic_phandle);
> +    for (i = 0; i < VIRT_UART_COUNT; ++i) {
> +        hwaddr base = VIRT_UART_BASE + i * VIRT_UART_SIZE;
> +        int irq = VIRT_UART_IRQ + i - VIRT_GSI_BASE;
> +        serial_mm_init(get_system_memory(), base, 0,
> +                       qdev_get_gpio_in(pch_pic, irq),
> +                       115200, serial_hd(VIRT_UART_COUNT - 1 - i),
> +                       DEVICE_LITTLE_ENDIAN);
> +        fdt_add_uart_node(lvms, pch_pic_phandle, base, irq, i == VIRT_UART_COUNT - 1 - i);
> +    }
>   
>       /* Network init */
>       pci_init_nic_devices(pci_bus, mc->default_nic);
> diff --git a/include/hw/pci-host/ls7a.h b/include/hw/pci-host/ls7a.h
> index cd7c9ec7bc..79d4ea8501 100644
> --- a/include/hw/pci-host/ls7a.h
> +++ b/include/hw/pci-host/ls7a.h
> @@ -36,17 +36,18 @@
>   #define VIRT_PCH_PIC_IRQ_NUM     32
>   #define VIRT_GSI_BASE            64
>   #define VIRT_DEVICE_IRQS         16
> +#define VIRT_UART_COUNT          4
>   #define VIRT_UART_IRQ            (VIRT_GSI_BASE + 2)
>   #define VIRT_UART_BASE           0x1fe001e0
> -#define VIRT_UART_SIZE           0X100
> -#define VIRT_RTC_IRQ             (VIRT_GSI_BASE + 3)
> +#define VIRT_UART_SIZE           0x100
> +#define VIRT_RTC_IRQ             (VIRT_GSI_BASE + 6)
>   #define VIRT_MISC_REG_BASE       (VIRT_PCH_REG_BASE + 0x00080000)
>   #define VIRT_RTC_REG_BASE        (VIRT_MISC_REG_BASE + 0x00050100)
>   #define VIRT_RTC_LEN             0x100
> -#define VIRT_SCI_IRQ             (VIRT_GSI_BASE + 4)
> +#define VIRT_SCI_IRQ             (VIRT_GSI_BASE + 7)
>   
>   #define VIRT_PLATFORM_BUS_BASEADDRESS   0x16000000
>   #define VIRT_PLATFORM_BUS_SIZE          0x2000000
>   #define VIRT_PLATFORM_BUS_NUM_IRQS      2
> -#define VIRT_PLATFORM_BUS_IRQ           (VIRT_GSI_BASE + 5)
> +#define VIRT_PLATFORM_BUS_IRQ           (VIRT_GSI_BASE + 8)
>   #endif
>
Jason A. Donenfeld Sept. 7, 2024, 2:44 p.m. UTC | #2 
On Sat, Sep 07, 2024 at 11:37:09AM +0800, maobibo wrote:
> Hi Jason,
> 
> It works well with ELF kernel, however it fails to boot with UEFI BIOS. 
> Maybe it is problem of UEFI BIOS, can we create UART in reverse order? 
> so that it can work well on both ELF kernel and UEFI BIOS.
> 
> Also for develops they usually use as earlycon with command line 
> -serial stdio --append "... earlycon=uart,mmio,0x1fe001e0", this 
> requires to uart with address 0x1fe001e0 as the first serial port.
> 
> Just small code base your patch like this:
> 
> -    for (i = 0; i < VIRT_UART_COUNT; ++i) {
> +    for (i = VIRT_UART_COUNT - 1; i >= 0; i--) {
>           hwaddr base = VIRT_UART_BASE + i * VIRT_UART_SIZE;
>           int irq = VIRT_UART_IRQ + i - VIRT_GSI_BASE;
>           serial_mm_init(get_system_memory(), base, 0,
>                          qdev_get_gpio_in(pch_pic, irq),
> -                       115200, serial_hd(VIRT_UART_COUNT - 1 - i),
> +                       115200, serial_hd(i),
>                          DEVICE_LITTLE_ENDIAN);
> -        fdt_add_uart_node(lvms, pch_pic_phandle, base, irq, i == 
> VIRT_UART_COUNT - 1 - i);
> +        fdt_add_uart_node(lvms, pch_pic_phandle, base, irq, i == 0);
>       }

Fixed: https://lore.kernel.org/all/20240907143439.2792924-1-Jason@zx2c4.com/
Jason A. Donenfeld Sept. 7, 2024, 2:57 p.m. UTC | #3 
On Sat, Sep 07, 2024 at 04:44:45PM +0200, Jason A. Donenfeld wrote:
> On Sat, Sep 07, 2024 at 11:37:09AM +0800, maobibo wrote:
> > Hi Jason,
> > 
> > It works well with ELF kernel, however it fails to boot with UEFI BIOS. 
> > Maybe it is problem of UEFI BIOS, can we create UART in reverse order? 
> > so that it can work well on both ELF kernel and UEFI BIOS.
> > 
> > Also for develops they usually use as earlycon with command line 
> > -serial stdio --append "... earlycon=uart,mmio,0x1fe001e0", this 
> > requires to uart with address 0x1fe001e0 as the first serial port.
> > 
> > Just small code base your patch like this:
> > 
> > -    for (i = 0; i < VIRT_UART_COUNT; ++i) {
> > +    for (i = VIRT_UART_COUNT - 1; i >= 0; i--) {
> >           hwaddr base = VIRT_UART_BASE + i * VIRT_UART_SIZE;
> >           int irq = VIRT_UART_IRQ + i - VIRT_GSI_BASE;
> >           serial_mm_init(get_system_memory(), base, 0,
> >                          qdev_get_gpio_in(pch_pic, irq),
> > -                       115200, serial_hd(VIRT_UART_COUNT - 1 - i),
> > +                       115200, serial_hd(i),
> >                          DEVICE_LITTLE_ENDIAN);
> > -        fdt_add_uart_node(lvms, pch_pic_phandle, base, irq, i == 
> > VIRT_UART_COUNT - 1 - i);
> > +        fdt_add_uart_node(lvms, pch_pic_phandle, base, irq, i == 0);
> >       }
> 
> Fixed: https://lore.kernel.org/all/20240907143439.2792924-1-Jason@zx2c4.com/

By the way, I don't know who is picking patches into which development
tree from the list, but this is starting to be kind of a lot of patches,
so I've queued them all up here:

   https://git.zx2c4.com/qemu/log/?h=loongarch64

In case that makes it easier for whoever the maintainer is to pick them.
diff mbox series

Patch

diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
index 2638f87434..2750c6e858 100644
--- a/hw/loongarch/acpi-build.c
+++ b/hw/loongarch/acpi-build.c
@@ -31,6 +31,7 @@ 
 
 #include "hw/acpi/generic_event_device.h"
 #include "hw/pci-host/gpex.h"
+#include "sysemu/sysemu.h"
 #include "sysemu/tpm.h"
 #include "hw/platform-bus.h"
 #include "hw/acpi/aml-build.h"
@@ -252,23 +253,27 @@  struct AcpiBuildState {
     MemoryRegion *linker_mr;
 } AcpiBuildState;
 
-static void build_uart_device_aml(Aml *table)
+static void build_uart_device_aml(Aml *table, int index)
 {
     Aml *dev;
     Aml *crs;
     Aml *pkg0, *pkg1, *pkg2;
-    uint32_t uart_irq = VIRT_UART_IRQ;
-
-    Aml *scope = aml_scope("_SB");
-    dev = aml_device("COMA");
+    Aml *scope;
+    uint32_t uart_irq;
+    uint64_t base;
+
+    uart_irq = VIRT_UART_IRQ + index;
+    base = VIRT_UART_BASE + index * VIRT_UART_SIZE;
+    scope = aml_scope("_SB");
+    dev = aml_device("COM%d", VIRT_UART_COUNT - 1 - index);
     aml_append(dev, aml_name_decl("_HID", aml_string("PNP0501")));
-    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+    aml_append(dev, aml_name_decl("_UID", aml_int(VIRT_UART_COUNT - 1 - index)));
     aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
     crs = aml_resource_template();
     aml_append(crs,
         aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
                          AML_NON_CACHEABLE, AML_READ_WRITE,
-                         0, VIRT_UART_BASE, VIRT_UART_BASE + VIRT_UART_SIZE - 1,
+                         0, base, base + VIRT_UART_SIZE - 1,
                          0, VIRT_UART_SIZE));
     aml_append(crs, aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
                                   AML_SHARED, &uart_irq, 1));
@@ -401,6 +406,7 @@  static void acpi_dsdt_add_tpm(Aml *scope, LoongArchVirtMachineState *vms)
 static void
 build_dsdt(GArray *table_data, BIOSLinker *linker, MachineState *machine)
 {
+    int i;
     Aml *dsdt, *scope, *pkg;
     LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(machine);
     AcpiTable table = { .sig = "DSDT", .rev = 1, .oem_id = lvms->oem_id,
@@ -408,7 +414,8 @@  build_dsdt(GArray *table_data, BIOSLinker *linker, MachineState *machine)
 
     acpi_table_begin(&table, table_data);
     dsdt = init_aml_allocator();
-    build_uart_device_aml(dsdt);
+    for (i = 0; i < VIRT_UART_COUNT; ++i)
+        build_uart_device_aml(dsdt, i);
     build_pci_device_aml(dsdt, lvms);
     build_la_ged_aml(dsdt, machine);
     build_flash_aml(dsdt, lvms);
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index 4151fc5e0c..6e8608874c 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -319,10 +319,10 @@  static void fdt_add_ged_reset(LoongArchVirtMachineState *lvms)
 }
 
 static void fdt_add_uart_node(LoongArchVirtMachineState *lvms,
-                              uint32_t *pch_pic_phandle)
+                              uint32_t *pch_pic_phandle, hwaddr base,
+                              int irq, bool chosen)
 {
     char *nodename;
-    hwaddr base = VIRT_UART_BASE;
     hwaddr size = VIRT_UART_SIZE;
     MachineState *ms = MACHINE(lvms);
 
@@ -331,9 +331,9 @@  static void fdt_add_uart_node(LoongArchVirtMachineState *lvms,
     qemu_fdt_setprop_string(ms->fdt, nodename, "compatible", "ns16550a");
     qemu_fdt_setprop_cells(ms->fdt, nodename, "reg", 0x0, base, 0x0, size);
     qemu_fdt_setprop_cell(ms->fdt, nodename, "clock-frequency", 100000000);
-    qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
-    qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
-                           VIRT_UART_IRQ - VIRT_GSI_BASE, 0x4);
+    if (chosen)
+        qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
+    qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts", irq, 0x4);
     qemu_fdt_setprop_cell(ms->fdt, nodename, "interrupt-parent",
                           *pch_pic_phandle);
     g_free(nodename);
@@ -750,11 +750,15 @@  static void virt_devices_init(DeviceState *pch_pic,
     /* Add pcie node */
     fdt_add_pcie_node(lvms, pch_pic_phandle, pch_msi_phandle);
 
-    serial_mm_init(get_system_memory(), VIRT_UART_BASE, 0,
-                   qdev_get_gpio_in(pch_pic,
-                                    VIRT_UART_IRQ - VIRT_GSI_BASE),
-                   115200, serial_hd(0), DEVICE_LITTLE_ENDIAN);
-    fdt_add_uart_node(lvms, pch_pic_phandle);
+    for (i = 0; i < VIRT_UART_COUNT; ++i) {
+        hwaddr base = VIRT_UART_BASE + i * VIRT_UART_SIZE;
+        int irq = VIRT_UART_IRQ + i - VIRT_GSI_BASE;
+        serial_mm_init(get_system_memory(), base, 0,
+                       qdev_get_gpio_in(pch_pic, irq),
+                       115200, serial_hd(VIRT_UART_COUNT - 1 - i),
+                       DEVICE_LITTLE_ENDIAN);
+        fdt_add_uart_node(lvms, pch_pic_phandle, base, irq, i == VIRT_UART_COUNT - 1 - i);
+    }
 
     /* Network init */
     pci_init_nic_devices(pci_bus, mc->default_nic);
diff --git a/include/hw/pci-host/ls7a.h b/include/hw/pci-host/ls7a.h
index cd7c9ec7bc..79d4ea8501 100644
--- a/include/hw/pci-host/ls7a.h
+++ b/include/hw/pci-host/ls7a.h
@@ -36,17 +36,18 @@ 
 #define VIRT_PCH_PIC_IRQ_NUM     32
 #define VIRT_GSI_BASE            64
 #define VIRT_DEVICE_IRQS         16
+#define VIRT_UART_COUNT          4
 #define VIRT_UART_IRQ            (VIRT_GSI_BASE + 2)
 #define VIRT_UART_BASE           0x1fe001e0
-#define VIRT_UART_SIZE           0X100
-#define VIRT_RTC_IRQ             (VIRT_GSI_BASE + 3)
+#define VIRT_UART_SIZE           0x100
+#define VIRT_RTC_IRQ             (VIRT_GSI_BASE + 6)
 #define VIRT_MISC_REG_BASE       (VIRT_PCH_REG_BASE + 0x00080000)
 #define VIRT_RTC_REG_BASE        (VIRT_MISC_REG_BASE + 0x00050100)
 #define VIRT_RTC_LEN             0x100
-#define VIRT_SCI_IRQ             (VIRT_GSI_BASE + 4)
+#define VIRT_SCI_IRQ             (VIRT_GSI_BASE + 7)
 
 #define VIRT_PLATFORM_BUS_BASEADDRESS   0x16000000
 #define VIRT_PLATFORM_BUS_SIZE          0x2000000
 #define VIRT_PLATFORM_BUS_NUM_IRQS      2
-#define VIRT_PLATFORM_BUS_IRQ           (VIRT_GSI_BASE + 5)
+#define VIRT_PLATFORM_BUS_IRQ           (VIRT_GSI_BASE + 8)
 #endif