Re: [PATCH 2/2] exec: split out non-softmmu-specific parts

[Paolo Bonzini]

On 08/10/20 13:02, Claudio Fontana wrote:
>>> What is the role of this new module?
>>
>> It's [...] "4-500 lines of code for the target
>> specific parts of the CPU QOM object, plus a few functions for user-mode
>> emulation that do not have a better place".
>>
>> It's basically sitting between hw/core/cpu.c and target/*/cpu.c.  Hence
>> the non-descriptive name. :)
>>
>>> Or its this basically a "leftovers" file for which we did not find a proper 
>>> role yet?
>>
>> The user-mode parts are, but most of it is implementing the QOM CPU
>> object.  We can move those functions to hw/core/cpu.c and make that file
>> target-dependent, I wouldn't object to that.
> 
> this gives me an idea, we already basically have a target-specific part of a 
> cpu QEMU object.

Which is? :)  Sorry I don't follow.  We have one that depends on the
target architecture (methods in the CPU class), but not one that depends
on the target kind.  We could add more methods in the CPU class for
that, but I'm not sure it would be useful because (unlike CPUs of which
in theory there could be >1 class in the system) the whole emulation
_has_ to be either user-level or system.

> I basically was looking for a place to graft accelerator-specific code in 
> order to refactor target/i386/cpu..., 
> to split between tcg stuff and non-tcg stuff, and thus refactor even more 
> code.
> 
> In the past I thought to put them here for example:
> 
> diff --git a/target/i386/cpu-qom.h b/target/i386/cpu-qom.h
> index 3e96f8d668..3716c3e949 100644
> --- a/target/i386/cpu-qom.h
> +++ b/target/i386/cpu-qom.h
> @@ -72,6 +72,12 @@ typedef struct X86CPUClass {
>      DeviceRealize parent_realize;
>      DeviceUnrealize parent_unrealize;
>      DeviceReset parent_reset;
> +
> +    /* methods operating on CPUX86State */
> +    uint32_t (*cpu_compute_eflags)(CPUX86State *env);
> +    void (*cpu_set_mxcsr)(CPUX86State *env, uint32_t mxcsr);
> +    void (*cpu_set_fpuc)(CPUX86State *env, uint16_t fpuc);
> +    void (*cpu_report_tpr_access)(CPUX86State *env, TPRAccess access);
>  } X86CPUClass;
>  
>  typedef struct X86CPU X86CPU;

I think in this case you would have an X86AccelOps struct and a global
variable pointing to it.

Paolo