Re: [Qemu-arm] [PATCH v6 4/4] hw/intc/arm_gicv3_kvm: Reset GICv3 cpu interface registers

[Vijay Kilari]

On Mon, Nov 28, 2016 at 6:31 PM, Peter Maydell <address@hidden> wrote:
> On 23 November 2016 at 12:39,  <address@hidden> wrote:
>> From: Vijaya Kumar K <address@hidden>
>>
>> Reset CPU interface registers of GICv3 when CPU is reset.
>> For this, object interface is used, which is called from
>> arm_cpu_reset function.
>>
>> Signed-off-by: Vijaya Kumar K <address@hidden>
>
> This approach doesn't handle the SMP case correctly --
> when a CPU is reset then the CPU interface for that CPU
> (and only that CPU) should be reset. Your code will
> reset every CPU interface every time any CPU is reset.

arm_cpu_reset is not called when particular cpu is reset?.
Is it called for all cpus?.
OR object_child_foreach_recursive() is calling to reset cpu interfaces of
all cpus?.

>
> I think it would be better to use the same approach that
> the arm_gicv3_cpuif.c code uses to arrange for cpu i/f
> registers to be reset, perhaps by moving the appropriate
> parts of that code into the common source file.
>
> Having the reset state depend implicitly on the kernel's
> internal state (as you have here for the ICC_CTLR_EL1
> state) is something I'm a bit unsure about -- what goes
> wrong if you don't do that?

During VM boots kvm_arm_gicv3_reset() writes all
the GIC registers with reset value. kernel does not allow writing ICC_CTLR_EL1
with zeros because it validates against hw supported values.
Similarly SRE_EL1.

>
> thanks
> -- PMM