Re: [PATCH] tools/ocaml: implement stub_xc_domain_create() for ARM

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 16/10/2026 12:52 pm, Julian Vetter wrote:
> From: Andrii Sultanov <andriy.sultanov@xxxxxxxxxx>
>
> stub_xc_domain_create() fails unconditionally with "Unhandled: ARM" for
> the ARM variant of arch_domainconfig, so no OCaml toolstack (e.g.
> xenopsd) can create an ARM domain.
>
> To fix this, convert the two fields of xen_arm_arch_domainconfig,
> gic_version and nr_spis, into the corresponding fields of struct
> xen_arch_domainconfig and reject values that do not fit, as is already
> done for other parameters. The remaining fields (sve_vl, tee_type,
> arm_sci_type and v8r_el1_msa) aren't exposed to OCaml yet, so they stay
> zero.

type xen_arm_arch_domainconfig contains clock_frequency: int32; but
you've just removed that from the hypercall.

I suggest that we use the fact that this hypercall doesn't work on ARM
at all to allow a minor breakage to remove that too.  It will want to be
a prerequisite to this patch.

>
> Also add a BUILD_BUG_ON() on the size of the createdomain struct, just
> like the x86 case, to catch future ABI changes.
>
> Signed-off-by: Andrii Sultanov <andriy.sultanov@xxxxxxxxxx>
> Signed-off-by: Julian Vetter <julian.vetter@xxxxxxxxxx>
> ---
>  tools/ocaml/libs/xc/xenctrl_stubs.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c 
> b/tools/ocaml/libs/xc/xenctrl_stubs.c
> index 180f2fbf45..5fa67cdf2f 100644
> --- a/tools/ocaml/libs/xc/xenctrl_stubs.c
> +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
> @@ -257,8 +257,34 @@ CAMLprim value stub_xc_domain_create(value xch_val, 
> value wanted_domid, value co
>       arch_domconfig = Field(VAL_ARCH, 0);
>       switch ( Tag_val(VAL_ARCH) )
>       {
> -     case 0: /* ARM - nothing to do */
> +     case 0: /* ARM - GIC version and number of SPIs in the block */

In hindsight, I think these comments are unhelpful, because all they'll
do is go stale.

Other than "not supported yet", I think these ought to just be the tag
names.  So, drop this to just /* ARM */.

> +#if defined(__arm__) || defined(__aarch64__)
> +     {
> +             /* Quick & dirty check for ABI changes. */
> +             BUILD_BUG_ON(sizeof(cfg) != 72);
> +
> +             /* Mnemonics for the named fields inside 
> xen_arm_arch_domainconfig */
> +#define VAL_GIC_VERSION         Field(arch_domconfig, 0)
> +#define VAL_NR_SPIS             Field(arch_domconfig, 1)
> +
> +             intnat gic_version = Int_val(VAL_GIC_VERSION);
> +             intnat nr_spis = Int_val(VAL_NR_SPIS);

This particular mix is buggy (not that it's obvious; Ocaml's API hide a
large number of spiketraps.)

Int_val(x) is ((int)Long_val(x)) so this expression is explicitly
truncating to int before widening back to long.

Either use Long_val(), or a narrower type.

> +
> +             if ( gic_version != (uint8_t)gic_version )
> +                     caml_invalid_argument("gic_version");
> +
> +             if ( nr_spis < 0 || nr_spis != (uint32_t)nr_spis )
> +                     caml_invalid_argument("nr_spis");

With the above, this needs to be the narrower type with:

    if ( $VAR != Long_val($VAL) )
        ...

which is the only way to see if any bits have gone missing from the
Ocaml representation.  I bet we have similar bugs elsewhere in the bindings.

~Andrew