Re: [PATCH 1/2] accel/tcg: add get_virtual_clock for TCG

["Alex Bennée" <alex.bennee@linaro.org>]

Mark Burton <mburton@qti.qualcomm.com> writes:

> In principle I like this, but 
> 1/ throughout the API can we please make everything consistent sure that all registrations take a handle (void *) and all callbacks functions pass that handle (and the ID)
>  - right now, some things do, some things dont, and this specific case
> seems to take a handle on registration, but does not provide it on
> callback (!)

The handle is something the plugin should have already. The plugin id is
needed so the framework knows who to deliver the callback back to.

>
> (This is the current implementation :
> typedef int64_t (*qemu_plugin_time_cb_t) (void);
> ...
> QEMU_PLUGIN_API void qemu_plugin_register_time_cb(qemu_plugin_id_t id, const void *handle, qemu_plugin_time_cb_t cb);
> )
>
> 2/ The current implementation makes use of the callback _ONLY_ in the
> case of single TCG — it’s most interesting when we have MTTCG enabled

Ahh - as I said compile tested only ;-)

I can fix that for v2.


> (and I see no reason not to provide the same mechanism for any other
> accelerator if/when anything in QEMU requests ’the time’.

That would mean making a clear separation in plugins for things that are
"events" which we do do from other hypervisors and "instrumentation"
which can only be done under TCG.


> 
>
> Cheers
> Mark.
>
>
>> On 3 Apr 2025, at 13:38, Alex Bennée <alex.bennee@linaro.org> wrote:
>> 
>> WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.
>> 
>> Rather than allowing cpus_get_virtual_clock() to fall through to
>> cpu_get_clock() introduce a TCG handler so it can make a decision
>> about what time it is.
>> 
>> Initially this just calls cpu_get_clock() as before but this will
>> change in later commits.
>> 
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>> accel/tcg/tcg-accel-ops.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>> 
>> diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
>> index d9b662efe3..1432d1c5b1 100644
>> --- a/accel/tcg/tcg-accel-ops.c
>> +++ b/accel/tcg/tcg-accel-ops.c
>> @@ -197,6 +197,11 @@ static inline void tcg_remove_all_breakpoints(CPUState *cpu)
>>     cpu_watchpoint_remove_all(cpu, BP_GDB);
>> }
>> 
>> +static int64_t tcg_get_virtual_clock(void)
>> +{
>> +    return cpu_get_clock();
>> +}
>> +
>> static void tcg_accel_ops_init(AccelOpsClass *ops)
>> {
>>     if (qemu_tcg_mttcg_enabled()) {
>> @@ -212,6 +217,7 @@ static void tcg_accel_ops_init(AccelOpsClass *ops)
>>             ops->get_virtual_clock = icount_get;
>>             ops->get_elapsed_ticks = icount_get;
>>         } else {
>> +            ops->get_virtual_clock = tcg_get_virtual_clock;
>>             ops->handle_interrupt = tcg_handle_interrupt;
>>         }
>>     }
>> --
>> 2.39.5
>> 

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro