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.li@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.li@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
>