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

[Vijay Kilari]

On Mon, Nov 28, 2016 at 10:05 PM, Peter Maydell
<address@hidden> wrote:
> On 28 November 2016 at 16:01, Vijay Kilari <address@hidden> wrote:
>> 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?.
>
> It's called to reset a particular CPU (so it will be called
> once for each CPU).
>
>> OR object_child_foreach_recursive() is calling to reset cpu
>> interfaces of
>> all cpus?.
>
> It does "look through the whole graph of objects in the
> simulation and call the function on anything in the
> graph that implements the interface". I've just seen that
> your code is doing "ignore the call if the CPU that
> triggered this isn't the one we care about", though --
> I missed that the first time reading the code.
>
> Still I would prefer it if we did this with the same
> mechanism for both TCG and KVM. A generic mechanism for
> "let the CPU reset trigger reset of many other devices in the
> system" isn't widely useful because real hardware doesn't
> have that kind of action-at-a-distance behaviour.

To make direct call from arm_cpu_reset() to reset CPUIF,
I could not find a way to get GICv3CPUState from CPUARMState or
ARMCPU struct.

Any idea how to get GICv3CPUState?

In  hw/intc/arm_gicv3_cpuif.c implementation,
el_hook function is registered to fetch GICv3CPUState from CPUARMState
struct, but it is for TCG

>
> thanks
> -- PMM