Re: modular tcg

From: Claudio Fontana <cfontana@suse.de>
To: Gerd Hoffmann <kraxel@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org, richard.henderson@linaro.org,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Jose R. Ziviani" <jziviani@suse.de>
Subject: Re: modular tcg
Date: Fri, 30 Jul 2021 12:02:18 +0200	[thread overview]
Message-ID: <9d4a9fde-2fd4-a7d3-4d8a-62a29ccc28d0@suse.de> (raw)
In-Reply-To: <20210730090538.ypohhzrd4nzqngk6@sirius.home.kraxel.org>

On 7/30/21 11:05 AM, Gerd Hoffmann wrote:
>   Hi,
> 
> On Thu, Jul 29, 2021 at 06:40:17PM +0200, Paolo Bonzini wrote:
>> On 29/07/21 11:14, Gerd Hoffmann wrote:
>>> The common functions could be added TCGCPUOps to allow them being called via
>>> CPUClass->tcg_ops->$name instead of a direct symbol reference.  Not sure this
>>> is a good idea though.  Experimental patch covering tcg_exec_realizefn +
>>> tcg_exec_unrealizefn below.
>>
>> A lot of these (though probably not all) are already stubbed out as "static
>> inline" in include/exec/exec-all.h.  I think they can be changed to function
>> pointers in the style of ui/spice-module.c (accel/tcg/tcg-module.c?).
> 
> Yep, was thinking about that too.  But then I noticed we already have
> TCGCPUOps and wondered whenever extending that would be the better
> option.

I'd read TCGCPUOps as TCG CPU-specific Operations in this case,
IIRC they are for the TCG Operations that have cpu target-specific behavior.

Maybe they should be called TCGCPUTargetOps to avoid confusion?

So, a TCGCPUOps will have an arm implementation, x86 implementation and so on.

In my view tcg_exec_realizefn does not fit, as this is just a set of additional
generic non-target-specific operations for the generic cpu_exec_realizefn() that needs to be called specifically and only for TCG.

> 
>>> No idea yet how to handle arch-specific bits best.  Seems there is no existing
>>> infrastructure to untangle target-specific code and tcg, so this probably needs
>>> something new.
>>
>> If they are few enough, I would just move them out of target/i386/tcg and
>> into target/i386/cpu-sysemu.c.
> 
> I'll have a look.
> 
>>> Noticed softmmu/physmem.c has lots of CONFIG_TCG #ifdefs, splitting this into
>>> softmmu/physmem-{common,tcg}.c is probably a good idea.
>>
>> I only count one, and those function should be easily moved  to
>> accel/tcg/cputlb.c (after all both physmem.c and cputlb.c used to be a
>> single file, exec.c, so this is just an oversight).
> 
> Well, I noticed one larger block covering multiple functions, didn't
> check the whole file ...
> 
> thanks & take care,
>   Gerd
> 