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

[Claudio Fontana]

On 10/8/20 1:36 PM, Paolo Bonzini wrote:
> 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
> 
> 

I was hoping to make use of some of the object model ... but lets get to this 
when we get to this later on.

Ciao, thanks!

Claudio