Re: [PATCH 03/10] accel/tcg: move i386 halt handling to sysemu_ops

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 20/3/23 16:23, Claudio Fontana wrote:
> Hi Alex, all,
> 
> again, this moves TCG-only code to common code, no?

Oh, good point.

> Even if this happens to work, the idea is to avoid adding unneeded accel TCG code to a KVM-only binary.

Could yet another AccelSysemuCPUOps *accel struct in SysemuCPUOps
help being stricter? ...

> We need to keep in mind all dimensions when we do refactorings:
> 
> user-mode vs sysemu,
> the architecture,
> the accel, in particular tcg, non-tcg (which could be not compiled in, built-in, or loaded as separate module).
> 
> In many cases, testing with --disable-tcg --enable-kvm helps to avoid breakages,
> but it is possible also to move in unneeded code in a way that does not generate compile or link-time errors, so we need to be a bit alert to that.
> 
> Ciao,
> 
> C
> 
> 
> On 3/20/23 11:10, Alex Bennée wrote:
>> We don't want to be polluting the core run loop code with target
>> specific handling, punt it to sysemu_ops where it belongs.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>   include/hw/core/sysemu-cpu-ops.h |  5 +++++
>>   target/i386/cpu-internal.h       |  1 +
>>   accel/tcg/cpu-exec.c             | 14 +++-----------
>>   target/i386/cpu-sysemu.c         | 12 ++++++++++++
>>   target/i386/cpu.c                |  1 +
>>   5 files changed, 22 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
>> index ee169b872c..c9d30172c4 100644
>> --- a/include/hw/core/sysemu-cpu-ops.h
>> +++ b/include/hw/core/sysemu-cpu-ops.h
>> @@ -48,6 +48,11 @@ typedef struct SysemuCPUOps {
>>        * GUEST_PANICKED events.
>>        */
>>       GuestPanicInformation* (*get_crash_info)(CPUState *cpu);
>> +    /**
>> +     * @handle_cpu_halt: Callback for special handling during cpu_handle_halt()
>> +     * @cs: The CPUState
>> +     */

Perhaps insert within a 'tcg' structure for now.

     #ifdef CONFIG_TCG
     struct {

>> +    void (*handle_cpu_halt)(CPUState *cpu);

     } tcg;
     #endif

Then we could extract as accel.

>>       /**
>>        * @write_elf32_note: Callback for writing a CPU-specific ELF note to a
>>        * 32-bit VM coredump.