"Bruno Larsen (billionai)" <bruno.lar...@eldorado.org.br> writes:
A general advice for this whole series is: make sure you add in some words explaining why you decided to make a particular change. It will be much easier to review if we know what were the logical steps leading to the change. > This commit does the necessary code motion from translate_init.c.inc For instance, I don't immediately see why these changes are necessary. I see that translate_init.c.inc already has some `#ifdef CONFIG_TCG`, so why do we need to move a bunch of code into cpu.c instead of just adding more code under ifdef CONFIG_TCG? (I'm not saying it's wrong, just trying to understand the reasoning). Is translate_init.c.inc intended to be TCG only? But then I see you moved TCG-only functions out of it (ppc_fixup_cpu) and left not TCG-only functions (gen_spr_generic). > This moves all functions that start with gdb_* into target/ppc/gdbstub.c > and creates a new function that calls those and is called by ppc_cpu_realize This looks like it makes sense regardless of disable-tcg, could we have it in a standalone patch? > All functions related to realizing the cpu have been moved to cpu.c, which > may call functions from gdbstub or translate_init Again, I don't disagree with this, but at first sight it doesn't seem entirely related to disabling TCG.
