Andreas Färber <afaer...@suse.de> writes:
> Am 02.08.2012 04:10, schrieb David Gibson:
>> A number of things need to occur during reset of the PAPR paravirtualized
>> platform in a specific order. For example, the hash table needs to be
>> cleared before the CPUs are reset, so that they initialize their register
>> state correctly, and the CPUs need to have their main reset called before
>> we set up the entry point state on the boot cpu. We also need to have
>> the main qdev reset happen before the creation and installation of the
>> device tree for the new boot, because we need the state of the devices
>> settled to correctly construct the device tree.
>>
>> Currently reset of pseries is broken in a number of ways, and in other
>> cases works largely by accident. This patch uses the new QEMUMachine reset
>> hook to correct these problems, by replacing the several existing spapr
>> reset hooks with one new machine hook which ensures that the various stages
>> happen in the correct order.
>>
>> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
>> ---
>> hw/spapr.c | 66
>> +++++++++++++++++++++++++++++++++---------------------------
>> 1 file changed, 36 insertions(+), 30 deletions(-)
>>
>> diff --git a/hw/spapr.c b/hw/spapr.c
>> index 2453bae..1e60ec1 100644
>> --- a/hw/spapr.c
>> +++ b/hw/spapr.c
>> @@ -582,29 +582,22 @@ static void spapr_reset_htab(sPAPREnvironment *spapr)
>> }
>> }
>>
>> -static void spapr_reset(void *opaque)
>> +static void spapr_reset_cpu(CPUPPCState *env)
>> {
>> - sPAPREnvironment *spapr = (sPAPREnvironment *)opaque;
>> -
>> - /* Reset the hash table & recalc the RMA */
>> - spapr_reset_htab(spapr);
>> -
>> - /* Load the fdt */
>> - spapr_finalize_fdt(spapr, spapr->fdt_addr, spapr->rtas_addr,
>> - spapr->rtas_size);
>> -}
>> -
>> -static void spapr_cpu_reset(void *opaque)
>> -{
>> - PowerPCCPU *cpu = opaque;
>> - CPUPPCState *env = &cpu->env;
>> + PowerPCCPU *cpu = container_of(env, PowerPCCPU, env);
>
> NACK. Please don't undo the cleanups I have applied! Functions should
> take a QOM PowerPCCPU, not its internal CPUPPCState. Fields are
> gradually being moved from CPUxxxState into CPUState.
>
>>
>> cpu_reset(CPU(cpu));
>
> Also note the current discussion about CPU reset and ordering, e.g.:
> http://patchwork.ozlabs.org/patch/174602/
>
> Anthony was favoring moving reset code out of machines and expressed
> dislike for looping through CPUs, which my above patch took into
> account. The ordering issue between CPU and devices is still unsolved there.
>
> Some on-list comments from Anthony would be nice, since we are moving
> into opposing directions here - having the sPAPR machine be more in
> control vs. moving code away from the PC machine into target-i386 CPU
> and/or common CPU code.
I already commented on the first patch because I had a feeling you'd
post something like this ;-)
Regarding reset:
1) Devices should implement DeviceState::reset()
2) If a device doesn't implement ::reset(), it should call
qemu_register_reset()
3) Reset should propagate through the device model, starting with the
top-level machine which is logically what's plugged into the wall and
is the source of power in the first place.
Regards,
Anthony Liguori
>
> Cheers,
> Andreas
>
>>
>> env->external_htab = spapr->htab;
>> env->htab_base = -1;
>> env->htab_mask = HTAB_SIZE(spapr) - 1;
>> + /* CPUs need to start halted at reset, the platform reset code
>> + * will activate CPU0 then the rest are explicitly started by the
>> + * guest using RTAS */
>> + env->halted = 1;
>>
>> + /* Secondary CPUs get the CPU ID in r3 on entry */
>> + env->gpr[3] = env->cpu_index;
>> env->spr[SPR_SDR1] = (unsigned long)spapr->htab |
>> (spapr->htab_shift - 18);
>>
>> @@ -612,14 +605,35 @@ static void spapr_cpu_reset(void *opaque)
>> kvmppc_update_sdr1(env);
>> }
>>
>> - /* Set up the entry state */
>> - if (env == first_cpu) {
>> - env->gpr[3] = spapr->fdt_addr;
>> - env->gpr[5] = 0;
>> - env->halted = 0;
>> - env->nip = spapr->entry_point;
>> + tb_flush(env);
>> +}
>> +
>> +static void spapr_reset(bool report)
>> +{
>> + CPUPPCState *env = first_cpu;
>> +
>> + /* Reset the qdevs */
>> + qemu_default_system_reset(report);
>> +
>> + /* Reset the hash table & recalc the RMA */
>> + spapr_reset_htab(spapr);
>> +
>> + /* Reset the CPUs */
>> + for (env = first_cpu; env != NULL; env = env->next_cpu) {
>> + spapr_reset_cpu(env);
>> }
>>
>> + /* Load the fdt */
>> + spapr_finalize_fdt(spapr, spapr->fdt_addr, spapr->rtas_addr,
>> + spapr->rtas_size);
>> +
>> + /* Set up the entry state on CPU0 */
>> + env = first_cpu;
>> +
>> + env->gpr[3] = spapr->fdt_addr;
>> + env->gpr[5] = 0;
>> + env->halted = 0;
>> + env->nip = spapr->entry_point;
>> tb_flush(env);
>> }
>>
>> @@ -718,8 +732,6 @@ static void ppc_spapr_init(ram_addr_t ram_size,
>> /* FIXME: we should change this default based on RAM size */
>> spapr->htab_shift = 24;
>>
>> - qemu_register_reset(spapr_reset, spapr);
>> -
>> /* init CPUs */
>> if (cpu_model == NULL) {
>> cpu_model = kvm_enabled() ? "host" : "POWER7";
>> @@ -734,11 +746,9 @@ static void ppc_spapr_init(ram_addr_t ram_size,
>>
>> /* Set time-base frequency to 512 MHz */
>> cpu_ppc_tb_init(env, TIMEBASE_FREQ);
>> - qemu_register_reset(spapr_cpu_reset, cpu);
>>
>> env->hreset_vector = 0x60;
>> env->hreset_excp_prefix = 0;
>> - env->gpr[3] = env->cpu_index;
>> }
>>
>> /* allocate RAM */
>> @@ -883,11 +893,6 @@ static void ppc_spapr_init(ram_addr_t ram_size,
>>
>> spapr->entry_point = 0x100;
>>
>> - /* SLOF will startup the secondary CPUs using RTAS */
>> - for (env = first_cpu; env != NULL; env = env->next_cpu) {
>> - env->halted = 1;
>> - }
>> -
>> /* Prepare the device tree */
>> spapr->fdt_skel = spapr_create_fdt_skel(cpu_model,
>> initrd_base, initrd_size,
>> @@ -900,6 +905,7 @@ static QEMUMachine spapr_machine = {
>> .name = "pseries",
>> .desc = "pSeries Logical Partition (PAPR compliant)",
>> .init = ppc_spapr_init,
>> + .reset = spapr_reset,
>> .max_cpus = MAX_CPUS,
>> .no_parallel = 1,
>> .use_scsi = 1,
>>
>
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
