| Message ID | 20200310050733.29805-2-aik@ozlabs.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | spapr: Kill SLOF | expand |
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 <aik@ozlabs.ru> > --- > 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 ? 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)); > >
On Tue, 10 Mar 2020 07:41:47 +0100 Cédric Le Goater <clg@kaod.org> 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 <aik@ozlabs.ru> > > --- > > 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)); > > > > > >
On Tue, 10 Mar 2020 16:07:31 +1100 Alexey Kardashevskiy <aik@ozlabs.ru> 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. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- Reviewed-by: Greg Kurz <groug@kaod.org> > 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; > > 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)); >
On Tue, Mar 10, 2020 at 04:07:31PM +1100, 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. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> LGTM independent of the other changes, so applied to ppc-for-5.0. > --- > 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; > > 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)); >
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; 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));
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. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- 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(-)