Re: [Qemu-devel] [PATCH target-arm v5 4/5] arm: Implement reset GPIO.

[Peter Crosthwaite]

On Tue, Jan 28, 2014 at 3:52 AM, Peter Maydell <address@hidden> wrote:
> On 15 January 2014 09:14, Peter Crosthwaite
> <address@hidden> wrote:
>> Implement a reset GPIO for ARM CPUs. This allows individual reset of ARM
>> CPUs from device land without the need for the much unwanted reset API
>> calls.
>>
>> The CPU is halted as long as the pin is held in reset. Releasing the
>> reset starts the CPU running again.
>
>> +static void arm_cpu_reset_gpio(void *opaque, int irq, int level)
>> +{
>> +    CPUState *cpu = opaque;
>> +
>> +    if (level) {
>> +        cpu_reset(cpu);
>> +        cpu_interrupt(cpu, CPU_INTERRUPT_HALT);
>> +    } else {
>> +        cpu_reset_interrupt(cpu, CPU_INTERRUPT_HALT);
>> +        cpu_interrupt(cpu, CPU_INTERRUPT_EXITTB);
>> +    }
>> +}
>
> I don't think this will work properly. For instance,
> cpu_exec() will bring the CPU out of halt if an inbound
> interrupt arrives, but we should stay in reset until
> the reset line is deasserted.
>

I see. I guess I'm going to have to save the reset pin state as a
boolean in CPU state (and I guess that means is should be migratable).
Then inhibit the true return from cpu_has_work when the pin is
asserted.

> Also ideally speaking we should probably do the reset
> actions on the falling edge of reset, not the rising edge.
>

Any particular reason? I would have thought that any externally
visible state would best be reset ASAP. For level sensitive behavior,
the transitionals should happen going into the active level. Unless
ARM CPU resets are actually falling edge sensitive (in which case the
CPU would continue to run while the reset is held).

> Does this work properly when we're running under KVM
> rather than using the TCG CPU?
>

I must confess no, I explicitly LOG_UNIMP for KVM, as I have no means
to develop or test ARM KVM.

> Is there anything really ARM-specific in this reset_gpio
> function, or could it be implemented at a common level for
> all target architectures?
>

Not yet, but probably will be ARM specific once I add the cpu reset
pin state. Unless Andreas is happy for that pin state and all this
code to go up to the base TYPE_CPU class.

I wonder however, whether different arch will have level/edge/high/low
variances in reset behavior that must be accommodated.

Andreas, you want this in CPU or should we leave it here in ARM land?

Regards,
Peter

>> -/* Meanings of the ARMCPU object's two inbound GPIO lines */
>> -#define ARM_CPU_IRQ 0
>> -#define ARM_CPU_FIQ 1
>> +/* Meanings of the ARMCPU object's inbound GPIO lines.  */
>> +#define ARM_CPU_IRQ         0
>> +#define ARM_CPU_FIQ         1
>> +/* reset GPIO is inited after irqs, so its index is one past FIQ */
>> +#define ARM_CPU_RESET       (ARM_CPU_FIQ + 1)
>
> ARMv8 also has an SError which probably ought to follow IRQ/FIQ,
> but I think we can safely renumber GPIO lines without breaking
> migration.
>
> thanks
> -- PMM
>