Re: [Qemu-devel] [PATCH 1/1] target-i386: Remove icc_bridge parameter from cpu_x86_create()

[Eduardo Habkost]

On Tue, Mar 10, 2015 at 01:51:26PM +0100, Andreas Färber wrote:
> Am 10.03.2015 um 13:42 schrieb Eduardo Habkost:
> > On Tue, Mar 10, 2015 at 12:50:07PM +0100, Andreas Färber wrote:
> >> Am 05.03.2015 um 18:26 schrieb Eduardo Habkost:
> >>> Instead of passing icc_bridge from the PC initialization code to
> >>> cpu_x86_create(), make the PC initialization code attach the CPU to
> >>> icc_bridge.
> >>>
> >>> The only difference here is that icc_bridge attachment will now be done
> >>> after x86_cpu_parse_featurestr() is called. But this shouldn't make any
> >>> difference, as property setters shouldn't depend on icc_bridge.
> >>>
> >>> Signed-off-by: Eduardo Habkost <address@hidden>
> >>
> >> Looks okay to me,
> >>
> >> Reviewed-by: Andreas Färber <address@hidden>
> >>
> >> But using this smaller patch will still make inlining pc_new_cpu(),
> >> where you are moving it to, bigger diffstat-wise (WIP).
> > 
> > I just see this it as a reason to not inline pc_new_cpu(). :)
> 
> Did you actually read my code? cpu_x86_create() does not allow in-place
> initialization, in addition to doing the realized=true.

I hadn't, sorry, I was simply thinking in general terms, where I
wouldn't want to inline a function if that would mean duplicating code.

I did read the qom-cpu-x86 code now, and I still don't see why you would
want to duplicate existing pc_new_cpu() code that is needed on both call
sites. I assume there is a way to avoid code duplication and still allow
in-place initialization.

Anyway, this discussion about diff sizes and duplication will be
obsolete as soon as we apply a new version of the icc-bus removal patch.
So I would prefer to discuss the details once we have a new patch

-- 
Eduardo