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 = &reg[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 = &reg[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

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

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

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

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
>