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

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

Posted by Yong Li 2 years, 9 months ago

The virt machine can have two UARTs and the second UART
can be used when host secure-mode support is enabled.

Signed-off-by: Yong Li <yong.li@intel.com>
Cc: "Zhiwei Liu" <zhiwei_liu@linux.alibaba.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));
diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index e5c474b26e..8d2f8f225d 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -74,6 +74,7 @@ enum {
     VIRT_APLIC_S,
     VIRT_UART0,
     VIRT_VIRTIO,
+    VIRT_UART1,
     VIRT_FW_CFG,
     VIRT_IMSIC_M,
     VIRT_IMSIC_S,
@@ -88,6 +89,7 @@ enum {
 enum {
     UART0_IRQ = 10,
     RTC_IRQ = 11,
+    UART1_IRQ = 12,
     VIRTIO_IRQ = 1, /* 1 to 8 */
     VIRTIO_COUNT = 8,
     PCIE_IRQ = 0x20, /* 32 to 35 */
-- 
2.25.1

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

Posted by Philippe Mathieu-Daudé 2 years, 9 months ago

On 24/4/23 03:01, Yong Li wrote:
> The virt machine can have two UARTs and the second UART
> can be used when host secure-mode support is enabled.
> 
> Signed-off-by: Yong Li <yong.li@intel.com>
> Cc: "Zhiwei Liu" <zhiwei_liu@linux.alibaba.com>
> ---
>   hw/riscv/virt.c         | 4 ++++
>   include/hw/riscv/virt.h | 2 ++
>   2 files changed, 6 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

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

Posted by LIU Zhiwei 2 years, 9 months ago

On 2023/4/24 9:01, Yong Li wrote:
> The virt machine can have two UARTs and the second UART
> can be used when host secure-mode support is enabled.
>
> Signed-off-by: Yong Li<yong.li@intel.com>
> Cc: "Zhiwei Liu"<zhiwei_liu@linux.alibaba.com>

Should  cc other Maintainers and Reviewers. Get the list by running the 
script

./scripts/get_maintainer.pl yours.patch

> ---
>   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 },

Can we move it a position adjacent to the VIRT_UART0, such as 0x10000100?

Otherwise,

Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>

Zhiwei

>       [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));
> diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
> index e5c474b26e..8d2f8f225d 100644
> --- a/include/hw/riscv/virt.h
> +++ b/include/hw/riscv/virt.h
> @@ -74,6 +74,7 @@ enum {
>       VIRT_APLIC_S,
>       VIRT_UART0,
>       VIRT_VIRTIO,
> +    VIRT_UART1,
>       VIRT_FW_CFG,
>       VIRT_IMSIC_M,
>       VIRT_IMSIC_S,
> @@ -88,6 +89,7 @@ enum {
>   enum {
>       UART0_IRQ = 10,
>       RTC_IRQ = 11,
> +    UART1_IRQ = 12,
>       VIRTIO_IRQ = 1, /* 1 to 8 */
>       VIRTIO_COUNT = 8,
>       PCIE_IRQ = 0x20, /* 32 to 35 */

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

Posted by Li, Yong 2 years, 9 months ago

On 2023/4/24 10:42, LIU Zhiwei wrote:
>
>
> On 2023/4/24 9:01, Yong Li wrote:
>> The virt machine can have two UARTs and the second UART
>> can be used when host secure-mode support is enabled.
>>
>> Signed-off-by: Yong Li<yong.li@intel.com>
>> Cc: "Zhiwei Liu"<zhiwei_liu@linux.alibaba.com>
>
> Should  cc other Maintainers and Reviewers. Get the list by running 
> the script
>
> ./scripts/get_maintainer.pl yours.patch
>
Sure,

>> ---
>>   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 },
>
> Can we move it a position adjacent to the VIRT_UART0, such as 0x10000100?
>
> Otherwise,
>
This probably cause back compatible issue to the firmware or other code 
who use VIRT_VIRTIO.

Actually, similar code in hw/arm/sbsa-ref.c,  the UART devices' address 
are also not continuously.

It should not be a problem. Thanks!


> Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
>
> Zhiwei
>
>>       [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));
>> diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
>> index e5c474b26e..8d2f8f225d 100644
>> --- a/include/hw/riscv/virt.h
>> +++ b/include/hw/riscv/virt.h
>> @@ -74,6 +74,7 @@ enum {
>>       VIRT_APLIC_S,
>>       VIRT_UART0,
>>       VIRT_VIRTIO,
>> +    VIRT_UART1,
>>       VIRT_FW_CFG,
>>       VIRT_IMSIC_M,
>>       VIRT_IMSIC_S,
>> @@ -88,6 +89,7 @@ enum {
>>   enum {
>>       UART0_IRQ = 10,
>>       RTC_IRQ = 11,
>> +    UART1_IRQ = 12,
>>       VIRTIO_IRQ = 1, /* 1 to 8 */
>>       VIRTIO_COUNT = 8,
>>       PCIE_IRQ = 0x20, /* 32 to 35 */