Re: [PATCH 2/2] tools/ocaml: add bindings for the paging mempool size

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 22/10/2026 3:27 pm, Julian Vetter wrote:
> The OCaml bindings only expose the shadow_allocation_{get,set}
> functions, which use XEN_DOMCTL_SHADOW_OP_{GET,SET}_ALLOCATION. These
> are x86-only, on ARM they fail with -ENOSYS. So, an OCaml toolstack has
> no way to size the P2M pool of an ARM domain, which libxl does via
> XEN_DOMCTL_set_paging_mempool_size.
>
> So, add a pair of functions paging_mempool_size_{get,set}, which just
> wrap xc_{get,set}_paging_mempool_size().
>
> Signed-off-by: Julian Vetter <julian.vetter@xxxxxxxxxx>
> ---
>  tools/ocaml/libs/xc/xenctrl.ml      |  5 +++++
>  tools/ocaml/libs/xc/xenctrl.mli     |  4 ++++
>  tools/ocaml/libs/xc/xenctrl_stubs.c | 33 +++++++++++++++++++++++++++++
>  3 files changed, 42 insertions(+)
>
> diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
> index 37e41879bb..55cfc2e9a2 100644
> --- a/tools/ocaml/libs/xc/xenctrl.ml
> +++ b/tools/ocaml/libs/xc/xenctrl.ml
> @@ -283,6 +283,11 @@ external shadow_allocation_set: handle -> domid -> int 
> -> unit
>  external shadow_allocation_get: handle -> domid -> int
>    = "stub_shadow_allocation_get"
>  
> +external paging_mempool_size_set: handle -> domid -> int64 -> unit
> +  = "stub_xc_set_paging_mempool_size"
> +external paging_mempool_size_get: handle -> domid -> int64
> +  = "stub_xc_get_paging_mempool_size"
> +
>  external evtchn_alloc_unbound: handle -> domid -> domid -> int
>    = "stub_xc_evtchn_alloc_unbound"
>  external evtchn_reset: handle -> domid -> unit = "stub_xc_evtchn_reset"
> diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
> index 7a0dd7d8ba..0d5cc94198 100644
> --- a/tools/ocaml/libs/xc/xenctrl.mli
> +++ b/tools/ocaml/libs/xc/xenctrl.mli
> @@ -210,6 +210,10 @@ external shadow_allocation_set : handle -> domid -> int 
> -> unit
>    = "stub_shadow_allocation_set"
>  external shadow_allocation_get : handle -> domid -> int
>    = "stub_shadow_allocation_get"
> +external paging_mempool_size_set : handle -> domid -> int64 -> unit
> +  = "stub_xc_set_paging_mempool_size"
> +external paging_mempool_size_get : handle -> domid -> int64
> +  = "stub_xc_get_paging_mempool_size"
>  external evtchn_alloc_unbound : handle -> domid -> domid -> int
>    = "stub_xc_evtchn_alloc_unbound"
>  external evtchn_reset : handle -> domid -> unit = "stub_xc_evtchn_reset"
> diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c 
> b/tools/ocaml/libs/xc/xenctrl_stubs.c
> index e0c435e346..ef3d16beaa 100644
> --- a/tools/ocaml/libs/xc/xenctrl_stubs.c
> +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
> @@ -1259,6 +1259,39 @@ CAMLprim value stub_shadow_allocation_set(value 
> xch_val, value domid,
>       CAMLreturn(Val_unit);
>  }
>  
> +CAMLprim value stub_xc_get_paging_mempool_size(value xch_val, value domid)
> +{
> +     CAMLparam2(xch_val, domid);
> +     xc_interface *xch = xch_of_val(xch_val);
> +     uint64_t c_size;
> +     int ret;
> +
> +     caml_enter_blocking_section();
> +     ret = xc_get_paging_mempool_size(xch, Int_val(domid), &c_size);
> +     caml_leave_blocking_section();
> +     if (ret != 0)
> +             failwith_xc(xch);
> +
> +     CAMLreturn(caml_copy_int64(c_size));
> +}
> +
> +CAMLprim value stub_xc_set_paging_mempool_size(value xch_val, value domid,
> +                                            value size)
> +{
> +     CAMLparam3(xch_val, domid, size);
> +     xc_interface *xch = xch_of_val(xch_val);
> +     uint64_t c_size = Int64_val(size);
> +     int ret;
> +
> +     caml_enter_blocking_section();
> +     ret = xc_set_paging_mempool_size(xch, Int_val(domid), c_size);
> +     caml_leave_blocking_section();
> +     if (ret != 0)
> +             failwith_xc(xch);
> +
> +     CAMLreturn(Val_unit);
> +}

I personally would consider these bindings fine.  But if we were to
accept the code like this, we'd have to make a change in due course.


It is not safe to use Int_val() inside a blocking section like this. 
Well, it is actually safe, but it also triggers ThreadSanitiser and
therefore the Ocaml devs have declared it to be unsafe.

During the mark/sweep phases, the Ocaml GC writes back every single
value, even the non-pointer ones (despite not changing value).  And
instead of fixing this behaviour, it's been declared to be "correct"
behaviour, forcing every C binding to be edited to avoid the pattern.

So, you need an `uint32_t c_domid = Long_val(domid)` outside of the
blocking section.  At some point we need to fix this pattern everywhere
else in our bindings.

Thinking about it further, we also need a truncation check on domid,
because a 64bit Ocaml could pass 0x100000000 which would get truncated
to 0 passing through the bindings.

~Andrew