Re: [Qemu-devel] [RFC PATCH v1 0/7] Reset and Halting modifications + Zynq SMP

[Peter Crosthwaite]

Hi Andreas,

On Mon, Mar 4, 2013 at 10:03 PM, Andreas Färber <address@hidden> wrote:
> Hi Peter,
>
> Am 04.03.2013 10:01, schrieb Peter Crosthwaite:
>> Hi All. The clock controller module in the Zynq platform has the ability to 
>> halt
>> and reset arbitrary devices, including the CPU. We use this feature to 
>> implement
>> SMP Linux - the kernel halts CPU1 then rewrites the vector table to the
>> secondary entry point and unhalts.
>>
>> The clock controller however is reasonable generic, in that in has the 
>> ability
>> to halt and reset any arbitrary device in the Zynq SoC. The interface for 
>> doing
>> this is reasonably consistent. So im looking for unified way to halt and 
>> reset
>> arbitrary devices CPUs included. So given we already have reset() defined on 
>> the
>> TYPE_DEVICE level, i've added halt as well.
>>
>> This series is based on v3 of the current qom-cpu queue to pick up the move 
>> of
>> cpu halt out of the env. From this I attach the cpu::halted bit to the
>> Device::(un)halt function.
>
> I have doubts whether that is a suitable API... My initial thought was:
> Isn't that pretty much realize/unrealize according to Anthony's
> definition via Vcc?

Loss of Vcc generally implies loss of state (unless your device state
is non volatile storage) which is not what I am going for. I'm trying
to suspend for later resumption. Could we hook up the cpu::halted bit
to unrealize/realize and it would work?

 The technical difference I see is that in your case
> devices are still migrated, not sure if that makes a difference,
> assuming unhalt/realize puts them into a defined state (i.e., reset).
>

The defined state for unhalt is exactly what it was when you halted.
Not sure of the migration issues here.

> Reminds me that some time ago we had a discussion about
> enabling/disabling/reconfiguring ISA devices. realize/unrealize might be
> a solution there too, but it may still be tied to the concept of
> hot-plug, which is disabled for ISA and SysBus.
>
Does hotplug support preservation of state? Or does hot-unplug imply
loss or power implying loss of state?

>> Next up, CPUs seem to have a different reset path to normal devices. So I 
>> have
>> trivially hooked up CPU::Reset to Device::Reset. This means that anyone who
>> holds a DeviceState pointer to the CPU can reset it properly without actually
>> knowing its a CPU.
>
> If we do that, it needs a better explanation of *why* this is okay.
>

Shouldn't CPUs implement device::reset anyway? Its mandated for all
regular devices that Device::reset function is implemented as a
power-on-reset. I'm not sure why CPUs should be any different
considering they are implementers of TYPE_DEVICE.

Regards,
Peter

> Regards,
> Andreas
>
>>
>> With the halt API I played with changing an existing device over to use it in
>> place of the CPU halted bit (sun4m).
>>
>> I then finally add SMP support to the Zynq machine, and patch the Zynq SLCR
>> (my clock controller) to have DeviceState pointers to the CPUs. 
>> device_reset()
>> and device_halt() is where the magic happens.
>>
>> Future work, more devices in Zynq will have halt and reset. My agenda for
>> abstracting away the fact that attached device is a CPU is to allow for
>> consistent implementation and a single code path for the clock controlled
>> devices.
>>
>>
>> Peter A. G. Crosthwaite (3):
>>   xilinx_zynq: added smp support
>>   zynq_slcr: Add links to the CPUs
>>   zynq_slcr: Implement CPU reset and halting
>>
>> Peter Crosthwaite (4):
>>   qdev: Define halting API
>>   qom/cpu.c: Encapsulate cpu halting
>>   qom/cpu.c: Hook CPU reset up to device reset
>>   sun4m: Use halting API to halt/unhalt CPUs
>>
>>  hw/qdev-core.h   |   17 ++++++++++++++
>>  hw/qdev.c        |   18 ++++++++++++++
>>  hw/sun4m.c       |   24 +++++++++---------
>>  hw/xilinx_zynq.c |   66 
>> ++++++++++++++++++++++++++++++++++++++++++++---------
>>  hw/zynq_slcr.c   |   30 ++++++++++++++++++++++++
>>  qom/cpu.c        |   22 ++++++++++++++++++
>>  6 files changed, 153 insertions(+), 24 deletions(-)
>>
>
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>