[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 04/11] target-i386: Rename cpu_x86_init() to cpu_
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PULL 04/11] target-i386: Rename cpu_x86_init() to cpu_x86_init_user() |
Date: |
Fri, 27 Feb 2015 20:10:40 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 |
Am 27.02.2015 um 20:01 schrieb 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.
Yes, that's why it's not done yet, it takes a lot of review and testing.
It is not going to be a pure Coccinelle patch. :) Doing that for the
inlining is an idea I had not yet considered, thanks.
On my qom-cpu-x86 branch I have already moved realized=true for the PC,
in order to make a CPU core object its parent.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
[Qemu-devel] [PULL 02/11] target-i386: Eliminate unnecessary get_cpuid_vendor() function, Eduardo Habkost, 2015/02/25
[Qemu-devel] [PULL 05/11] target-i386: Eliminate cpu_init() function, Eduardo Habkost, 2015/02/25
[Qemu-devel] [PULL 06/11] target-i386: Simplify error handling on cpu_x86_init_user(), Eduardo Habkost, 2015/02/25
[Qemu-devel] [PULL 08/11] linux-user: Check for cpu_init() errors, Eduardo Habkost, 2015/02/25