Re: [PATCH v1 03/27] xen/riscv: implement prerequisites for domain_create()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.03.2026 18:08, Oleksii Kurochko wrote:
> arch_domain_create() and arch_sanitise_domain_config() are prerequisites for
> domain_create().
> 
> arch_sanitise_domain_config() currently returns 0, as there is no specific
> work required at this stage.
> 
> arch_domain_create() performs basic initialization, such as setting up the P2M
> and initializing the domain's virtual timer.

Does it? I can spot only the former; instead there is ...

> --- a/xen/arch/riscv/domain.c
> +++ b/xen/arch/riscv/domain.c
> @@ -288,6 +288,33 @@ void sync_vcpu_execstate(struct vcpu *v)
>      /* Nothing to do -- no lazy switching */
>  }
>  
> +int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
> +{
> +    return 0;
> +}
> +
> +int arch_domain_create(struct domain *d,
> +                       struct xen_domctl_createdomain *config,
> +                       unsigned int flags)
> +{
> +    int rc = 0;
> +
> +    if ( is_idle_domain(d) )
> +        return 0;
> +
> +    if ( (rc = p2m_init(d)) != 0)
> +        goto fail;
> +
> +    d->arch.next_phandle = GUEST_PHANDLE_LAST + 1;

... this, which I can't make any sense of. I can't find matching Arm code
either, which might otherwise have helped.

> +    return rc;
> +
> + fail:
> +    d->is_dying = DOMDYING_dead;
> +    arch_domain_destroy(d);

(At least) for the use here, that other function would better also move out
of stubs.c at the same time (and no longer have unconditional BUG_ON() in it).

> --- a/xen/arch/riscv/stubs.c
> +++ b/xen/arch/riscv/stubs.c
> @@ -101,14 +101,7 @@ void dump_pageframe_info(struct domain *d)
>      BUG_ON("unimplemented");
>  }
>  
> -int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
> -{
> -    BUG_ON("unimplemented");
> -}
> -
> -int arch_domain_create(struct domain *d,
> -                       struct xen_domctl_createdomain *config,
> -                       unsigned int flags)
> +void vcpu_switch_to_aarch64_mode(struct vcpu *v)

What is this? Surely nothing with this name should exist under riscv/.

> --- a/xen/include/public/device_tree_defs.h
> +++ b/xen/include/public/device_tree_defs.h
> @@ -14,6 +14,7 @@
>   */
>  #define GUEST_PHANDLE_GIC (65000)
>  #define GUEST_PHANDLE_IOMMU (GUEST_PHANDLE_GIC + 1)
> +#define GUEST_PHANDLE_LAST GUEST_PHANDLE_IOMMU

This, to me, looks like a questionable addition to the public interface.
Yet I'm not a DT person, so I may simply be missing why something like
this might be wanted (and how stable it then would be, long term).

Jan