[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 6/6] Add gicversion option to virt machine
From: |
Shlomo Pongratz |
Subject: |
Re: [Qemu-devel] [PATCH v7 6/6] Add gicversion option to virt machine |
Date: |
Mon, 3 Aug 2015 14:41:30 +0300 |
On Monday, August 3, 2015, Peter Maydell <address@hidden> wrote:
On 3 August 2015 at 08:19, Pavel Fedin <address@hidden> wrote:
> Hello!
>
>> > gicdev = qdev_create(NULL, gictype);
>> > - qdev_prop_set_uint32(gicdev, "revision", 2);
>> > +
>> > + for (i = 0; i < vbi->smp_cpus; i++) {
>> > + CPUState *cpu = qemu_get_cpu(i);
>> > + CPUARMState *env = cpu->env_ptr;
>> > + env->nvic = gicdev;
>> > + }
>>
>> We definitely need to come up with a something cleaner
>> than this (which is ugly for two reasons
>
> This could be done:
> a) as property
> b) as global variable because 'gicdev' is a single of its kind.
>
> But, actually, this is currently only for TCG, which needs it in
> order to forward system register accesses to GICv3 code. Would it
> be OK if i just omit this assignment ?
Yes, I think so.
I'm surprised we tell the CPU about the GIC pointer for the
system register stuff -- I was expecting that we'd give the
GIC a CPU pointer. (We could in theory implement some
equivalent of the architected protocol between the redistributors
and the CPU interfaces, but I think that would be overkill.)
-- PMM
The problem is that each function added as a system's instruction helper to target-arm/cpu64.c has the signature of void set(CPUARMState *env, ARMCPRegInfo *ri, uint64_t value) or uint64_t get(CPUARMState *env, ARMCPRegInfo *ri)
I just mimicked the way armv7m_nvic_XXXX API works.
So in a sense the CPU must be familiar with the GIC (as an opaque object of course).
Best regards,
S.P.