* [PATCH] hw/arm/virt: support both pl011 and 16550 uart
@ 2023-03-22 23:27 Patrick Venture
2023-03-23 8:13 ` Philippe Mathieu-Daudé
2023-03-23 11:26 ` Peter Maydell
0 siblings, 2 replies; 3+ messages in thread
From: Patrick Venture @ 2023-03-22 23:27 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-arm, qemu-devel, Shu-Chun Weng, Patrick Venture
From: Shu-Chun Weng <scw@google.com>
Select uart for virt machine from pl011 and ns16550a with
-M virt,uart={pl011|ns16550a}.
Signed-off-by: Shu-Chun Weng <scw@google.com>
Signed-off-by: Patrick Venture <venture@google.com>
---
hw/arm/virt.c | 85 ++++++++++++++++++++++++++++++++++++++++++-
include/hw/arm/virt.h | 6 +++
2 files changed, 89 insertions(+), 2 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ac626b3bef..84b335a5d7 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -80,6 +80,7 @@
#include "hw/virtio/virtio-iommu.h"
#include "hw/char/pl011.h"
#include "qemu/guest-random.h"
+#include "hw/char/serial.h"
#define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
@@ -847,8 +848,37 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
}
}
-static void create_uart(const VirtMachineState *vms, int uart,
- MemoryRegion *mem, Chardev *chr)
+static void create_uart_ns16550a(const VirtMachineState *vms, int uart,
+ MemoryRegion *mem, Chardev *chr)
+{
+ char *nodename;
+ hwaddr base = vms->memmap[uart].base;
+ hwaddr size = vms->memmap[uart].size;
+ int irq = vms->irqmap[uart];
+ const char compat[] = "ns16550a";
+
+ serial_mm_init(get_system_memory(), base, 0,
+ qdev_get_gpio_in(vms->gic, irq), 19200, serial_hd(0),
+ DEVICE_LITTLE_ENDIAN);
+
+ nodename = g_strdup_printf("/serial@%" PRIx64, base);
+
+ MachineState *ms = MACHINE(vms);
+
+ qemu_fdt_add_subnode(ms->fdt, nodename);
+ qemu_fdt_setprop(ms->fdt, nodename, "compatible", compat, sizeof(compat));
+ qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg", 2, base, 2, size);
+ qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "clock-frequency",
+ 1, 0x825f0);
+ qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
+ GIC_FDT_IRQ_TYPE_SPI, irq,
+ GIC_FDT_IRQ_FLAGS_LEVEL_HI);
+
+ g_free(nodename);
+}
+
+static void create_uart_pl011(const VirtMachineState *vms, int uart,
+ MemoryRegion *mem, Chardev *chr)
{
char *nodename;
hwaddr base = vms->memmap[uart].base;
@@ -895,6 +925,16 @@ static void create_uart(const VirtMachineState *vms, int uart,
g_free(nodename);
}
+static void create_uart(const VirtMachineState *vms, int uart,
+ MemoryRegion *mem, Chardev *chr)
+{
+ if (vms->uart == UART_NS16550A) {
+ create_uart_ns16550a(vms, uart, mem, chr);
+ } else {
+ create_uart_pl011(vms, uart, mem, chr);
+ }
+}
+
static void create_rtc(const VirtMachineState *vms)
{
char *nodename;
@@ -2601,6 +2641,39 @@ static void virt_set_gic_version(Object *obj, const char *value, Error **errp)
}
}
+static char *virt_get_uart_type(Object *obj, Error **errp)
+{
+ VirtMachineState *vms = VIRT_MACHINE(obj);
+ const char *val = NULL;
+
+ switch (vms->uart) {
+ case UART_PL011:
+ val = "pl011";
+ break;
+ case UART_NS16550A:
+ val = "ns16550a";
+ break;
+ default:
+ error_setg(errp, "Invalid uart value");
+ }
+
+ return g_strdup(val);
+}
+
+static void virt_set_uart_type(Object *obj, const char *value, Error **errp)
+{
+ VirtMachineState *vms = VIRT_MACHINE(obj);
+
+ if (!strcmp(value, "pl011")) {
+ vms->uart = UART_PL011;
+ } else if (!strcmp(value, "ns16550a")) {
+ vms->uart = UART_NS16550A;
+ } else {
+ error_setg(errp, "Invalid uart type");
+ error_append_hint(errp, "Valid values are pl011, and ns16550a.\n");
+ }
+}
+
static char *virt_get_iommu(Object *obj, Error **errp)
{
VirtMachineState *vms = VIRT_MACHINE(obj);
@@ -3172,6 +3245,14 @@ static void virt_instance_init(Object *obj)
vms->highmem_compact = !vmc->no_highmem_compact;
vms->gic_version = VIRT_GIC_VERSION_NOSEL;
+ /* Default uart type is pl011 */
+ vms->uart = UART_PL011;
+ object_property_add_str(obj, "uart", virt_get_uart_type,
+ virt_set_uart_type);
+ object_property_set_description(obj, "uart",
+ "Set uart type. "
+ "Valid values are pl011 and ns16550a");
+
vms->highmem_ecam = !vmc->no_highmem_ecam;
vms->highmem_mmio = true;
vms->highmem_redists = true;
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index e1ddbea96b..04539f347d 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -122,6 +122,11 @@ typedef enum VirtGICType {
#define VIRT_GIC_VERSION_3_MASK BIT(VIRT_GIC_VERSION_3)
#define VIRT_GIC_VERSION_4_MASK BIT(VIRT_GIC_VERSION_4)
+typedef enum UartType {
+ UART_PL011,
+ UART_NS16550A,
+} UartType;
+
struct VirtMachineClass {
MachineClass parent;
bool disallow_affinity_adjustment;
@@ -183,6 +188,7 @@ struct VirtMachineState {
PCIBus *bus;
char *oem_id;
char *oem_table_id;
+ UartType uart;
};
#define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] hw/arm/virt: support both pl011 and 16550 uart
2023-03-22 23:27 [PATCH] hw/arm/virt: support both pl011 and 16550 uart Patrick Venture
@ 2023-03-23 8:13 ` Philippe Mathieu-Daudé
2023-03-23 11:26 ` Peter Maydell
1 sibling, 0 replies; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-03-23 8:13 UTC (permalink / raw)
To: Patrick Venture, peter.maydell; +Cc: qemu-arm, qemu-devel, Shu-Chun Weng
On 23/3/23 00:27, Patrick Venture wrote:
> From: Shu-Chun Weng <scw@google.com>
>
> Select uart for virt machine from pl011 and ns16550a with
> -M virt,uart={pl011|ns16550a}.
This commit description is missing the "why" we want this change.
In which case the pl011 isn't enough or we don't want it?
> Signed-off-by: Shu-Chun Weng <scw@google.com>
> Signed-off-by: Patrick Venture <venture@google.com>
> ---
> hw/arm/virt.c | 85 ++++++++++++++++++++++++++++++++++++++++++-
> include/hw/arm/virt.h | 6 +++
> 2 files changed, 89 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index ac626b3bef..84b335a5d7 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -80,6 +80,7 @@
> #include "hw/virtio/virtio-iommu.h"
> #include "hw/char/pl011.h"
> #include "qemu/guest-random.h"
> +#include "hw/char/serial.h"
>
> #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
> static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
> @@ -847,8 +848,37 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
> }
> }
>
> -static void create_uart(const VirtMachineState *vms, int uart,
> - MemoryRegion *mem, Chardev *chr)
> +static void create_uart_ns16550a(const VirtMachineState *vms, int uart,
> + MemoryRegion *mem, Chardev *chr)
> +{
> + char *nodename;
> + hwaddr base = vms->memmap[uart].base;
> + hwaddr size = vms->memmap[uart].size;
> + int irq = vms->irqmap[uart];
> + const char compat[] = "ns16550a";
> +
> + serial_mm_init(get_system_memory(), base, 0,
> + qdev_get_gpio_in(vms->gic, irq), 19200, serial_hd(0),
> + DEVICE_LITTLE_ENDIAN);
> +
> + nodename = g_strdup_printf("/serial@%" PRIx64, base);
> +
> + MachineState *ms = MACHINE(vms);
> +
> + qemu_fdt_add_subnode(ms->fdt, nodename);
> + qemu_fdt_setprop(ms->fdt, nodename, "compatible", compat, sizeof(compat));
> + qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg", 2, base, 2, size);
> + qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "clock-frequency",
> + 1, 0x825f0);
> + qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
> + GIC_FDT_IRQ_TYPE_SPI, irq,
> + GIC_FDT_IRQ_FLAGS_LEVEL_HI);
> +
> + g_free(nodename);
> +}
> +
> +static void create_uart_pl011(const VirtMachineState *vms, int uart,
> + MemoryRegion *mem, Chardev *chr)
> {
> char *nodename;
> hwaddr base = vms->memmap[uart].base;
> @@ -895,6 +925,16 @@ static void create_uart(const VirtMachineState *vms, int uart,
> g_free(nodename);
> }
>
> +static void create_uart(const VirtMachineState *vms, int uart,
> + MemoryRegion *mem, Chardev *chr)
> +{
> + if (vms->uart == UART_NS16550A) {
> + create_uart_ns16550a(vms, uart, mem, chr);
> + } else {
> + create_uart_pl011(vms, uart, mem, chr);
> + }
> +}
> +
> static void create_rtc(const VirtMachineState *vms)
> {
> char *nodename;
> @@ -2601,6 +2641,39 @@ static void virt_set_gic_version(Object *obj, const char *value, Error **errp)
> }
> }
>
> +static char *virt_get_uart_type(Object *obj, Error **errp)
> +{
> + VirtMachineState *vms = VIRT_MACHINE(obj);
> + const char *val = NULL;
> +
> + switch (vms->uart) {
> + case UART_PL011:
> + val = "pl011";
> + break;
> + case UART_NS16550A:
> + val = "ns16550a";
> + break;
> + default:
> + error_setg(errp, "Invalid uart value");
> + }
> +
> + return g_strdup(val);
> +}
> +
> +static void virt_set_uart_type(Object *obj, const char *value, Error **errp)
> +{
> + VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> + if (!strcmp(value, "pl011")) {
> + vms->uart = UART_PL011;
> + } else if (!strcmp(value, "ns16550a")) {
> + vms->uart = UART_NS16550A;
> + } else {
> + error_setg(errp, "Invalid uart type");
> + error_append_hint(errp, "Valid values are pl011, and ns16550a.\n");
> + }
> +}
> +
> static char *virt_get_iommu(Object *obj, Error **errp)
> {
> VirtMachineState *vms = VIRT_MACHINE(obj);
> @@ -3172,6 +3245,14 @@ static void virt_instance_init(Object *obj)
> vms->highmem_compact = !vmc->no_highmem_compact;
> vms->gic_version = VIRT_GIC_VERSION_NOSEL;
>
> + /* Default uart type is pl011 */
> + vms->uart = UART_PL011;
> + object_property_add_str(obj, "uart", virt_get_uart_type,
> + virt_set_uart_type);
> + object_property_set_description(obj, "uart",
> + "Set uart type. "
> + "Valid values are pl011 and ns16550a");
> +
> vms->highmem_ecam = !vmc->no_highmem_ecam;
> vms->highmem_mmio = true;
> vms->highmem_redists = true;
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index e1ddbea96b..04539f347d 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -122,6 +122,11 @@ typedef enum VirtGICType {
> #define VIRT_GIC_VERSION_3_MASK BIT(VIRT_GIC_VERSION_3)
> #define VIRT_GIC_VERSION_4_MASK BIT(VIRT_GIC_VERSION_4)
>
> +typedef enum UartType {
> + UART_PL011,
> + UART_NS16550A,
> +} UartType;
> +
> struct VirtMachineClass {
> MachineClass parent;
> bool disallow_affinity_adjustment;
> @@ -183,6 +188,7 @@ struct VirtMachineState {
> PCIBus *bus;
> char *oem_id;
> char *oem_table_id;
> + UartType uart;
> };
>
> #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] hw/arm/virt: support both pl011 and 16550 uart
2023-03-22 23:27 [PATCH] hw/arm/virt: support both pl011 and 16550 uart Patrick Venture
2023-03-23 8:13 ` Philippe Mathieu-Daudé
@ 2023-03-23 11:26 ` Peter Maydell
1 sibling, 0 replies; 3+ messages in thread
From: Peter Maydell @ 2023-03-23 11:26 UTC (permalink / raw)
To: Patrick Venture; +Cc: qemu-arm, qemu-devel, Shu-Chun Weng
On Wed, 22 Mar 2023 at 23:27, Patrick Venture <venture@google.com> wrote:
>
> From: Shu-Chun Weng <scw@google.com>
>
> Select uart for virt machine from pl011 and ns16550a with
> -M virt,uart={pl011|ns16550a}.
Firm "no". The PL011 is a perfectly good UART widely
supported by guest OSes, and there is no reason
why we should want to also provide a 16550.
thanks
-- PMM
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-03-23 11:27 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-22 23:27 [PATCH] hw/arm/virt: support both pl011 and 16550 uart Patrick Venture
2023-03-23 8:13 ` Philippe Mathieu-Daudé
2023-03-23 11:26 ` Peter Maydell
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.