Re: [PATCH 1/4] arm/domain: Update SBSA_VUART_CONSOLE config option handling

["Orzel, Michal" <michal.orzel@xxxxxxx>]

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 <milan_djokic@xxxxxxxx>
> ---
>  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.

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

> +
>          /* 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