Re: [PATCH] tools/ocaml/xc: Drop the GC lock for all hypercalls

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 02/09/2024 9:10 am, Edwin Torok wrote:
> On Fri, Aug 30, 2024 at 6:53 PM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> 
> wrote:
>> We should be doing this unilaterally.
> Agreed, but we should do it safely, since last time I did this I
> learned about a few more instances of behaviours I previously thought
> to be safe, but that are undefined behaviour.
> Which probably means we have a bunch of other code to fixup (I should
> really finish my static analyzer project, and update it with the newly
> learned rules to catch all these...).
> See below for comments.
>
> Although there is one bug here we've previously known to avoid:
> String_val cannot be dereferenced with the lock released, that one is
> an OCaml value and will cause actual problems,
> so we need to caml_copy_string that one.
>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> ---
>> CC: Christian Lindig <christian.lindig@xxxxxxxxxx>
>> CC: David Scott <dave@xxxxxxxxxx>
>> CC: Edwin Török <edwin.torok@xxxxxxxxx>
>> CC: Rob Hoes <Rob.Hoes@xxxxxxxxxx>
>> CC: Andrii Sultanov <andrii.sultanov@xxxxxxxxx>
>>
>> Also pulled out of a larger cleanup series.
>> ---
>>  tools/ocaml/libs/xc/xenctrl_stubs.c | 63 +++++++++++++++++++++++++++--
>>  1 file changed, 60 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c 
>> b/tools/ocaml/libs/xc/xenctrl_stubs.c
>> index c78191f95abc..20487b21008f 100644
>> --- a/tools/ocaml/libs/xc/xenctrl_stubs.c
>> +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
>> @@ -312,7 +312,10 @@ CAMLprim value stub_xc_domain_max_vcpus(value xch_val, 
>> value domid,
>>         xc_interface *xch = xch_of_val(xch_val);
>>         int r;
>>
>> +       caml_enter_blocking_section();
>>         r = xc_domain_max_vcpus(xch, Int_val(domid), Int_val(max_vcpus));
> We need to move the Int_val macros out of here, domid is registered as
> a GC root, so the GC *will* write to it (it'll write the same value).

How?

As value's, both domid and max_vcpu are integers living in GPRs.

These expressions are not dereferencing  into the Ocaml Heap.

~Andrew