Re: [PATCH v2 02/14] plugins: scoreboard API

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 1/31/24 11:44, Pierrick Bouvier wrote:
> On 1/26/24 19:14, Alex Bennée wrote:
>>> +        need_realloc = TRUE;
>>> +    }
>>> +    plugin.scoreboard_size = cpu->cpu_index + 1;
>>> +    g_assert(plugin.scoreboard_size <= plugin.scoreboard_alloc_size);
>>> +
>>> +    if (g_hash_table_size(plugin.scoreboards) == 0) {
>>> +        /* nothing to do, we just updated sizes for future scoreboards */
>>> +        return;
>>> +    }
>>> +
>>> +    if (need_realloc) {
>>> +#ifdef CONFIG_USER_ONLY
>>> +        /**
>>> +         * cpus must be stopped, as some tb might still use an existing
>>> +         * scoreboard.
>>> +         */
>>> +        start_exclusive();
>>> +#endif
>>
>> Hmm this seems wrong to be USER_ONLY. While we don't expect to resize in
>> system mode if we did we certainly want to do it during exclusive
>> periods.
>>
> 
> After investigation, current_cpu TLS var is not set in cpus-common.c at
> this point.
> 
> Indeed we are not on any cpu_exec path, but in the cpu_realize_fn when
> calling this (through qemu_plugin_vcpu_init_hook).
> 
> One obvious fix is to check if it's NULL or not, like:
> --- a/cpu-common.c
> +++ b/cpu-common.c
> @@ -193,7 +193,7 @@ void start_exclusive(void)
>        CPUState *other_cpu;
>        int running_cpus;
> 
> -    if (current_cpu->exclusive_context_count) {
> +    if (current_cpu && current_cpu->exclusive_context_count) {
>            current_cpu->exclusive_context_count++;
>            return;
>        }
> 
> Does anyone suggest another possible fix? (like define current_cpu
> somewhere, or moving qemu_plugin_vcpu_init_hook call).

Running init_hook asynchronously on cpu works and solves the problem, 
without any need to modify start/end exclusive code.