Re: [Qemu-devel] [PULL 04/11] target-i386: Rename cpu_x86_init() to cpu_x86_init_user()

[Eduardo Habkost]

On Fri, Feb 27, 2015 at 03:10:27PM +0100, Andreas Färber wrote:
> Am 26.02.2015 um 16:59 schrieb Eduardo Habkost:
> > On Wed, Feb 25, 2015 at 11:06:55PM +0100, Andreas Färber wrote:
> >> Am 25.02.2015 um 20:58 schrieb Eduardo Habkost:
> >>> The function is used only for CONFIG_USER, so make its purpose clear.
> >>>
> >>> Reviewed-by: Paolo Bonzini <address@hidden>
> >>> Signed-off-by: Eduardo Habkost <address@hidden>
> >>> ---
> >>>  target-i386/cpu.c | 2 +-
> >>>  target-i386/cpu.h | 4 ++--
> >>>  2 files changed, 3 insertions(+), 3 deletions(-)
> >>
> >> Please don't. I happily got all architectures aligned that it's at least
> >> cpu_something_init, and it only happens to be user-only for x86. It is
> >> rather the legacy function that was used in both system and user.
> > 
> > If that's a legacy function, what are the steps you plan to follow to
> > eliminate it? I would be glad to help eliminating legacy code.
> > 
> > Initialization of CPUs in *-user and *-softmmu is different in i386, so
> > we are going to have different code for both. How do you think I should
> > name the *-user-specific CPU init function in target-i386, then?
> 
> I would prefer to leave its name as it is (unless we are renaming all,
> which would probably be a waste of effort giving the next steps) and
> simply not use it in PC code. If you want to enforce this, you could
> #ifdef CONFIG_USER_ONLY it.
> 
> For some targets - as can be seen in your uc32 patch - there is already
> a cpu_generic_init() that calls into the CPUClass hooks of the given CPU
> type. I would like to call that from linux-user directly (or from a
> lightweight wrapper to be shared between linux-user and bsd-user, I
> assume we're going need some target-specific #ifdefs) and drop
> cpu_init() in its current form. In particular I want to somehow move the
> realized=true part out of it, which means either inlining it into dozens
> of machines or finishing the recursive realization work so that we only
> need one central realized=true for /machine.

Moving realized=true outside cpu_generic_init() would make lots of sense
for PC, as we already need to perform additional steps in the PC init
code before realizing the CPU (and I expect the list of PC-specific CPU
initialization steps to grow). And when we do this, cpu_x86_init()
(used by CONFIG_USER) and cpu_x86_create() (used by PC) can become the
same function.

But I don't see how to implement this without requiring changing machine
code case-by-case, as existing machine code may expect realized=true to
be set very early and break if you don't set it immediately after
cpu_*_init(). In other words, even if we implement recursive
realization, we may need to inline realize=true on all machines as an
intermediate (and automated) step, and then change machines to just rely
on recursive realization, one by one.

-- 
Eduardo