Re: [PATCH] hw/riscv/virt: Add a second UART for secure world

[Li, Yong]

On 2023/4/24 16:59, Peter Maydell wrote:
On Mon, 24 Apr 2023 at 07:24, Yong Li<yong...@intel.com>  wrote:
The virt machine can have two UARTs and the second UART
can be used when host secure-mode support is enabled.
Do you mean "host" here, or "guest" ?

Currently, it is an UEFI secure firmware residing in a secure 
partition.  Let me update the message with a v2 patch

Signed-off-by: Yong Li<yong...@intel.com>
---
  hw/riscv/virt.c         | 4 ++++
  include/hw/riscv/virt.h | 2 ++
  2 files changed, 6 insertions(+)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index b38b41e685..02475e1678 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -88,6 +88,7 @@ static const MemMapEntry virt_memmap[] = {
      [VIRT_APLIC_S] =      {  0xd000000, APLIC_SIZE(VIRT_CPUS_MAX) },
      [VIRT_UART0] =        { 0x10000000,         0x100 },
      [VIRT_VIRTIO] =       { 0x10001000,        0x1000 },
+    [VIRT_UART1] =        { 0x10002000,         0x100 },
      [VIRT_FW_CFG] =       { 0x10100000,          0x18 },
      [VIRT_FLASH] =        { 0x20000000,     0x4000000 },
      [VIRT_IMSIC_M] =      { 0x24000000, VIRT_IMSIC_MAX_SIZE },
@@ -1508,6 +1509,9 @@ static void virt_machine_init(MachineState *machine)
      serial_mm_init(system_memory, memmap[VIRT_UART0].base,
          0, qdev_get_gpio_in(DEVICE(mmio_irqchip), UART0_IRQ), 399193,
          serial_hd(0), DEVICE_LITTLE_ENDIAN);
+    serial_mm_init(system_memory, memmap[VIRT_UART1].base,
+        0, qdev_get_gpio_in(DEVICE(mmio_irqchip), UART1_IRQ), 399193,
+        serial_hd(1), DEVICE_LITTLE_ENDIAN);

      sysbus_create_simple("goldfish_rtc", memmap[VIRT_RTC].base,
          qdev_get_gpio_in(DEVICE(mmio_irqchip), RTC_IRQ));
Is it intentional that you only create the UART device
and do not add it to the FDT ? (UART0 doesn't seem to be
in the ACPI table generation code so I guess that part
is OK.)

You probably also want to test for compatibility:
for arm we found that different bits of software
(firmware, Linux kernel) annoyingly iterated through
the dtb in different orders.

Thanks for the comments.

Yes, tested with our software stack which has edk2 , u-boot firmware,  
and also linux/debian os.  No compatibility issue.



thanks
-- PMM