Re: [Qemu-devel] [PATCH 0/8] ppc: compute cpu_dt_id in the machine code

[Igor Mammedov <imammedo@redhat.com>]

On Wed, 29 Jun 2016 22:50:06 +0200
Greg Kurz <groug@kaod.org> wrote:

> This series is a sequel to the discussion on a patch from Ben's powernv
> patchset:
> 
>         http://patchwork.ozlabs.org/patch/597153/
> 
> Indeed, since the DT is a machine abstraction, it should definitely sit
> under hw/ppc and not in the target code:
> - all machine types are forced to share the same numbering logic
> - user mode does not need that => there are #ifdef everywhere
> 
> So this series moves all the current numbering logic to the machine
> code. It also provides the ability for each machine type to provide
> its own numbering logic instead of using the legacy one.
> 
> The current code computes cpu_dt_id out of cpu_index, both getting
> their values during cpu realization.
>
> The idea is to compute cpu_index at cpu initialization time, so
> that the machine can compute cpu_dt_id before realizing the cpu.
Series goes to considerable lengths to reuse cpu_index,
maybe other way around would be better (like we did for x86) where
the end goal was the same as here i.e. make machine code assign/manage
apic_id (x86 equivalent of cpu_dt_id). So what we did to make it happen.

 - do not try to reuse cpu_index and leave it alone
 - do not use cpu_init() in machine code, leave it for creating a CPU only for usermode
 - make target code use dt_id_property instead of cpu_index internally
 - make machine code to construct cpu manually
     cpu = object_new(CPU)
     set_dt_id_property(cpu)
     set other properties if needed
     realize(cpu)
   see pc_new_cpu() as example
 - make machine code do mgmt/bookkeeping of DT_IDs
     possibly you could reuse CPUArchIdList for it,
     see pc_cpus_init() + usage of pcms->possible_cpus

> I had to fix some issues and to do some more structural changes to have
> this working, hence the RFC tag. The first 4 patches are bug fixes. The
> next ones do what the subject says.
> 
> This patchset is based on David's ppc-for-2.7 branch. I could test it plays
> nicely with Bharata's CPU hotplug code for sPAPR.
> 
> There are some more places where the cpu_dt_id logic is open coded in the
> sPAPR code. It may be worth to consolidate that in follow-up patches. Also
> maybe other machine types may provide their own numbering logic if the
> current one is not appropriate.
> 
> Please comment.
> 
> ---
> 
> Greg Kurz (8):
>       spapr: drop reference on child object during core realization
>       spapr: do proper error propagation in spapr_cpu_core_realize_child()
>       spapr: drop duplicate variable in spapr_core_release()
>       exec: add missing conditional compilation
>       exec: move cpu_index init and exit to their own function
>       ppc: move cpu index setup to instance_init/finalize
>       cpu: add initialization helper without realize
>       hw/ppc: move DT cpu id generation to machine code
> 
> 
>  exec.c                      |   72 +++++++++++++++++++++++++++++++++----------
>  hw/ppc/e500.c               |    2 +
>  hw/ppc/mac_newworld.c       |    2 +
>  hw/ppc/mac_oldworld.c       |    2 +
>  hw/ppc/ppc.c                |   60 ++++++++++++++++++++++++++++++++++++
>  hw/ppc/ppc440_bamboo.c      |    2 +
>  hw/ppc/ppc4xx_devs.c        |    2 +
>  hw/ppc/prep.c               |    2 +
>  hw/ppc/spapr.c              |    2 +
>  hw/ppc/spapr_cpu_core.c     |   23 ++++++++++----
>  hw/ppc/virtex_ml507.c       |    2 +
>  include/exec/exec-all.h     |    2 +
>  include/hw/ppc/ppc.h        |    4 ++
>  include/qom/cpu.h           |   14 ++++++++
>  qom/cpu.c                   |   19 ++++++++++-
>  target-ppc/translate_init.c |   42 ++++++-------------------
>  16 files changed, 185 insertions(+), 67 deletions(-)
> 
> --
> Greg
> 
>