On Thu, 30 Jun 2016 09:34:35 +0200 Igor Mammedov <imamm...@redhat.com> wrote:
> On Wed, 29 Jun 2016 22:50:06 +0200 > Greg Kurz <gr...@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, I fully agree ! > 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 Looking again at patches 5/8 and 6/8, I'm more than convinced, yeah :) > - do not use cpu_init() in machine code, leave it for creating a CPU only > for usermode Makes sense, cpu_ppc_init() should only be used by usermode. I'll start with that I guess. > - make target code use dt_id_property instead of cpu_index internally Agreed. > - 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 With the recent addition of CPU hotplug, the latest pseries machine type is now constructing cpus that way. The idea in patches 7-8/8 was to provide a helper to do the same for other machines. > - 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'll have look. Thanks a lot for your feedback ! Cheers. -- Greg > > 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 > > > > >
