* [PATCH v1 01/16] arm/vpl011: rename virtual PL011 Kconfig option
2025-06-24 3:54 [PATCH v1 00/16] xen: framework for UART emulators dmkhn
@ 2025-06-24 3:55 ` dmkhn
2025-06-24 6:13 ` Orzel, Michal
2025-06-24 3:55 ` [PATCH v1 02/16] arm/vpl011: move DT node parsing to PL011 emulator code dmkhn
` (14 subsequent siblings)
15 siblings, 1 reply; 59+ messages in thread
From: dmkhn @ 2025-06-24 3:55 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
From: Denis Mukhin <dmukhin@ford.com>
Rename CONFIG_SBSA_VUART_CONSOLE to CONFIG_HAS_VUART_PL011.
No functional change.
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
xen/arch/arm/Kconfig | 2 +-
xen/arch/arm/Makefile | 2 +-
xen/arch/arm/configs/tiny64_defconfig | 2 +-
xen/arch/arm/dom0less-build.c | 4 ++--
xen/arch/arm/include/asm/domain.h | 2 +-
xen/arch/arm/include/asm/vpl011.h | 2 +-
xen/drivers/char/console.c | 4 ++--
7 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 3f25da3ca5fd..03888569f38c 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -170,7 +170,7 @@ config NEW_VGIC
problems with the standard emulation.
At the moment this implementation is not security supported.
-config SBSA_VUART_CONSOLE
+config HAS_VUART_PL011
bool "Emulated SBSA UART console support"
default y
help
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index ab0a0c2be6d8..2d6787fb03bc 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -65,7 +65,7 @@ obj-$(CONFIG_HAS_ITS) += vgic-v3-its.o
endif
obj-$(CONFIG_VM_EVENT) += vm_event.o
obj-y += vtimer.o
-obj-$(CONFIG_SBSA_VUART_CONSOLE) += vpl011.o
+obj-$(CONFIG_HAS_VUART_PL011) += vpl011.o
obj-y += vsmc.o
obj-y += vpsci.o
obj-$(CONFIG_HWDOM_VUART) += vuart.o
diff --git a/xen/arch/arm/configs/tiny64_defconfig b/xen/arch/arm/configs/tiny64_defconfig
index 469a1eb9f99d..acc227262d81 100644
--- a/xen/arch/arm/configs/tiny64_defconfig
+++ b/xen/arch/arm/configs/tiny64_defconfig
@@ -6,7 +6,7 @@ CONFIG_ARM=y
#
# CONFIG_GICV3 is not set
# CONFIG_VM_EVENT is not set
-# CONFIG_SBSA_VUART_CONSOLE is not set
+# CONFIG_HAS_VUART_PL011 is not set
#
# Common Features
diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index 4b285cff5ee2..2a5531a2b892 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -167,7 +167,7 @@ int __init make_intc_domU_node(struct kernel_info *kinfo)
}
}
-#ifdef CONFIG_SBSA_VUART_CONSOLE
+#ifdef CONFIG_HAS_VUART_PL011
static int __init make_vpl011_uart_node(struct kernel_info *kinfo)
{
void *fdt = kinfo->fdt;
@@ -226,7 +226,7 @@ int __init make_arch_nodes(struct kernel_info *kinfo)
if ( kinfo->arch.vpl011 )
{
-#ifdef CONFIG_SBSA_VUART_CONSOLE
+#ifdef CONFIG_HAS_VUART_PL011
ret = make_vpl011_uart_node(kinfo);
#endif
if ( ret )
diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
index a3487ca71303..746ea687d523 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -113,7 +113,7 @@ struct arch_domain
uint8_t privileged_call_enabled : 1;
} monitor;
-#ifdef CONFIG_SBSA_VUART_CONSOLE
+#ifdef CONFIG_HAS_VUART_PL011
struct vpl011 vpl011;
#endif
diff --git a/xen/arch/arm/include/asm/vpl011.h b/xen/arch/arm/include/asm/vpl011.h
index cc838682815c..be64883b8628 100644
--- a/xen/arch/arm/include/asm/vpl011.h
+++ b/xen/arch/arm/include/asm/vpl011.h
@@ -65,7 +65,7 @@ struct vpl011_init_info {
evtchn_port_t evtchn;
};
-#ifdef CONFIG_SBSA_VUART_CONSOLE
+#ifdef CONFIG_HAS_VUART_PL011
int domain_vpl011_init(struct domain *d,
struct vpl011_init_info *info);
void domain_vpl011_deinit(struct domain *d);
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 5879e31786ba..0f37badc471e 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -37,7 +37,7 @@
#ifdef CONFIG_X86
#include <asm/guest.h>
#endif
-#ifdef CONFIG_SBSA_VUART_CONSOLE
+#ifdef CONFIG_HAS_VUART_PL011
#include <asm/vpl011.h>
#endif
@@ -606,7 +606,7 @@ static void __serial_rx(char c)
*/
send_global_virq(VIRQ_CONSOLE);
}
-#ifdef CONFIG_SBSA_VUART_CONSOLE
+#ifdef CONFIG_HAS_VUART_PL011
else
/* Deliver input to the emulated UART. */
rc = vpl011_rx_char_xen(d, c);
--
2.34.1
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH v1 01/16] arm/vpl011: rename virtual PL011 Kconfig option
2025-06-24 3:55 ` [PATCH v1 01/16] arm/vpl011: rename virtual PL011 Kconfig option dmkhn
@ 2025-06-24 6:13 ` Orzel, Michal
2025-06-24 7:24 ` dmkhn
0 siblings, 1 reply; 59+ messages in thread
From: Orzel, Michal @ 2025-06-24 6:13 UTC (permalink / raw)
To: dmkhn, xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
On 24/06/2025 05:55, dmkhn@proton.me wrote:
> From: Denis Mukhin <dmukhin@ford.com>
>
> Rename CONFIG_SBSA_VUART_CONSOLE to CONFIG_HAS_VUART_PL011.
Why? We emulate SBSA UART and not PL011. Despite the similarities (the former is
a subset of the latter) they are not the same. I find it confusing and drivers
for PL011 might not work with SBSA UART. Also, in the future we may want to
emulate full PL011 in which case it will be even more confusing.
Also, why HAS_?
~Michal
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v1 01/16] arm/vpl011: rename virtual PL011 Kconfig option
2025-06-24 6:13 ` Orzel, Michal
@ 2025-06-24 7:24 ` dmkhn
2025-07-17 15:43 ` Alejandro Vallejo
0 siblings, 1 reply; 59+ messages in thread
From: dmkhn @ 2025-06-24 7:24 UTC (permalink / raw)
To: Orzel, Michal
Cc: xen-devel, andrew.cooper3, anthony.perard, jbeulich, julien,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
On Tue, Jun 24, 2025 at 08:13:08AM +0200, Orzel, Michal wrote:
>
>
> On 24/06/2025 05:55, dmkhn@proton.me wrote:
> > From: Denis Mukhin <dmukhin@ford.com>
> >
> > Rename CONFIG_SBSA_VUART_CONSOLE to CONFIG_HAS_VUART_PL011.
> Why? We emulate SBSA UART and not PL011. Despite the similarities (the former is
> a subset of the latter) they are not the same. I find it confusing and drivers
> for PL011 might not work with SBSA UART. Also, in the future we may want to
> emulate full PL011 in which case it will be even more confusing.
That's because the emulator is called vpl011, but yes, it is SBSA UART.
I will adjust to SBSA, thanks!
>
> Also, why HAS_?
My understanding is that HAS_ is the desired naming convention throughout the
code (not documented, though), e.g. all Arm UART drivers are gated by HAS_XXX
(e.g. arch/arm/platforms/Kconfig).
>
> ~Michal
>
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v1 01/16] arm/vpl011: rename virtual PL011 Kconfig option
2025-06-24 7:24 ` dmkhn
@ 2025-07-17 15:43 ` Alejandro Vallejo
2025-07-17 19:58 ` dmkhn
0 siblings, 1 reply; 59+ messages in thread
From: Alejandro Vallejo @ 2025-07-17 15:43 UTC (permalink / raw)
To: dmkhn, Orzel, Michal
Cc: xen-devel, andrew.cooper3, anthony.perard, jbeulich, julien,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, Xen-devel
On Tue Jun 24, 2025 at 9:24 AM CEST, dmkhn wrote:
> On Tue, Jun 24, 2025 at 08:13:08AM +0200, Orzel, Michal wrote:
>>
>>
>> On 24/06/2025 05:55, dmkhn@proton.me wrote:
>> > From: Denis Mukhin <dmukhin@ford.com>
>> >
>> > Rename CONFIG_SBSA_VUART_CONSOLE to CONFIG_HAS_VUART_PL011.
>> Why? We emulate SBSA UART and not PL011. Despite the similarities (the former is
>> a subset of the latter) they are not the same. I find it confusing and drivers
>> for PL011 might not work with SBSA UART. Also, in the future we may want to
>> emulate full PL011 in which case it will be even more confusing.
>
> That's because the emulator is called vpl011, but yes, it is SBSA UART.
> I will adjust to SBSA, thanks!
>
>>
>> Also, why HAS_?
>
> My understanding is that HAS_ is the desired naming convention throughout the
> code (not documented, though), e.g. all Arm UART drivers are gated by HAS_XXX
> (e.g. arch/arm/platforms/Kconfig).
HAS_ is a non-selectable property dependent on the arch. Think HAS_PCI, or
HAS_EHCI, etc. IOW: HAS_X means "you may implement feature X on this arch",
which is why some options have X and HAS_X variants, where X is selectable
while HAS_X is not.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v1 01/16] arm/vpl011: rename virtual PL011 Kconfig option
2025-07-17 15:43 ` Alejandro Vallejo
@ 2025-07-17 19:58 ` dmkhn
2025-07-18 11:10 ` Alejandro Vallejo
0 siblings, 1 reply; 59+ messages in thread
From: dmkhn @ 2025-07-17 19:58 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Orzel, Michal, xen-devel, andrew.cooper3, anthony.perard,
jbeulich, julien, oleksii.kurochko, roger.pau, sstabellini,
dmukhin, Xen-devel
On Thu, Jul 17, 2025 at 05:43:22PM +0200, Alejandro Vallejo wrote:
> On Tue Jun 24, 2025 at 9:24 AM CEST, dmkhn wrote:
> > On Tue, Jun 24, 2025 at 08:13:08AM +0200, Orzel, Michal wrote:
> >>
> >>
> >> On 24/06/2025 05:55, dmkhn@proton.me wrote:
> >> > From: Denis Mukhin <dmukhin@ford.com>
> >> >
> >> > Rename CONFIG_SBSA_VUART_CONSOLE to CONFIG_HAS_VUART_PL011.
> >> Why? We emulate SBSA UART and not PL011. Despite the similarities (the former is
> >> a subset of the latter) they are not the same. I find it confusing and drivers
> >> for PL011 might not work with SBSA UART. Also, in the future we may want to
> >> emulate full PL011 in which case it will be even more confusing.
> >
> > That's because the emulator is called vpl011, but yes, it is SBSA UART.
> > I will adjust to SBSA, thanks!
> >
> >>
> >> Also, why HAS_?
> >
> > My understanding is that HAS_ is the desired naming convention throughout the
> > code (not documented, though), e.g. all Arm UART drivers are gated by HAS_XXX
> > (e.g. arch/arm/platforms/Kconfig).
>
> HAS_ is a non-selectable property dependent on the arch. Think HAS_PCI, or
> HAS_EHCI, etc. IOW: HAS_X means "you may implement feature X on this arch",
> which is why some options have X and HAS_X variants, where X is selectable
> while HAS_X is not.
Thanks; I will fix that.
Jan explained the difference here [1] and [2]:
[1] https://lore.kernel.org/xen-devel/6d33355c-477f-4ef3-8f17-b7f1dd1164ce@suse.com/
[2] https://lore.kernel.org/xen-devel/a63ac9d5-152e-47b0-8169-bf470611c059@suse.com/
It's just there's a lot of drivers (UARTs) which are selectable by the user via
HAS_ symbols (drivers/char/Kconfig)
>
> Cheers,
> Alejandro
>
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v1 01/16] arm/vpl011: rename virtual PL011 Kconfig option
2025-07-17 19:58 ` dmkhn
@ 2025-07-18 11:10 ` Alejandro Vallejo
0 siblings, 0 replies; 59+ messages in thread
From: Alejandro Vallejo @ 2025-07-18 11:10 UTC (permalink / raw)
To: dmkhn
Cc: Orzel, Michal, xen-devel, andrew.cooper3, anthony.perard,
jbeulich, julien, oleksii.kurochko, roger.pau, sstabellini,
dmukhin, Xen-devel
On Thu Jul 17, 2025 at 9:58 PM CEST, dmkhn wrote:
> On Thu, Jul 17, 2025 at 05:43:22PM +0200, Alejandro Vallejo wrote:
>> On Tue Jun 24, 2025 at 9:24 AM CEST, dmkhn wrote:
>> > On Tue, Jun 24, 2025 at 08:13:08AM +0200, Orzel, Michal wrote:
>> >>
>> >>
>> >> On 24/06/2025 05:55, dmkhn@proton.me wrote:
>> >> > From: Denis Mukhin <dmukhin@ford.com>
>> >> >
>> >> > Rename CONFIG_SBSA_VUART_CONSOLE to CONFIG_HAS_VUART_PL011.
>> >> Why? We emulate SBSA UART and not PL011. Despite the similarities (the former is
>> >> a subset of the latter) they are not the same. I find it confusing and drivers
>> >> for PL011 might not work with SBSA UART. Also, in the future we may want to
>> >> emulate full PL011 in which case it will be even more confusing.
>> >
>> > That's because the emulator is called vpl011, but yes, it is SBSA UART.
>> > I will adjust to SBSA, thanks!
>> >
>> >>
>> >> Also, why HAS_?
>> >
>> > My understanding is that HAS_ is the desired naming convention throughout the
>> > code (not documented, though), e.g. all Arm UART drivers are gated by HAS_XXX
>> > (e.g. arch/arm/platforms/Kconfig).
>>
>> HAS_ is a non-selectable property dependent on the arch. Think HAS_PCI, or
>> HAS_EHCI, etc. IOW: HAS_X means "you may implement feature X on this arch",
>> which is why some options have X and HAS_X variants, where X is selectable
>> while HAS_X is not.
>
> Thanks; I will fix that.
>
> Jan explained the difference here [1] and [2]:
> [1] https://lore.kernel.org/xen-devel/6d33355c-477f-4ef3-8f17-b7f1dd1164ce@suse.com/
> [2] https://lore.kernel.org/xen-devel/a63ac9d5-152e-47b0-8169-bf470611c059@suse.com/
>
> It's just there's a lot of drivers (UARTs) which are selectable by the user via
> HAS_ symbols (drivers/char/Kconfig)
IMO, HAS_IMX_LPUART should be selected by ARM_64, then IMX_LPUART would depend
on HAS_IMX_LPUART. I don't know how those (and you're right, there's lots) got
there. Same with NS16550, etc.
That allows to decouple the device from the architectures where it might be
found.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v1 02/16] arm/vpl011: move DT node parsing to PL011 emulator code
2025-06-24 3:54 [PATCH v1 00/16] xen: framework for UART emulators dmkhn
2025-06-24 3:55 ` [PATCH v1 01/16] arm/vpl011: rename virtual PL011 Kconfig option dmkhn
@ 2025-06-24 3:55 ` dmkhn
2025-06-24 7:49 ` Orzel, Michal
2025-06-24 3:55 ` [PATCH v1 03/16] arm/vpl011: use vuart_ prefix in vpl011 public calls dmkhn
` (13 subsequent siblings)
15 siblings, 1 reply; 59+ messages in thread
From: dmkhn @ 2025-06-24 3:55 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
From: Denis Mukhin <dmukhin@ford.com>
Move vpl011 DT node parsing from common Arm code to PL011 emulator code.
While doing it pick the generic name vuart_add_fwnode() for DT parser function
and place the declaration in the common header in include/xen/vuart.h.
No functional change.
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
xen/arch/arm/dom0less-build.c | 52 ++---------------------------------
xen/arch/arm/vpl011.c | 52 +++++++++++++++++++++++++++++++++++
xen/include/xen/vuart.h | 23 ++++++++++++++++
3 files changed, 77 insertions(+), 50 deletions(-)
create mode 100644 xen/include/xen/vuart.h
diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index 2a5531a2b892..7c1b59750fb5 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -15,6 +15,7 @@
#include <xen/static-memory.h>
#include <xen/static-shmem.h>
#include <xen/vmap.h>
+#include <xen/vuart.h>
#include <public/bootfdt.h>
#include <public/io/xs_wire.h>
@@ -167,55 +168,6 @@ int __init make_intc_domU_node(struct kernel_info *kinfo)
}
}
-#ifdef CONFIG_HAS_VUART_PL011
-static int __init make_vpl011_uart_node(struct kernel_info *kinfo)
-{
- void *fdt = kinfo->fdt;
- int res;
- gic_interrupt_t intr;
- __be32 reg[GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS];
- __be32 *cells;
- struct domain *d = kinfo->d;
-
- res = domain_fdt_begin_node(fdt, "sbsa-uart", d->arch.vpl011.base_addr);
- if ( res )
- return res;
-
- res = fdt_property_string(fdt, "compatible", "arm,sbsa-uart");
- if ( res )
- return res;
-
- cells = ®[0];
- dt_child_set_range(&cells, GUEST_ROOT_ADDRESS_CELLS,
- GUEST_ROOT_SIZE_CELLS, d->arch.vpl011.base_addr,
- GUEST_PL011_SIZE);
-
- res = fdt_property(fdt, "reg", reg, sizeof(reg));
- if ( res )
- return res;
-
- set_interrupt(intr, d->arch.vpl011.virq, 0xf, DT_IRQ_TYPE_LEVEL_HIGH);
-
- res = fdt_property(fdt, "interrupts", intr, sizeof (intr));
- if ( res )
- return res;
-
- res = fdt_property_cell(fdt, "interrupt-parent",
- kinfo->phandle_intc);
- if ( res )
- return res;
-
- /* Use a default baud rate of 115200. */
- fdt_property_u32(fdt, "current-speed", 115200);
-
- res = fdt_end_node(fdt);
- if ( res )
- return res;
-
- return 0;
-}
-#endif
-
int __init make_arch_nodes(struct kernel_info *kinfo)
{
int ret;
@@ -227,7 +179,7 @@ int __init make_arch_nodes(struct kernel_info *kinfo)
if ( kinfo->arch.vpl011 )
{
#ifdef CONFIG_HAS_VUART_PL011
- ret = make_vpl011_uart_node(kinfo);
+ ret = vuart_add_fwnode(kinfo->d, kinfo);
#endif
if ( ret )
return -EINVAL;
diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 480fc664fc62..cafc532cf028 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -12,15 +12,20 @@
#include <xen/errno.h>
#include <xen/event.h>
+#include <xen/device_tree.h>
#include <xen/guest_access.h>
#include <xen/init.h>
#include <xen/lib.h>
+#include <xen/libfdt/libfdt.h>
#include <xen/mm.h>
#include <xen/sched.h>
#include <xen/console.h>
#include <xen/serial.h>
+#include <xen/vuart.h>
#include <public/domctl.h>
#include <public/io/console.h>
+#include <asm/domain_build.h>
+#include <asm/kernel.h>
#include <asm/pl011-uart.h>
#include <asm/vgic-emul.h>
#include <asm/vpl011.h>
@@ -784,6 +789,53 @@ void domain_vpl011_deinit(struct domain *d)
XFREE(vpl011->backend.xen);
}
+int __init vuart_add_fwnode(struct domain *d, void *node)
+{
+ struct kernel_info *kinfo = node;
+ void *fdt = kinfo->fdt;
+ int res;
+ gic_interrupt_t intr;
+ __be32 reg[GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS];
+ __be32 *cells;
+
+ res = domain_fdt_begin_node(fdt, "sbsa-uart", d->arch.vpl011.base_addr);
+ if ( res )
+ return res;
+
+ res = fdt_property_string(fdt, "compatible", "arm,sbsa-uart");
+ if ( res )
+ return res;
+
+ cells = ®[0];
+ dt_child_set_range(&cells, GUEST_ROOT_ADDRESS_CELLS,
+ GUEST_ROOT_SIZE_CELLS, d->arch.vpl011.base_addr,
+ GUEST_PL011_SIZE);
+
+ res = fdt_property(fdt, "reg", reg, sizeof(reg));
+ if ( res )
+ return res;
+
+ set_interrupt(intr, d->arch.vpl011.virq, 0xf, DT_IRQ_TYPE_LEVEL_HIGH);
+
+ res = fdt_property(fdt, "interrupts", intr, sizeof (intr));
+ if ( res )
+ return res;
+
+ res = fdt_property_cell(fdt, "interrupt-parent",
+ kinfo->phandle_intc);
+ if ( res )
+ return res;
+
+ /* Use a default baud rate of 115200. */
+ fdt_property_u32(fdt, "current-speed", 115200);
+
+ res = fdt_end_node(fdt);
+ if ( res )
+ return res;
+
+ return 0;
+}
+
/*
* Local variables:
* mode: C
diff --git a/xen/include/xen/vuart.h b/xen/include/xen/vuart.h
new file mode 100644
index 000000000000..bb883823ea31
--- /dev/null
+++ b/xen/include/xen/vuart.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef XEN_VUART_H
+#define XEN_VUART_H
+
+#ifdef CONFIG_HAS_VUART_PL011
+int __init vuart_add_fwnode(struct domain *d, void *node);
+#else
+static inline int __init vuart_add_fwnode(struct domain *d, void *node)
+{
+ return 0;
+}
+#endif
+
+#endif /* XEN_VUART_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.34.1
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH v1 02/16] arm/vpl011: move DT node parsing to PL011 emulator code
2025-06-24 3:55 ` [PATCH v1 02/16] arm/vpl011: move DT node parsing to PL011 emulator code dmkhn
@ 2025-06-24 7:49 ` Orzel, Michal
2025-06-24 21:56 ` dmkhn
0 siblings, 1 reply; 59+ messages in thread
From: Orzel, Michal @ 2025-06-24 7:49 UTC (permalink / raw)
To: dmkhn, xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
On 24/06/2025 05:55, dmkhn@proton.me wrote:
> From: Denis Mukhin <dmukhin@ford.com>
>
> Move vpl011 DT node parsing from common Arm code to PL011 emulator code.
It's not parsing, it's DT node generation.
We usually keep all the DT node generation functions in one place. I'm not sure
if we want to move them to respective drivers (i.e. vpl011 to vpl011.c, gicv3 to
gicv3.c, etc.). Not sure what other maintainers think.
>
> While doing it pick the generic name vuart_add_fwnode() for DT parser function
What 'fw' stands for? Firmware? This function creates DT node for domU, so it
should better be sth like vuart_add_dt_node().
~Michal
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v1 02/16] arm/vpl011: move DT node parsing to PL011 emulator code
2025-06-24 7:49 ` Orzel, Michal
@ 2025-06-24 21:56 ` dmkhn
2025-06-25 6:57 ` Orzel, Michal
0 siblings, 1 reply; 59+ messages in thread
From: dmkhn @ 2025-06-24 21:56 UTC (permalink / raw)
To: Orzel, Michal
Cc: xen-devel, andrew.cooper3, anthony.perard, jbeulich, julien,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
On Tue, Jun 24, 2025 at 09:49:39AM +0200, Orzel, Michal wrote:
>
>
> On 24/06/2025 05:55, dmkhn@proton.me wrote:
> > From: Denis Mukhin <dmukhin@ford.com>
> >
> > Move vpl011 DT node parsing from common Arm code to PL011 emulator code.
> It's not parsing, it's DT node generation.
Oh, that's right, overlooked.
Thanks, will update.
>
> We usually keep all the DT node generation functions in one place. I'm not sure
> if we want to move them to respective drivers (i.e. vpl011 to vpl011.c, gicv3 to
> gicv3.c, etc.). Not sure what other maintainers think.
>
> >
> > While doing it pick the generic name vuart_add_fwnode() for DT parser function
> What 'fw' stands for? Firmware? This function creates DT node for domU, so it
> should better be sth like vuart_add_dt_node().
'fw' stands for 'firmware'.
It should be some generic name because the function will be used on x86 to
generate to generate the guest ACPI tables.
>
> ~Michal
>
>
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v1 02/16] arm/vpl011: move DT node parsing to PL011 emulator code
2025-06-24 21:56 ` dmkhn
@ 2025-06-25 6:57 ` Orzel, Michal
2025-07-31 21:02 ` dmkhn
0 siblings, 1 reply; 59+ messages in thread
From: Orzel, Michal @ 2025-06-25 6:57 UTC (permalink / raw)
To: dmkhn
Cc: xen-devel, andrew.cooper3, anthony.perard, jbeulich, julien,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
On 24/06/2025 23:56, dmkhn@proton.me wrote:
> On Tue, Jun 24, 2025 at 09:49:39AM +0200, Orzel, Michal wrote:
>>
>>
>> On 24/06/2025 05:55, dmkhn@proton.me wrote:
>>> From: Denis Mukhin <dmukhin@ford.com>
>>>
>>> Move vpl011 DT node parsing from common Arm code to PL011 emulator code.
>> It's not parsing, it's DT node generation.
>
> Oh, that's right, overlooked.
> Thanks, will update.
>
>>
>> We usually keep all the DT node generation functions in one place. I'm not sure
>> if we want to move them to respective drivers (i.e. vpl011 to vpl011.c, gicv3 to
>> gicv3.c, etc.). Not sure what other maintainers think.
>>
>>>
>>> While doing it pick the generic name vuart_add_fwnode() for DT parser function
>> What 'fw' stands for? Firmware? This function creates DT node for domU, so it
>> should better be sth like vuart_add_dt_node().
>
> 'fw' stands for 'firmware'.
>
> It should be some generic name because the function will be used on x86 to
> generate to generate the guest ACPI tables.
I see but maybe vuart_add_node() would be a better choice here.
~Michal
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v1 02/16] arm/vpl011: move DT node parsing to PL011 emulator code
2025-06-25 6:57 ` Orzel, Michal
@ 2025-07-31 21:02 ` dmkhn
0 siblings, 0 replies; 59+ messages in thread
From: dmkhn @ 2025-07-31 21:02 UTC (permalink / raw)
To: Orzel, Michal
Cc: xen-devel, andrew.cooper3, anthony.perard, jbeulich, julien,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
On Wed, Jun 25, 2025 at 08:57:27AM +0200, Orzel, Michal wrote:
>
>
> On 24/06/2025 23:56, dmkhn@proton.me wrote:
> > On Tue, Jun 24, 2025 at 09:49:39AM +0200, Orzel, Michal wrote:
> >>
> >>
> >> On 24/06/2025 05:55, dmkhn@proton.me wrote:
> >>> From: Denis Mukhin <dmukhin@ford.com>
> >>>
> >>> Move vpl011 DT node parsing from common Arm code to PL011 emulator code.
> >> It's not parsing, it's DT node generation.
> >
> > Oh, that's right, overlooked.
> > Thanks, will update.
> >
> >>
> >> We usually keep all the DT node generation functions in one place. I'm not sure
> >> if we want to move them to respective drivers (i.e. vpl011 to vpl011.c, gicv3 to
> >> gicv3.c, etc.). Not sure what other maintainers think.
> >>
> >>>
> >>> While doing it pick the generic name vuart_add_fwnode() for DT parser function
> >> What 'fw' stands for? Firmware? This function creates DT node for domU, so it
> >> should better be sth like vuart_add_dt_node().
> >
> > 'fw' stands for 'firmware'.
> >
> > It should be some generic name because the function will be used on x86 to
> > generate to generate the guest ACPI tables.
> I see but maybe vuart_add_node() would be a better choice here.
Ack.
>
> ~Michal
>
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v1 03/16] arm/vpl011: use vuart_ prefix in vpl011 public calls
2025-06-24 3:54 [PATCH v1 00/16] xen: framework for UART emulators dmkhn
2025-06-24 3:55 ` [PATCH v1 01/16] arm/vpl011: rename virtual PL011 Kconfig option dmkhn
2025-06-24 3:55 ` [PATCH v1 02/16] arm/vpl011: move DT node parsing to PL011 emulator code dmkhn
@ 2025-06-24 3:55 ` dmkhn
2025-06-24 10:11 ` Orzel, Michal
2025-06-24 3:55 ` [PATCH v1 04/16] arm/vpl011: use raw spin_lock_{irqrestore,irqsave} dmkhn
` (12 subsequent siblings)
15 siblings, 1 reply; 59+ messages in thread
From: dmkhn @ 2025-06-24 3:55 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
From: Denis Mukhin <dmukhin@ford.com>
Use generic names prefixed with 'vuart_' in public PL011 emulator data
structures and functions.
No functional change intended.
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
xen/arch/arm/dom0less-build.c | 4 ++--
xen/arch/arm/domain.c | 3 ++-
xen/arch/arm/domctl.c | 14 +++++++------
xen/arch/arm/include/asm/vpl011.h | 20 ------------------
xen/arch/arm/vpl011.c | 24 +++++++++++-----------
xen/drivers/char/console.c | 6 ++----
xen/include/xen/vuart.h | 34 ++++++++++++++++++++++++++++++-
7 files changed, 59 insertions(+), 46 deletions(-)
diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index 7c1b59750fb5..11b8498d3b22 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -216,7 +216,7 @@ int __init init_vuart(struct domain *d, struct kernel_info *kinfo,
*/
if ( kinfo->arch.vpl011 )
{
- rc = domain_vpl011_init(d, NULL);
+ rc = vuart_init(d, NULL);
if ( rc < 0 )
return rc;
}
@@ -247,7 +247,7 @@ void __init arch_create_domUs(struct dt_device_node *node,
* d->arch.vpl011.irq. So the logic to find the vIRQ has to
* be hardcoded.
* The logic here shall be consistent with the one in
- * domain_vpl011_init().
+ * vuart_init().
*/
if ( flags & CDF_directmap )
{
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index be58a23dd725..68297e619bad 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -11,6 +11,7 @@
#include <xen/sched.h>
#include <xen/softirq.h>
#include <xen/wait.h>
+#include <xen/vuart.h>
#include <asm/arm64/sve.h>
#include <asm/cpuerrata.h>
@@ -1072,7 +1073,7 @@ int domain_relinquish_resources(struct domain *d)
* Release the resources allocated for vpl011 which were
* allocated via a DOMCTL call XEN_DOMCTL_vuart_op.
*/
- domain_vpl011_deinit(d);
+ vuart_exit(d);
#ifdef CONFIG_IOREQ_SERVER
ioreq_server_destroy_all(d);
diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
index ad914c915f81..dde25ceff6d0 100644
--- a/xen/arch/arm/domctl.c
+++ b/xen/arch/arm/domctl.c
@@ -14,6 +14,7 @@
#include <xen/mm.h>
#include <xen/sched.h>
#include <xen/types.h>
+#include <xen/vuart.h>
#include <xsm/xsm.h>
#include <public/domctl.h>
@@ -30,10 +31,11 @@ static int handle_vuart_init(struct domain *d,
struct xen_domctl_vuart_op *vuart_op)
{
int rc;
- struct vpl011_init_info info;
-
- info.console_domid = vuart_op->console_domid;
- info.gfn = _gfn(vuart_op->gfn);
+ struct vuart_params params = {
+ .console_domid = vuart_op->console_domid,
+ .gfn = _gfn(vuart_op->gfn),
+ .evtchn = 0,
+ };
if ( d->creation_finished )
return -EPERM;
@@ -41,10 +43,10 @@ static int handle_vuart_init(struct domain *d,
if ( vuart_op->type != XEN_DOMCTL_VUART_TYPE_VPL011 )
return -EOPNOTSUPP;
- rc = domain_vpl011_init(d, &info);
+ rc = vuart_init(d, ¶ms);
if ( !rc )
- vuart_op->evtchn = info.evtchn;
+ vuart_op->evtchn = params.evtchn;
return rc;
}
diff --git a/xen/arch/arm/include/asm/vpl011.h b/xen/arch/arm/include/asm/vpl011.h
index be64883b8628..5c308cc8c148 100644
--- a/xen/arch/arm/include/asm/vpl011.h
+++ b/xen/arch/arm/include/asm/vpl011.h
@@ -59,26 +59,6 @@ struct vpl011 {
evtchn_port_t evtchn;
};
-struct vpl011_init_info {
- domid_t console_domid;
- gfn_t gfn;
- evtchn_port_t evtchn;
-};
-
-#ifdef CONFIG_HAS_VUART_PL011
-int domain_vpl011_init(struct domain *d,
- struct vpl011_init_info *info);
-void domain_vpl011_deinit(struct domain *d);
-int vpl011_rx_char_xen(struct domain *d, char c);
-#else
-static inline int domain_vpl011_init(struct domain *d,
- struct vpl011_init_info *info)
-{
- return -ENOSYS;
-}
-
-static inline void domain_vpl011_deinit(struct domain *d) { }
-#endif
#endif /* _VPL011_H_ */
/*
diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index cafc532cf028..2cf88a70ecdb 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -134,7 +134,7 @@ static void vpl011_write_data_xen(struct domain *d, uint8_t data)
/*
* vpl011_read_data_xen reads data when the backend is xen. Characters
- * are added to the vpl011 receive buffer by vpl011_rx_char_xen.
+ * are added to the vpl011 receive buffer by vuart_putchar.
*/
static uint8_t vpl011_read_data_xen(struct domain *d)
{
@@ -571,9 +571,9 @@ static void vpl011_data_avail(struct domain *d,
}
/*
- * vpl011_rx_char_xen adds a char to a domain's vpl011 receive buffer.
+ * vuart_putchar adds a char to a domain's vpl011 receive buffer.
*/
-int vpl011_rx_char_xen(struct domain *d, char c)
+int vuart_putchar(struct domain *d, char c)
{
unsigned long flags;
struct vpl011 *vpl011 = &d->arch.vpl011;
@@ -642,7 +642,7 @@ static void vpl011_notification(struct vcpu *v, unsigned int port)
VPL011_UNLOCK(d, flags);
}
-int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
+int vuart_init(struct domain *d, struct vuart_params *params)
{
int rc;
struct vpl011 *vpl011 = &d->arch.vpl011;
@@ -694,27 +694,27 @@ int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
}
/*
- * info is NULL when the backend is in Xen.
- * info is != NULL when the backend is in a domain.
+ * params is NULL when the backend is in Xen.
+ * params is != NULL when the backend is in a domain.
*/
- if ( info != NULL )
+ if ( params != NULL )
{
vpl011->backend_in_domain = true;
/* Map the guest PFN to Xen address space. */
rc = prepare_ring_for_helper(d,
- gfn_x(info->gfn),
+ gfn_x(params->gfn),
&vpl011->backend.dom.ring_page,
&vpl011->backend.dom.ring_buf);
if ( rc < 0 )
goto out;
- rc = alloc_unbound_xen_event_channel(d, 0, info->console_domid,
+ rc = alloc_unbound_xen_event_channel(d, 0, params->console_domid,
vpl011_notification);
if ( rc < 0 )
goto out1;
- vpl011->evtchn = info->evtchn = rc;
+ vpl011->evtchn = params->evtchn = rc;
}
else
{
@@ -746,13 +746,13 @@ int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
return 0;
out1:
- domain_vpl011_deinit(d);
+ vuart_exit(d);
out:
return rc;
}
-void domain_vpl011_deinit(struct domain *d)
+void vuart_exit(struct domain *d)
{
struct vpl011 *vpl011 = &d->arch.vpl011;
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 0f37badc471e..f322d59515ab 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -33,13 +33,11 @@
#include <asm/setup.h>
#include <xen/sections.h>
#include <xen/consoled.h>
+#include <xen/vuart.h>
#ifdef CONFIG_X86
#include <asm/guest.h>
#endif
-#ifdef CONFIG_HAS_VUART_PL011
-#include <asm/vpl011.h>
-#endif
/* Internal console flags. */
enum {
@@ -609,7 +607,7 @@ static void __serial_rx(char c)
#ifdef CONFIG_HAS_VUART_PL011
else
/* Deliver input to the emulated UART. */
- rc = vpl011_rx_char_xen(d, c);
+ rc = vuart_putchar(d, c);
#endif
if ( consoled_is_enabled() )
diff --git a/xen/include/xen/vuart.h b/xen/include/xen/vuart.h
index bb883823ea31..cae72ac9c6b9 100644
--- a/xen/include/xen/vuart.h
+++ b/xen/include/xen/vuart.h
@@ -2,14 +2,46 @@
#ifndef XEN_VUART_H
#define XEN_VUART_H
+#include <public/xen.h>
+#include <public/event_channel.h>
+#include <xen/types.h>
+
+struct vuart_params {
+ domid_t console_domid;
+ gfn_t gfn;
+ evtchn_port_t evtchn;
+};
+
#ifdef CONFIG_HAS_VUART_PL011
+
int __init vuart_add_fwnode(struct domain *d, void *node);
+int vuart_init(struct domain *d, struct vuart_params *params);
+void vuart_exit(struct domain *d);
+int vuart_putchar(struct domain *d, char c);
+
#else
+
static inline int __init vuart_add_fwnode(struct domain *d, void *node)
{
return 0;
}
-#endif
+
+static inline int vuart_init(struct domain *d, struct vuart_params *params)
+{
+ return 0;
+}
+
+static inline void vuart_exit(struct domain *d)
+{
+}
+
+static inline int vuart_putchar(struct domain *d, char c)
+{
+ ASSERT_UNREACHABLE();
+ return -ENODEV;
+}
+
+#endif /* CONFIG_HAS_VUART_PL011 */
#endif /* XEN_VUART_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH v1 03/16] arm/vpl011: use vuart_ prefix in vpl011 public calls
2025-06-24 3:55 ` [PATCH v1 03/16] arm/vpl011: use vuart_ prefix in vpl011 public calls dmkhn
@ 2025-06-24 10:11 ` Orzel, Michal
2025-06-24 22:17 ` dmkhn
2025-07-31 21:11 ` dmkhn
0 siblings, 2 replies; 59+ messages in thread
From: Orzel, Michal @ 2025-06-24 10:11 UTC (permalink / raw)
To: dmkhn, xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
On 24/06/2025 05:55, dmkhn@proton.me wrote:
> From: Denis Mukhin <dmukhin@ford.com>
>
> Use generic names prefixed with 'vuart_' in public PL011 emulator data
> structures and functions.
>
> No functional change intended.
>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
> xen/arch/arm/dom0less-build.c | 4 ++--
> xen/arch/arm/domain.c | 3 ++-
> xen/arch/arm/domctl.c | 14 +++++++------
> xen/arch/arm/include/asm/vpl011.h | 20 ------------------
> xen/arch/arm/vpl011.c | 24 +++++++++++-----------
> xen/drivers/char/console.c | 6 ++----
> xen/include/xen/vuart.h | 34 ++++++++++++++++++++++++++++++-
> 7 files changed, 59 insertions(+), 46 deletions(-)
>
> diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> index 7c1b59750fb5..11b8498d3b22 100644
> --- a/xen/arch/arm/dom0less-build.c
> +++ b/xen/arch/arm/dom0less-build.c
> @@ -216,7 +216,7 @@ int __init init_vuart(struct domain *d, struct kernel_info *kinfo,
As can be seen here ...
> */
> if ( kinfo->arch.vpl011 )
> {
> - rc = domain_vpl011_init(d, NULL);
> + rc = vuart_init(d, NULL);
we end up with init_vuart() and vuart_init(). That's quite confusing. Maybe
domain_vuart_init() or alike?
> if ( rc < 0 )
> return rc;
> }
> @@ -247,7 +247,7 @@ void __init arch_create_domUs(struct dt_device_node *node,
> * d->arch.vpl011.irq. So the logic to find the vIRQ has to
> * be hardcoded.
> * The logic here shall be consistent with the one in
> - * domain_vpl011_init().
> + * vuart_init().
> */
> if ( flags & CDF_directmap )
> {
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index be58a23dd725..68297e619bad 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -11,6 +11,7 @@
> #include <xen/sched.h>
> #include <xen/softirq.h>
> #include <xen/wait.h>
> +#include <xen/vuart.h>
>
> #include <asm/arm64/sve.h>
> #include <asm/cpuerrata.h>
> @@ -1072,7 +1073,7 @@ int domain_relinquish_resources(struct domain *d)
> * Release the resources allocated for vpl011 which were
> * allocated via a DOMCTL call XEN_DOMCTL_vuart_op.
> */
> - domain_vpl011_deinit(d);
> + vuart_exit(d);
IMO, deinit is more meaningful here.
>
> #ifdef CONFIG_IOREQ_SERVER
> ioreq_server_destroy_all(d);
> diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
> index ad914c915f81..dde25ceff6d0 100644
> --- a/xen/arch/arm/domctl.c
> +++ b/xen/arch/arm/domctl.c
> @@ -14,6 +14,7 @@
> #include <xen/mm.h>
> #include <xen/sched.h>
> #include <xen/types.h>
> +#include <xen/vuart.h>
> #include <xsm/xsm.h>
> #include <public/domctl.h>
>
> @@ -30,10 +31,11 @@ static int handle_vuart_init(struct domain *d,
> struct xen_domctl_vuart_op *vuart_op)
> {
> int rc;
> - struct vpl011_init_info info;
> -
> - info.console_domid = vuart_op->console_domid;
> - info.gfn = _gfn(vuart_op->gfn);
> + struct vuart_params params = {
> + .console_domid = vuart_op->console_domid,
> + .gfn = _gfn(vuart_op->gfn),
> + .evtchn = 0,
> + };
>
> if ( d->creation_finished )
> return -EPERM;
> @@ -41,10 +43,10 @@ static int handle_vuart_init(struct domain *d,
> if ( vuart_op->type != XEN_DOMCTL_VUART_TYPE_VPL011 )
> return -EOPNOTSUPP;
>
> - rc = domain_vpl011_init(d, &info);
> + rc = vuart_init(d, ¶ms);
>
> if ( !rc )
> - vuart_op->evtchn = info.evtchn;
> + vuart_op->evtchn = params.evtchn;
>
> return rc;
> }
> diff --git a/xen/arch/arm/include/asm/vpl011.h b/xen/arch/arm/include/asm/vpl011.h
> index be64883b8628..5c308cc8c148 100644
> --- a/xen/arch/arm/include/asm/vpl011.h
> +++ b/xen/arch/arm/include/asm/vpl011.h
> @@ -59,26 +59,6 @@ struct vpl011 {
> evtchn_port_t evtchn;
> };
>
> -struct vpl011_init_info {
> - domid_t console_domid;
> - gfn_t gfn;
> - evtchn_port_t evtchn;
> -};
> -
> -#ifdef CONFIG_HAS_VUART_PL011
> -int domain_vpl011_init(struct domain *d,
> - struct vpl011_init_info *info);
> -void domain_vpl011_deinit(struct domain *d);
> -int vpl011_rx_char_xen(struct domain *d, char c);
> -#else
> -static inline int domain_vpl011_init(struct domain *d,
> - struct vpl011_init_info *info)
> -{
> - return -ENOSYS;
> -}
> -
> -static inline void domain_vpl011_deinit(struct domain *d) { }
> -#endif
> #endif /* _VPL011_H_ */
>
> /*
> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> index cafc532cf028..2cf88a70ecdb 100644
> --- a/xen/arch/arm/vpl011.c
> +++ b/xen/arch/arm/vpl011.c
> @@ -134,7 +134,7 @@ static void vpl011_write_data_xen(struct domain *d, uint8_t data)
>
> /*
> * vpl011_read_data_xen reads data when the backend is xen. Characters
> - * are added to the vpl011 receive buffer by vpl011_rx_char_xen.
> + * are added to the vpl011 receive buffer by vuart_putchar.
> */
> static uint8_t vpl011_read_data_xen(struct domain *d)
> {
> @@ -571,9 +571,9 @@ static void vpl011_data_avail(struct domain *d,
> }
>
> /*
> - * vpl011_rx_char_xen adds a char to a domain's vpl011 receive buffer.
> + * vuart_putchar adds a char to a domain's vpl011 receive buffer.
> */
> -int vpl011_rx_char_xen(struct domain *d, char c)
> +int vuart_putchar(struct domain *d, char c)
How can putchar refer to RX? By definition putchar() is used to print data to
STDOUT. Here we receive a character and put it in the RX FIFO.
~Michal
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [PATCH v1 03/16] arm/vpl011: use vuart_ prefix in vpl011 public calls
2025-06-24 10:11 ` Orzel, Michal
@ 2025-06-24 22:17 ` dmkhn
2025-07-31 21:11 ` dmkhn
1 sibling, 0 replies; 59+ messages in thread
From: dmkhn @ 2025-06-24 22:17 UTC (permalink / raw)
To: Orzel, Michal
Cc: xen-devel, andrew.cooper3, anthony.perard, jbeulich, julien,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
On Tue, Jun 24, 2025 at 12:11:10PM +0200, Orzel, Michal wrote:
>
>
> On 24/06/2025 05:55, dmkhn@proton.me wrote:
> > From: Denis Mukhin <dmukhin@ford.com>
> >
> > Use generic names prefixed with 'vuart_' in public PL011 emulator data
> > structures and functions.
> >
> > No functional change intended.
> >
> > Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> > ---
> > xen/arch/arm/dom0less-build.c | 4 ++--
> > xen/arch/arm/domain.c | 3 ++-
> > xen/arch/arm/domctl.c | 14 +++++++------
> > xen/arch/arm/include/asm/vpl011.h | 20 ------------------
> > xen/arch/arm/vpl011.c | 24 +++++++++++-----------
> > xen/drivers/char/console.c | 6 ++----
> > xen/include/xen/vuart.h | 34 ++++++++++++++++++++++++++++++-
> > 7 files changed, 59 insertions(+), 46 deletions(-)
> >
> > diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> > index 7c1b59750fb5..11b8498d3b22 100644
> > --- a/xen/arch/arm/dom0less-build.c
> > +++ b/xen/arch/arm/dom0less-build.c
> > @@ -216,7 +216,7 @@ int __init init_vuart(struct domain *d, struct kernel_info *kinfo,
> As can be seen here ...
>
> > */
> > if ( kinfo->arch.vpl011 )
> > {
> > - rc = domain_vpl011_init(d, NULL);
> > + rc = vuart_init(d, NULL);
> we end up with init_vuart() and vuart_init(). That's quite confusing. Maybe
> domain_vuart_init() or alike?
That's right!
But domain_vuart_init() is used by MMIO-based variant :)
I will write an extra patch and put it to the end of the series to update the
name here.
>
> > if ( rc < 0 )
> > return rc;
> > }
> > @@ -247,7 +247,7 @@ void __init arch_create_domUs(struct dt_device_node *node,
> > * d->arch.vpl011.irq. So the logic to find the vIRQ has to
> > * be hardcoded.
> > * The logic here shall be consistent with the one in
> > - * domain_vpl011_init().
> > + * vuart_init().
> > */
> > if ( flags & CDF_directmap )
> > {
> > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> > index be58a23dd725..68297e619bad 100644
> > --- a/xen/arch/arm/domain.c
> > +++ b/xen/arch/arm/domain.c
> > @@ -11,6 +11,7 @@
> > #include <xen/sched.h>
> > #include <xen/softirq.h>
> > #include <xen/wait.h>
> > +#include <xen/vuart.h>
> >
> > #include <asm/arm64/sve.h>
> > #include <asm/cpuerrata.h>
> > @@ -1072,7 +1073,7 @@ int domain_relinquish_resources(struct domain *d)
> > * Release the resources allocated for vpl011 which were
> > * allocated via a DOMCTL call XEN_DOMCTL_vuart_op.
> > */
> > - domain_vpl011_deinit(d);
> > + vuart_exit(d);
> IMO, deinit is more meaningful here.
Yeah, it's just MMIO UART uses init/free, here it is init/deinit.
So I just picked init/exit similar to module_{init,exit} in Linux driver model
and applied to all existing vUARTs.
Can update all vUARTs to deinit()
>
> >
> > #ifdef CONFIG_IOREQ_SERVER
> > ioreq_server_destroy_all(d);
> > diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
> > index ad914c915f81..dde25ceff6d0 100644
> > --- a/xen/arch/arm/domctl.c
> > +++ b/xen/arch/arm/domctl.c
> > @@ -14,6 +14,7 @@
> > #include <xen/mm.h>
> > #include <xen/sched.h>
> > #include <xen/types.h>
> > +#include <xen/vuart.h>
> > #include <xsm/xsm.h>
> > #include <public/domctl.h>
> >
> > @@ -30,10 +31,11 @@ static int handle_vuart_init(struct domain *d,
> > struct xen_domctl_vuart_op *vuart_op)
> > {
> > int rc;
> > - struct vpl011_init_info info;
> > -
> > - info.console_domid = vuart_op->console_domid;
> > - info.gfn = _gfn(vuart_op->gfn);
> > + struct vuart_params params = {
> > + .console_domid = vuart_op->console_domid,
> > + .gfn = _gfn(vuart_op->gfn),
> > + .evtchn = 0,
> > + };
> >
> > if ( d->creation_finished )
> > return -EPERM;
> > @@ -41,10 +43,10 @@ static int handle_vuart_init(struct domain *d,
> > if ( vuart_op->type != XEN_DOMCTL_VUART_TYPE_VPL011 )
> > return -EOPNOTSUPP;
> >
> > - rc = domain_vpl011_init(d, &info);
> > + rc = vuart_init(d, ¶ms);
> >
> > if ( !rc )
> > - vuart_op->evtchn = info.evtchn;
> > + vuart_op->evtchn = params.evtchn;
> >
> > return rc;
> > }
> > diff --git a/xen/arch/arm/include/asm/vpl011.h b/xen/arch/arm/include/asm/vpl011.h
> > index be64883b8628..5c308cc8c148 100644
> > --- a/xen/arch/arm/include/asm/vpl011.h
> > +++ b/xen/arch/arm/include/asm/vpl011.h
> > @@ -59,26 +59,6 @@ struct vpl011 {
> > evtchn_port_t evtchn;
> > };
> >
> > -struct vpl011_init_info {
> > - domid_t console_domid;
> > - gfn_t gfn;
> > - evtchn_port_t evtchn;
> > -};
> > -
> > -#ifdef CONFIG_HAS_VUART_PL011
> > -int domain_vpl011_init(struct domain *d,
> > - struct vpl011_init_info *info);
> > -void domain_vpl011_deinit(struct domain *d);
> > -int vpl011_rx_char_xen(struct domain *d, char c);
> > -#else
> > -static inline int domain_vpl011_init(struct domain *d,
> > - struct vpl011_init_info *info)
> > -{
> > - return -ENOSYS;
> > -}
> > -
> > -static inline void domain_vpl011_deinit(struct domain *d) { }
> > -#endif
> > #endif /* _VPL011_H_ */
> >
> > /*
> > diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> > index cafc532cf028..2cf88a70ecdb 100644
> > --- a/xen/arch/arm/vpl011.c
> > +++ b/xen/arch/arm/vpl011.c
> > @@ -134,7 +134,7 @@ static void vpl011_write_data_xen(struct domain *d, uint8_t data)
> >
> > /*
> > * vpl011_read_data_xen reads data when the backend is xen. Characters
> > - * are added to the vpl011 receive buffer by vpl011_rx_char_xen.
> > + * are added to the vpl011 receive buffer by vuart_putchar.
> > */
> > static uint8_t vpl011_read_data_xen(struct domain *d)
> > {
> > @@ -571,9 +571,9 @@ static void vpl011_data_avail(struct domain *d,
> > }
> >
> > /*
> > - * vpl011_rx_char_xen adds a char to a domain's vpl011 receive buffer.
> > + * vuart_putchar adds a char to a domain's vpl011 receive buffer.
> > */
> > -int vpl011_rx_char_xen(struct domain *d, char c)
> > +int vuart_putchar(struct domain *d, char c)
> How can putchar refer to RX? By definition putchar() is used to print data to
> STDOUT. Here we receive a character and put it in the RX FIFO.
That's confusing, I agree; I think this is a leftover from the earlier vUART
series.
Will update, thanks!
>
> ~Michal
>
>
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [PATCH v1 03/16] arm/vpl011: use vuart_ prefix in vpl011 public calls
2025-06-24 10:11 ` Orzel, Michal
2025-06-24 22:17 ` dmkhn
@ 2025-07-31 21:11 ` dmkhn
1 sibling, 0 replies; 59+ messages in thread
From: dmkhn @ 2025-07-31 21:11 UTC (permalink / raw)
To: Orzel, Michal
Cc: xen-devel, andrew.cooper3, anthony.perard, jbeulich, julien,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
On Tue, Jun 24, 2025 at 12:11:10PM +0200, Orzel, Michal wrote:
>
>
> On 24/06/2025 05:55, dmkhn@proton.me wrote:
> > From: Denis Mukhin <dmukhin@ford.com>
> >
> > Use generic names prefixed with 'vuart_' in public PL011 emulator data
> > structures and functions.
> >
> > No functional change intended.
> >
> > Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> > ---
> > xen/arch/arm/dom0less-build.c | 4 ++--
> > xen/arch/arm/domain.c | 3 ++-
> > xen/arch/arm/domctl.c | 14 +++++++------
> > xen/arch/arm/include/asm/vpl011.h | 20 ------------------
> > xen/arch/arm/vpl011.c | 24 +++++++++++-----------
> > xen/drivers/char/console.c | 6 ++----
> > xen/include/xen/vuart.h | 34 ++++++++++++++++++++++++++++++-
> > 7 files changed, 59 insertions(+), 46 deletions(-)
> >
> > diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> > index 7c1b59750fb5..11b8498d3b22 100644
> > --- a/xen/arch/arm/dom0less-build.c
> > +++ b/xen/arch/arm/dom0less-build.c
> > @@ -216,7 +216,7 @@ int __init init_vuart(struct domain *d, struct kernel_info *kinfo,
> As can be seen here ...
>
> > */
> > if ( kinfo->arch.vpl011 )
> > {
> > - rc = domain_vpl011_init(d, NULL);
> > + rc = vuart_init(d, NULL);
> we end up with init_vuart() and vuart_init(). That's quite confusing. Maybe
> domain_vuart_init() or alike?
Agreed, will update.
>
> > if ( rc < 0 )
> > return rc;
> > }
> > @@ -247,7 +247,7 @@ void __init arch_create_domUs(struct dt_device_node *node,
> > * d->arch.vpl011.irq. So the logic to find the vIRQ has to
> > * be hardcoded.
> > * The logic here shall be consistent with the one in
> > - * domain_vpl011_init().
> > + * vuart_init().
> > */
> > if ( flags & CDF_directmap )
> > {
> > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> > index be58a23dd725..68297e619bad 100644
> > --- a/xen/arch/arm/domain.c
> > +++ b/xen/arch/arm/domain.c
> > @@ -11,6 +11,7 @@
> > #include <xen/sched.h>
> > #include <xen/softirq.h>
> > #include <xen/wait.h>
> > +#include <xen/vuart.h>
> >
> > #include <asm/arm64/sve.h>
> > #include <asm/cpuerrata.h>
> > @@ -1072,7 +1073,7 @@ int domain_relinquish_resources(struct domain *d)
> > * Release the resources allocated for vpl011 which were
> > * allocated via a DOMCTL call XEN_DOMCTL_vuart_op.
> > */
> > - domain_vpl011_deinit(d);
> > + vuart_exit(d);
> IMO, deinit is more meaningful here.
Ack
>
> >
> > #ifdef CONFIG_IOREQ_SERVER
> > ioreq_server_destroy_all(d);
> > diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
> > index ad914c915f81..dde25ceff6d0 100644
> > --- a/xen/arch/arm/domctl.c
> > +++ b/xen/arch/arm/domctl.c
> > @@ -14,6 +14,7 @@
> > #include <xen/mm.h>
> > #include <xen/sched.h>
> > #include <xen/types.h>
> > +#include <xen/vuart.h>
> > #include <xsm/xsm.h>
> > #include <public/domctl.h>
> >
> > @@ -30,10 +31,11 @@ static int handle_vuart_init(struct domain *d,
> > struct xen_domctl_vuart_op *vuart_op)
> > {
> > int rc;
> > - struct vpl011_init_info info;
> > -
> > - info.console_domid = vuart_op->console_domid;
> > - info.gfn = _gfn(vuart_op->gfn);
> > + struct vuart_params params = {
> > + .console_domid = vuart_op->console_domid,
> > + .gfn = _gfn(vuart_op->gfn),
> > + .evtchn = 0,
> > + };
> >
> > if ( d->creation_finished )
> > return -EPERM;
> > @@ -41,10 +43,10 @@ static int handle_vuart_init(struct domain *d,
> > if ( vuart_op->type != XEN_DOMCTL_VUART_TYPE_VPL011 )
> > return -EOPNOTSUPP;
> >
> > - rc = domain_vpl011_init(d, &info);
> > + rc = vuart_init(d, ¶ms);
> >
> > if ( !rc )
> > - vuart_op->evtchn = info.evtchn;
> > + vuart_op->evtchn = params.evtchn;
> >
> > return rc;
> > }
> > diff --git a/xen/arch/arm/include/asm/vpl011.h b/xen/arch/arm/include/asm/vpl011.h
> > index be64883b8628..5c308cc8c148 100644
> > --- a/xen/arch/arm/include/asm/vpl011.h
> > +++ b/xen/arch/arm/include/asm/vpl011.h
> > @@ -59,26 +59,6 @@ struct vpl011 {
> > evtchn_port_t evtchn;
> > };
> >
> > -struct vpl011_init_info {
> > - domid_t console_domid;
> > - gfn_t gfn;
> > - evtchn_port_t evtchn;
> > -};
> > -
> > -#ifdef CONFIG_HAS_VUART_PL011
> > -int domain_vpl011_init(struct domain *d,
> > - struct vpl011_init_info *info);
> > -void domain_vpl011_deinit(struct domain *d);
> > -int vpl011_rx_char_xen(struct domain *d, char c);
> > -#else
> > -static inline int domain_vpl011_init(struct domain *d,
> > - struct vpl011_init_info *info)
> > -{
> > - return -ENOSYS;
> > -}
> > -
> > -static inline void domain_vpl011_deinit(struct domain *d) { }
> > -#endif
> > #endif /* _VPL011_H_ */
> >
> > /*
> > diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> > index cafc532cf028..2cf88a70ecdb 100644
> > --- a/xen/arch/arm/vpl011.c
> > +++ b/xen/arch/arm/vpl011.c
> > @@ -134,7 +134,7 @@ static void vpl011_write_data_xen(struct domain *d, uint8_t data)
> >
> > /*
> > * vpl011_read_data_xen reads data when the backend is xen. Characters
> > - * are added to the vpl011 receive buffer by vpl011_rx_char_xen.
> > + * are added to the vpl011 receive buffer by vuart_putchar.
> > */
> > static uint8_t vpl011_read_data_xen(struct domain *d)
> > {
> > @@ -571,9 +571,9 @@ static void vpl011_data_avail(struct domain *d,
> > }
> >
> > /*
> > - * vpl011_rx_char_xen adds a char to a domain's vpl011 receive buffer.
> > + * vuart_putchar adds a char to a domain's vpl011 receive buffer.
> > */
> > -int vpl011_rx_char_xen(struct domain *d, char c)
> > +int vuart_putchar(struct domain *d, char c)
> How can putchar refer to RX? By definition putchar() is used to print data to
> STDOUT. Here we receive a character and put it in the RX FIFO.
Yeah, that may be confusing.
I will use vuart_put_rx().
>
> ~Michal
>
>
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v1 04/16] arm/vpl011: use raw spin_lock_{irqrestore,irqsave}
2025-06-24 3:54 [PATCH v1 00/16] xen: framework for UART emulators dmkhn
` (2 preceding siblings ...)
2025-06-24 3:55 ` [PATCH v1 03/16] arm/vpl011: use vuart_ prefix in vpl011 public calls dmkhn
@ 2025-06-24 3:55 ` dmkhn
2025-06-24 5:46 ` Jan Beulich
2025-06-24 3:55 ` [PATCH v1 05/16] arm/vpl011: use void pointer in domain struct dmkhn
` (11 subsequent siblings)
15 siblings, 1 reply; 59+ messages in thread
From: dmkhn @ 2025-06-24 3:55 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
From: Denis Mukhin <dmukhin@ford.com>
Replace VPL011_{LOCK,UNLOCK} macros with raw spinlock calls to improve
readability.
No functional change.
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
xen/arch/arm/include/asm/vpl011.h | 4 ---
xen/arch/arm/vpl011.c | 50 +++++++++++++++----------------
2 files changed, 25 insertions(+), 29 deletions(-)
diff --git a/xen/arch/arm/include/asm/vpl011.h b/xen/arch/arm/include/asm/vpl011.h
index 5c308cc8c148..8f6ea0005e72 100644
--- a/xen/arch/arm/include/asm/vpl011.h
+++ b/xen/arch/arm/include/asm/vpl011.h
@@ -24,10 +24,6 @@
#include <public/io/console.h>
#include <xen/mm.h>
-/* helper macros */
-#define VPL011_LOCK(d,flags) spin_lock_irqsave(&(d)->arch.vpl011.lock, flags)
-#define VPL011_UNLOCK(d,flags) spin_unlock_irqrestore(&(d)->arch.vpl011.lock, flags)
-
#define SBSA_UART_FIFO_SIZE 32
/* Same size as VUART_BUF_SIZE, used in vuart.c */
#define SBSA_UART_OUT_BUF_SIZE 128
diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 2cf88a70ecdb..a97c3b74208c 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -85,7 +85,7 @@ static void vpl011_write_data_xen(struct domain *d, uint8_t data)
struct vpl011_xen_backend *intf = vpl011->backend.xen;
struct domain *input = console_get_domain();
- VPL011_LOCK(d, flags);
+ spin_lock_irqsave(&vpl011->lock, flags);
intf->out[intf->out_prod++] = data;
if ( d == input )
@@ -127,7 +127,7 @@ static void vpl011_write_data_xen(struct domain *d, uint8_t data)
vpl011->uartfr |= TXFE;
vpl011_update_interrupt_status(d);
- VPL011_UNLOCK(d, flags);
+ spin_unlock_irqrestore(&vpl011->lock, flags);
console_put_domain(input);
}
@@ -144,7 +144,7 @@ static uint8_t vpl011_read_data_xen(struct domain *d)
struct vpl011_xen_backend *intf = vpl011->backend.xen;
XENCONS_RING_IDX in_cons, in_prod;
- VPL011_LOCK(d, flags);
+ spin_lock_irqsave(&vpl011->lock, flags);
in_cons = intf->in_cons;
in_prod = intf->in_prod;
@@ -190,7 +190,7 @@ static uint8_t vpl011_read_data_xen(struct domain *d)
*/
vpl011->uartfr &= ~RXFF;
- VPL011_UNLOCK(d, flags);
+ spin_unlock_irqrestore(&vpl011->lock, flags);
return data;
}
@@ -203,7 +203,7 @@ static uint8_t vpl011_read_data(struct domain *d)
struct xencons_interface *intf = vpl011->backend.dom.ring_buf;
XENCONS_RING_IDX in_cons, in_prod;
- VPL011_LOCK(d, flags);
+ spin_lock_irqsave(&vpl011->lock, flags);
in_cons = intf->in_cons;
in_prod = intf->in_prod;
@@ -249,7 +249,7 @@ static uint8_t vpl011_read_data(struct domain *d)
*/
vpl011->uartfr &= ~RXFF;
- VPL011_UNLOCK(d, flags);
+ spin_unlock_irqrestore(&vpl011->lock, flags);
/*
* Send an event to console backend to indicate that data has been
@@ -288,7 +288,7 @@ static void vpl011_write_data(struct domain *d, uint8_t data)
struct xencons_interface *intf = vpl011->backend.dom.ring_buf;
XENCONS_RING_IDX out_cons, out_prod;
- VPL011_LOCK(d, flags);
+ spin_lock_irqsave(&vpl011->lock, flags);
out_cons = intf->out_cons;
out_prod = intf->out_prod;
@@ -336,7 +336,7 @@ static void vpl011_write_data(struct domain *d, uint8_t data)
vpl011->uartfr &= ~TXFE;
- VPL011_UNLOCK(d, flags);
+ spin_unlock_irqrestore(&vpl011->lock, flags);
/*
* Send an event to console backend to indicate that there is
@@ -378,34 +378,34 @@ static int vpl011_mmio_read(struct vcpu *v,
case FR:
if ( !vpl011_reg32_check_access(dabt) ) goto bad_width;
- VPL011_LOCK(d, flags);
+ spin_lock_irqsave(&vpl011->lock, flags);
*r = vreg_reg32_extract(vpl011->uartfr, info);
- VPL011_UNLOCK(d, flags);
+ spin_unlock_irqrestore(&vpl011->lock, flags);
return 1;
case RIS:
if ( !vpl011_reg32_check_access(dabt) ) goto bad_width;
- VPL011_LOCK(d, flags);
+ spin_lock_irqsave(&vpl011->lock, flags);
*r = vreg_reg32_extract(vpl011->uartris, info);
- VPL011_UNLOCK(d, flags);
+ spin_unlock_irqrestore(&vpl011->lock, flags);
return 1;
case MIS:
if ( !vpl011_reg32_check_access(dabt) ) goto bad_width;
- VPL011_LOCK(d, flags);
+ spin_lock_irqsave(&vpl011->lock, flags);
*r = vreg_reg32_extract(vpl011->uartris & vpl011->uartimsc,
info);
- VPL011_UNLOCK(d, flags);
+ spin_unlock_irqrestore(&vpl011->lock, flags);
return 1;
case IMSC:
if ( !vpl011_reg32_check_access(dabt) ) goto bad_width;
- VPL011_LOCK(d, flags);
+ spin_lock_irqsave(&vpl011->lock, flags);
*r = vreg_reg32_extract(vpl011->uartimsc, info);
- VPL011_UNLOCK(d, flags);
+ spin_unlock_irqrestore(&vpl011->lock, flags);
return 1;
case ICR:
@@ -476,19 +476,19 @@ static int vpl011_mmio_write(struct vcpu *v,
case IMSC:
if ( !vpl011_reg32_check_access(dabt) ) goto bad_width;
- VPL011_LOCK(d, flags);
+ spin_lock_irqsave(&vpl011->lock, flags);
vreg_reg32_update(&vpl011->uartimsc, r, info);
vpl011_update_interrupt_status(v->domain);
- VPL011_UNLOCK(d, flags);
+ spin_unlock_irqrestore(&vpl011->lock, flags);
return 1;
case ICR:
if ( !vpl011_reg32_check_access(dabt) ) goto bad_width;
- VPL011_LOCK(d, flags);
+ spin_lock_irqsave(&vpl011->lock, flags);
vreg_reg32_clearbits(&vpl011->uartris, r, info);
vpl011_update_interrupt_status(d);
- VPL011_UNLOCK(d, flags);
+ spin_unlock_irqrestore(&vpl011->lock, flags);
return 1;
default:
@@ -587,13 +587,13 @@ int vuart_putchar(struct domain *d, char c)
if ( intf == NULL )
return -ENODEV;
- VPL011_LOCK(d, flags);
+ spin_lock_irqsave(&vpl011->lock, flags);
in_cons = intf->in_cons;
in_prod = intf->in_prod;
if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) == sizeof(intf->in) )
{
- VPL011_UNLOCK(d, flags);
+ spin_unlock_irqrestore(&vpl011->lock, flags);
return -ENOSPC;
}
@@ -605,7 +605,7 @@ int vuart_putchar(struct domain *d, char c)
sizeof(intf->in));
vpl011_data_avail(d, in_fifo_level, sizeof(intf->in), 0, SBSA_UART_FIFO_SIZE);
- VPL011_UNLOCK(d, flags);
+ spin_unlock_irqrestore(&vpl011->lock, flags);
return 0;
}
@@ -619,7 +619,7 @@ static void vpl011_notification(struct vcpu *v, unsigned int port)
XENCONS_RING_IDX in_cons, in_prod, out_cons, out_prod;
XENCONS_RING_IDX in_fifo_level, out_fifo_level;
- VPL011_LOCK(d, flags);
+ spin_lock_irqsave(&vpl011->lock, flags);
in_cons = intf->in_cons;
in_prod = intf->in_prod;
@@ -639,7 +639,7 @@ static void vpl011_notification(struct vcpu *v, unsigned int port)
vpl011_data_avail(v->domain, in_fifo_level, sizeof(intf->in),
out_fifo_level, sizeof(intf->out));
- VPL011_UNLOCK(d, flags);
+ spin_unlock_irqrestore(&vpl011->lock, flags);
}
int vuart_init(struct domain *d, struct vuart_params *params)
--
2.34.1
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH v1 04/16] arm/vpl011: use raw spin_lock_{irqrestore,irqsave}
2025-06-24 3:55 ` [PATCH v1 04/16] arm/vpl011: use raw spin_lock_{irqrestore,irqsave} dmkhn
@ 2025-06-24 5:46 ` Jan Beulich
2025-06-24 7:50 ` Orzel, Michal
0 siblings, 1 reply; 59+ messages in thread
From: Jan Beulich @ 2025-06-24 5:46 UTC (permalink / raw)
To: dmkhn
Cc: andrew.cooper3, anthony.perard, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
On 24.06.2025 05:55, dmkhn@proton.me wrote:
> From: Denis Mukhin <dmukhin@ford.com>
>
> Replace VPL011_{LOCK,UNLOCK} macros with raw spinlock calls to improve
> readability.
I'm not an Arm maintainer, so I have limited say here, but: How is this
improving readability? It better utilizes available local variables, yes,
so this may be a little bit of an optimization, but otherwise to me this
looks to rather hamper readability.
Jan
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [PATCH v1 04/16] arm/vpl011: use raw spin_lock_{irqrestore,irqsave}
2025-06-24 5:46 ` Jan Beulich
@ 2025-06-24 7:50 ` Orzel, Michal
2025-06-24 21:46 ` dmkhn
0 siblings, 1 reply; 59+ messages in thread
From: Orzel, Michal @ 2025-06-24 7:50 UTC (permalink / raw)
To: Jan Beulich, dmkhn
Cc: andrew.cooper3, anthony.perard, julien, oleksii.kurochko,
roger.pau, sstabellini, dmukhin, xen-devel
On 24/06/2025 07:46, Jan Beulich wrote:
> On 24.06.2025 05:55, dmkhn@proton.me wrote:
>> From: Denis Mukhin <dmukhin@ford.com>
>>
>> Replace VPL011_{LOCK,UNLOCK} macros with raw spinlock calls to improve
>> readability.
>
> I'm not an Arm maintainer, so I have limited say here, but: How is this
> improving readability? It better utilizes available local variables, yes,
> so this may be a little bit of an optimization, but otherwise to me this
> looks to rather hamper readability.
I agree with Jan here. I don't think it improves readability, therefore I don't
think such change is needed.
~Michal
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [PATCH v1 04/16] arm/vpl011: use raw spin_lock_{irqrestore,irqsave}
2025-06-24 7:50 ` Orzel, Michal
@ 2025-06-24 21:46 ` dmkhn
2025-06-25 6:52 ` Orzel, Michal
0 siblings, 1 reply; 59+ messages in thread
From: dmkhn @ 2025-06-24 21:46 UTC (permalink / raw)
To: Orzel, Michal
Cc: Jan Beulich, andrew.cooper3, anthony.perard, julien,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
On Tue, Jun 24, 2025 at 09:50:54AM +0200, Orzel, Michal wrote:
>
>
> On 24/06/2025 07:46, Jan Beulich wrote:
> > On 24.06.2025 05:55, dmkhn@proton.me wrote:
> >> From: Denis Mukhin <dmukhin@ford.com>
> >>
> >> Replace VPL011_{LOCK,UNLOCK} macros with raw spinlock calls to improve
> >> readability.
> >
> > I'm not an Arm maintainer, so I have limited say here, but: How is this
> > improving readability? It better utilizes available local variables, yes,
> > so this may be a little bit of an optimization, but otherwise to me this
> > looks to rather hamper readability.
> I agree with Jan here. I don't think it improves readability, therefore I don't
> think such change is needed.
I think exdanding macros helps to understand the code since is explicitly
shows what kind of locking *really* used, so this aspect is actually getting
more readable; yes, that's a bit of more text.
But, MMIO-based flavor does not define such helpers for example, so now vUARTs
follow similar coding pattern which is easy to read/follow.
>
> ~Michal
>
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [PATCH v1 04/16] arm/vpl011: use raw spin_lock_{irqrestore,irqsave}
2025-06-24 21:46 ` dmkhn
@ 2025-06-25 6:52 ` Orzel, Michal
2025-07-31 21:03 ` dmkhn
0 siblings, 1 reply; 59+ messages in thread
From: Orzel, Michal @ 2025-06-25 6:52 UTC (permalink / raw)
To: dmkhn
Cc: Jan Beulich, andrew.cooper3, anthony.perard, julien,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
On 24/06/2025 23:46, dmkhn@proton.me wrote:
> On Tue, Jun 24, 2025 at 09:50:54AM +0200, Orzel, Michal wrote:
>>
>>
>> On 24/06/2025 07:46, Jan Beulich wrote:
>>> On 24.06.2025 05:55, dmkhn@proton.me wrote:
>>>> From: Denis Mukhin <dmukhin@ford.com>
>>>>
>>>> Replace VPL011_{LOCK,UNLOCK} macros with raw spinlock calls to improve
>>>> readability.
>>>
>>> I'm not an Arm maintainer, so I have limited say here, but: How is this
>>> improving readability? It better utilizes available local variables, yes,
>>> so this may be a little bit of an optimization, but otherwise to me this
>>> looks to rather hamper readability.
>> I agree with Jan here. I don't think it improves readability, therefore I don't
>> think such change is needed.
>
> I think exdanding macros helps to understand the code since is explicitly
> shows what kind of locking *really* used, so this aspect is actually getting
> more readable; yes, that's a bit of more text.
>
> But, MMIO-based flavor does not define such helpers for example, so now vUARTs
> follow similar coding pattern which is easy to read/follow.
I understand your point of view. It's more like a matter of taste here, so I
won't oppose to it. Others may chime in.
~Michal
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [PATCH v1 04/16] arm/vpl011: use raw spin_lock_{irqrestore,irqsave}
2025-06-25 6:52 ` Orzel, Michal
@ 2025-07-31 21:03 ` dmkhn
0 siblings, 0 replies; 59+ messages in thread
From: dmkhn @ 2025-07-31 21:03 UTC (permalink / raw)
To: Orzel, Michal
Cc: Jan Beulich, andrew.cooper3, anthony.perard, julien,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
On Wed, Jun 25, 2025 at 08:52:39AM +0200, Orzel, Michal wrote:
>
>
> On 24/06/2025 23:46, dmkhn@proton.me wrote:
> > On Tue, Jun 24, 2025 at 09:50:54AM +0200, Orzel, Michal wrote:
> >>
> >>
> >> On 24/06/2025 07:46, Jan Beulich wrote:
> >>> On 24.06.2025 05:55, dmkhn@proton.me wrote:
> >>>> From: Denis Mukhin <dmukhin@ford.com>
> >>>>
> >>>> Replace VPL011_{LOCK,UNLOCK} macros with raw spinlock calls to improve
> >>>> readability.
> >>>
> >>> I'm not an Arm maintainer, so I have limited say here, but: How is this
> >>> improving readability? It better utilizes available local variables, yes,
> >>> so this may be a little bit of an optimization, but otherwise to me this
> >>> looks to rather hamper readability.
> >> I agree with Jan here. I don't think it improves readability, therefore I don't
> >> think such change is needed.
> >
> > I think exdanding macros helps to understand the code since is explicitly
> > shows what kind of locking *really* used, so this aspect is actually getting
> > more readable; yes, that's a bit of more text.
> >
> > But, MMIO-based flavor does not define such helpers for example, so now vUARTs
> > follow similar coding pattern which is easy to read/follow.
> I understand your point of view. It's more like a matter of taste here, so I
> won't oppose to it. Others may chime in.
Thank you.
>
> ~Michal
>
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v1 05/16] arm/vpl011: use void pointer in domain struct
2025-06-24 3:54 [PATCH v1 00/16] xen: framework for UART emulators dmkhn
` (3 preceding siblings ...)
2025-06-24 3:55 ` [PATCH v1 04/16] arm/vpl011: use raw spin_lock_{irqrestore,irqsave} dmkhn
@ 2025-06-24 3:55 ` dmkhn
2025-06-25 7:21 ` Orzel, Michal
2025-06-24 3:55 ` [PATCH v1 06/16] arm/vpl011: remove vpl011 header file dmkhn
` (10 subsequent siblings)
15 siblings, 1 reply; 59+ messages in thread
From: dmkhn @ 2025-06-24 3:55 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
From: Denis Mukhin <dmukhin@ford.com>
Switch to using void pointer in domain struct to reduce compile-time
dependencies for PL011 emulator.
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
xen/arch/arm/include/asm/domain.h | 3 +-
xen/arch/arm/vpl011.c | 139 +++++++++++++++++-------------
2 files changed, 79 insertions(+), 63 deletions(-)
diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
index 746ea687d523..2ee9976b55a8 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -9,7 +9,6 @@
#include <asm/mmio.h>
#include <asm/gic.h>
#include <asm/vgic.h>
-#include <asm/vpl011.h>
#include <public/hvm/params.h>
struct hvm_domain
@@ -114,7 +113,7 @@ struct arch_domain
} monitor;
#ifdef CONFIG_HAS_VUART_PL011
- struct vpl011 vpl011;
+ void *vpl011;
#endif
#ifdef CONFIG_TEE
diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index a97c3b74208c..3c027ccf0b4e 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -22,6 +22,7 @@
#include <xen/console.h>
#include <xen/serial.h>
#include <xen/vuart.h>
+#include <xen/xvmalloc.h>
#include <public/domctl.h>
#include <public/io/console.h>
#include <asm/domain_build.h>
@@ -31,6 +32,43 @@
#include <asm/vpl011.h>
#include <asm/vreg.h>
+static void __vpl011_exit(struct vpl011 *vpl011, struct domain *d)
+{
+ if ( vpl011->virq )
+ {
+ vgic_free_virq(d, vpl011->virq);
+
+ /*
+ * Set to invalid irq (we use SPI) to prevent extra free and to avoid
+ * freeing irq that could have already been reserved by someone else.
+ */
+ vpl011->virq = 0;
+ }
+
+ if ( vpl011->backend_in_domain )
+ {
+ if ( vpl011->backend.dom.ring_buf )
+ destroy_ring_for_helper(&vpl011->backend.dom.ring_buf,
+ vpl011->backend.dom.ring_page);
+
+ if ( vpl011->evtchn )
+ {
+ free_xen_event_channel(d, vpl011->evtchn);
+
+ /*
+ * Set to invalid event channel port to prevent extra free and to
+ * avoid freeing port that could have already been allocated for
+ * other purposes.
+ */
+ vpl011->evtchn = 0;
+ }
+ }
+ else
+ XFREE(vpl011->backend.xen);
+
+ xfree(vpl011);
+}
+
/*
* Since pl011 registers are 32-bit registers, all registers
* are handled similarly allowing 8-bit, 16-bit and 32-bit
@@ -43,7 +81,7 @@ static bool vpl011_reg32_check_access(struct hsr_dabt dabt)
static void vpl011_update_interrupt_status(struct domain *d)
{
- struct vpl011 *vpl011 = &d->arch.vpl011;
+ struct vpl011 *vpl011 = d->arch.vpl011;
uint32_t uartmis = vpl011->uartris & vpl011->uartimsc;
/*
@@ -81,7 +119,7 @@ static void vpl011_update_interrupt_status(struct domain *d)
static void vpl011_write_data_xen(struct domain *d, uint8_t data)
{
unsigned long flags;
- struct vpl011 *vpl011 = &d->arch.vpl011;
+ struct vpl011 *vpl011 = d->arch.vpl011;
struct vpl011_xen_backend *intf = vpl011->backend.xen;
struct domain *input = console_get_domain();
@@ -140,7 +178,7 @@ static uint8_t vpl011_read_data_xen(struct domain *d)
{
unsigned long flags;
uint8_t data = 0;
- struct vpl011 *vpl011 = &d->arch.vpl011;
+ struct vpl011 *vpl011 = d->arch.vpl011;
struct vpl011_xen_backend *intf = vpl011->backend.xen;
XENCONS_RING_IDX in_cons, in_prod;
@@ -199,7 +237,7 @@ static uint8_t vpl011_read_data(struct domain *d)
{
unsigned long flags;
uint8_t data = 0;
- struct vpl011 *vpl011 = &d->arch.vpl011;
+ struct vpl011 *vpl011 = d->arch.vpl011;
struct xencons_interface *intf = vpl011->backend.dom.ring_buf;
XENCONS_RING_IDX in_cons, in_prod;
@@ -284,7 +322,7 @@ static void vpl011_update_tx_fifo_status(struct vpl011 *vpl011,
static void vpl011_write_data(struct domain *d, uint8_t data)
{
unsigned long flags;
- struct vpl011 *vpl011 = &d->arch.vpl011;
+ struct vpl011 *vpl011 = d->arch.vpl011;
struct xencons_interface *intf = vpl011->backend.dom.ring_buf;
XENCONS_RING_IDX out_cons, out_prod;
@@ -350,10 +388,9 @@ static int vpl011_mmio_read(struct vcpu *v,
register_t *r,
void *priv)
{
+ struct vpl011 *vpl011 = v->domain->arch.vpl011;
struct hsr_dabt dabt = info->dabt;
- uint32_t vpl011_reg = (uint32_t)(info->gpa -
- v->domain->arch.vpl011.base_addr);
- struct vpl011 *vpl011 = &v->domain->arch.vpl011;
+ uint32_t vpl011_reg = (uint32_t)(info->gpa - vpl011->base_addr);
struct domain *d = v->domain;
unsigned long flags;
@@ -439,10 +476,9 @@ static int vpl011_mmio_write(struct vcpu *v,
register_t r,
void *priv)
{
+ struct vpl011 *vpl011 = v->domain->arch.vpl011;
struct hsr_dabt dabt = info->dabt;
- uint32_t vpl011_reg = (uint32_t)(info->gpa -
- v->domain->arch.vpl011.base_addr);
- struct vpl011 *vpl011 = &v->domain->arch.vpl011;
+ uint32_t vpl011_reg = (uint32_t)(info->gpa - vpl011->base_addr);
struct domain *d = v->domain;
unsigned long flags;
@@ -518,7 +554,7 @@ static void vpl011_data_avail(struct domain *d,
XENCONS_RING_IDX out_fifo_level,
XENCONS_RING_IDX out_size)
{
- struct vpl011 *vpl011 = &d->arch.vpl011;
+ struct vpl011 *vpl011 = d->arch.vpl011;
/**** Update the UART RX state ****/
@@ -576,7 +612,7 @@ static void vpl011_data_avail(struct domain *d,
int vuart_putchar(struct domain *d, char c)
{
unsigned long flags;
- struct vpl011 *vpl011 = &d->arch.vpl011;
+ struct vpl011 *vpl011 = d->arch.vpl011;
struct vpl011_xen_backend *intf = vpl011->backend.xen;
XENCONS_RING_IDX in_cons, in_prod, in_fifo_level;
@@ -614,7 +650,7 @@ static void vpl011_notification(struct vcpu *v, unsigned int port)
{
unsigned long flags;
struct domain *d = v->domain;
- struct vpl011 *vpl011 = &d->arch.vpl011;
+ struct vpl011 *vpl011 = d->arch.vpl011;
struct xencons_interface *intf = vpl011->backend.dom.ring_buf;
XENCONS_RING_IDX in_cons, in_prod, out_cons, out_prod;
XENCONS_RING_IDX in_fifo_level, out_fifo_level;
@@ -644,11 +680,14 @@ static void vpl011_notification(struct vcpu *v, unsigned int port)
int vuart_init(struct domain *d, struct vuart_params *params)
{
+ struct vpl011 *vpl011;
int rc;
- struct vpl011 *vpl011 = &d->arch.vpl011;
- if ( vpl011->backend.dom.ring_buf )
- return -EINVAL;
+ BUG_ON(d->arch.vpl011);
+
+ vpl011 = xvzalloc(typeof(*vpl011));
+ if ( !vpl011 )
+ return -ENOMEM;
/*
* The VPL011 virq is GUEST_VPL011_SPI, except for direct-map domains
@@ -670,7 +709,8 @@ int vuart_init(struct domain *d, struct vuart_params *params)
{
printk(XENLOG_ERR
"vpl011: Unable to re-use the Xen UART information.\n");
- return -EINVAL;
+ rc = -EINVAL;
+ goto err_out;
}
/*
@@ -684,7 +724,8 @@ int vuart_init(struct domain *d, struct vuart_params *params)
{
printk(XENLOG_ERR
"vpl011: Can't re-use the Xen UART MMIO region as it is too small.\n");
- return -EINVAL;
+ rc = -EINVAL;
+ goto err_out;
}
}
else
@@ -707,12 +748,12 @@ int vuart_init(struct domain *d, struct vuart_params *params)
&vpl011->backend.dom.ring_page,
&vpl011->backend.dom.ring_buf);
if ( rc < 0 )
- goto out;
+ goto err_out;
rc = alloc_unbound_xen_event_channel(d, 0, params->console_domid,
vpl011_notification);
if ( rc < 0 )
- goto out1;
+ goto err_out;
vpl011->evtchn = params->evtchn = rc;
}
@@ -725,7 +766,7 @@ int vuart_init(struct domain *d, struct vuart_params *params)
if ( vpl011->backend.xen == NULL )
{
rc = -ENOMEM;
- goto out;
+ goto err_out;
}
}
@@ -733,7 +774,7 @@ int vuart_init(struct domain *d, struct vuart_params *params)
if ( !rc )
{
rc = -EINVAL;
- goto out1;
+ goto err_out;
}
vpl011->uartfr = TXFE | RXFE;
@@ -743,50 +784,22 @@ int vuart_init(struct domain *d, struct vuart_params *params)
register_mmio_handler(d, &vpl011_mmio_handler,
vpl011->base_addr, GUEST_PL011_SIZE, NULL);
+ d->arch.vpl011 = vpl011;
+
return 0;
-out1:
- vuart_exit(d);
-
-out:
+err_out:
+ __vpl011_exit(vpl011, d);
return rc;
}
void vuart_exit(struct domain *d)
{
- struct vpl011 *vpl011 = &d->arch.vpl011;
-
- if ( vpl011->virq )
+ if ( d->arch.vpl011 )
{
- vgic_free_virq(d, vpl011->virq);
-
- /*
- * Set to invalid irq (we use SPI) to prevent extra free and to avoid
- * freeing irq that could have already been reserved by someone else.
- */
- vpl011->virq = 0;
+ __vpl011_exit(d->arch.vpl011, d);
+ d->arch.vpl011 = NULL;
}
-
- if ( vpl011->backend_in_domain )
- {
- if ( vpl011->backend.dom.ring_buf )
- destroy_ring_for_helper(&vpl011->backend.dom.ring_buf,
- vpl011->backend.dom.ring_page);
-
- if ( vpl011->evtchn )
- {
- free_xen_event_channel(d, vpl011->evtchn);
-
- /*
- * Set to invalid event channel port to prevent extra free and to
- * avoid freeing port that could have already been allocated for
- * other purposes.
- */
- vpl011->evtchn = 0;
- }
- }
- else
- XFREE(vpl011->backend.xen);
}
int __init vuart_add_fwnode(struct domain *d, void *node)
@@ -797,8 +810,12 @@ int __init vuart_add_fwnode(struct domain *d, void *node)
gic_interrupt_t intr;
__be32 reg[GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS];
__be32 *cells;
+ struct vpl011 *vpl011 = d->arch.vpl011;
- res = domain_fdt_begin_node(fdt, "sbsa-uart", d->arch.vpl011.base_addr);
+ if ( !vpl011 )
+ return 0;
+
+ res = domain_fdt_begin_node(fdt, "sbsa-uart", vpl011->base_addr);
if ( res )
return res;
@@ -808,14 +825,14 @@ int __init vuart_add_fwnode(struct domain *d, void *node)
cells = ®[0];
dt_child_set_range(&cells, GUEST_ROOT_ADDRESS_CELLS,
- GUEST_ROOT_SIZE_CELLS, d->arch.vpl011.base_addr,
+ GUEST_ROOT_SIZE_CELLS, vpl011->base_addr,
GUEST_PL011_SIZE);
res = fdt_property(fdt, "reg", reg, sizeof(reg));
if ( res )
return res;
- set_interrupt(intr, d->arch.vpl011.virq, 0xf, DT_IRQ_TYPE_LEVEL_HIGH);
+ set_interrupt(intr, vpl011->virq, 0xf, DT_IRQ_TYPE_LEVEL_HIGH);
res = fdt_property(fdt, "interrupts", intr, sizeof (intr));
if ( res )
--
2.34.1
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH v1 05/16] arm/vpl011: use void pointer in domain struct
2025-06-24 3:55 ` [PATCH v1 05/16] arm/vpl011: use void pointer in domain struct dmkhn
@ 2025-06-25 7:21 ` Orzel, Michal
0 siblings, 0 replies; 59+ messages in thread
From: Orzel, Michal @ 2025-06-25 7:21 UTC (permalink / raw)
To: dmkhn, xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
On 24/06/2025 05:55, dmkhn@proton.me wrote:
> From: Denis Mukhin <dmukhin@ford.com>
>
> Switch to using void pointer in domain struct to reduce compile-time
> dependencies for PL011 emulator.
I don't understand the rationale very well. Could you provide more details?
Why can't we keep struct vpl011 in domain struct given? I would understand the
need for void if we used a single member that could map to different vUARTs
depending on selection. That's clearly not the case. If it is just to avoid the
header, then I don't think we need such churn.
>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
> xen/arch/arm/include/asm/domain.h | 3 +-
> xen/arch/arm/vpl011.c | 139 +++++++++++++++++-------------
> 2 files changed, 79 insertions(+), 63 deletions(-)
>
> diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
> index 746ea687d523..2ee9976b55a8 100644
> --- a/xen/arch/arm/include/asm/domain.h
> +++ b/xen/arch/arm/include/asm/domain.h
> @@ -9,7 +9,6 @@
> #include <asm/mmio.h>
> #include <asm/gic.h>
> #include <asm/vgic.h>
> -#include <asm/vpl011.h>
> #include <public/hvm/params.h>
>
> struct hvm_domain
> @@ -114,7 +113,7 @@ struct arch_domain
> } monitor;
>
> #ifdef CONFIG_HAS_VUART_PL011
> - struct vpl011 vpl011;
> + void *vpl011;
> #endif
>
> #ifdef CONFIG_TEE
> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> index a97c3b74208c..3c027ccf0b4e 100644
> --- a/xen/arch/arm/vpl011.c
> +++ b/xen/arch/arm/vpl011.c
> @@ -22,6 +22,7 @@
> #include <xen/console.h>
> #include <xen/serial.h>
> #include <xen/vuart.h>
> +#include <xen/xvmalloc.h>
> #include <public/domctl.h>
> #include <public/io/console.h>
> #include <asm/domain_build.h>
> @@ -31,6 +32,43 @@
> #include <asm/vpl011.h>
> #include <asm/vreg.h>
>
> +static void __vpl011_exit(struct vpl011 *vpl011, struct domain *d
Names starting with '__' are reserved and forbidden by MISRA C rule that AFAIR
we accepted (don't remember what rule exactly).
> +{
> + if ( vpl011->virq )
> + {
> + vgic_free_virq(d, vpl011->virq);
> +
> + /*
> + * Set to invalid irq (we use SPI) to prevent extra free and to avoid
> + * freeing irq that could have already been reserved by someone else.
> + */
> + vpl011->virq = 0;
> + }
> +
> + if ( vpl011->backend_in_domain )
> + {
> + if ( vpl011->backend.dom.ring_buf )
> + destroy_ring_for_helper(&vpl011->backend.dom.ring_buf,
> + vpl011->backend.dom.ring_page);
> +
> + if ( vpl011->evtchn )
> + {
> + free_xen_event_channel(d, vpl011->evtchn);
> +
> + /*
> + * Set to invalid event channel port to prevent extra free and to
> + * avoid freeing port that could have already been allocated for
> + * other purposes.
> + */
> + vpl011->evtchn = 0;
> + }
> + }
> + else
> + XFREE(vpl011->backend.xen);
> +
> + xfree(vpl011);
> +}
> +
> /*
> * Since pl011 registers are 32-bit registers, all registers
> * are handled similarly allowing 8-bit, 16-bit and 32-bit
> @@ -43,7 +81,7 @@ static bool vpl011_reg32_check_access(struct hsr_dabt dabt)
>
> static void vpl011_update_interrupt_status(struct domain *d)
> {
> - struct vpl011 *vpl011 = &d->arch.vpl011;
> + struct vpl011 *vpl011 = d->arch.vpl011;
> uint32_t uartmis = vpl011->uartris & vpl011->uartimsc;
>
> /*
> @@ -81,7 +119,7 @@ static void vpl011_update_interrupt_status(struct domain *d)
> static void vpl011_write_data_xen(struct domain *d, uint8_t data)
> {
> unsigned long flags;
> - struct vpl011 *vpl011 = &d->arch.vpl011;
> + struct vpl011 *vpl011 = d->arch.vpl011;
> struct vpl011_xen_backend *intf = vpl011->backend.xen;
> struct domain *input = console_get_domain();
>
> @@ -140,7 +178,7 @@ static uint8_t vpl011_read_data_xen(struct domain *d)
> {
> unsigned long flags;
> uint8_t data = 0;
> - struct vpl011 *vpl011 = &d->arch.vpl011;
> + struct vpl011 *vpl011 = d->arch.vpl011;
> struct vpl011_xen_backend *intf = vpl011->backend.xen;
> XENCONS_RING_IDX in_cons, in_prod;
>
> @@ -199,7 +237,7 @@ static uint8_t vpl011_read_data(struct domain *d)
> {
> unsigned long flags;
> uint8_t data = 0;
> - struct vpl011 *vpl011 = &d->arch.vpl011;
> + struct vpl011 *vpl011 = d->arch.vpl011;
> struct xencons_interface *intf = vpl011->backend.dom.ring_buf;
> XENCONS_RING_IDX in_cons, in_prod;
>
> @@ -284,7 +322,7 @@ static void vpl011_update_tx_fifo_status(struct vpl011 *vpl011,
> static void vpl011_write_data(struct domain *d, uint8_t data)
> {
> unsigned long flags;
> - struct vpl011 *vpl011 = &d->arch.vpl011;
> + struct vpl011 *vpl011 = d->arch.vpl011;
> struct xencons_interface *intf = vpl011->backend.dom.ring_buf;
> XENCONS_RING_IDX out_cons, out_prod;
>
> @@ -350,10 +388,9 @@ static int vpl011_mmio_read(struct vcpu *v,
> register_t *r,
> void *priv)
> {
> + struct vpl011 *vpl011 = v->domain->arch.vpl011;
> struct hsr_dabt dabt = info->dabt;
> - uint32_t vpl011_reg = (uint32_t)(info->gpa -
> - v->domain->arch.vpl011.base_addr);
> - struct vpl011 *vpl011 = &v->domain->arch.vpl011;
> + uint32_t vpl011_reg = (uint32_t)(info->gpa - vpl011->base_addr);
> struct domain *d = v->domain;
> unsigned long flags;
>
> @@ -439,10 +476,9 @@ static int vpl011_mmio_write(struct vcpu *v,
> register_t r,
> void *priv)
> {
> + struct vpl011 *vpl011 = v->domain->arch.vpl011;
> struct hsr_dabt dabt = info->dabt;
> - uint32_t vpl011_reg = (uint32_t)(info->gpa -
> - v->domain->arch.vpl011.base_addr);
> - struct vpl011 *vpl011 = &v->domain->arch.vpl011;
> + uint32_t vpl011_reg = (uint32_t)(info->gpa - vpl011->base_addr);
> struct domain *d = v->domain;
> unsigned long flags;
>
> @@ -518,7 +554,7 @@ static void vpl011_data_avail(struct domain *d,
> XENCONS_RING_IDX out_fifo_level,
> XENCONS_RING_IDX out_size)
> {
> - struct vpl011 *vpl011 = &d->arch.vpl011;
> + struct vpl011 *vpl011 = d->arch.vpl011;
>
> /**** Update the UART RX state ****/
>
> @@ -576,7 +612,7 @@ static void vpl011_data_avail(struct domain *d,
> int vuart_putchar(struct domain *d, char c)
> {
> unsigned long flags;
> - struct vpl011 *vpl011 = &d->arch.vpl011;
> + struct vpl011 *vpl011 = d->arch.vpl011;
> struct vpl011_xen_backend *intf = vpl011->backend.xen;
> XENCONS_RING_IDX in_cons, in_prod, in_fifo_level;
>
> @@ -614,7 +650,7 @@ static void vpl011_notification(struct vcpu *v, unsigned int port)
> {
> unsigned long flags;
> struct domain *d = v->domain;
> - struct vpl011 *vpl011 = &d->arch.vpl011;
> + struct vpl011 *vpl011 = d->arch.vpl011;
> struct xencons_interface *intf = vpl011->backend.dom.ring_buf;
> XENCONS_RING_IDX in_cons, in_prod, out_cons, out_prod;
> XENCONS_RING_IDX in_fifo_level, out_fifo_level;
> @@ -644,11 +680,14 @@ static void vpl011_notification(struct vcpu *v, unsigned int port)
>
> int vuart_init(struct domain *d, struct vuart_params *params)
> {
> + struct vpl011 *vpl011;
> int rc;
> - struct vpl011 *vpl011 = &d->arch.vpl011;
>
> - if ( vpl011->backend.dom.ring_buf )
> - return -EINVAL;
> + BUG_ON(d->arch.vpl011);
> +
> + vpl011 = xvzalloc(typeof(*vpl011));
> + if ( !vpl011 )
> + return -ENOMEM;
>
> /*
> * The VPL011 virq is GUEST_VPL011_SPI, except for direct-map domains
> @@ -670,7 +709,8 @@ int vuart_init(struct domain *d, struct vuart_params *params)
> {
> printk(XENLOG_ERR
> "vpl011: Unable to re-use the Xen UART information.\n");
> - return -EINVAL;
> + rc = -EINVAL;
> + goto err_out;
> }
>
> /*
> @@ -684,7 +724,8 @@ int vuart_init(struct domain *d, struct vuart_params *params)
> {
> printk(XENLOG_ERR
> "vpl011: Can't re-use the Xen UART MMIO region as it is too small.\n");
> - return -EINVAL;
> + rc = -EINVAL;
> + goto err_out;
> }
> }
> else
> @@ -707,12 +748,12 @@ int vuart_init(struct domain *d, struct vuart_params *params)
> &vpl011->backend.dom.ring_page,
> &vpl011->backend.dom.ring_buf);
> if ( rc < 0 )
> - goto out;
> + goto err_out;
>
> rc = alloc_unbound_xen_event_channel(d, 0, params->console_domid,
> vpl011_notification);
> if ( rc < 0 )
> - goto out1;
> + goto err_out;
>
> vpl011->evtchn = params->evtchn = rc;
> }
> @@ -725,7 +766,7 @@ int vuart_init(struct domain *d, struct vuart_params *params)
> if ( vpl011->backend.xen == NULL )
> {
> rc = -ENOMEM;
> - goto out;
> + goto err_out;
> }
> }
>
> @@ -733,7 +774,7 @@ int vuart_init(struct domain *d, struct vuart_params *params)
> if ( !rc )
> {
> rc = -EINVAL;
> - goto out1;
> + goto err_out;
> }
>
> vpl011->uartfr = TXFE | RXFE;
> @@ -743,50 +784,22 @@ int vuart_init(struct domain *d, struct vuart_params *params)
> register_mmio_handler(d, &vpl011_mmio_handler,
> vpl011->base_addr, GUEST_PL011_SIZE, NULL);
>
> + d->arch.vpl011 = vpl011;
> +
> return 0;
>
> -out1:
> - vuart_exit(d);
> -
> -out:
> +err_out:
Labels should be indented by one space.
~Michal
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v1 06/16] arm/vpl011: remove vpl011 header file
2025-06-24 3:54 [PATCH v1 00/16] xen: framework for UART emulators dmkhn
` (4 preceding siblings ...)
2025-06-24 3:55 ` [PATCH v1 05/16] arm/vpl011: use void pointer in domain struct dmkhn
@ 2025-06-24 3:55 ` dmkhn
2025-06-24 3:55 ` [PATCH v1 07/16] arm/vuart: rename 'virtual UART' Kconfig option dmkhn
` (9 subsequent siblings)
15 siblings, 0 replies; 59+ messages in thread
From: dmkhn @ 2025-06-24 3:55 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
From: Denis Mukhin <dmukhin@ford.com>
Make all PL011 emulator declarations private to emulator's code.
No functional change.
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
xen/arch/arm/include/asm/vpl011.h | 67 -------------------------------
xen/arch/arm/vpl011.c | 39 ++++++++++++++++--
2 files changed, 35 insertions(+), 71 deletions(-)
delete mode 100644 xen/arch/arm/include/asm/vpl011.h
diff --git a/xen/arch/arm/include/asm/vpl011.h b/xen/arch/arm/include/asm/vpl011.h
deleted file mode 100644
index 8f6ea0005e72..000000000000
--- a/xen/arch/arm/include/asm/vpl011.h
+++ /dev/null
@@ -1,67 +0,0 @@
-/*
- * include/xen/vpl011.h
- *
- * Virtual PL011 UART
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program; If not, see <http://www.gnu.org/licenses/>.
- */
-
-#ifndef _VPL011_H_
-#define _VPL011_H_
-
-#include <public/domctl.h>
-#include <public/io/ring.h>
-#include <public/io/console.h>
-#include <xen/mm.h>
-
-#define SBSA_UART_FIFO_SIZE 32
-/* Same size as VUART_BUF_SIZE, used in vuart.c */
-#define SBSA_UART_OUT_BUF_SIZE 128
-struct vpl011_xen_backend {
- char in[SBSA_UART_FIFO_SIZE];
- char out[SBSA_UART_OUT_BUF_SIZE];
- XENCONS_RING_IDX in_cons, in_prod;
- XENCONS_RING_IDX out_prod;
-};
-
-struct vpl011 {
- bool backend_in_domain;
- union {
- struct {
- void *ring_buf;
- struct page_info *ring_page;
- } dom;
- struct vpl011_xen_backend *xen;
- } backend;
- uint32_t uartfr; /* Flag register */
- uint32_t uartcr; /* Control register */
- uint32_t uartimsc; /* Interrupt mask register*/
- uint32_t uarticr; /* Interrupt clear register */
- uint32_t uartris; /* Raw interrupt status register */
- uint32_t shadow_uartmis; /* shadow masked interrupt register */
- paddr_t base_addr;
- unsigned int virq;
- spinlock_t lock;
- evtchn_port_t evtchn;
-};
-
-#endif /* _VPL011_H_ */
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 3c027ccf0b4e..bebfb5e0365c 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -7,9 +7,6 @@
#define XEN_WANT_FLEX_CONSOLE_RING 1
-/* We assume the PL011 default of "1/2 way" for the FIFO trigger level. */
-#define SBSA_UART_FIFO_LEVEL (SBSA_UART_FIFO_SIZE / 2)
-
#include <xen/errno.h>
#include <xen/event.h>
#include <xen/device_tree.h>
@@ -25,13 +22,47 @@
#include <xen/xvmalloc.h>
#include <public/domctl.h>
#include <public/io/console.h>
+#include <public/io/ring.h>
#include <asm/domain_build.h>
#include <asm/kernel.h>
#include <asm/pl011-uart.h>
#include <asm/vgic-emul.h>
-#include <asm/vpl011.h>
#include <asm/vreg.h>
+#define SBSA_UART_FIFO_SIZE 32
+/* Same size as VUART_BUF_SIZE, used in simple MMIO-based vUART */
+#define SBSA_UART_OUT_BUF_SIZE 128
+/* We assume the PL011 default of "1/2 way" for the FIFO trigger level. */
+#define SBSA_UART_FIFO_LEVEL (SBSA_UART_FIFO_SIZE / 2)
+
+struct vpl011_xen_backend {
+ char in[SBSA_UART_FIFO_SIZE];
+ char out[SBSA_UART_OUT_BUF_SIZE];
+ XENCONS_RING_IDX in_cons, in_prod;
+ XENCONS_RING_IDX out_prod;
+};
+
+struct vpl011 {
+ bool backend_in_domain;
+ union {
+ struct {
+ void *ring_buf;
+ struct page_info *ring_page;
+ } dom;
+ struct vpl011_xen_backend *xen;
+ } backend;
+ uint32_t uartfr; /* Flag register */
+ uint32_t uartcr; /* Control register */
+ uint32_t uartimsc; /* Interrupt mask register*/
+ uint32_t uarticr; /* Interrupt clear register */
+ uint32_t uartris; /* Raw interrupt status register */
+ uint32_t shadow_uartmis; /* shadow masked interrupt register */
+ paddr_t base_addr;
+ unsigned int virq;
+ spinlock_t lock;
+ evtchn_port_t evtchn;
+};
+
static void __vpl011_exit(struct vpl011 *vpl011, struct domain *d)
{
if ( vpl011->virq )
--
2.34.1
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH v1 07/16] arm/vuart: rename 'virtual UART' Kconfig option
2025-06-24 3:54 [PATCH v1 00/16] xen: framework for UART emulators dmkhn
` (5 preceding siblings ...)
2025-06-24 3:55 ` [PATCH v1 06/16] arm/vpl011: remove vpl011 header file dmkhn
@ 2025-06-24 3:55 ` dmkhn
2025-06-24 6:37 ` Orzel, Michal
2025-06-24 3:56 ` [PATCH v1 08/16] arm/vuart: move simple MMIO-based vUART declarations to common header dmkhn
` (8 subsequent siblings)
15 siblings, 1 reply; 59+ messages in thread
From: dmkhn @ 2025-06-24 3:55 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
From: Denis Mukhin <dmukhin@ford.com>
Rename HWDOM_VUART to HAS_VUART_MMIO.
This emulator emulates only one register and the use of the emulator is
limited to early boot console in the guest OS.
No functional change.
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
xen/arch/arm/Kconfig | 2 +-
xen/arch/arm/Makefile | 2 +-
xen/arch/arm/include/asm/domain.h | 2 +-
xen/arch/arm/vuart.h | 4 ++--
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 03888569f38c..b11cb583a763 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -177,7 +177,7 @@ config HAS_VUART_PL011
Allows a guest to use SBSA Generic UART as a console. The
SBSA Generic UART implements a subset of ARM PL011 UART.
-config HWDOM_VUART
+config HAS_VUART_MMIO
bool "Emulated UART for hardware domain"
default y
help
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 2d6787fb03bc..dd015a2a19e8 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -68,7 +68,7 @@ obj-y += vtimer.o
obj-$(CONFIG_HAS_VUART_PL011) += vpl011.o
obj-y += vsmc.o
obj-y += vpsci.o
-obj-$(CONFIG_HWDOM_VUART) += vuart.o
+obj-$(CONFIG_HAS_VUART_MMIO) += vuart.o
extra-y += xen.lds
diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
index 2ee9976b55a8..d668c11d7e2c 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -90,7 +90,7 @@ struct arch_domain
struct vgic_dist vgic;
-#ifdef CONFIG_HWDOM_VUART
+#ifdef CONFIG_HAS_VUART_MMIO
struct vuart {
#define VUART_BUF_SIZE 128
char *buf;
diff --git a/xen/arch/arm/vuart.h b/xen/arch/arm/vuart.h
index e90d84c6eddb..726846355c3b 100644
--- a/xen/arch/arm/vuart.h
+++ b/xen/arch/arm/vuart.h
@@ -22,7 +22,7 @@
struct domain;
-#ifdef CONFIG_HWDOM_VUART
+#ifdef CONFIG_HAS_VUART_MMIO
int domain_vuart_init(struct domain *d);
void domain_vuart_free(struct domain *d);
@@ -40,7 +40,7 @@ static inline int domain_vuart_init(struct domain *d)
static inline void domain_vuart_free(struct domain *d) {};
-#endif /* CONFIG_HWDOM_VUART */
+#endif /* CONFIG_HAS_VUART_MMIO */
#endif /* __ARCH_ARM_VUART_H__ */
--
2.34.1
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH v1 07/16] arm/vuart: rename 'virtual UART' Kconfig option
2025-06-24 3:55 ` [PATCH v1 07/16] arm/vuart: rename 'virtual UART' Kconfig option dmkhn
@ 2025-06-24 6:37 ` Orzel, Michal
2025-06-24 7:14 ` dmkhn
0 siblings, 1 reply; 59+ messages in thread
From: Orzel, Michal @ 2025-06-24 6:37 UTC (permalink / raw)
To: dmkhn, xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
On 24/06/2025 05:55, dmkhn@proton.me wrote:
> From: Denis Mukhin <dmukhin@ford.com>
>
> Rename HWDOM_VUART to HAS_VUART_MMIO.
>
> This emulator emulates only one register and the use of the emulator is
> limited to early boot console in the guest OS.
>
> No functional change.
>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
> xen/arch/arm/Kconfig | 2 +-
> xen/arch/arm/Makefile | 2 +-
> xen/arch/arm/include/asm/domain.h | 2 +-
> xen/arch/arm/vuart.h | 4 ++--
> 4 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 03888569f38c..b11cb583a763 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -177,7 +177,7 @@ config HAS_VUART_PL011
> Allows a guest to use SBSA Generic UART as a console. The
> SBSA Generic UART implements a subset of ARM PL011 UART.
>
> -config HWDOM_VUART
> +config HAS_VUART_MMIO
I personally don't like this change. The current config option name reads much
better and clearly denotes the purpose.
~Michal
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v1 07/16] arm/vuart: rename 'virtual UART' Kconfig option
2025-06-24 6:37 ` Orzel, Michal
@ 2025-06-24 7:14 ` dmkhn
2025-06-25 7:07 ` Orzel, Michal
0 siblings, 1 reply; 59+ messages in thread
From: dmkhn @ 2025-06-24 7:14 UTC (permalink / raw)
To: Orzel, Michal
Cc: xen-devel, andrew.cooper3, anthony.perard, jbeulich, julien,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
On Tue, Jun 24, 2025 at 08:37:22AM +0200, Orzel, Michal wrote:
>
>
> On 24/06/2025 05:55, dmkhn@proton.me wrote:
> > From: Denis Mukhin <dmukhin@ford.com>
> >
> > Rename HWDOM_VUART to HAS_VUART_MMIO.
> >
> > This emulator emulates only one register and the use of the emulator is
> > limited to early boot console in the guest OS.
> >
> > No functional change.
> >
> > Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> > ---
> > xen/arch/arm/Kconfig | 2 +-
> > xen/arch/arm/Makefile | 2 +-
> > xen/arch/arm/include/asm/domain.h | 2 +-
> > xen/arch/arm/vuart.h | 4 ++--
> > 4 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index 03888569f38c..b11cb583a763 100644
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
> > @@ -177,7 +177,7 @@ config HAS_VUART_PL011
> > Allows a guest to use SBSA Generic UART as a console. The
> > SBSA Generic UART implements a subset of ARM PL011 UART.
> >
> > -config HWDOM_VUART
> > +config HAS_VUART_MMIO
> I personally don't like this change. The current config option name reads much
> better and clearly denotes the purpose.
In my opinion, the MMIO-based UART is a useful debugging tool for early guest
boot, even when the guest doesn't run in hwdom or on Arm system.
>
> ~Michal
>
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v1 07/16] arm/vuart: rename 'virtual UART' Kconfig option
2025-06-24 7:14 ` dmkhn
@ 2025-06-25 7:07 ` Orzel, Michal
2025-07-31 21:02 ` dmkhn
0 siblings, 1 reply; 59+ messages in thread
From: Orzel, Michal @ 2025-06-25 7:07 UTC (permalink / raw)
To: dmkhn
Cc: xen-devel, andrew.cooper3, anthony.perard, jbeulich, julien,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
On 24/06/2025 09:14, dmkhn@proton.me wrote:
> On Tue, Jun 24, 2025 at 08:37:22AM +0200, Orzel, Michal wrote:
>>
>>
>> On 24/06/2025 05:55, dmkhn@proton.me wrote:
>>> From: Denis Mukhin <dmukhin@ford.com>
>>>
>>> Rename HWDOM_VUART to HAS_VUART_MMIO.
>>>
>>> This emulator emulates only one register and the use of the emulator is
>>> limited to early boot console in the guest OS.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
>>> ---
>>> xen/arch/arm/Kconfig | 2 +-
>>> xen/arch/arm/Makefile | 2 +-
>>> xen/arch/arm/include/asm/domain.h | 2 +-
>>> xen/arch/arm/vuart.h | 4 ++--
>>> 4 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>>> index 03888569f38c..b11cb583a763 100644
>>> --- a/xen/arch/arm/Kconfig
>>> +++ b/xen/arch/arm/Kconfig
>>> @@ -177,7 +177,7 @@ config HAS_VUART_PL011
>>> Allows a guest to use SBSA Generic UART as a console. The
>>> SBSA Generic UART implements a subset of ARM PL011 UART.
>>>
>>> -config HWDOM_VUART
>>> +config HAS_VUART_MMIO
>> I personally don't like this change. The current config option name reads much
>> better and clearly denotes the purpose.
>
> In my opinion, the MMIO-based UART is a useful debugging tool for early guest
> boot, even when the guest doesn't run in hwdom or on Arm system.
The reason why this vUART is for hwdom is that is uses information from dtuart
(physical UART used by Xen probed from DT). This is to enable kernels used as
dom0 that had early printk/earlycon set for this serial device (as if they run
baremetal). Regular domUs have vPL011 and don't need hwdom vUART.
~Michal
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v1 07/16] arm/vuart: rename 'virtual UART' Kconfig option
2025-06-25 7:07 ` Orzel, Michal
@ 2025-07-31 21:02 ` dmkhn
0 siblings, 0 replies; 59+ messages in thread
From: dmkhn @ 2025-07-31 21:02 UTC (permalink / raw)
To: Orzel, Michal
Cc: xen-devel, andrew.cooper3, anthony.perard, jbeulich, julien,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
Hi Michal,
On Wed, Jun 25, 2025 at 09:07:48AM +0200, Orzel, Michal wrote:
>
>
> On 24/06/2025 09:14, dmkhn@proton.me wrote:
> > On Tue, Jun 24, 2025 at 08:37:22AM +0200, Orzel, Michal wrote:
> >>
> >>
> >> On 24/06/2025 05:55, dmkhn@proton.me wrote:
> >>> From: Denis Mukhin <dmukhin@ford.com>
> >>>
> >>> Rename HWDOM_VUART to HAS_VUART_MMIO.
> >>>
> >>> This emulator emulates only one register and the use of the emulator is
> >>> limited to early boot console in the guest OS.
> >>>
> >>> No functional change.
> >>>
> >>> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> >>> ---
> >>> xen/arch/arm/Kconfig | 2 +-
> >>> xen/arch/arm/Makefile | 2 +-
> >>> xen/arch/arm/include/asm/domain.h | 2 +-
> >>> xen/arch/arm/vuart.h | 4 ++--
> >>> 4 files changed, 5 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> >>> index 03888569f38c..b11cb583a763 100644
> >>> --- a/xen/arch/arm/Kconfig
> >>> +++ b/xen/arch/arm/Kconfig
> >>> @@ -177,7 +177,7 @@ config HAS_VUART_PL011
> >>> Allows a guest to use SBSA Generic UART as a console. The
> >>> SBSA Generic UART implements a subset of ARM PL011 UART.
> >>>
> >>> -config HWDOM_VUART
> >>> +config HAS_VUART_MMIO
> >> I personally don't like this change. The current config option name reads much
> >> better and clearly denotes the purpose.
> >
> > In my opinion, the MMIO-based UART is a useful debugging tool for early guest
> > boot, even when the guest doesn't run in hwdom or on Arm system.
> The reason why this vUART is for hwdom is that is uses information from dtuart
> (physical UART used by Xen probed from DT). This is to enable kernels used as
> dom0 that had early printk/earlycon set for this serial device (as if they run
> baremetal). Regular domUs have vPL011 and don't need hwdom vUART.
OK, I'll keep hwdom, hope there will be no need to change it again.
I think dtuart may be useful for bringing up some exotic OSes which do
not have pl011 driver.
But then, I want to do s/HWDOM_VUART/VUART_HWDOM/g so all vUART build-time settings
have the same naming convention:
VUART_SBSA
VUART_NS16550
Will that be OK with you?
--
Denis
>
> ~Michal
>
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v1 08/16] arm/vuart: move simple MMIO-based vUART declarations to common header
2025-06-24 3:54 [PATCH v1 00/16] xen: framework for UART emulators dmkhn
` (6 preceding siblings ...)
2025-06-24 3:55 ` [PATCH v1 07/16] arm/vuart: rename 'virtual UART' Kconfig option dmkhn
@ 2025-06-24 3:56 ` dmkhn
2025-06-24 3:56 ` [PATCH v1 09/16] arm/vuart: use void pointer in domain struct dmkhn
` (7 subsequent siblings)
15 siblings, 0 replies; 59+ messages in thread
From: dmkhn @ 2025-06-24 3:56 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
From: Denis Mukhin <dmukhin@ford.com>
Merge arch/arm/vuart.h with include/xen/vuart.h.
No functional change.
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
xen/arch/arm/domain.c | 1 -
xen/arch/arm/vuart.c | 3 +--
xen/arch/arm/vuart.h | 54 -----------------------------------------
xen/include/xen/vuart.h | 20 +++++++++++++++
4 files changed, 21 insertions(+), 57 deletions(-)
delete mode 100644 xen/arch/arm/vuart.h
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 68297e619bad..3579d10d7e1d 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -31,7 +31,6 @@
#include <asm/vtimer.h>
#include "vpci.h"
-#include "vuart.h"
DEFINE_PER_CPU(struct vcpu *, curr_vcpu);
diff --git a/xen/arch/arm/vuart.c b/xen/arch/arm/vuart.c
index bd2f425214b7..5403ed284846 100644
--- a/xen/arch/arm/vuart.c
+++ b/xen/arch/arm/vuart.c
@@ -28,8 +28,7 @@
#include <xen/serial.h>
#include <asm/mmio.h>
#include <xen/perfc.h>
-
-#include "vuart.h"
+#include <xen/vuart.h>
#define domain_has_vuart(d) ((d)->arch.vuart.info != NULL)
diff --git a/xen/arch/arm/vuart.h b/xen/arch/arm/vuart.h
deleted file mode 100644
index 726846355c3b..000000000000
--- a/xen/arch/arm/vuart.h
+++ /dev/null
@@ -1,54 +0,0 @@
-/*
- * xen/arch/arm/vuart.h
- *
- * Virtual UART Emulation Support
- *
- * Ian Campbell <ian.campbell@citrix.com>
- * Copyright (c) 2012 Citrix Systems.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- */
-
-#ifndef __ARCH_ARM_VUART_H__
-#define __ARCH_ARM_VUART_H__
-
-struct domain;
-
-#ifdef CONFIG_HAS_VUART_MMIO
-
-int domain_vuart_init(struct domain *d);
-void domain_vuart_free(struct domain *d);
-
-#else
-
-static inline int domain_vuart_init(struct domain *d)
-{
- /*
- * The vUART is unconditionally inialized for the hw domain. So we
- * can't return an error.
- */
- return 0;
-}
-
-static inline void domain_vuart_free(struct domain *d) {};
-
-#endif /* CONFIG_HAS_VUART_MMIO */
-
-#endif /* __ARCH_ARM_VUART_H__ */
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/include/xen/vuart.h b/xen/include/xen/vuart.h
index cae72ac9c6b9..928b60bbb4e2 100644
--- a/xen/include/xen/vuart.h
+++ b/xen/include/xen/vuart.h
@@ -43,6 +43,26 @@ static inline int vuart_putchar(struct domain *d, char c)
#endif /* CONFIG_HAS_VUART_PL011 */
+#ifdef CONFIG_HAS_VUART_MMIO
+
+int domain_vuart_init(struct domain *d);
+void domain_vuart_free(struct domain *d);
+
+#else
+
+static inline int domain_vuart_init(struct domain *d)
+{
+ /*
+ * The vUART is unconditionally inialized for the hw domain. So we
+ * can't return an error.
+ */
+ return 0;
+}
+
+static inline void domain_vuart_free(struct domain *d) {};
+
+#endif /* CONFIG_HAS_VUART_MMIO */
+
#endif /* XEN_VUART_H */
/*
--
2.34.1
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH v1 09/16] arm/vuart: use void pointer in domain struct
2025-06-24 3:54 [PATCH v1 00/16] xen: framework for UART emulators dmkhn
` (7 preceding siblings ...)
2025-06-24 3:56 ` [PATCH v1 08/16] arm/vuart: move simple MMIO-based vUART declarations to common header dmkhn
@ 2025-06-24 3:56 ` dmkhn
2025-06-24 3:56 ` [PATCH v1 10/16] arm/vuart: merge vuart_print_char() with vuart_mmio_write() dmkhn
` (6 subsequent siblings)
15 siblings, 0 replies; 59+ messages in thread
From: dmkhn @ 2025-06-24 3:56 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
From: Denis Mukhin <dmukhin@ford.com>
Make all public data structures private to simple MMIO-based vUART
implementation and switch struct domain to using void pointer to reduce
compile-time dependencies.
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
xen/arch/arm/include/asm/domain.h | 14 ++-----
xen/arch/arm/vuart.c | 68 ++++++++++++++++++++++---------
2 files changed, 52 insertions(+), 30 deletions(-)
diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
index d668c11d7e2c..38873c66f1f8 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -90,16 +90,6 @@ struct arch_domain
struct vgic_dist vgic;
-#ifdef CONFIG_HAS_VUART_MMIO
- struct vuart {
-#define VUART_BUF_SIZE 128
- char *buf;
- int idx;
- const struct vuart_info *info;
- spinlock_t lock;
- } vuart;
-#endif
-
unsigned int evtchn_irq;
#ifdef CONFIG_ACPI
void *efi_acpi_table;
@@ -116,6 +106,10 @@ struct arch_domain
void *vpl011;
#endif
+#ifdef CONFIG_HAS_VUART_MMIO
+ void *vuart;
+#endif
+
#ifdef CONFIG_TEE
void *tee;
#endif
diff --git a/xen/arch/arm/vuart.c b/xen/arch/arm/vuart.c
index 5403ed284846..d2f90ab0c64f 100644
--- a/xen/arch/arm/vuart.c
+++ b/xen/arch/arm/vuart.c
@@ -29,8 +29,16 @@
#include <asm/mmio.h>
#include <xen/perfc.h>
#include <xen/vuart.h>
+#include <xen/xvmalloc.h>
-#define domain_has_vuart(d) ((d)->arch.vuart.info != NULL)
+#define VUART_BUF_SIZE 128
+
+struct vuart {
+ char *buf;
+ int idx;
+ const struct vuart_info *info;
+ spinlock_t lock;
+};
static int vuart_mmio_read(struct vcpu *v, mmio_info_t *info,
register_t *r, void *priv);
@@ -44,39 +52,57 @@ static const struct mmio_handler_ops vuart_mmio_handler = {
int domain_vuart_init(struct domain *d)
{
- ASSERT( is_hardware_domain(d) );
+ const struct vuart_info *info;
+ struct vuart *vdev;
- d->arch.vuart.info = serial_vuart_info(SERHND_DTUART);
- if ( !d->arch.vuart.info )
+ if ( !is_hardware_domain(d) )
return 0;
- spin_lock_init(&d->arch.vuart.lock);
- d->arch.vuart.idx = 0;
+ info = serial_vuart_info(SERHND_DTUART);
+ if ( !info )
+ return 0;
- d->arch.vuart.buf = xzalloc_array(char, VUART_BUF_SIZE);
- if ( !d->arch.vuart.buf )
+ vdev = xvzalloc(typeof(*vdev));
+ if ( !vdev )
return -ENOMEM;
- register_mmio_handler(d, &vuart_mmio_handler,
- d->arch.vuart.info->base_addr,
- d->arch.vuart.info->size,
+ vdev->buf = xzalloc_array(char, VUART_BUF_SIZE);
+ if ( !vdev->buf )
+ {
+ xfree(vdev);
+ return -ENOMEM;
+ }
+
+ spin_lock_init(&vdev->lock);
+
+ register_mmio_handler(d,
+ &vuart_mmio_handler,
+ info->base_addr,
+ info->size,
NULL);
+ vdev->info = info;
+ d->arch.vuart = vdev;
+
return 0;
}
void domain_vuart_free(struct domain *d)
{
- if ( !domain_has_vuart(d) )
- return;
+ struct vuart *vdev = d->arch.vuart;
- xfree(d->arch.vuart.buf);
+ if ( vdev )
+ {
+ xfree(vdev->buf);
+ xfree(vdev);
+ d->arch.vuart = NULL;
+ }
}
static void vuart_print_char(struct vcpu *v, char c)
{
struct domain *d = v->domain;
- struct vuart *uart = &d->arch.vuart;
+ struct vuart *uart = d->arch.vuart;
if ( !is_console_printable(c) )
return ;
@@ -98,16 +124,17 @@ static int vuart_mmio_read(struct vcpu *v, mmio_info_t *info,
register_t *r, void *priv)
{
struct domain *d = v->domain;
- paddr_t offset = info->gpa - d->arch.vuart.info->base_addr;
+ struct vuart *vdev = d->arch.vuart;
+ paddr_t offset = info->gpa - vdev->info->base_addr;
perfc_incr(vuart_reads);
/* By default zeroed the register */
*r = 0;
- if ( offset == d->arch.vuart.info->status_off )
+ if ( offset == vdev->info->status_off )
/* All holding registers empty, ready to send etc */
- *r = d->arch.vuart.info->status;
+ *r = vdev->info->status;
return 1;
}
@@ -116,11 +143,12 @@ static int vuart_mmio_write(struct vcpu *v, mmio_info_t *info,
register_t r, void *priv)
{
struct domain *d = v->domain;
- paddr_t offset = info->gpa - d->arch.vuart.info->base_addr;
+ struct vuart *vdev = d->arch.vuart;
+ paddr_t offset = info->gpa - vdev->info->base_addr;
perfc_incr(vuart_writes);
- if ( offset == d->arch.vuart.info->data_off )
+ if ( offset == vdev->info->data_off )
/* ignore any status bits */
vuart_print_char(v, r & 0xFF);
--
2.34.1
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH v1 10/16] arm/vuart: merge vuart_print_char() with vuart_mmio_write()
2025-06-24 3:54 [PATCH v1 00/16] xen: framework for UART emulators dmkhn
` (8 preceding siblings ...)
2025-06-24 3:56 ` [PATCH v1 09/16] arm/vuart: use void pointer in domain struct dmkhn
@ 2025-06-24 3:56 ` dmkhn
2025-06-24 3:56 ` [PATCH v1 11/16] xen/domain: introduce common emulation flags dmkhn
` (5 subsequent siblings)
15 siblings, 0 replies; 59+ messages in thread
From: dmkhn @ 2025-06-24 3:56 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
From: Denis Mukhin <dmukhin@ford.com>
Simplify code a bit since there's only one user of vuart_print_char().
No functional change.
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
xen/arch/arm/vuart.c | 45 ++++++++++++++++++--------------------------
1 file changed, 18 insertions(+), 27 deletions(-)
diff --git a/xen/arch/arm/vuart.c b/xen/arch/arm/vuart.c
index d2f90ab0c64f..66fac6c994ce 100644
--- a/xen/arch/arm/vuart.c
+++ b/xen/arch/arm/vuart.c
@@ -1,7 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0-or-later */
/*
- * xen/arch/arm/vuart.c
- *
* Virtual UART Emulator.
*
* This emulator uses the information from dtuart. This is not intended to be
@@ -21,6 +19,7 @@
* Ian Campbell <ian.campbell@citrix.com>
* Copyright (c) 2012 Citrix Systems.
*/
+
#include <xen/lib.h>
#include <xen/sched.h>
#include <xen/errno.h>
@@ -99,27 +98,6 @@ void domain_vuart_free(struct domain *d)
}
}
-static void vuart_print_char(struct vcpu *v, char c)
-{
- struct domain *d = v->domain;
- struct vuart *uart = d->arch.vuart;
-
- if ( !is_console_printable(c) )
- return ;
-
- spin_lock(&uart->lock);
- uart->buf[uart->idx++] = c;
- if ( (uart->idx == (VUART_BUF_SIZE - 2)) || (c == '\n') )
- {
- if ( c != '\n' )
- uart->buf[uart->idx++] = '\n';
- uart->buf[uart->idx] = '\0';
- printk(XENLOG_G_DEBUG "DOM%u: %s", d->domain_id, uart->buf);
- uart->idx = 0;
- }
- spin_unlock(&uart->lock);
-}
-
static int vuart_mmio_read(struct vcpu *v, mmio_info_t *info,
register_t *r, void *priv)
{
@@ -145,12 +123,26 @@ static int vuart_mmio_write(struct vcpu *v, mmio_info_t *info,
struct domain *d = v->domain;
struct vuart *vdev = d->arch.vuart;
paddr_t offset = info->gpa - vdev->info->base_addr;
+ char ch = r & 0xFF; /* ignore any status bits */
perfc_incr(vuart_writes);
- if ( offset == vdev->info->data_off )
- /* ignore any status bits */
- vuart_print_char(v, r & 0xFF);
+ if ( !vdev || offset != vdev->info->data_off || !is_console_printable(ch) )
+ return 1;
+
+ spin_lock(&vdev->lock);
+
+ vdev->buf[vdev->idx++] = ch;
+ if ( (vdev->idx == (VUART_BUF_SIZE - 2)) || (ch == '\n') )
+ {
+ if ( ch != '\n' )
+ vdev->buf[vdev->idx++] = '\n';
+ vdev->buf[vdev->idx] = '\0';
+ printk(XENLOG_G_DEBUG "DOM%u: %s", d->domain_id, vdev->buf);
+ vdev->idx = 0;
+ }
+
+ spin_unlock(&vdev->lock);
return 1;
}
@@ -163,4 +155,3 @@ static int vuart_mmio_write(struct vcpu *v, mmio_info_t *info,
* indent-tabs-mode: nil
* End:
*/
-
--
2.34.1
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH v1 11/16] xen/domain: introduce common emulation flags
2025-06-24 3:54 [PATCH v1 00/16] xen: framework for UART emulators dmkhn
` (9 preceding siblings ...)
2025-06-24 3:56 ` [PATCH v1 10/16] arm/vuart: merge vuart_print_char() with vuart_mmio_write() dmkhn
@ 2025-06-24 3:56 ` dmkhn
2025-06-24 3:56 ` [PATCH v1 12/16] xen/domain: introduce domain-emu.h dmkhn
` (4 subsequent siblings)
15 siblings, 0 replies; 59+ messages in thread
From: dmkhn @ 2025-06-24 3:56 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
From: Denis Mukhin <dmukhin@ford.com>
Add common emulation_flags for configuring domain emulation features.
Print d->emulation_flags from 'q' keyhandler for better traceability while
debugging.
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
Original patch: https://lore.kernel.org/r/20250602191717.148361-2-dmukhin@ford.com
---
xen/arch/x86/domain.c | 2 +-
xen/arch/x86/domctl.c | 2 +-
xen/arch/x86/include/asm/domain.h | 25 +++++++++++--------------
xen/common/keyhandler.c | 1 +
xen/include/xen/sched.h | 2 ++
5 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index d025befe3d8e..fc310b1164f4 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -881,7 +881,7 @@ int arch_domain_create(struct domain *d,
emflags);
return -EOPNOTSUPP;
}
- d->arch.emulation_flags = emflags;
+ d->emulation_flags = emflags;
#ifdef CONFIG_PV32
HYPERVISOR_COMPAT_VIRT_START(d) =
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 3044f706de1c..37d848f68339 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -144,7 +144,7 @@ void arch_get_domain_info(const struct domain *d,
if ( paging_mode_hap(d) )
info->flags |= XEN_DOMINF_hap;
- info->arch_config.emulation_flags = d->arch.emulation_flags;
+ info->arch_config.emulation_flags = d->emulation_flags;
info->gpaddr_bits = hap_paddr_bits;
}
diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h
index 8c0dea12a526..eafd5cfc903d 100644
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -455,9 +455,6 @@ struct arch_domain
/* Don't unconditionally inject #GP for unhandled MSRs. */
bool msr_relaxed;
-
- /* Emulated devices enabled bitmap. */
- uint32_t emulation_flags;
} __cacheline_aligned;
#ifdef CONFIG_HVM
@@ -494,17 +491,17 @@ struct arch_domain
X86_EMU_PIT | X86_EMU_USE_PIRQ | \
X86_EMU_VPCI)
-#define has_vlapic(d) (!!((d)->arch.emulation_flags & X86_EMU_LAPIC))
-#define has_vhpet(d) (!!((d)->arch.emulation_flags & X86_EMU_HPET))
-#define has_vpm(d) (!!((d)->arch.emulation_flags & X86_EMU_PM))
-#define has_vrtc(d) (!!((d)->arch.emulation_flags & X86_EMU_RTC))
-#define has_vioapic(d) (!!((d)->arch.emulation_flags & X86_EMU_IOAPIC))
-#define has_vpic(d) (!!((d)->arch.emulation_flags & X86_EMU_PIC))
-#define has_vvga(d) (!!((d)->arch.emulation_flags & X86_EMU_VGA))
-#define has_viommu(d) (!!((d)->arch.emulation_flags & X86_EMU_IOMMU))
-#define has_vpit(d) (!!((d)->arch.emulation_flags & X86_EMU_PIT))
-#define has_pirq(d) (!!((d)->arch.emulation_flags & X86_EMU_USE_PIRQ))
-#define has_vpci(d) (!!((d)->arch.emulation_flags & X86_EMU_VPCI))
+#define has_vlapic(d) (!!((d)->emulation_flags & X86_EMU_LAPIC))
+#define has_vhpet(d) (!!((d)->emulation_flags & X86_EMU_HPET))
+#define has_vpm(d) (!!((d)->emulation_flags & X86_EMU_PM))
+#define has_vrtc(d) (!!((d)->emulation_flags & X86_EMU_RTC))
+#define has_vioapic(d) (!!((d)->emulation_flags & X86_EMU_IOAPIC))
+#define has_vpic(d) (!!((d)->emulation_flags & X86_EMU_PIC))
+#define has_vvga(d) (!!((d)->emulation_flags & X86_EMU_VGA))
+#define has_viommu(d) (!!((d)->emulation_flags & X86_EMU_IOMMU))
+#define has_vpit(d) (!!((d)->emulation_flags & X86_EMU_PIT))
+#define has_pirq(d) (!!((d)->emulation_flags & X86_EMU_USE_PIRQ))
+#define has_vpci(d) (!!((d)->emulation_flags & X86_EMU_VPCI))
#define gdt_ldt_pt_idx(v) \
((v)->vcpu_id >> (PAGETABLE_ORDER - GDT_LDT_VCPU_SHIFT))
diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index b0a2051408d5..eccd97c565c6 100644
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -306,6 +306,7 @@ static void cf_check dump_domains(unsigned char key)
if ( test_bit(i, &d->watchdog_inuse_map) )
printk(" watchdog %d expires in %d seconds\n",
i, (u32)((d->watchdog_timer[i].expires - NOW()) >> 30));
+ printk(" emulation_flags %#x\n", d->emulation_flags);
arch_dump_domain_info(d);
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index fe53d4fab7ba..8b16efe6afd8 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -652,6 +652,8 @@ struct domain
const unsigned int *llc_colors;
#endif
+ uint32_t emulation_flags;
+
/* Console settings. */
struct {
/* Permission to take ownership of the physical console input. */
--
2.34.1
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH v1 12/16] xen/domain: introduce domain-emu.h
2025-06-24 3:54 [PATCH v1 00/16] xen: framework for UART emulators dmkhn
` (10 preceding siblings ...)
2025-06-24 3:56 ` [PATCH v1 11/16] xen/domain: introduce common emulation flags dmkhn
@ 2025-06-24 3:56 ` dmkhn
2025-07-09 14:57 ` Jan Beulich
2025-06-24 3:56 ` [PATCH v1 13/16] drivers/vuart: move PL011 emulator code dmkhn
` (3 subsequent siblings)
15 siblings, 1 reply; 59+ messages in thread
From: dmkhn @ 2025-06-24 3:56 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
From: Denis Mukhin <dmukhin@ford.com>
Define an architecture-independent location for describing domain emulation
flags.
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Original code: https://lore.kernel.org/xen-devel/20250103-vuart-ns8250-v3-v1-6-c5d36b31d66c@ford.com/
---
xen/arch/x86/include/asm/domain.h | 23 ++++++++++++-----------
xen/include/xen/domain-emu.h | 30 ++++++++++++++++++++++++++++++
2 files changed, 42 insertions(+), 11 deletions(-)
create mode 100644 xen/include/xen/domain-emu.h
diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h
index eafd5cfc903d..c6bc83b4abe0 100644
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -3,6 +3,7 @@
#include <xen/mm.h>
#include <xen/radix-tree.h>
+#include <xen/domain-emu.h>
#include <asm/hvm/vcpu.h>
#include <asm/hvm/domain.h>
#include <asm/e820.h>
@@ -458,16 +459,16 @@ struct arch_domain
} __cacheline_aligned;
#ifdef CONFIG_HVM
-#define X86_EMU_LAPIC XEN_X86_EMU_LAPIC
-#define X86_EMU_HPET XEN_X86_EMU_HPET
-#define X86_EMU_PM XEN_X86_EMU_PM
-#define X86_EMU_RTC XEN_X86_EMU_RTC
-#define X86_EMU_IOAPIC XEN_X86_EMU_IOAPIC
-#define X86_EMU_PIC XEN_X86_EMU_PIC
-#define X86_EMU_VGA XEN_X86_EMU_VGA
-#define X86_EMU_IOMMU XEN_X86_EMU_IOMMU
-#define X86_EMU_USE_PIRQ XEN_X86_EMU_USE_PIRQ
-#define X86_EMU_VPCI XEN_X86_EMU_VPCI
+#define X86_EMU_LAPIC DOMAIN_EMU_LAPIC
+#define X86_EMU_HPET DOMAIN_EMU_HPET
+#define X86_EMU_PM DOMAIN_EMU_PM
+#define X86_EMU_RTC DOMAIN_EMU_RTC
+#define X86_EMU_IOAPIC DOMAIN_EMU_IOAPIC
+#define X86_EMU_PIC DOMAIN_EMU_PIC
+#define X86_EMU_VGA DOMAIN_EMU_VGA
+#define X86_EMU_IOMMU DOMAIN_EMU_IOMMU
+#define X86_EMU_USE_PIRQ DOMAIN_EMU_PIRQ
+#define X86_EMU_VPCI DOMAIN_EMU_PCI
#else
#define X86_EMU_LAPIC 0
#define X86_EMU_HPET 0
@@ -481,7 +482,7 @@ struct arch_domain
#define X86_EMU_VPCI 0
#endif
-#define X86_EMU_PIT XEN_X86_EMU_PIT
+#define X86_EMU_PIT DOMAIN_EMU_PIT
/* This must match XEN_X86_EMU_ALL in xen.h */
#define X86_EMU_ALL (X86_EMU_LAPIC | X86_EMU_HPET | \
diff --git a/xen/include/xen/domain-emu.h b/xen/include/xen/domain-emu.h
new file mode 100644
index 000000000000..410963ff480c
--- /dev/null
+++ b/xen/include/xen/domain-emu.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef XEN_DOMAIN_EMU_H
+#define XEN_DOMAIN_EMU_H
+
+/*
+ * Domain emulation flags.
+ */
+#define DOMAIN_EMU_LAPIC (1U << 0)
+#define DOMAIN_EMU_HPET (1U << 1)
+#define DOMAIN_EMU_PM (1U << 2)
+#define DOMAIN_EMU_RTC (1U << 3)
+#define DOMAIN_EMU_IOAPIC (1U << 4)
+#define DOMAIN_EMU_PIC (1U << 5)
+#define DOMAIN_EMU_VGA (1U << 6)
+#define DOMAIN_EMU_IOMMU (1U << 7)
+#define DOMAIN_EMU_PIT (1U << 8)
+#define DOMAIN_EMU_PIRQ (1U << 9)
+#define DOMAIN_EMU_PCI (1U << 10)
+
+#endif /* XEN_DOMAIN_EMU_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.34.1
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH v1 12/16] xen/domain: introduce domain-emu.h
2025-06-24 3:56 ` [PATCH v1 12/16] xen/domain: introduce domain-emu.h dmkhn
@ 2025-07-09 14:57 ` Jan Beulich
2025-07-31 20:55 ` dmkhn
0 siblings, 1 reply; 59+ messages in thread
From: Jan Beulich @ 2025-07-09 14:57 UTC (permalink / raw)
To: dmkhn
Cc: andrew.cooper3, anthony.perard, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
On 24.06.2025 05:56, dmkhn@proton.me wrote:
> @@ -458,16 +459,16 @@ struct arch_domain
> } __cacheline_aligned;
>
> #ifdef CONFIG_HVM
> -#define X86_EMU_LAPIC XEN_X86_EMU_LAPIC
> -#define X86_EMU_HPET XEN_X86_EMU_HPET
> -#define X86_EMU_PM XEN_X86_EMU_PM
> -#define X86_EMU_RTC XEN_X86_EMU_RTC
> -#define X86_EMU_IOAPIC XEN_X86_EMU_IOAPIC
> -#define X86_EMU_PIC XEN_X86_EMU_PIC
> -#define X86_EMU_VGA XEN_X86_EMU_VGA
> -#define X86_EMU_IOMMU XEN_X86_EMU_IOMMU
> -#define X86_EMU_USE_PIRQ XEN_X86_EMU_USE_PIRQ
> -#define X86_EMU_VPCI XEN_X86_EMU_VPCI
The old code deliberately used values from the public interface.
> --- /dev/null
> +++ b/xen/include/xen/domain-emu.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef XEN_DOMAIN_EMU_H
> +#define XEN_DOMAIN_EMU_H
> +
> +/*
> + * Domain emulation flags.
> + */
> +#define DOMAIN_EMU_LAPIC (1U << 0)
> +#define DOMAIN_EMU_HPET (1U << 1)
> +#define DOMAIN_EMU_PM (1U << 2)
> +#define DOMAIN_EMU_RTC (1U << 3)
> +#define DOMAIN_EMU_IOAPIC (1U << 4)
> +#define DOMAIN_EMU_PIC (1U << 5)
> +#define DOMAIN_EMU_VGA (1U << 6)
> +#define DOMAIN_EMU_IOMMU (1U << 7)
> +#define DOMAIN_EMU_PIT (1U << 8)
> +#define DOMAIN_EMU_PIRQ (1U << 9)
> +#define DOMAIN_EMU_PCI (1U << 10)
There's nothing in the new code making sure that the values won't go out
of sync.
Jan
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v1 12/16] xen/domain: introduce domain-emu.h
2025-07-09 14:57 ` Jan Beulich
@ 2025-07-31 20:55 ` dmkhn
2025-08-01 6:02 ` Jan Beulich
0 siblings, 1 reply; 59+ messages in thread
From: dmkhn @ 2025-07-31 20:55 UTC (permalink / raw)
To: Jan Beulich
Cc: andrew.cooper3, anthony.perard, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
Hi Jan,
On Wed, Jul 09, 2025 at 04:57:44PM +0200, Jan Beulich wrote:
> On 24.06.2025 05:56, dmkhn@proton.me wrote:
> > @@ -458,16 +459,16 @@ struct arch_domain
> > } __cacheline_aligned;
> >
> > #ifdef CONFIG_HVM
> > -#define X86_EMU_LAPIC XEN_X86_EMU_LAPIC
> > -#define X86_EMU_HPET XEN_X86_EMU_HPET
> > -#define X86_EMU_PM XEN_X86_EMU_PM
> > -#define X86_EMU_RTC XEN_X86_EMU_RTC
> > -#define X86_EMU_IOAPIC XEN_X86_EMU_IOAPIC
> > -#define X86_EMU_PIC XEN_X86_EMU_PIC
> > -#define X86_EMU_VGA XEN_X86_EMU_VGA
> > -#define X86_EMU_IOMMU XEN_X86_EMU_IOMMU
> > -#define X86_EMU_USE_PIRQ XEN_X86_EMU_USE_PIRQ
> > -#define X86_EMU_VPCI XEN_X86_EMU_VPCI
>
> The old code deliberately used values from the public interface.
In next version I am building, I moved all of XEN_X86_EMU_XXX definitions as
is to a new public header under include/public/xen-emu.h:
https://gitlab.com/xen-project/people/dmukhin/xen/-/commit/9b0bc5ffa5710114df8523ae2aa7680b7c6f0942
That looks less invasive.
Will that work?
There should be a common header with emulation flags somewhere, since
there will be SBSA and hwdom vUART definitions there.
--
Denis
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v1 12/16] xen/domain: introduce domain-emu.h
2025-07-31 20:55 ` dmkhn
@ 2025-08-01 6:02 ` Jan Beulich
2025-08-01 19:30 ` dmkhn
0 siblings, 1 reply; 59+ messages in thread
From: Jan Beulich @ 2025-08-01 6:02 UTC (permalink / raw)
To: dmkhn
Cc: andrew.cooper3, anthony.perard, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
On 31.07.2025 22:55, dmkhn@proton.me wrote:
> On Wed, Jul 09, 2025 at 04:57:44PM +0200, Jan Beulich wrote:
>> On 24.06.2025 05:56, dmkhn@proton.me wrote:
>>> @@ -458,16 +459,16 @@ struct arch_domain
>>> } __cacheline_aligned;
>>>
>>> #ifdef CONFIG_HVM
>>> -#define X86_EMU_LAPIC XEN_X86_EMU_LAPIC
>>> -#define X86_EMU_HPET XEN_X86_EMU_HPET
>>> -#define X86_EMU_PM XEN_X86_EMU_PM
>>> -#define X86_EMU_RTC XEN_X86_EMU_RTC
>>> -#define X86_EMU_IOAPIC XEN_X86_EMU_IOAPIC
>>> -#define X86_EMU_PIC XEN_X86_EMU_PIC
>>> -#define X86_EMU_VGA XEN_X86_EMU_VGA
>>> -#define X86_EMU_IOMMU XEN_X86_EMU_IOMMU
>>> -#define X86_EMU_USE_PIRQ XEN_X86_EMU_USE_PIRQ
>>> -#define X86_EMU_VPCI XEN_X86_EMU_VPCI
>>
>> The old code deliberately used values from the public interface.
>
> In next version I am building, I moved all of XEN_X86_EMU_XXX definitions as
> is to a new public header under include/public/xen-emu.h:
>
> https://gitlab.com/xen-project/people/dmukhin/xen/-/commit/9b0bc5ffa5710114df8523ae2aa7680b7c6f0942
>
> That looks less invasive.
>
> Will that work?
>
> There should be a common header with emulation flags somewhere, since
> there will be SBSA and hwdom vUART definitions there.
Yet will there be a strict need for any constants to be identical (i.e.
not only have the same name, but also the same value) across architectures?
Jan
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v1 12/16] xen/domain: introduce domain-emu.h
2025-08-01 6:02 ` Jan Beulich
@ 2025-08-01 19:30 ` dmkhn
2025-08-04 7:13 ` Jan Beulich
0 siblings, 1 reply; 59+ messages in thread
From: dmkhn @ 2025-08-01 19:30 UTC (permalink / raw)
To: Jan Beulich
Cc: andrew.cooper3, anthony.perard, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
On Fri, Aug 01, 2025 at 08:02:56AM +0200, Jan Beulich wrote:
> On 31.07.2025 22:55, dmkhn@proton.me wrote:
> > On Wed, Jul 09, 2025 at 04:57:44PM +0200, Jan Beulich wrote:
> >> On 24.06.2025 05:56, dmkhn@proton.me wrote:
> >>> @@ -458,16 +459,16 @@ struct arch_domain
> >>> } __cacheline_aligned;
> >>>
> >>> #ifdef CONFIG_HVM
> >>> -#define X86_EMU_LAPIC XEN_X86_EMU_LAPIC
> >>> -#define X86_EMU_HPET XEN_X86_EMU_HPET
> >>> -#define X86_EMU_PM XEN_X86_EMU_PM
> >>> -#define X86_EMU_RTC XEN_X86_EMU_RTC
> >>> -#define X86_EMU_IOAPIC XEN_X86_EMU_IOAPIC
> >>> -#define X86_EMU_PIC XEN_X86_EMU_PIC
> >>> -#define X86_EMU_VGA XEN_X86_EMU_VGA
> >>> -#define X86_EMU_IOMMU XEN_X86_EMU_IOMMU
> >>> -#define X86_EMU_USE_PIRQ XEN_X86_EMU_USE_PIRQ
> >>> -#define X86_EMU_VPCI XEN_X86_EMU_VPCI
> >>
> >> The old code deliberately used values from the public interface.
> >
> > In next version I am building, I moved all of XEN_X86_EMU_XXX definitions as
> > is to a new public header under include/public/xen-emu.h:
> >
> > https://gitlab.com/xen-project/people/dmukhin/xen/-/commit/9b0bc5ffa5710114df8523ae2aa7680b7c6f0942
> >
> > That looks less invasive.
> >
> > Will that work?
> >
> > There should be a common header with emulation flags somewhere, since
> > there will be SBSA and hwdom vUART definitions there.
>
> Yet will there be a strict need for any constants to be identical (i.e.
> not only have the same name, but also the same value) across architectures?
I don't think there's strict need for identical values across achitectures.
But some of the constants _may_ be reused for non-x86 arches, like VPCI bit
and, perhaps, IOMMU, PIRQ and future NS16550 (after adding MMIO).
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v1 12/16] xen/domain: introduce domain-emu.h
2025-08-01 19:30 ` dmkhn
@ 2025-08-04 7:13 ` Jan Beulich
2025-08-05 0:24 ` dmkhn
0 siblings, 1 reply; 59+ messages in thread
From: Jan Beulich @ 2025-08-04 7:13 UTC (permalink / raw)
To: dmkhn
Cc: andrew.cooper3, anthony.perard, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
On 01.08.2025 21:30, dmkhn@proton.me wrote:
> On Fri, Aug 01, 2025 at 08:02:56AM +0200, Jan Beulich wrote:
>> On 31.07.2025 22:55, dmkhn@proton.me wrote:
>>> On Wed, Jul 09, 2025 at 04:57:44PM +0200, Jan Beulich wrote:
>>>> On 24.06.2025 05:56, dmkhn@proton.me wrote:
>>>>> @@ -458,16 +459,16 @@ struct arch_domain
>>>>> } __cacheline_aligned;
>>>>>
>>>>> #ifdef CONFIG_HVM
>>>>> -#define X86_EMU_LAPIC XEN_X86_EMU_LAPIC
>>>>> -#define X86_EMU_HPET XEN_X86_EMU_HPET
>>>>> -#define X86_EMU_PM XEN_X86_EMU_PM
>>>>> -#define X86_EMU_RTC XEN_X86_EMU_RTC
>>>>> -#define X86_EMU_IOAPIC XEN_X86_EMU_IOAPIC
>>>>> -#define X86_EMU_PIC XEN_X86_EMU_PIC
>>>>> -#define X86_EMU_VGA XEN_X86_EMU_VGA
>>>>> -#define X86_EMU_IOMMU XEN_X86_EMU_IOMMU
>>>>> -#define X86_EMU_USE_PIRQ XEN_X86_EMU_USE_PIRQ
>>>>> -#define X86_EMU_VPCI XEN_X86_EMU_VPCI
>>>>
>>>> The old code deliberately used values from the public interface.
>>>
>>> In next version I am building, I moved all of XEN_X86_EMU_XXX definitions as
>>> is to a new public header under include/public/xen-emu.h:
>>>
>>> https://gitlab.com/xen-project/people/dmukhin/xen/-/commit/9b0bc5ffa5710114df8523ae2aa7680b7c6f0942
>>>
>>> That looks less invasive.
>>>
>>> Will that work?
>>>
>>> There should be a common header with emulation flags somewhere, since
>>> there will be SBSA and hwdom vUART definitions there.
>>
>> Yet will there be a strict need for any constants to be identical (i.e.
>> not only have the same name, but also the same value) across architectures?
>
> I don't think there's strict need for identical values across achitectures.
That's what I was expecting.
> But some of the constants _may_ be reused for non-x86 arches, like VPCI bit
> and, perhaps, IOMMU, PIRQ and future NS16550 (after adding MMIO).
Right, but as you say - they want to use the same name, but they could easily
have a different value there. I hope you understand that what I'm questioning
is the introduction of a single header covering all architectures.
Jan
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v1 12/16] xen/domain: introduce domain-emu.h
2025-08-04 7:13 ` Jan Beulich
@ 2025-08-05 0:24 ` dmkhn
0 siblings, 0 replies; 59+ messages in thread
From: dmkhn @ 2025-08-05 0:24 UTC (permalink / raw)
To: Jan Beulich
Cc: andrew.cooper3, anthony.perard, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
On Mon, Aug 04, 2025 at 09:13:07AM +0200, Jan Beulich wrote:
> On 01.08.2025 21:30, dmkhn@proton.me wrote:
> > On Fri, Aug 01, 2025 at 08:02:56AM +0200, Jan Beulich wrote:
> >> On 31.07.2025 22:55, dmkhn@proton.me wrote:
> >>> On Wed, Jul 09, 2025 at 04:57:44PM +0200, Jan Beulich wrote:
> >>>> On 24.06.2025 05:56, dmkhn@proton.me wrote:
> >>>>> @@ -458,16 +459,16 @@ struct arch_domain
> >>>>> } __cacheline_aligned;
> >>>>>
> >>>>> #ifdef CONFIG_HVM
> >>>>> -#define X86_EMU_LAPIC XEN_X86_EMU_LAPIC
> >>>>> -#define X86_EMU_HPET XEN_X86_EMU_HPET
> >>>>> -#define X86_EMU_PM XEN_X86_EMU_PM
> >>>>> -#define X86_EMU_RTC XEN_X86_EMU_RTC
> >>>>> -#define X86_EMU_IOAPIC XEN_X86_EMU_IOAPIC
> >>>>> -#define X86_EMU_PIC XEN_X86_EMU_PIC
> >>>>> -#define X86_EMU_VGA XEN_X86_EMU_VGA
> >>>>> -#define X86_EMU_IOMMU XEN_X86_EMU_IOMMU
> >>>>> -#define X86_EMU_USE_PIRQ XEN_X86_EMU_USE_PIRQ
> >>>>> -#define X86_EMU_VPCI XEN_X86_EMU_VPCI
> >>>>
> >>>> The old code deliberately used values from the public interface.
> >>>
> >>> In next version I am building, I moved all of XEN_X86_EMU_XXX definitions as
> >>> is to a new public header under include/public/xen-emu.h:
> >>>
> >>> https://gitlab.com/xen-project/people/dmukhin/xen/-/commit/9b0bc5ffa5710114df8523ae2aa7680b7c6f0942
> >>>
> >>> That looks less invasive.
> >>>
> >>> Will that work?
> >>>
> >>> There should be a common header with emulation flags somewhere, since
> >>> there will be SBSA and hwdom vUART definitions there.
> >>
> >> Yet will there be a strict need for any constants to be identical (i.e.
> >> not only have the same name, but also the same value) across architectures?
> >
> > I don't think there's strict need for identical values across achitectures.
>
> That's what I was expecting.
>
> > But some of the constants _may_ be reused for non-x86 arches, like VPCI bit
> > and, perhaps, IOMMU, PIRQ and future NS16550 (after adding MMIO).
>
> Right, but as you say - they want to use the same name, but they could easily
> have a different value there. I hope you understand that what I'm questioning
> is the introduction of a single header covering all architectures.
Yes, I understand your point wrt common header and identical values in
emulation flags.
I think I can add missing per-vUART fields to per-arch xen_arch_domainconfig,
populate them by the toolstack and be done for now, i.e. I can drop that
patch.
--
Denis
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v1 13/16] drivers/vuart: move PL011 emulator code
2025-06-24 3:54 [PATCH v1 00/16] xen: framework for UART emulators dmkhn
` (11 preceding siblings ...)
2025-06-24 3:56 ` [PATCH v1 12/16] xen/domain: introduce domain-emu.h dmkhn
@ 2025-06-24 3:56 ` dmkhn
2025-06-24 5:50 ` Jan Beulich
2025-06-24 3:57 ` [PATCH v1 14/16] drivers/vuart: move simple MMIO-based UART emulator dmkhn
` (2 subsequent siblings)
15 siblings, 1 reply; 59+ messages in thread
From: dmkhn @ 2025-06-24 3:56 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
From: Denis Mukhin <dmukhin@ford.com>
Move PL011 emulator to the new location for UART emulators.
No functional change intended.
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
xen/arch/arm/Kconfig | 7 -------
xen/arch/arm/Makefile | 1 -
xen/drivers/Kconfig | 2 ++
xen/drivers/Makefile | 1 +
xen/drivers/vuart/Kconfig | 14 ++++++++++++++
xen/drivers/vuart/Makefile | 1 +
.../arm/vpl011.c => drivers/vuart/vuart-pl011.c} | 0
7 files changed, 18 insertions(+), 8 deletions(-)
create mode 100644 xen/drivers/vuart/Kconfig
create mode 100644 xen/drivers/vuart/Makefile
rename xen/{arch/arm/vpl011.c => drivers/vuart/vuart-pl011.c} (100%)
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index b11cb583a763..6eeae97293f2 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -170,13 +170,6 @@ config NEW_VGIC
problems with the standard emulation.
At the moment this implementation is not security supported.
-config HAS_VUART_PL011
- bool "Emulated SBSA UART console support"
- default y
- help
- Allows a guest to use SBSA Generic UART as a console. The
- SBSA Generic UART implements a subset of ARM PL011 UART.
-
config HAS_VUART_MMIO
bool "Emulated UART for hardware domain"
default y
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index dd015a2a19e8..8475043d8701 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -65,7 +65,6 @@ obj-$(CONFIG_HAS_ITS) += vgic-v3-its.o
endif
obj-$(CONFIG_VM_EVENT) += vm_event.o
obj-y += vtimer.o
-obj-$(CONFIG_HAS_VUART_PL011) += vpl011.o
obj-y += vsmc.o
obj-y += vpsci.o
obj-$(CONFIG_HAS_VUART_MMIO) += vuart.o
diff --git a/xen/drivers/Kconfig b/xen/drivers/Kconfig
index 20050e9bb8b3..5e533b260004 100644
--- a/xen/drivers/Kconfig
+++ b/xen/drivers/Kconfig
@@ -12,6 +12,8 @@ source "drivers/pci/Kconfig"
source "drivers/video/Kconfig"
+source "drivers/vuart/Kconfig"
+
config HAS_VPCI
bool
diff --git a/xen/drivers/Makefile b/xen/drivers/Makefile
index 2a1ae8ad130a..510820472938 100644
--- a/xen/drivers/Makefile
+++ b/xen/drivers/Makefile
@@ -2,6 +2,7 @@ obj-y += char/
obj-$(CONFIG_HAS_CPUFREQ) += cpufreq/
obj-$(CONFIG_HAS_PCI) += pci/
obj-$(CONFIG_HAS_VPCI) += vpci/
+obj-$(CONFIG_HAS_VUART) += vuart/
obj-$(CONFIG_HAS_PASSTHROUGH) += passthrough/
obj-$(CONFIG_ACPI) += acpi/
obj-$(CONFIG_VIDEO) += video/
diff --git a/xen/drivers/vuart/Kconfig b/xen/drivers/vuart/Kconfig
new file mode 100644
index 000000000000..d8df0f6d1b3c
--- /dev/null
+++ b/xen/drivers/vuart/Kconfig
@@ -0,0 +1,14 @@
+config HAS_VUART
+ bool
+
+if (ARM_32 || ARM_64)
+
+config HAS_VUART_PL011
+ bool "Emulated PL011 UART support"
+ default y
+ select HAS_VUART
+ help
+ Allows a guest to use SBSA Generic UART as a console. The
+ SBSA Generic UART implements a subset of ARM PL011 UART.
+
+endif
diff --git a/xen/drivers/vuart/Makefile b/xen/drivers/vuart/Makefile
new file mode 100644
index 000000000000..3b7069f1cf95
--- /dev/null
+++ b/xen/drivers/vuart/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_HAS_VUART_PL011) += vuart-pl011.o
diff --git a/xen/arch/arm/vpl011.c b/xen/drivers/vuart/vuart-pl011.c
similarity index 100%
rename from xen/arch/arm/vpl011.c
rename to xen/drivers/vuart/vuart-pl011.c
--
2.34.1
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH v1 13/16] drivers/vuart: move PL011 emulator code
2025-06-24 3:56 ` [PATCH v1 13/16] drivers/vuart: move PL011 emulator code dmkhn
@ 2025-06-24 5:50 ` Jan Beulich
2025-06-24 7:31 ` dmkhn
0 siblings, 1 reply; 59+ messages in thread
From: Jan Beulich @ 2025-06-24 5:50 UTC (permalink / raw)
To: dmkhn
Cc: andrew.cooper3, anthony.perard, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
On 24.06.2025 05:56, dmkhn@proton.me wrote:
> From: Denis Mukhin <dmukhin@ford.com>
>
> Move PL011 emulator to the new location for UART emulators.
>
> No functional change intended.
>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
> xen/arch/arm/Kconfig | 7 -------
> xen/arch/arm/Makefile | 1 -
> xen/drivers/Kconfig | 2 ++
> xen/drivers/Makefile | 1 +
> xen/drivers/vuart/Kconfig | 14 ++++++++++++++
> xen/drivers/vuart/Makefile | 1 +
> .../arm/vpl011.c => drivers/vuart/vuart-pl011.c} | 0
> 7 files changed, 18 insertions(+), 8 deletions(-)
> create mode 100644 xen/drivers/vuart/Kconfig
> create mode 100644 xen/drivers/vuart/Makefile
> rename xen/{arch/arm/vpl011.c => drivers/vuart/vuart-pl011.c} (100%)
I question the placement under drivers/. To me, driver != emulator. I
wonder what others think. But yes, we already have drivers/vpci/. That
may want moving then ...
Jan
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [PATCH v1 13/16] drivers/vuart: move PL011 emulator code
2025-06-24 5:50 ` Jan Beulich
@ 2025-06-24 7:31 ` dmkhn
2025-06-24 7:33 ` Jan Beulich
0 siblings, 1 reply; 59+ messages in thread
From: dmkhn @ 2025-06-24 7:31 UTC (permalink / raw)
To: Jan Beulich
Cc: andrew.cooper3, anthony.perard, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
On Tue, Jun 24, 2025 at 07:50:33AM +0200, Jan Beulich wrote:
> On 24.06.2025 05:56, dmkhn@proton.me wrote:
> > From: Denis Mukhin <dmukhin@ford.com>
> >
> > Move PL011 emulator to the new location for UART emulators.
> >
> > No functional change intended.
> >
> > Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> > ---
> > xen/arch/arm/Kconfig | 7 -------
> > xen/arch/arm/Makefile | 1 -
> > xen/drivers/Kconfig | 2 ++
> > xen/drivers/Makefile | 1 +
> > xen/drivers/vuart/Kconfig | 14 ++++++++++++++
> > xen/drivers/vuart/Makefile | 1 +
> > .../arm/vpl011.c => drivers/vuart/vuart-pl011.c} | 0
> > 7 files changed, 18 insertions(+), 8 deletions(-)
> > create mode 100644 xen/drivers/vuart/Kconfig
> > create mode 100644 xen/drivers/vuart/Makefile
> > rename xen/{arch/arm/vpl011.c => drivers/vuart/vuart-pl011.c} (100%)
>
> I question the placement under drivers/. To me, driver != emulator. I
> wonder what others think. But yes, we already have drivers/vpci/. That
> may want moving then ...
re: driver != emulator: I agree; but I followed drivers/vpci.
Do you think common/vuart would be a better location?
>
> Jan
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [PATCH v1 13/16] drivers/vuart: move PL011 emulator code
2025-06-24 7:31 ` dmkhn
@ 2025-06-24 7:33 ` Jan Beulich
2025-07-10 1:59 ` dmkhn
0 siblings, 1 reply; 59+ messages in thread
From: Jan Beulich @ 2025-06-24 7:33 UTC (permalink / raw)
To: dmkhn
Cc: andrew.cooper3, anthony.perard, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
On 24.06.2025 09:31, dmkhn@proton.me wrote:
> On Tue, Jun 24, 2025 at 07:50:33AM +0200, Jan Beulich wrote:
>> On 24.06.2025 05:56, dmkhn@proton.me wrote:
>>> From: Denis Mukhin <dmukhin@ford.com>
>>>
>>> Move PL011 emulator to the new location for UART emulators.
>>>
>>> No functional change intended.
>>>
>>> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
>>> ---
>>> xen/arch/arm/Kconfig | 7 -------
>>> xen/arch/arm/Makefile | 1 -
>>> xen/drivers/Kconfig | 2 ++
>>> xen/drivers/Makefile | 1 +
>>> xen/drivers/vuart/Kconfig | 14 ++++++++++++++
>>> xen/drivers/vuart/Makefile | 1 +
>>> .../arm/vpl011.c => drivers/vuart/vuart-pl011.c} | 0
>>> 7 files changed, 18 insertions(+), 8 deletions(-)
>>> create mode 100644 xen/drivers/vuart/Kconfig
>>> create mode 100644 xen/drivers/vuart/Makefile
>>> rename xen/{arch/arm/vpl011.c => drivers/vuart/vuart-pl011.c} (100%)
>>
>> I question the placement under drivers/. To me, driver != emulator. I
>> wonder what others think. But yes, we already have drivers/vpci/. That
>> may want moving then ...
>
> re: driver != emulator: I agree; but I followed drivers/vpci.
>
> Do you think common/vuart would be a better location?
Or maybe common/emul/... This wants discussing, I think.
Jan
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [PATCH v1 13/16] drivers/vuart: move PL011 emulator code
2025-06-24 7:33 ` Jan Beulich
@ 2025-07-10 1:59 ` dmkhn
2025-07-10 8:15 ` Jan Beulich
0 siblings, 1 reply; 59+ messages in thread
From: dmkhn @ 2025-07-10 1:59 UTC (permalink / raw)
To: Jan Beulich
Cc: andrew.cooper3, anthony.perard, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
On Tue, Jun 24, 2025 at 09:33:04AM +0200, Jan Beulich wrote:
> On 24.06.2025 09:31, dmkhn@proton.me wrote:
> > On Tue, Jun 24, 2025 at 07:50:33AM +0200, Jan Beulich wrote:
> >> On 24.06.2025 05:56, dmkhn@proton.me wrote:
> >>> From: Denis Mukhin <dmukhin@ford.com>
> >>>
> >>> Move PL011 emulator to the new location for UART emulators.
> >>>
> >>> No functional change intended.
> >>>
> >>> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> >>> ---
> >>> xen/arch/arm/Kconfig | 7 -------
> >>> xen/arch/arm/Makefile | 1 -
> >>> xen/drivers/Kconfig | 2 ++
> >>> xen/drivers/Makefile | 1 +
> >>> xen/drivers/vuart/Kconfig | 14 ++++++++++++++
> >>> xen/drivers/vuart/Makefile | 1 +
> >>> .../arm/vpl011.c => drivers/vuart/vuart-pl011.c} | 0
> >>> 7 files changed, 18 insertions(+), 8 deletions(-)
> >>> create mode 100644 xen/drivers/vuart/Kconfig
> >>> create mode 100644 xen/drivers/vuart/Makefile
> >>> rename xen/{arch/arm/vpl011.c => drivers/vuart/vuart-pl011.c} (100%)
> >>
> >> I question the placement under drivers/. To me, driver != emulator. I
> >> wonder what others think. But yes, we already have drivers/vpci/. That
> >> may want moving then ...
> >
> > re: driver != emulator: I agree; but I followed drivers/vpci.
> >
> > Do you think common/vuart would be a better location?
>
> Or maybe common/emul/... This wants discussing, I think.
Will something like the following work
common/hvm/vuart
?
>
> Jan
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [PATCH v1 13/16] drivers/vuart: move PL011 emulator code
2025-07-10 1:59 ` dmkhn
@ 2025-07-10 8:15 ` Jan Beulich
2025-07-10 17:00 ` Stefano Stabellini
2025-07-10 18:32 ` dmkhn
0 siblings, 2 replies; 59+ messages in thread
From: Jan Beulich @ 2025-07-10 8:15 UTC (permalink / raw)
To: dmkhn
Cc: andrew.cooper3, anthony.perard, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
On 10.07.2025 03:59, dmkhn@proton.me wrote:
> On Tue, Jun 24, 2025 at 09:33:04AM +0200, Jan Beulich wrote:
>> On 24.06.2025 09:31, dmkhn@proton.me wrote:
>>> On Tue, Jun 24, 2025 at 07:50:33AM +0200, Jan Beulich wrote:
>>>> On 24.06.2025 05:56, dmkhn@proton.me wrote:
>>>>> From: Denis Mukhin <dmukhin@ford.com>
>>>>>
>>>>> Move PL011 emulator to the new location for UART emulators.
>>>>>
>>>>> No functional change intended.
>>>>>
>>>>> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
>>>>> ---
>>>>> xen/arch/arm/Kconfig | 7 -------
>>>>> xen/arch/arm/Makefile | 1 -
>>>>> xen/drivers/Kconfig | 2 ++
>>>>> xen/drivers/Makefile | 1 +
>>>>> xen/drivers/vuart/Kconfig | 14 ++++++++++++++
>>>>> xen/drivers/vuart/Makefile | 1 +
>>>>> .../arm/vpl011.c => drivers/vuart/vuart-pl011.c} | 0
>>>>> 7 files changed, 18 insertions(+), 8 deletions(-)
>>>>> create mode 100644 xen/drivers/vuart/Kconfig
>>>>> create mode 100644 xen/drivers/vuart/Makefile
>>>>> rename xen/{arch/arm/vpl011.c => drivers/vuart/vuart-pl011.c} (100%)
>>>>
>>>> I question the placement under drivers/. To me, driver != emulator. I
>>>> wonder what others think. But yes, we already have drivers/vpci/. That
>>>> may want moving then ...
>>>
>>> re: driver != emulator: I agree; but I followed drivers/vpci.
>>>
>>> Do you think common/vuart would be a better location?
>>
>> Or maybe common/emul/... This wants discussing, I think.
>
> Will something like the following work
> common/hvm/vuart
> ?
Not really, emulators may not be limited to HVM. But iirc common/emul/ is
what we settled on anyway at the last Community Call?
Jan
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [PATCH v1 13/16] drivers/vuart: move PL011 emulator code
2025-07-10 8:15 ` Jan Beulich
@ 2025-07-10 17:00 ` Stefano Stabellini
2025-07-31 20:47 ` dmkhn
2025-07-10 18:32 ` dmkhn
1 sibling, 1 reply; 59+ messages in thread
From: Stefano Stabellini @ 2025-07-10 17:00 UTC (permalink / raw)
To: Jan Beulich
Cc: dmkhn, andrew.cooper3, anthony.perard, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
On Thu, 10 Jul 2025, Jan Beulich wrote:
> On 10.07.2025 03:59, dmkhn@proton.me wrote:
> > On Tue, Jun 24, 2025 at 09:33:04AM +0200, Jan Beulich wrote:
> >> On 24.06.2025 09:31, dmkhn@proton.me wrote:
> >>> On Tue, Jun 24, 2025 at 07:50:33AM +0200, Jan Beulich wrote:
> >>>> On 24.06.2025 05:56, dmkhn@proton.me wrote:
> >>>>> From: Denis Mukhin <dmukhin@ford.com>
> >>>>>
> >>>>> Move PL011 emulator to the new location for UART emulators.
> >>>>>
> >>>>> No functional change intended.
> >>>>>
> >>>>> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> >>>>> ---
> >>>>> xen/arch/arm/Kconfig | 7 -------
> >>>>> xen/arch/arm/Makefile | 1 -
> >>>>> xen/drivers/Kconfig | 2 ++
> >>>>> xen/drivers/Makefile | 1 +
> >>>>> xen/drivers/vuart/Kconfig | 14 ++++++++++++++
> >>>>> xen/drivers/vuart/Makefile | 1 +
> >>>>> .../arm/vpl011.c => drivers/vuart/vuart-pl011.c} | 0
> >>>>> 7 files changed, 18 insertions(+), 8 deletions(-)
> >>>>> create mode 100644 xen/drivers/vuart/Kconfig
> >>>>> create mode 100644 xen/drivers/vuart/Makefile
> >>>>> rename xen/{arch/arm/vpl011.c => drivers/vuart/vuart-pl011.c} (100%)
> >>>>
> >>>> I question the placement under drivers/. To me, driver != emulator. I
> >>>> wonder what others think. But yes, we already have drivers/vpci/. That
> >>>> may want moving then ...
> >>>
> >>> re: driver != emulator: I agree; but I followed drivers/vpci.
> >>>
> >>> Do you think common/vuart would be a better location?
> >>
> >> Or maybe common/emul/... This wants discussing, I think.
> >
> > Will something like the following work
> > common/hvm/vuart
> > ?
>
> Not really, emulators may not be limited to HVM. But iirc common/emul/ is
> what we settled on anyway at the last Community Call?
Yes, that is what I recall as well
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [PATCH v1 13/16] drivers/vuart: move PL011 emulator code
2025-07-10 17:00 ` Stefano Stabellini
@ 2025-07-31 20:47 ` dmkhn
0 siblings, 0 replies; 59+ messages in thread
From: dmkhn @ 2025-07-31 20:47 UTC (permalink / raw)
To: Stefano Stabellini
Cc: Jan Beulich, andrew.cooper3, anthony.perard, julien, michal.orzel,
oleksii.kurochko, roger.pau, dmukhin, xen-devel
On Thu, Jul 10, 2025 at 10:00:10AM -0700, Stefano Stabellini wrote:
> On Thu, 10 Jul 2025, Jan Beulich wrote:
> > On 10.07.2025 03:59, dmkhn@proton.me wrote:
> > > On Tue, Jun 24, 2025 at 09:33:04AM +0200, Jan Beulich wrote:
> > >> On 24.06.2025 09:31, dmkhn@proton.me wrote:
> > >>> On Tue, Jun 24, 2025 at 07:50:33AM +0200, Jan Beulich wrote:
> > >>>> On 24.06.2025 05:56, dmkhn@proton.me wrote:
> > >>>>> From: Denis Mukhin <dmukhin@ford.com>
> > >>>>>
> > >>>>> Move PL011 emulator to the new location for UART emulators.
> > >>>>>
> > >>>>> No functional change intended.
> > >>>>>
> > >>>>> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> > >>>>> ---
> > >>>>> xen/arch/arm/Kconfig | 7 -------
> > >>>>> xen/arch/arm/Makefile | 1 -
> > >>>>> xen/drivers/Kconfig | 2 ++
> > >>>>> xen/drivers/Makefile | 1 +
> > >>>>> xen/drivers/vuart/Kconfig | 14 ++++++++++++++
> > >>>>> xen/drivers/vuart/Makefile | 1 +
> > >>>>> .../arm/vpl011.c => drivers/vuart/vuart-pl011.c} | 0
> > >>>>> 7 files changed, 18 insertions(+), 8 deletions(-)
> > >>>>> create mode 100644 xen/drivers/vuart/Kconfig
> > >>>>> create mode 100644 xen/drivers/vuart/Makefile
> > >>>>> rename xen/{arch/arm/vpl011.c => drivers/vuart/vuart-pl011.c} (100%)
> > >>>>
> > >>>> I question the placement under drivers/. To me, driver != emulator. I
> > >>>> wonder what others think. But yes, we already have drivers/vpci/. That
> > >>>> may want moving then ...
> > >>>
> > >>> re: driver != emulator: I agree; but I followed drivers/vpci.
> > >>>
> > >>> Do you think common/vuart would be a better location?
> > >>
> > >> Or maybe common/emul/... This wants discussing, I think.
> > >
> > > Will something like the following work
> > > common/hvm/vuart
> > > ?
> >
> > Not really, emulators may not be limited to HVM. But iirc common/emul/ is
> > what we settled on anyway at the last Community Call?
>
> Yes, that is what I recall as well
Ack, will use common/emul/
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v1 13/16] drivers/vuart: move PL011 emulator code
2025-07-10 8:15 ` Jan Beulich
2025-07-10 17:00 ` Stefano Stabellini
@ 2025-07-10 18:32 ` dmkhn
1 sibling, 0 replies; 59+ messages in thread
From: dmkhn @ 2025-07-10 18:32 UTC (permalink / raw)
To: Jan Beulich
Cc: andrew.cooper3, anthony.perard, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
On Thu, Jul 10, 2025 at 10:15:54AM +0200, Jan Beulich wrote:
> On 10.07.2025 03:59, dmkhn@proton.me wrote:
> > On Tue, Jun 24, 2025 at 09:33:04AM +0200, Jan Beulich wrote:
> >> On 24.06.2025 09:31, dmkhn@proton.me wrote:
> >>> On Tue, Jun 24, 2025 at 07:50:33AM +0200, Jan Beulich wrote:
> >>>> On 24.06.2025 05:56, dmkhn@proton.me wrote:
> >>>>> From: Denis Mukhin <dmukhin@ford.com>
> >>>>>
> >>>>> Move PL011 emulator to the new location for UART emulators.
> >>>>>
> >>>>> No functional change intended.
> >>>>>
> >>>>> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> >>>>> ---
> >>>>> xen/arch/arm/Kconfig | 7 -------
> >>>>> xen/arch/arm/Makefile | 1 -
> >>>>> xen/drivers/Kconfig | 2 ++
> >>>>> xen/drivers/Makefile | 1 +
> >>>>> xen/drivers/vuart/Kconfig | 14 ++++++++++++++
> >>>>> xen/drivers/vuart/Makefile | 1 +
> >>>>> .../arm/vpl011.c => drivers/vuart/vuart-pl011.c} | 0
> >>>>> 7 files changed, 18 insertions(+), 8 deletions(-)
> >>>>> create mode 100644 xen/drivers/vuart/Kconfig
> >>>>> create mode 100644 xen/drivers/vuart/Makefile
> >>>>> rename xen/{arch/arm/vpl011.c => drivers/vuart/vuart-pl011.c} (100%)
> >>>>
> >>>> I question the placement under drivers/. To me, driver != emulator. I
> >>>> wonder what others think. But yes, we already have drivers/vpci/. That
> >>>> may want moving then ...
> >>>
> >>> re: driver != emulator: I agree; but I followed drivers/vpci.
> >>>
> >>> Do you think common/vuart would be a better location?
> >>
> >> Or maybe common/emul/... This wants discussing, I think.
> >
> > Will something like the following work
> > common/hvm/vuart
> > ?
>
> Not really, emulators may not be limited to HVM. But iirc common/emul/ is
> what we settled on anyway at the last Community Call?
Sorry, I missed today's call, did not know about the decision.
>
> Jan
>
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v1 14/16] drivers/vuart: move simple MMIO-based UART emulator
2025-06-24 3:54 [PATCH v1 00/16] xen: framework for UART emulators dmkhn
` (12 preceding siblings ...)
2025-06-24 3:56 ` [PATCH v1 13/16] drivers/vuart: move PL011 emulator code dmkhn
@ 2025-06-24 3:57 ` dmkhn
2025-06-24 5:53 ` Jan Beulich
2025-06-24 3:57 ` [PATCH v1 15/16] drivers/vuart: introduce framework for UART emulators dmkhn
2025-06-24 3:57 ` [PATCH v1 16/16] drivers/vuart: hook simple MMIO-based UART to vUART framework dmkhn
15 siblings, 1 reply; 59+ messages in thread
From: dmkhn @ 2025-06-24 3:57 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
From: Denis Mukhin <dmukhin@ford.com>
Move simple MMIO-based UART emulator under drivers/vuart and rename it to
vuart-mmio.c to keep "vuart" for the vUART framework.
No functional change intended.
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
xen/arch/arm/Kconfig | 8 --------
xen/arch/arm/Makefile | 1 -
xen/drivers/vuart/Kconfig | 9 +++++++++
xen/drivers/vuart/Makefile | 1 +
xen/{arch/arm/vuart.c => drivers/vuart/vuart-mmio.c} | 0
5 files changed, 10 insertions(+), 9 deletions(-)
rename xen/{arch/arm/vuart.c => drivers/vuart/vuart-mmio.c} (100%)
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 6eeae97293f2..7b915abc6f18 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -170,14 +170,6 @@ config NEW_VGIC
problems with the standard emulation.
At the moment this implementation is not security supported.
-config HAS_VUART_MMIO
- bool "Emulated UART for hardware domain"
- default y
- help
- Allows a hardware domain to use a minimalistic UART (single transmit
- and status register) which takes information from dtuart. Note that this
- UART is not intended to be exposed (e.g. via device-tree) to a domain.
-
config ARM_SSBD
bool "Speculative Store Bypass Disable" if EXPERT
depends on HAS_ALTERNATIVE
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 8475043d8701..24bc9c88f7ac 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -67,7 +67,6 @@ obj-$(CONFIG_VM_EVENT) += vm_event.o
obj-y += vtimer.o
obj-y += vsmc.o
obj-y += vpsci.o
-obj-$(CONFIG_HAS_VUART_MMIO) += vuart.o
extra-y += xen.lds
diff --git a/xen/drivers/vuart/Kconfig b/xen/drivers/vuart/Kconfig
index d8df0f6d1b3c..6002817152df 100644
--- a/xen/drivers/vuart/Kconfig
+++ b/xen/drivers/vuart/Kconfig
@@ -3,6 +3,15 @@ config HAS_VUART
if (ARM_32 || ARM_64)
+config HAS_VUART_MMIO
+ bool "Simple MMIO-based emulated UART support"
+ default y
+ select HAS_VUART
+ help
+ Enables minimalistic UART (single transmit and status register) which
+ takes information from dtuart. Note that this UART is not intended to
+ be exposed (e.g. via device-tree) to a domain.
+
config HAS_VUART_PL011
bool "Emulated PL011 UART support"
default y
diff --git a/xen/drivers/vuart/Makefile b/xen/drivers/vuart/Makefile
index 3b7069f1cf95..1c775ffb7f1d 100644
--- a/xen/drivers/vuart/Makefile
+++ b/xen/drivers/vuart/Makefile
@@ -1 +1,2 @@
+obj-$(CONFIG_HAS_VUART_MMIO) += vuart-mmio.o
obj-$(CONFIG_HAS_VUART_PL011) += vuart-pl011.o
diff --git a/xen/arch/arm/vuart.c b/xen/drivers/vuart/vuart-mmio.c
similarity index 100%
rename from xen/arch/arm/vuart.c
rename to xen/drivers/vuart/vuart-mmio.c
--
2.34.1
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH v1 14/16] drivers/vuart: move simple MMIO-based UART emulator
2025-06-24 3:57 ` [PATCH v1 14/16] drivers/vuart: move simple MMIO-based UART emulator dmkhn
@ 2025-06-24 5:53 ` Jan Beulich
2025-06-24 7:36 ` dmkhn
0 siblings, 1 reply; 59+ messages in thread
From: Jan Beulich @ 2025-06-24 5:53 UTC (permalink / raw)
To: dmkhn
Cc: andrew.cooper3, anthony.perard, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
On 24.06.2025 05:57, dmkhn@proton.me wrote:
> --- a/xen/drivers/vuart/Kconfig
> +++ b/xen/drivers/vuart/Kconfig
> @@ -3,6 +3,15 @@ config HAS_VUART
>
> if (ARM_32 || ARM_64)
>
> +config HAS_VUART_MMIO
> + bool "Simple MMIO-based emulated UART support"
Perhaps in a separate change this should be renamed. HAS_* should never
have prompts.
> + default y
> + select HAS_VUART
This is questionable too (for still being controlled by a prompt), but
may need to remain as is.
Jan
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v1 14/16] drivers/vuart: move simple MMIO-based UART emulator
2025-06-24 5:53 ` Jan Beulich
@ 2025-06-24 7:36 ` dmkhn
2025-06-24 7:40 ` Jan Beulich
0 siblings, 1 reply; 59+ messages in thread
From: dmkhn @ 2025-06-24 7:36 UTC (permalink / raw)
To: Jan Beulich
Cc: andrew.cooper3, anthony.perard, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
On Tue, Jun 24, 2025 at 07:53:04AM +0200, Jan Beulich wrote:
> On 24.06.2025 05:57, dmkhn@proton.me wrote:
> > --- a/xen/drivers/vuart/Kconfig
> > +++ b/xen/drivers/vuart/Kconfig
> > @@ -3,6 +3,15 @@ config HAS_VUART
> >
> > if (ARM_32 || ARM_64)
> >
> > +config HAS_VUART_MMIO
> > + bool "Simple MMIO-based emulated UART support"
>
> Perhaps in a separate change this should be renamed. HAS_* should never
> have prompts.
Oh, so HAS_ flags are non-interactive selectors by design?
I did not find explanation in the docs :-/
Can you please explain?
(and I will add a note to docs for that)
>
> > + default y
> > + select HAS_VUART
>
> This is questionable too (for still being controlled by a prompt), but
> may need to remain as is.
>
> Jan
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v1 14/16] drivers/vuart: move simple MMIO-based UART emulator
2025-06-24 7:36 ` dmkhn
@ 2025-06-24 7:40 ` Jan Beulich
2025-06-24 22:54 ` dmkhn
0 siblings, 1 reply; 59+ messages in thread
From: Jan Beulich @ 2025-06-24 7:40 UTC (permalink / raw)
To: dmkhn
Cc: andrew.cooper3, anthony.perard, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
On 24.06.2025 09:36, dmkhn@proton.me wrote:
> On Tue, Jun 24, 2025 at 07:53:04AM +0200, Jan Beulich wrote:
>> On 24.06.2025 05:57, dmkhn@proton.me wrote:
>>> --- a/xen/drivers/vuart/Kconfig
>>> +++ b/xen/drivers/vuart/Kconfig
>>> @@ -3,6 +3,15 @@ config HAS_VUART
>>>
>>> if (ARM_32 || ARM_64)
>>>
>>> +config HAS_VUART_MMIO
>>> + bool "Simple MMIO-based emulated UART support"
>>
>> Perhaps in a separate change this should be renamed. HAS_* should never
>> have prompts.
>
> Oh, so HAS_ flags are non-interactive selectors by design?
Well "has" simply by the word means "this is available". Any user-selectable item
deriving from the mere availability would then have a "depends on HAS_...", thus
hiding the option in situation where the functionality isn't available (be it per
arch or for other reasons).
Jan
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v1 14/16] drivers/vuart: move simple MMIO-based UART emulator
2025-06-24 7:40 ` Jan Beulich
@ 2025-06-24 22:54 ` dmkhn
2025-06-25 5:25 ` Jan Beulich
0 siblings, 1 reply; 59+ messages in thread
From: dmkhn @ 2025-06-24 22:54 UTC (permalink / raw)
To: Jan Beulich
Cc: andrew.cooper3, anthony.perard, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
On Tue, Jun 24, 2025 at 09:40:02AM +0200, Jan Beulich wrote:
> On 24.06.2025 09:36, dmkhn@proton.me wrote:
> > On Tue, Jun 24, 2025 at 07:53:04AM +0200, Jan Beulich wrote:
> >> On 24.06.2025 05:57, dmkhn@proton.me wrote:
> >>> --- a/xen/drivers/vuart/Kconfig
> >>> +++ b/xen/drivers/vuart/Kconfig
> >>> @@ -3,6 +3,15 @@ config HAS_VUART
> >>>
> >>> if (ARM_32 || ARM_64)
> >>>
> >>> +config HAS_VUART_MMIO
> >>> + bool "Simple MMIO-based emulated UART support"
> >>
> >> Perhaps in a separate change this should be renamed. HAS_* should never
> >> have prompts.
> >
> > Oh, so HAS_ flags are non-interactive selectors by design?
>
> Well "has" simply by the word means "this is available". Any user-selectable item
> deriving from the mere availability would then have a "depends on HAS_...", thus
> hiding the option in situation where the functionality isn't available (be it per
> arch or for other reasons).
I see there's a lot of drivers (UARTs) which are selectable by the user via
HAS_ symbols (drivers/char/Kconfig), e.g:
CONFIG_HAS_NS16550: │
│ │
│ This selects the 16550-series UART support. For most systems, say Y. │
│ │
│ Symbol: HAS_NS16550 [=y] │
│ Type : bool │
│ Prompt: NS16550 UART driver │
│ Location: │
│ -> Device Drivers │
│ Defined at drivers/char/Kconfig:4
>
> Jan
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v1 14/16] drivers/vuart: move simple MMIO-based UART emulator
2025-06-24 22:54 ` dmkhn
@ 2025-06-25 5:25 ` Jan Beulich
2025-07-10 1:52 ` dmkhn
0 siblings, 1 reply; 59+ messages in thread
From: Jan Beulich @ 2025-06-25 5:25 UTC (permalink / raw)
To: dmkhn
Cc: andrew.cooper3, anthony.perard, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
On 25.06.2025 00:54, dmkhn@proton.me wrote:
> On Tue, Jun 24, 2025 at 09:40:02AM +0200, Jan Beulich wrote:
>> On 24.06.2025 09:36, dmkhn@proton.me wrote:
>>> On Tue, Jun 24, 2025 at 07:53:04AM +0200, Jan Beulich wrote:
>>>> On 24.06.2025 05:57, dmkhn@proton.me wrote:
>>>>> --- a/xen/drivers/vuart/Kconfig
>>>>> +++ b/xen/drivers/vuart/Kconfig
>>>>> @@ -3,6 +3,15 @@ config HAS_VUART
>>>>>
>>>>> if (ARM_32 || ARM_64)
>>>>>
>>>>> +config HAS_VUART_MMIO
>>>>> + bool "Simple MMIO-based emulated UART support"
>>>>
>>>> Perhaps in a separate change this should be renamed. HAS_* should never
>>>> have prompts.
>>>
>>> Oh, so HAS_ flags are non-interactive selectors by design?
>>
>> Well "has" simply by the word means "this is available". Any user-selectable item
>> deriving from the mere availability would then have a "depends on HAS_...", thus
>> hiding the option in situation where the functionality isn't available (be it per
>> arch or for other reasons).
>
> I see there's a lot of drivers (UARTs) which are selectable by the user via
> HAS_ symbols (drivers/char/Kconfig), e.g:
>
> CONFIG_HAS_NS16550:
Iirc it was prompt-less up to some point. And when the prompt was added, the name
wasn't changed / split. Other UARTs then followed suit (when they shouldn't have).
Jan
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v1 14/16] drivers/vuart: move simple MMIO-based UART emulator
2025-06-25 5:25 ` Jan Beulich
@ 2025-07-10 1:52 ` dmkhn
0 siblings, 0 replies; 59+ messages in thread
From: dmkhn @ 2025-07-10 1:52 UTC (permalink / raw)
To: Jan Beulich
Cc: andrew.cooper3, anthony.perard, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin, xen-devel
On Wed, Jun 25, 2025 at 07:25:29AM +0200, Jan Beulich wrote:
> On 25.06.2025 00:54, dmkhn@proton.me wrote:
> > On Tue, Jun 24, 2025 at 09:40:02AM +0200, Jan Beulich wrote:
> >> On 24.06.2025 09:36, dmkhn@proton.me wrote:
> >>> On Tue, Jun 24, 2025 at 07:53:04AM +0200, Jan Beulich wrote:
> >>>> On 24.06.2025 05:57, dmkhn@proton.me wrote:
> >>>>> --- a/xen/drivers/vuart/Kconfig
> >>>>> +++ b/xen/drivers/vuart/Kconfig
> >>>>> @@ -3,6 +3,15 @@ config HAS_VUART
> >>>>>
> >>>>> if (ARM_32 || ARM_64)
> >>>>>
> >>>>> +config HAS_VUART_MMIO
> >>>>> + bool "Simple MMIO-based emulated UART support"
> >>>>
> >>>> Perhaps in a separate change this should be renamed. HAS_* should never
> >>>> have prompts.
> >>>
> >>> Oh, so HAS_ flags are non-interactive selectors by design?
> >>
> >> Well "has" simply by the word means "this is available". Any user-selectable item
> >> deriving from the mere availability would then have a "depends on HAS_...", thus
> >> hiding the option in situation where the functionality isn't available (be it per
> >> arch or for other reasons).
> >
> > I see there's a lot of drivers (UARTs) which are selectable by the user via
> > HAS_ symbols (drivers/char/Kconfig), e.g:
> >
> > CONFIG_HAS_NS16550:
>
> Iirc it was prompt-less up to some point. And when the prompt was added, the name
> wasn't changed / split. Other UARTs then followed suit (when they shouldn't have).
I can submit a separate patch to address the naming, if there are no
objections.
>
> Jan
>
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v1 15/16] drivers/vuart: introduce framework for UART emulators
2025-06-24 3:54 [PATCH v1 00/16] xen: framework for UART emulators dmkhn
` (13 preceding siblings ...)
2025-06-24 3:57 ` [PATCH v1 14/16] drivers/vuart: move simple MMIO-based UART emulator dmkhn
@ 2025-06-24 3:57 ` dmkhn
2025-06-24 3:57 ` [PATCH v1 16/16] drivers/vuart: hook simple MMIO-based UART to vUART framework dmkhn
15 siblings, 0 replies; 59+ messages in thread
From: dmkhn @ 2025-06-24 3:57 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
From: Denis Mukhin <dmukhin@ford.com>
Introduce a driver framework to abstract UART emulators in the hypervisor.
That allows for architecture-independent handling of virtual UARTs in the
console driver and simplifies enabling new UART emulators.
The framework is built under CONFIG_HAS_VUART, which is automatically enabled
once the user enables any UART emulator.
Current implementation supports maximum of one vUART of each kind per domain.
All domains with enabled virtual PL011 will have DOMAIN_EMU_UART_PL011 bit set
in d->arch.emulation_flags. Introduce domain_has_vuart() in arch code to check
whether domain has virtual UART.
Use domain_has_vuart() in the console driver code to check whether to forward
console input to the domain using vUART.
Hook existing PL011 UART emulator to the new driver framework.
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Original code: https://lore.kernel.org/xen-devel/20250103-vuart-ns8250-v3-v1-7-c5d36b31d66c@ford.com/
---
xen/arch/arm/dom0less-build.c | 22 +++----
xen/arch/arm/domctl.c | 3 +-
xen/arch/arm/include/asm/domain.h | 1 +
xen/arch/arm/include/asm/kernel.h | 3 -
xen/arch/arm/xen.lds.S | 1 +
xen/arch/ppc/include/asm/domain.h | 1 +
xen/arch/ppc/xen.lds.S | 1 +
xen/arch/riscv/include/asm/domain.h | 1 +
xen/arch/riscv/xen.lds.S | 1 +
xen/arch/x86/xen.lds.S | 1 +
xen/common/domain.c | 10 +++
xen/common/keyhandler.c | 3 +
xen/drivers/char/console.c | 5 +-
xen/drivers/vuart/Makefile | 1 +
xen/drivers/vuart/vuart-pl011.c | 71 +++++++++++++++++++--
xen/drivers/vuart/vuart.c | 95 +++++++++++++++++++++++++++++
xen/include/xen/domain-emu.h | 2 +
xen/include/xen/domain.h | 2 +
xen/include/xen/vuart.h | 25 ++++++--
xen/include/xen/xen.lds.h | 10 +++
20 files changed, 227 insertions(+), 32 deletions(-)
create mode 100644 xen/drivers/vuart/vuart.c
diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index 11b8498d3b22..c04441ef4657 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -176,14 +176,9 @@ int __init make_arch_nodes(struct kernel_info *kinfo)
if ( ret )
return -EINVAL;
- if ( kinfo->arch.vpl011 )
- {
-#ifdef CONFIG_HAS_VUART_PL011
- ret = vuart_add_fwnode(kinfo->d, kinfo);
-#endif
- if ( ret )
- return -EINVAL;
- }
+ ret = vuart_add_fwnode(kinfo->d, kinfo);
+ if ( ret )
+ return ret;
return 0;
}
@@ -207,19 +202,16 @@ int __init init_vuart(struct domain *d, struct kernel_info *kinfo,
{
int rc = 0;
- kinfo->arch.vpl011 = dt_property_read_bool(node, "vpl011");
-
/*
* Base address and irq number are needed when creating vpl011 device
* tree node in prepare_dtb_domU, so initialization on related variables
* shall be done first.
*/
- if ( kinfo->arch.vpl011 )
- {
+ if ( dt_property_read_bool(node, "vpl011") )
+ d->emulation_flags |= DOMAIN_EMU_UART_PL011;
+
+ if ( domain_has_vuart(d) )
rc = vuart_init(d, NULL);
- if ( rc < 0 )
- return rc;
- }
return rc;
}
diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
index dde25ceff6d0..45144eca1ae2 100644
--- a/xen/arch/arm/domctl.c
+++ b/xen/arch/arm/domctl.c
@@ -43,8 +43,9 @@ static int handle_vuart_init(struct domain *d,
if ( vuart_op->type != XEN_DOMCTL_VUART_TYPE_VPL011 )
return -EOPNOTSUPP;
- rc = vuart_init(d, ¶ms);
+ d->emulation_flags |= DOMAIN_EMU_UART_PL011;
+ rc = vuart_init(d, ¶ms);
if ( !rc )
vuart_op->evtchn = params.evtchn;
diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
index 38873c66f1f8..32ef281151d8 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -2,6 +2,7 @@
#define __ASM_DOMAIN_H__
#include <xen/cache.h>
+#include <xen/domain-emu.h>
#include <xen/timer.h>
#include <asm/page.h>
#include <asm/p2m.h>
diff --git a/xen/arch/arm/include/asm/kernel.h b/xen/arch/arm/include/asm/kernel.h
index 7c3b7fde5b64..cfeab792c76e 100644
--- a/xen/arch/arm/include/asm/kernel.h
+++ b/xen/arch/arm/include/asm/kernel.h
@@ -13,9 +13,6 @@ struct arch_kernel_info
#ifdef CONFIG_ARM_64
enum domain_type type;
#endif
-
- /* Enable pl011 emulation */
- bool vpl011;
};
#endif /* #ifdef __ARCH_ARM_KERNEL_H__ */
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 5bfbe1e92c1e..e876e3efbe4c 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -66,6 +66,7 @@ SECTIONS
__proc_info_end = .;
VPCI_ARRAY
+ VUART_ARRAY
} :text
#if defined(BUILD_ID)
diff --git a/xen/arch/ppc/include/asm/domain.h b/xen/arch/ppc/include/asm/domain.h
index 3a447272c6f2..426e6297c935 100644
--- a/xen/arch/ppc/include/asm/domain.h
+++ b/xen/arch/ppc/include/asm/domain.h
@@ -2,6 +2,7 @@
#ifndef __ASM_PPC_DOMAIN_H__
#define __ASM_PPC_DOMAIN_H__
+#include <xen/domain-emu.h>
#include <xen/xmalloc.h>
#include <public/hvm/params.h>
diff --git a/xen/arch/ppc/xen.lds.S b/xen/arch/ppc/xen.lds.S
index 1366e2819eed..56bf4a0638d5 100644
--- a/xen/arch/ppc/xen.lds.S
+++ b/xen/arch/ppc/xen.lds.S
@@ -55,6 +55,7 @@ SECTIONS
*(.data.rel.ro.*)
VPCI_ARRAY
+ VUART_ARRAY
. = ALIGN(POINTER_ALIGN);
} :text
diff --git a/xen/arch/riscv/include/asm/domain.h b/xen/arch/riscv/include/asm/domain.h
index c3d965a559b6..fcde3bc78dc6 100644
--- a/xen/arch/riscv/include/asm/domain.h
+++ b/xen/arch/riscv/include/asm/domain.h
@@ -2,6 +2,7 @@
#ifndef ASM__RISCV__DOMAIN_H
#define ASM__RISCV__DOMAIN_H
+#include <xen/domain-emu.h>
#include <xen/xmalloc.h>
#include <public/hvm/params.h>
diff --git a/xen/arch/riscv/xen.lds.S b/xen/arch/riscv/xen.lds.S
index 8c3c06de01f6..45592fca128f 100644
--- a/xen/arch/riscv/xen.lds.S
+++ b/xen/arch/riscv/xen.lds.S
@@ -50,6 +50,7 @@ SECTIONS
*(.data.rel.ro.*)
VPCI_ARRAY
+ VUART_ARRAY
. = ALIGN(POINTER_ALIGN);
} :text
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index bf956b6c5fc0..b04ed1e6138c 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -149,6 +149,7 @@ SECTIONS
__note_gnu_build_id_end = .;
#endif
VPCI_ARRAY
+ VUART_ARRAY
} PHDR(text)
#if defined(CONFIG_PVH_GUEST) && !defined(EFI)
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 8c8f70347a91..071fee81fe2c 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -2436,6 +2436,16 @@ void thaw_domains(void)
rcu_read_unlock(&domlist_read_lock);
}
+bool domain_has_vuart(const struct domain *d)
+{
+ uint32_t mask = 0;
+
+ if ( IS_ENABLED(CONFIG_HAS_VUART_PL011) )
+ mask |= DOMAIN_EMU_UART_PL011;
+
+ return !!(d->emulation_flags & mask);
+}
+
/*
* Local variables:
* mode: C
diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index eccd97c565c6..21e970aded9a 100644
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -22,6 +22,7 @@
#include <xen/mm.h>
#include <xen/watchdog.h>
#include <xen/init.h>
+#include <xen/vuart.h>
#include <asm/div64.h>
static unsigned char keypress_key;
@@ -354,6 +355,8 @@ static void cf_check dump_domains(unsigned char key)
v->periodic_period / 1000000);
}
}
+
+ vuart_dump(d);
}
for_each_domain ( d )
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index f322d59515ab..5ddc05cc44da 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -594,6 +594,7 @@ static void __serial_rx(char c)
/*
* Deliver input to the hardware domain buffer, unless it is
* already full.
+ * NB: must be the first check: hardware domain may have emulated UART.
*/
if ( (serial_rx_prod - serial_rx_cons) != SERIAL_RX_SIZE )
serial_rx_ring[SERIAL_RX_MASK(serial_rx_prod++)] = c;
@@ -604,11 +605,9 @@ static void __serial_rx(char c)
*/
send_global_virq(VIRQ_CONSOLE);
}
-#ifdef CONFIG_HAS_VUART_PL011
- else
+ else if ( domain_has_vuart(d) )
/* Deliver input to the emulated UART. */
rc = vuart_putchar(d, c);
-#endif
if ( consoled_is_enabled() )
/* Deliver input to the PV shim console. */
diff --git a/xen/drivers/vuart/Makefile b/xen/drivers/vuart/Makefile
index 1c775ffb7f1d..fa9aab666c2a 100644
--- a/xen/drivers/vuart/Makefile
+++ b/xen/drivers/vuart/Makefile
@@ -1,2 +1,3 @@
+obj-$(CONFIG_HAS_VUART) += vuart.o
obj-$(CONFIG_HAS_VUART_MMIO) += vuart-mmio.o
obj-$(CONFIG_HAS_VUART_PL011) += vuart-pl011.o
diff --git a/xen/drivers/vuart/vuart-pl011.c b/xen/drivers/vuart/vuart-pl011.c
index bebfb5e0365c..312618ad1b6f 100644
--- a/xen/drivers/vuart/vuart-pl011.c
+++ b/xen/drivers/vuart/vuart-pl011.c
@@ -638,9 +638,9 @@ static void vpl011_data_avail(struct domain *d,
}
/*
- * vuart_putchar adds a char to a domain's vpl011 receive buffer.
+ * vpl011_putchar adds a char to a domain's vpl011 receive buffer.
*/
-int vuart_putchar(struct domain *d, char c)
+static int cf_check vpl011_putchar(struct domain *d, char c)
{
unsigned long flags;
struct vpl011 *vpl011 = d->arch.vpl011;
@@ -709,7 +709,8 @@ static void vpl011_notification(struct vcpu *v, unsigned int port)
spin_unlock_irqrestore(&vpl011->lock, flags);
}
-int vuart_init(struct domain *d, struct vuart_params *params)
+static int cf_check vpl011_init(struct domain *d,
+ struct vuart_params *params)
{
struct vpl011 *vpl011;
int rc;
@@ -824,7 +825,7 @@ err_out:
return rc;
}
-void vuart_exit(struct domain *d)
+static void cf_check vpl011_exit(struct domain *d)
{
if ( d->arch.vpl011 )
{
@@ -833,9 +834,57 @@ void vuart_exit(struct domain *d)
}
}
-int __init vuart_add_fwnode(struct domain *d, void *node)
+static void cf_check vpl011_dump(const struct domain *d)
{
- struct kernel_info *kinfo = node;
+ struct vpl011 *vdev = d->arch.vpl011;
+
+ if ( !vdev )
+ return;
+
+ /* Allow printing state in case of a deadlock. */
+ if ( !spin_trylock(&vdev->lock) )
+ return;
+
+ printk("Virtual PL011@%"PRIpaddr" IRQ#%d owner %pd\n",
+ vdev->base_addr, vdev->virq, d);
+
+ if ( vdev->backend_in_domain )
+ {
+ printk(" Event channel %"PRIu32"\n", vdev->evtchn);
+ }
+ else
+ {
+ const struct vpl011_xen_backend *cons = vdev->backend.xen;
+
+ printk(" RX FIFO size %u in_prod %u in_cons %u used %u\n",
+ SBSA_UART_FIFO_SIZE, cons->in_prod, cons->in_cons,
+ cons->in_prod - cons->in_cons);
+
+ printk(" TX FIFO size %u out_prod %u used %u\n",
+ SBSA_UART_OUT_BUF_SIZE, cons->out_prod, cons->out_prod);
+ }
+
+ printk(" %02"PRIx8" DR %08"PRIx8"\n", DR , 0);
+ printk(" %02"PRIx8" RSR %08"PRIx8"\n", RSR , 0);
+ printk(" %02"PRIx8" FR %08"PRIx8"\n", FR , vdev->uartfr);
+ printk(" %02"PRIx8" ILPR %08"PRIx8"\n", ILPR , 0);
+ printk(" %02"PRIx8" IBRD %08"PRIx8"\n", IBRD , 0);
+ printk(" %02"PRIx8" FBRD %08"PRIx8"\n", FBRD , 0);
+ printk(" %02"PRIx8" LCR_H %08"PRIx8"\n", LCR_H, 0);
+ printk(" %02"PRIx8" CR %08"PRIx8"\n", CR , vdev->uartcr);
+ printk(" %02"PRIx8" IFLS %08"PRIx8"\n", IFLS , 0);
+ printk(" %02"PRIx8" IMSC %08"PRIx8"\n", IMSC , vdev->uartimsc);
+ printk(" %02"PRIx8" RIS %08"PRIx8"\n", RIS , vdev->uartris);
+ printk(" %02"PRIX8" MIS %08"PRIX8"\n", MIS , vdev->shadow_uartmis);
+ printk(" %02"PRIx8" ICR %08"PRIx8"\n", ICR , vdev->uarticr);
+ printk(" %02"PRIx8" DMACR %08"PRIx8"\n", DMACR, 0);
+
+ spin_unlock(&vdev->lock);
+}
+
+static int cf_check vpl011_add_fwnode(struct domain *d, void *info)
+{
+ struct kernel_info *kinfo = info;
void *fdt = kinfo->fdt;
int res;
gic_interrupt_t intr;
@@ -884,6 +933,16 @@ int __init vuart_add_fwnode(struct domain *d, void *node)
return 0;
}
+static const struct vuart_ops vpl011_ops = {
+ .add_fwnode = vpl011_add_fwnode,
+ .init = vpl011_init,
+ .exit = vpl011_exit,
+ .dump = vpl011_dump,
+ .putchar = vpl011_putchar,
+};
+
+VUART_REGISTER(vpl011, &vpl011_ops);
+
/*
* Local variables:
* mode: C
diff --git a/xen/drivers/vuart/vuart.c b/xen/drivers/vuart/vuart.c
new file mode 100644
index 000000000000..ab3c2504bff5
--- /dev/null
+++ b/xen/drivers/vuart/vuart.c
@@ -0,0 +1,95 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <xen/errno.h>
+#include <xen/sched.h>
+#include <xen/vuart.h>
+
+extern const struct vuart_ops *const __vuart_array_start[];
+extern const struct vuart_ops *const __vuart_array_end[];
+
+#define VUART_ARRAY_SIZE (__vuart_array_end - __vuart_array_start)
+
+#define for_each_vuart(vdev) \
+ for (unsigned __i = 0; \
+ __i < VUART_ARRAY_SIZE && (vdev = __vuart_array_start[__i], 1); \
+ __i++)
+
+int vuart_add_fwnode(struct domain *d, void *node)
+{
+ const struct vuart_ops *vdev;
+ int rc;
+
+ for_each_vuart(vdev)
+ {
+ if ( !vdev->add_fwnode )
+ continue;
+
+ rc = vdev->add_fwnode(d, node);
+ if ( rc )
+ return rc;
+ }
+
+ return 0;
+}
+
+int vuart_init(struct domain *d, struct vuart_params *params)
+{
+ const struct vuart_ops *vdev;
+ int rc;
+
+ for_each_vuart(vdev)
+ {
+ rc = vdev->init(d, params);
+ if ( rc )
+ return rc;
+ }
+
+ d->console.input_allowed = true;
+
+ return 0;
+}
+
+/*
+ * Release any resources taken by UART emulators.
+ *
+ * NB: no flags are cleared, since currently exit() is called only during
+ * domain destroy.
+ */
+void vuart_exit(struct domain *d)
+{
+ const struct vuart_ops *vdev;
+
+ for_each_vuart(vdev)
+ vdev->exit(d);
+}
+
+void vuart_dump(const struct domain *d)
+{
+ const struct vuart_ops *vdev;
+
+ for_each_vuart(vdev)
+ vdev->dump(d);
+}
+
+/*
+ * Put character to the first suitable emulated UART's FIFO.
+ */
+int vuart_putchar(struct domain *d, char c)
+{
+ const struct vuart_ops *vdev = NULL;
+
+ for_each_vuart(vdev)
+ if ( vdev->putchar )
+ break;
+
+ return vdev ? vdev->putchar(d, c) : -ENODEV;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/xen/domain-emu.h b/xen/include/xen/domain-emu.h
index 410963ff480c..1d3a6c80fadd 100644
--- a/xen/include/xen/domain-emu.h
+++ b/xen/include/xen/domain-emu.h
@@ -17,6 +17,8 @@
#define DOMAIN_EMU_PIRQ (1U << 9)
#define DOMAIN_EMU_PCI (1U << 10)
+#define DOMAIN_EMU_UART_PL011 (1U << 15)
+
#endif /* XEN_DOMAIN_EMU_H */
/*
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index e10baf2615fd..e40906380760 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -59,6 +59,8 @@ domid_t get_initial_domain_id(void);
#define is_domain_direct_mapped(d) ((d)->cdf & CDF_directmap)
#define is_domain_using_staticmem(d) ((d)->cdf & CDF_staticmem)
+bool domain_has_vuart(const struct domain *d);
+
/*
* Arch-specifics.
*/
diff --git a/xen/include/xen/vuart.h b/xen/include/xen/vuart.h
index 928b60bbb4e2..1f4b47575359 100644
--- a/xen/include/xen/vuart.h
+++ b/xen/include/xen/vuart.h
@@ -12,16 +12,29 @@ struct vuart_params {
evtchn_port_t evtchn;
};
-#ifdef CONFIG_HAS_VUART_PL011
+struct vuart_ops {
+ int (*add_fwnode)(struct domain *d, void *node);
+ int (*init)(struct domain *d, struct vuart_params *params);
+ void (*exit)(struct domain *d);
+ void (*dump)(const struct domain *d);
+ int (*putchar)(struct domain *d, char c);
+};
-int __init vuart_add_fwnode(struct domain *d, void *node);
+#define VUART_REGISTER(name, x) \
+ static const struct vuart_ops *const __name##_entry \
+ __used_section(".data.vuart." #name) = (x);
+
+#ifdef CONFIG_HAS_VUART
+
+int vuart_add_fwnode(struct domain *d, void *node);
int vuart_init(struct domain *d, struct vuart_params *params);
void vuart_exit(struct domain *d);
+void vuart_dump(const struct domain *d);
int vuart_putchar(struct domain *d, char c);
#else
-static inline int __init vuart_add_fwnode(struct domain *d, void *node)
+static inline int vuart_add_fwnode(struct domain *d, void *node)
{
return 0;
}
@@ -35,13 +48,17 @@ static inline void vuart_exit(struct domain *d)
{
}
+static inline void vuart_dump(const struct domain *d)
+{
+}
+
static inline int vuart_putchar(struct domain *d, char c)
{
ASSERT_UNREACHABLE();
return -ENODEV;
}
-#endif /* CONFIG_HAS_VUART_PL011 */
+#endif /* CONFIG_HAS_VUART */
#ifdef CONFIG_HAS_VUART_MMIO
diff --git a/xen/include/xen/xen.lds.h b/xen/include/xen/xen.lds.h
index 793d0e11450c..1671257e19ee 100644
--- a/xen/include/xen/xen.lds.h
+++ b/xen/include/xen/xen.lds.h
@@ -194,4 +194,14 @@
#define VPCI_ARRAY
#endif
+#ifdef CONFIG_HAS_VUART
+#define VUART_ARRAY \
+ . = ALIGN(POINTER_ALIGN); \
+ __vuart_array_start = .; \
+ *(SORT(.data.vuart.*)) \
+ __vuart_array_end = .;
+#else
+#define VUART_ARRAY
+#endif
+
#endif /* __XEN_LDS_H__ */
--
2.34.1
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH v1 16/16] drivers/vuart: hook simple MMIO-based UART to vUART framework
2025-06-24 3:54 [PATCH v1 00/16] xen: framework for UART emulators dmkhn
` (14 preceding siblings ...)
2025-06-24 3:57 ` [PATCH v1 15/16] drivers/vuart: introduce framework for UART emulators dmkhn
@ 2025-06-24 3:57 ` dmkhn
15 siblings, 0 replies; 59+ messages in thread
From: dmkhn @ 2025-06-24 3:57 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
oleksii.kurochko, roger.pau, sstabellini, dmukhin
From: Denis Mukhin <dmukhin@ford.com>
Add new emulation flag DOMAIN_EMU_UART_MMIO and add it to domain_has_vuart().
Add needed shims for vuart framework integration to MMIO-based UART emulator.
Remove domain_vuart_{init,free}() and use generic vuart_{init,exit}() calls.
No functional change intended.
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
xen/arch/arm/domain.c | 7 +++++--
xen/common/domain.c | 3 +++
xen/drivers/vuart/vuart-mmio.c | 36 ++++++++++++++++++++++++++++++++--
xen/include/xen/domain-emu.h | 1 +
xen/include/xen/vuart.h | 20 -------------------
5 files changed, 43 insertions(+), 24 deletions(-)
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 3579d10d7e1d..5d7006241be0 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -780,7 +780,10 @@ int arch_domain_create(struct domain *d,
* Only use it for the hardware domain because the linux kernel may not
* support multi-platform.
*/
- if ( is_hardware_domain(d) && (rc = domain_vuart_init(d)) )
+ if ( is_hardware_domain(d) && IS_ENABLED(CONFIG_HAS_VUART_MMIO) )
+ d->emulation_flags |= DOMAIN_EMU_UART_MMIO;
+
+ if ( domain_has_vuart(d) && (rc = vuart_init(d, NULL)) != 0 )
goto fail;
if ( (rc = domain_vpci_init(d)) != 0 )
@@ -849,7 +852,7 @@ void arch_domain_destroy(struct domain *d)
iommu_domain_destroy(d);
p2m_final_teardown(d);
domain_vgic_free(d);
- domain_vuart_free(d);
+ vuart_exit(d);
free_xenheap_page(d->shared_info);
#ifdef CONFIG_ACPI
free_xenheap_pages(d->arch.efi_acpi_table,
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 071fee81fe2c..fc0ceb266d88 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -2443,6 +2443,9 @@ bool domain_has_vuart(const struct domain *d)
if ( IS_ENABLED(CONFIG_HAS_VUART_PL011) )
mask |= DOMAIN_EMU_UART_PL011;
+ if ( IS_ENABLED(CONFIG_HAS_VUART_MMIO) )
+ mask |= DOMAIN_EMU_UART_MMIO;
+
return !!(d->emulation_flags & mask);
}
diff --git a/xen/drivers/vuart/vuart-mmio.c b/xen/drivers/vuart/vuart-mmio.c
index 66fac6c994ce..1888e44e3d94 100644
--- a/xen/drivers/vuart/vuart-mmio.c
+++ b/xen/drivers/vuart/vuart-mmio.c
@@ -49,7 +49,7 @@ static const struct mmio_handler_ops vuart_mmio_handler = {
.write = vuart_mmio_write,
};
-int domain_vuart_init(struct domain *d)
+static int cf_check vuart_mmio_init(struct domain *d, struct vuart_params *params)
{
const struct vuart_info *info;
struct vuart *vdev;
@@ -86,7 +86,7 @@ int domain_vuart_init(struct domain *d)
return 0;
}
-void domain_vuart_free(struct domain *d)
+static void cf_check vuart_mmio_exit(struct domain *d)
{
struct vuart *vdev = d->arch.vuart;
@@ -147,6 +147,38 @@ static int vuart_mmio_write(struct vcpu *v, mmio_info_t *info,
return 1;
}
+static void cf_check vuart_mmio_dump(const struct domain *d)
+{
+ struct vuart *vdev = d->arch.vuart;
+
+ if ( !vdev )
+ return;
+
+ /* Allow printing state in case of a deadlock. */
+ if ( !spin_trylock(&vdev->lock) )
+ return;
+
+ printk("Virtual MMIO UART@%"PRIpaddr" owner %pd\n",
+ vdev->info->base_addr, d);
+ printk(" RX FIFO size %u idx %u\n",
+ VUART_BUF_SIZE, vdev->idx);
+ printk(" status 0x%lx 0x%lx\n",
+ vdev->info->status_off, vdev->info->status);
+
+ spin_unlock(&vdev->lock);
+}
+
+static const struct vuart_ops vuart_mmio_ops = {
+ .add_fwnode = NULL,
+ .init = vuart_mmio_init,
+ .exit = vuart_mmio_exit,
+ .dump = vuart_mmio_dump,
+ /* Physical console focus is not supported */
+ .putchar = NULL,
+};
+
+VUART_REGISTER(mmio, &vuart_mmio_ops);
+
/*
* Local variables:
* mode: C
diff --git a/xen/include/xen/domain-emu.h b/xen/include/xen/domain-emu.h
index 1d3a6c80fadd..9ef607666842 100644
--- a/xen/include/xen/domain-emu.h
+++ b/xen/include/xen/domain-emu.h
@@ -18,6 +18,7 @@
#define DOMAIN_EMU_PCI (1U << 10)
#define DOMAIN_EMU_UART_PL011 (1U << 15)
+#define DOMAIN_EMU_UART_MMIO (1U << 16)
#endif /* XEN_DOMAIN_EMU_H */
diff --git a/xen/include/xen/vuart.h b/xen/include/xen/vuart.h
index 1f4b47575359..4c923025b4eb 100644
--- a/xen/include/xen/vuart.h
+++ b/xen/include/xen/vuart.h
@@ -60,26 +60,6 @@ static inline int vuart_putchar(struct domain *d, char c)
#endif /* CONFIG_HAS_VUART */
-#ifdef CONFIG_HAS_VUART_MMIO
-
-int domain_vuart_init(struct domain *d);
-void domain_vuart_free(struct domain *d);
-
-#else
-
-static inline int domain_vuart_init(struct domain *d)
-{
- /*
- * The vUART is unconditionally inialized for the hw domain. So we
- * can't return an error.
- */
- return 0;
-}
-
-static inline void domain_vuart_free(struct domain *d) {};
-
-#endif /* CONFIG_HAS_VUART_MMIO */
-
#endif /* XEN_VUART_H */
/*
--
2.34.1
^ permalink raw reply related [flat|nested] 59+ messages in thread