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

[Philippe Mathieu-Daudé]

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.