Hi Michal,
On 12/8/25 15:43, Orzel, Michal wrote:


On 05/12/2025 21:36, Milan Djokic wrote:
Update XEN_DOMCTL_vuart_op command handling to return -EOPNOTSUPP when
vpl011 is disabled, informing the control domain that this feature
is unavailable.
Added dom0less config sanity check for vpl011 property

Signed-off-by: Milan Djokic <[email protected]>
---
  xen/arch/arm/dom0less-build.c     | 4 ++++
  xen/arch/arm/domctl.c             | 3 +++
  xen/arch/arm/include/asm/vpl011.h | 2 +-
  3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index 4181c10538..57980d2abe 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -322,6 +322,10 @@ int __init arch_parse_dom0less_node(struct dt_device_node 
*node,
      if ( domu_dt_sci_parse(node, d_cfg) )
          panic("Error getting SCI configuration\n");
+ if ( dt_property_read_bool(node, "vpl011") &&
+         !IS_ENABLED(CONFIG_SBSA_VUART_CONSOLE) )
+        panic("'vpl011' property found, but CONFIG_SBSA_VUART_CONSOLE not 
selected\n");
I don't think there is a need for another dt property reading. In init_vuart()
we read this property and if present we call domain_vpl011_init. This function
returns (thanks to your change) -EOPNOTSUPP, so we already cover both cases. No
need for an explicit panic.


I added this check since it seemed to me that this specific case should be covered as part of the config sanity check. But yes, if there's no need for an explicit panic, it is handled later in domain_vpl011_init() anyway. Will remove it.

+
      if ( !dt_property_read_u32(node, "nr_spis", &d_cfg->arch.nr_spis) )
      {
          int vpl011_virq = GUEST_VPL011_SPI;
diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
index ad914c915f..250e20a9fb 100644
--- a/xen/arch/arm/domctl.c
+++ b/xen/arch/arm/domctl.c
@@ -156,6 +156,9 @@ long arch_do_domctl(struct xen_domctl *domctl, struct 
domain *d,
          unsigned int i;
          struct xen_domctl_vuart_op *vuart_op = &domctl->u.vuart_op;
+ if ( !IS_ENABLED(CONFIG_SBSA_VUART_CONSOLE) )
+            return -EOPNOTSUPP;
Why is this needed? Later on in this path we will call domain_vpl011_init that
will return exactly the same if CONFIG_SBSA_VUART_CONSOLE is not enabled.


Yes, this is not necessary. I will exclude this part.

+
          /* check that structure padding must be 0. */
          for ( i = 0; i < sizeof(vuart_op->pad); i++ )
              if ( vuart_op->pad[i] )
diff --git a/xen/arch/arm/include/asm/vpl011.h 
b/xen/arch/arm/include/asm/vpl011.h
index cc83868281..b8f4d85651 100644
--- a/xen/arch/arm/include/asm/vpl011.h
+++ b/xen/arch/arm/include/asm/vpl011.h
@@ -74,7 +74,7 @@ int vpl011_rx_char_xen(struct domain *d, char c);
  static inline int domain_vpl011_init(struct domain *d,
                                       struct vpl011_init_info *info)
  {
-    return -ENOSYS;
+    return -EOPNOTSUPP;
This change is ok.

~Michal


BR,
Milan


Reply via email to