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

[Claudio Fontana]

On 10/8/20 9:56 AM, Paolo Bonzini wrote:
> On 08/10/20 09:47, Claudio Fontana wrote:
>> On 10/6/20 11:19 AM, Paolo Bonzini wrote:
>>> Over the years, most parts of exec.c that were not specific to softmmu
>>> have been moved to accel/tcg; what's left is mostly the low-level part
>>> of the memory API, which includes RAMBlock and AddressSpaceDispatch.
>>> However exec.c also hosts 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 (they are not TCG-specific so
>>> accel/tcg/user-exec.c is not a good place either).
>>>
>>> Move these parts to a new file, so that exec.c can be moved to
>>> softmmu/physmem.c.
>>>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>
>> Hi Paolo,
>>
>> the comment does not talk about cpu.c, which is now created in the top 
>> source directory.
>> What is the role of this new module?
> 
> It's actually in the commit message: "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. :)
> 
>> Also, could we find a more descriptive file name than cpu.c?
>> Do you plan further renaming of this new module functions?
>>
>> 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.  But since there are some
> opportunities for simplification, I'd rather do that in a separate patch
> and keep the pure code-movement in this one.
> 
> Paolo
> 

Ciao Paolo,

this gives me an idea, we already basically have a target-specific part of a 
cpu QEMU object.

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;


But maybe that is the right component?

Just thinking out loud here.

Ciao,

Claudio