[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PATCH 0/8] ppc: compute cpu_dt_id in the ma
From: |
Greg Kurz |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PATCH 0/8] ppc: compute cpu_dt_id in the machine code |
Date: |
Thu, 30 Jun 2016 11:43:28 +0200 |
On Thu, 30 Jun 2016 09:34:35 +0200
Igor Mammedov <address@hidden> wrote:
> On Wed, 29 Jun 2016 22:50:06 +0200
> Greg Kurz <address@hidden> 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
> >
> >
>
- [Qemu-ppc] [PATCH 3/8] spapr: drop duplicate variable in spapr_core_release(), (continued)
- [Qemu-ppc] [PATCH 3/8] spapr: drop duplicate variable in spapr_core_release(), Greg Kurz, 2016/06/29
- [Qemu-ppc] [PATCH 4/8] exec: add missing conditional compilation, Greg Kurz, 2016/06/29
- [Qemu-ppc] [PATCH 5/8] exec: move cpu_index init and exit to their own function, Greg Kurz, 2016/06/29
- [Qemu-ppc] [PATCH 6/8] ppc: move cpu index setup to instance_init/finalize, Greg Kurz, 2016/06/29
- [Qemu-ppc] [PATCH 7/8] cpu: add initialization helper without realize, Greg Kurz, 2016/06/29
- [Qemu-ppc] [PATCH 8/8] hw/ppc: move DT cpu id generation to machine code, Greg Kurz, 2016/06/29
- Re: [Qemu-ppc] [PATCH 0/8] ppc: compute cpu_dt_id in the machine code, Greg Kurz, 2016/06/29
- Re: [Qemu-ppc] [PATCH 0/8] ppc: compute cpu_dt_id in the machine code, David Gibson, 2016/06/30
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 0/8] ppc: compute cpu_dt_id in the machine code, Igor Mammedov, 2016/06/30
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 0/8] ppc: compute cpu_dt_id in the machine code,
Greg Kurz <=