Re: [PATCH qemu v8 1/3] ppc/spapr: Move GPRs setup to one place

[Greg Kurz]

On Tue, 10 Mar 2020 07:41:47 +0100
Cédric Le Goater <address@hidden> wrote:

> On 3/10/20 6:07 AM, Alexey Kardashevskiy wrote:
> > At the moment "pseries" starts in SLOF which only expects the FDT blob
> > pointer in r3. As we are going to introduce a OpenFirmware support in
> > QEMU, we will be booting OF clients directly and these expect a stack
> > pointer in r1, Linux looks at r3/r4 for the initramdisk location
> > (although vmlinux can find this from the device tree but zImage from
> > distro kernels cannot).
> > 
> > This extends spapr_cpu_set_entry_state() to take more registers. This
> > should cause no behavioral change.
> 
> LGTM, one question below.
> 
> > 
> > Signed-off-by: Alexey Kardashevskiy <address@hidden>
> > ---
> > Changes:
> > v7:
> > * removed r5 as it points to prom entry which is now provided by
> > a new firmware in later patches
> > ---
> >  include/hw/ppc/spapr_cpu_core.h | 4 +++-
> >  hw/ppc/spapr.c                  | 2 +-
> >  hw/ppc/spapr_cpu_core.c         | 6 +++++-
> >  hw/ppc/spapr_rtas.c             | 2 +-
> >  4 files changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/hw/ppc/spapr_cpu_core.h 
> > b/include/hw/ppc/spapr_cpu_core.h
> > index 1c4cc6559c52..7aed8f555b4f 100644
> > --- a/include/hw/ppc/spapr_cpu_core.h
> > +++ b/include/hw/ppc/spapr_cpu_core.h
> > @@ -40,7 +40,9 @@ typedef struct SpaprCpuCoreClass {
> >  } SpaprCpuCoreClass;
> >  
> >  const char *spapr_get_cpu_core_type(const char *cpu_type);
> > -void spapr_cpu_set_entry_state(PowerPCCPU *cpu, target_ulong nip, 
> > target_ulong r3);
> > +void spapr_cpu_set_entry_state(PowerPCCPU *cpu, target_ulong nip,
> > +                               target_ulong r1, target_ulong r3,
> > +                               target_ulong r4);
> >  
> >  typedef struct SpaprCpuState {
> >      uint64_t vpa_addr;
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 2eb0d8f70de6..64bc8b83e91e 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1698,7 +1698,7 @@ static void spapr_machine_reset(MachineState *machine)
> >      spapr->fdt_blob = fdt;
> >  
> >      /* Set up the entry state */
> > -    spapr_cpu_set_entry_state(first_ppc_cpu, SPAPR_ENTRY_POINT, fdt_addr);
> > +    spapr_cpu_set_entry_state(first_ppc_cpu, SPAPR_ENTRY_POINT, 0, 
> > fdt_addr, 0);
> >      first_ppc_cpu->env.gpr[5] = 0;
> 
> 
> Why is this done in the machine reset handler and not in the CPU reset 
> handler ? 
> 

Because spapr_cpu_set_entry_state() starts the CPU. The machine code is
responsible to start the boot CPU (first_ppc_cpu) at system reset, other
CPUs will be started by the guest with the "start-cpu" RTAS call.

> C. 
> 
> 
> >      spapr->cas_reboot = false;
> > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > index 36ed3a2b665b..ac1c10942771 100644
> > --- a/hw/ppc/spapr_cpu_core.c
> > +++ b/hw/ppc/spapr_cpu_core.c
> > @@ -76,13 +76,17 @@ static void spapr_reset_vcpu(PowerPCCPU *cpu)
> >      spapr_irq_cpu_intc_reset(spapr, cpu);
> >  }
> >  
> > -void spapr_cpu_set_entry_state(PowerPCCPU *cpu, target_ulong nip, 
> > target_ulong r3)
> > +void spapr_cpu_set_entry_state(PowerPCCPU *cpu, target_ulong nip,
> > +                               target_ulong r1, target_ulong r3,
> > +                               target_ulong r4)
> >  {
> >      PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> >      CPUPPCState *env = &cpu->env;
> >  
> >      env->nip = nip;
> > +    env->gpr[1] = r1;
> >      env->gpr[3] = r3;
> > +    env->gpr[4] = r4;
> >      kvmppc_set_reg_ppc_online(cpu, 1);
> >      CPU(cpu)->halted = 0;
> >      /* Enable Power-saving mode Exit Cause exceptions */
> > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> > index 656fdd221665..fe83b50c6629 100644
> > --- a/hw/ppc/spapr_rtas.c
> > +++ b/hw/ppc/spapr_rtas.c
> > @@ -190,7 +190,7 @@ static void rtas_start_cpu(PowerPCCPU *callcpu, 
> > SpaprMachineState *spapr,
> >       */
> >      newcpu->env.tb_env->tb_offset = callcpu->env.tb_env->tb_offset;
> >  
> > -    spapr_cpu_set_entry_state(newcpu, start, r3);
> > +    spapr_cpu_set_entry_state(newcpu, start, 0, r3, 0);
> >  
> >      qemu_cpu_kick(CPU(newcpu));
> >  
> > 
> 
>