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