On 12/12/20 11:00 AM, Claudio Fontana wrote: > On 12/11/20 9:02 PM, Eduardo Habkost wrote: >> On Fri, Dec 11, 2020 at 07:51:54PM +0100, Claudio Fontana wrote: >>> On 12/11/20 7:26 PM, Philippe Mathieu-Daudé wrote: >>>> On 12/11/20 7:22 PM, Richard Henderson wrote: >>>>> On 12/11/20 12:15 PM, Claudio Fontana wrote: >>>>>> Should I return this file to the original state (without the extra >>>>>> #includes that pretend it to be a standalone header file, >>>>>> and call it >>>>>> >>>>>> tcg-cpu-ops.h.inc >>>>>> >>>>>> ? >>>>> >>>>> If this header can work with qemu/typedefs.h, then no, because the >>>>> circularity >>>>> has been resolved. Otherwise, yes. >>>> >>>> My editor got confused with TranslationBlock, which is why I asked >>>> to include its declaration. >>>> >>>> Easier to forward-declare TranslationBlock in qemu/typedefs.h? >>>> >>>> Regards, >>>> >>>> Phil. >>>> >>> >>> Hello Philippe, >>> >>> ok you propose to move the existing fwd declaration of TranslationBlock >>> from cpu.h to qemu/typedefs.h . >> >> It seems simpler to just add a >> >> typedef struct TranslationBlock TranslationBlock; >> >> line to tcg-cpu-ops.h. >> >> Or, an even simpler solution: just use `struct TranslationBlock` >> instead of `TranslationBlock` in the declarations being moved to >> tcg-cpu-ops.h. >> >> We don't need to move declarations to typedefs.h anymore, because >> now the compilers we support don't warn about typedef >> redefinitions: >> https://lore.kernel.org/qemu-devel/20200914134636.gz1618...@habkost.net/ >> >> >>> >>> And what about #include "exec/memattrs.h"? >>> >>> I assume you propose to put struct MemTxAttrs there as a fwd declaration >>> too, >> >> This can't be done, because MemTxAttrs can't be an incomplete >> type in the code you are moving (the methods get a MemTxAttrs >> value, not a pointer). > > > > I'm confused now on what we are trying to do: if we want the file to be a > "proper header" or just a TCG-ops-only convenience split of cpu.h. > > I thought that we were only solving a highlighting issue in some editor > (Philippe), > and I wonder if these changes in qemu/typedef.h help with that? > > I tried adding both to qemu/typedef.h, and since cpu.h is the only user of > the file, and it already includes memattrs.h, everything is fine. > > But here maybe you are proposing to make it a regular header, and include > this instead of just hw/core/cpu.h in the targets? > > I am thinking whether it is the case to scrap this whole mess, make TCGCPUOps > a pointer in CPUClass, and in the targets say for example: > > #include "tcg-cpu-ops.h" > > ... > > +static struct TCGCPUOps cris_tcg_ops = { > + .initialize = cris_initialize_tcg, > +}; > + > static void cris_cpu_class_init(ObjectClass *oc, void *data) > { > DeviceClass *dc = DEVICE_CLASS(oc); > @@ -284,7 +292,7 @@ static void cris_cpu_class_init(ObjectClass *oc, void > *data) > cc->gdb_stop_before_watchpoint = true; > > cc->disas_set_info = cris_disas_set_info; > - cc->tcg_ops.initialize = cris_initialize_tcg; > + cc->tcg_ops = &cris_tcg_ops; > } > > > What do you all think of this? > > Thanks, > > Claudio
Not sure it solves all problems: the MMUAccessType is still a cpu.h enum, so we are back to the circular dependency. Will try the .inc in the next spin, and I hope that the discussion can go on from there, with Eduardo, Philippe and Richard laying out more clearly what your requirements are. Thanks, Claudio