* [PATCH qemu.git 0/1] hw/arm/virt: make second UART available @ 2022-11-30 17:00 ~axelheider 2022-11-14 12:06 ` [PATCH qemu.git 1/1] " ~axelheider 2022-11-30 19:11 ` [PATCH qemu.git 0/1] " Alex Bennée 0 siblings, 2 replies; 6+ messages in thread From: ~axelheider @ 2022-11-30 17:00 UTC (permalink / raw) To: qemu-devel; +Cc: qemu-arm, peter.maydell, daniel.thompson This is a follow-up on older attempts to make a second UART available for the arm-virt machine in normal world. The use case is, that this give a simple I/O channel in addition to stdout, as this simplifies various test scenarios. Especially for non-Linux operating system,s (e.g. seL4) where arm-virt is handy as a generic machine for testing purposes. There are existing discussions about this topic at: - https://lists.gnu.org/archive/html/qemu-arm/2017-12/msg00063.html - https://lists.nongnu.org/archive/html/qemu- discuss/2018-11/msg00001.html - https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg01613.html This patch tries to address the concerns raised there and also avoid breaking compatibility with existing setups. Axel Heider (1): hw/arm/virt: make second UART available hw/arm/virt-acpi-build.c | 12 ++++----- hw/arm/virt.c | 55 ++++++++++++++++++++++++++++++---------- include/hw/arm/virt.h | 4 +-- 3 files changed, 49 insertions(+), 22 deletions(-) -- 2.34.5 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH qemu.git 1/1] hw/arm/virt: make second UART available 2022-11-30 17:00 [PATCH qemu.git 0/1] hw/arm/virt: make second UART available ~axelheider @ 2022-11-14 12:06 ` ~axelheider 2022-11-30 18:15 ` Philippe Mathieu-Daudé 2022-11-30 19:11 ` [PATCH qemu.git 0/1] " Alex Bennée 1 sibling, 1 reply; 6+ messages in thread From: ~axelheider @ 2022-11-14 12:06 UTC (permalink / raw) To: qemu-devel; +Cc: qemu-arm, peter.maydell, daniel.thompson From: Axel Heider <axel.heider@hensoldt.net> The first UART always always exists. If the security extensions are enabled, the second UART also always exists. Otherwise, it only exists if a backend is configured explicitly via '-serial <backend>', where 'null' creates a dummy backend. This allows enabling the second UART explicitly on demand and does not interfere with any existing setup that just expect one (or two if security extensions are enabled) UARTs. Signed-off-by: Axel Heider <axel.heider@hensoldt.net> --- hw/arm/virt-acpi-build.c | 12 ++++----- hw/arm/virt.c | 55 ++++++++++++++++++++++++++++++---------- include/hw/arm/virt.h | 4 +-- 3 files changed, 49 insertions(+), 22 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 4156111d49..3e1852a1b9 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -482,14 +482,14 @@ build_spcr(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) build_append_int_noprefix(table_data, 0, 3); /* Reserved */ /* Base Address */ build_append_gas(table_data, AML_AS_SYSTEM_MEMORY, 8, 0, 1, - vms->memmap[VIRT_UART].base); + vms->memmap[VIRT_UART0].base); /* Interrupt Type */ build_append_int_noprefix(table_data, (1 << 3) /* Bit[3] ARMH GIC interrupt */, 1); build_append_int_noprefix(table_data, 0, 1); /* IRQ */ /* Global System Interrupt */ build_append_int_noprefix(table_data, - vms->irqmap[VIRT_UART] + ARM_SPI_BASE, 4); + vms->irqmap[VIRT_UART0] + ARM_SPI_BASE, 4); build_append_int_noprefix(table_data, 3 /* 9600 */, 1); /* Baud Rate */ build_append_int_noprefix(table_data, 0 /* No Parity */, 1); /* Parity */ /* Stop Bits */ @@ -673,11 +673,11 @@ build_dbg2(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) /* BaseAddressRegister[] */ build_append_gas(table_data, AML_AS_SYSTEM_MEMORY, 8, 0, 1, - vms->memmap[VIRT_UART].base); + vms->memmap[VIRT_UART0].base); /* AddressSize[] */ build_append_int_noprefix(table_data, - vms->memmap[VIRT_UART].size, 4); + vms->memmap[VIRT_UART0].size, 4); /* NamespaceString[] */ g_array_append_vals(table_data, name, namespace_length); @@ -858,8 +858,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) */ scope = aml_scope("\\_SB"); acpi_dsdt_add_cpus(scope, vms); - acpi_dsdt_add_uart(scope, &memmap[VIRT_UART], - (irqmap[VIRT_UART] + ARM_SPI_BASE)); + acpi_dsdt_add_uart(scope, &memmap[VIRT_UART0], + (irqmap[VIRT_UART0] + ARM_SPI_BASE)); if (vmc->acpi_expose_flash) { acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]); } diff --git a/hw/arm/virt.c b/hw/arm/virt.c index b871350856..59959c75b0 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -143,11 +143,11 @@ static const MemMapEntry base_memmap[] = { [VIRT_GIC_ITS] = { 0x08080000, 0x00020000 }, /* This redistributor space allows up to 2*64kB*123 CPUs */ [VIRT_GIC_REDIST] = { 0x080A0000, 0x00F60000 }, - [VIRT_UART] = { 0x09000000, 0x00001000 }, + [VIRT_UART0] = { 0x09000000, 0x00001000 }, [VIRT_RTC] = { 0x09010000, 0x00001000 }, [VIRT_FW_CFG] = { 0x09020000, 0x00000018 }, [VIRT_GPIO] = { 0x09030000, 0x00001000 }, - [VIRT_SECURE_UART] = { 0x09040000, 0x00001000 }, + [VIRT_UART1] = { 0x09040000, 0x00001000 }, /* secure UART */ [VIRT_SMMU] = { 0x09050000, 0x00020000 }, [VIRT_PCDIMM_ACPI] = { 0x09070000, MEMORY_HOTPLUG_IO_LEN }, [VIRT_ACPI_GED] = { 0x09080000, ACPI_GED_EVT_SEL_LEN }, @@ -184,11 +184,11 @@ static MemMapEntry extended_memmap[] = { }; static const int a15irqmap[] = { - [VIRT_UART] = 1, + [VIRT_UART0] = 1, [VIRT_RTC] = 2, [VIRT_PCIE] = 3, /* ... to 6 */ [VIRT_GPIO] = 7, - [VIRT_SECURE_UART] = 8, + [VIRT_UART1] = 8, [VIRT_ACPI_GED] = 9, [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */ [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */ @@ -843,6 +843,27 @@ static void create_uart(const VirtMachineState *vms, int uart, MemoryRegion *mem, Chardev *chr) { char *nodename; + /* + * The first UART always always exists. If the security extensions are + * enabled, the second UART also always exists. Otherwise, it only exists + * if a backend is configured explicitly via '-serial <backend>', where + * 'null' creates a dummy backend. This allows enabling the second UART + * explicitly on demand and does not interfere with any existing setup that + * just expect one (or two if security extensions are enabled) UARTs. + */ + switch(uart) { + case VIRT_UART0: + break; + case VIRT_UART1: + if (!vms->secure && !chr) { + return; + } + break; + default: + error_report("unsupported UART ID %d", uart); + exit(1); + } + hwaddr base = vms->memmap[uart].base; hwaddr size = vms->memmap[uart].size; int irq = vms->irqmap[uart]; @@ -854,6 +875,7 @@ static void create_uart(const VirtMachineState *vms, int uart, qdev_prop_set_chr(dev, "chardev", chr); sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); + /* if security extensions are enabled, 'mem' is 'secure_sysmem' for UART1 */ memory_region_add_subregion(mem, base, sysbus_mmio_get_region(s, 0)); sysbus_connect_irq(s, 0, qdev_get_gpio_in(vms->gic, irq)); @@ -873,15 +895,19 @@ static void create_uart(const VirtMachineState *vms, int uart, qemu_fdt_setprop(ms->fdt, nodename, "clock-names", clocknames, sizeof(clocknames)); - if (uart == VIRT_UART) { + switch(uart) { + case VIRT_UART0: qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename); - } else { - /* Mark as not usable by the normal world */ - qemu_fdt_setprop_string(ms->fdt, nodename, "status", "disabled"); - qemu_fdt_setprop_string(ms->fdt, nodename, "secure-status", "okay"); - - qemu_fdt_setprop_string(ms->fdt, "/secure-chosen", "stdout-path", - nodename); + break; + case VIRT_UART1: + if (vms->secure) { + qemu_fdt_setprop_string(ms->fdt, nodename, "status", "disabled"); + qemu_fdt_setprop_string(ms->fdt, nodename, "secure-status", "okay"); + qemu_fdt_setprop_string(ms->fdt, "/secure-chosen", "stdout-path", + nodename); + } + default: + break; } g_free(nodename); @@ -2222,11 +2248,12 @@ static void machvirt_init(MachineState *machine) fdt_add_pmu_nodes(vms); - create_uart(vms, VIRT_UART, sysmem, serial_hd(0)); + create_uart(vms, VIRT_UART0, sysmem, serial_hd(0)); + create_uart(vms, VIRT_UART1, vms->secure ? secure_sysmem : sysmem, + serial_hd(1)); if (vms->secure) { create_secure_ram(vms, secure_sysmem, secure_tag_sysmem); - create_uart(vms, VIRT_SECURE_UART, secure_sysmem, serial_hd(1)); } if (tag_sysmem) { diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h index 6ec479ca2b..90563c132b 100644 --- a/include/hw/arm/virt.h +++ b/include/hw/arm/virt.h @@ -69,7 +69,7 @@ enum { VIRT_GIC_ITS, VIRT_GIC_REDIST, VIRT_SMMU, - VIRT_UART, + VIRT_UART0, VIRT_MMIO, VIRT_RTC, VIRT_FW_CFG, @@ -79,7 +79,7 @@ enum { VIRT_PCIE_ECAM, VIRT_PLATFORM_BUS, VIRT_GPIO, - VIRT_SECURE_UART, + VIRT_UART1, /* secure UART if vms->secure */ VIRT_SECURE_MEM, VIRT_SECURE_GPIO, VIRT_PCDIMM_ACPI, -- 2.34.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH qemu.git 1/1] hw/arm/virt: make second UART available 2022-11-14 12:06 ` [PATCH qemu.git 1/1] " ~axelheider @ 2022-11-30 18:15 ` Philippe Mathieu-Daudé 2022-11-30 18:45 ` Axel Heider 0 siblings, 1 reply; 6+ messages in thread From: Philippe Mathieu-Daudé @ 2022-11-30 18:15 UTC (permalink / raw) To: ~axelheider, qemu-devel; +Cc: qemu-arm, peter.maydell, daniel.thompson Hi Axel, On 14/11/22 13:06, ~axelheider wrote: > From: Axel Heider <axel.heider@hensoldt.net> > > The first UART always always exists. If the security extensions are > enabled, the second UART also always exists. Otherwise, it only exists > if a backend is configured explicitly via '-serial <backend>', where > 'null' creates a dummy backend. This allows enabling the second UART > explicitly on demand and does not interfere with any existing setup > that just expect one (or two if security extensions are enabled) > UARTs. > > Signed-off-by: Axel Heider <axel.heider@hensoldt.net> > --- > hw/arm/virt-acpi-build.c | 12 ++++----- > hw/arm/virt.c | 55 ++++++++++++++++++++++++++++++---------- > include/hw/arm/virt.h | 4 +-- > 3 files changed, 49 insertions(+), 22 deletions(-) > @@ -843,6 +843,27 @@ static void create_uart(const VirtMachineState *vms, int uart, > MemoryRegion *mem, Chardev *chr) > { > char *nodename; > + /* > + * The first UART always always exists. If the security extensions are > + * enabled, the second UART also always exists. Otherwise, it only exists > + * if a backend is configured explicitly via '-serial <backend>', where > + * 'null' creates a dummy backend. This allows enabling the second UART > + * explicitly on demand and does not interfere with any existing setup that > + * just expect one (or two if security extensions are enabled) UARTs. > + */ > + switch(uart) { > + case VIRT_UART0: > + break; > + case VIRT_UART1: Maybe pass a 'is_secure' boolean? > + if (!vms->secure && !chr) { > + return; > + } > + break; > + default: > + error_report("unsupported UART ID %d", uart); > + exit(1); > + } > @@ -2222,11 +2248,12 @@ static void machvirt_init(MachineState *machine) > > fdt_add_pmu_nodes(vms); > > - create_uart(vms, VIRT_UART, sysmem, serial_hd(0)); > + create_uart(vms, VIRT_UART0, sysmem, serial_hd(0)); > + create_uart(vms, VIRT_UART1, vms->secure ? secure_sysmem : sysmem, > + serial_hd(1)); > > if (vms->secure) { > create_secure_ram(vms, secure_sysmem, secure_tag_sysmem); > - create_uart(vms, VIRT_SECURE_UART, secure_sysmem, serial_hd(1)); Correct. > } > > if (tag_sysmem) { > diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h > index 6ec479ca2b..90563c132b 100644 > --- a/include/hw/arm/virt.h > +++ b/include/hw/arm/virt.h > @@ -69,7 +69,7 @@ enum { > VIRT_GIC_ITS, > VIRT_GIC_REDIST, > VIRT_SMMU, > - VIRT_UART, > + VIRT_UART0, > VIRT_MMIO, > VIRT_RTC, > VIRT_FW_CFG, > @@ -79,7 +79,7 @@ enum { > VIRT_PCIE_ECAM, > VIRT_PLATFORM_BUS, > VIRT_GPIO, > - VIRT_SECURE_UART, > + VIRT_UART1, /* secure UART if vms->secure */ (I'm not sure changing the name is worth the churn). Regards, Phil. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH qemu.git 1/1] hw/arm/virt: make second UART available 2022-11-30 18:15 ` Philippe Mathieu-Daudé @ 2022-11-30 18:45 ` Axel Heider 0 siblings, 0 replies; 6+ messages in thread From: Axel Heider @ 2022-11-30 18:45 UTC (permalink / raw) To: qemu-devel Cc: qemu-arm, peter.maydell, daniel.thompson, Philippe Mathieu-Daudé Hi, >> + switch(uart) { >> + case VIRT_UART0: >> + break; >> + case VIRT_UART1: >> > Maybe pass a 'is_secure' boolean? I don't think this would really make things easier. I wanted to avoid too many changes in this patch. The price is, that there are two places where decisions about the configuration are made. But these are also two independent decisions: (a) which memory this belongs to and (b) if the UART exists at all. I think the code is easier to maintain this way, because more UARTs can be added with a small patch then. Note also, that a parameter 'is_secure' would still not avoid mis-usage, as we still need the 'mem' parameter for 'secure_sysmem' or 'sysmem', because this is no available from the 'vms' object. >> - VIRT_SECURE_UART, >> + VIRT_UART1, /* secure UART if vms->secure */ >> > (I'm not sure changing the name is worth the churn). After this patch is merged, we have two UARTs, so the naming is a bit more consistent. It's no longer a secure UART only. And when adding even more UARTs in further customization, it's easier to understand the order of the UARTs. Axel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH qemu.git 0/1] hw/arm/virt: make second UART available 2022-11-30 17:00 [PATCH qemu.git 0/1] hw/arm/virt: make second UART available ~axelheider 2022-11-14 12:06 ` [PATCH qemu.git 1/1] " ~axelheider @ 2022-11-30 19:11 ` Alex Bennée 2022-11-30 19:24 ` Axel Heider 1 sibling, 1 reply; 6+ messages in thread From: Alex Bennée @ 2022-11-30 19:11 UTC (permalink / raw) To: ~axelheider Cc: qemu-devel, peter.maydell, daniel.thompson, ~axelheider, qemu-arm ~axelheider <axelheider@git.sr.ht> writes: > This is a follow-up on older attempts to make a second UART > available for the arm-virt machine in normal world. The use case > is, that this give a simple I/O channel in addition to stdout, as this > simplifies various test scenarios. Especially for non-Linux operating > system,s (e.g. seL4) where arm-virt is handy as a generic machine > for testing purposes. > > There are existing discussions about this topic at: > - https://lists.gnu.org/archive/html/qemu-arm/2017-12/msg00063.html > - https://lists.nongnu.org/archive/html/qemu- > discuss/2018-11/msg00001.html > - https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg01613.html > > This patch tries to address the concerns raised there and also > avoid breaking compatibility with existing setups. > > Axel Heider (1): > hw/arm/virt: make second UART available > > hw/arm/virt-acpi-build.c | 12 ++++----- > hw/arm/virt.c | 55 ++++++++++++++++++++++++++++++---------- > include/hw/arm/virt.h | 4 +-- > 3 files changed, 49 insertions(+), 22 deletions(-) It would also be worth updating ./docs/system/arm/virt.rst to document this feature. -- Alex Bennée ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH qemu.git 0/1] hw/arm/virt: make second UART available 2022-11-30 19:11 ` [PATCH qemu.git 0/1] " Alex Bennée @ 2022-11-30 19:24 ` Axel Heider 0 siblings, 0 replies; 6+ messages in thread From: Axel Heider @ 2022-11-30 19:24 UTC (permalink / raw) To: qemu-devel Cc: peter.maydell, daniel.thompson, ~axelheider, qemu-arm, Alex Bennée Alex, > It would also be worth updating ./docs/system/arm/virt.rst to document > this feature. Good point. I will add this in the next iteration of the patch. Until then, the proposed doc changes can be found here: https://gitlab.com/axel-h/qemu/-/merge_requests/1/diffs Axel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-11-30 19:24 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-11-30 17:00 [PATCH qemu.git 0/1] hw/arm/virt: make second UART available ~axelheider 2022-11-14 12:06 ` [PATCH qemu.git 1/1] " ~axelheider 2022-11-30 18:15 ` Philippe Mathieu-Daudé 2022-11-30 18:45 ` Axel Heider 2022-11-30 19:11 ` [PATCH qemu.git 0/1] " Alex Bennée 2022-11-30 19:24 ` Axel Heider
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.