[Qemu-devel] PING: [PATCH v2 0/2] cpu_arm: Implement irqchip property for ARM CPU

[Qemu-devel] PING: [PATCH v2 0/2] cpu_arm: Implement irqchip property for ARM CPU

[Pavel Fedin]

 Hi! This message is mainly for Peter. I say you reviewed my major sets, but looks like missed this
one. If it is OK, we could apply it, and i could successfully bring back the missing part in
vGICv3-enabled hw/arm/virt.c which attaches irqchip to CPUs. This would make us more ready for TCG
version of GICv3.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


> -----Original Message-----
> From: qemu-devel-bounces+p.fedin=samsung.com@nongnu.org [mailto:qemu-devel-
> bounces+p.fedin=samsung.com@nongnu.org] On Behalf Of Pavel Fedin
> Sent: Tuesday, August 25, 2015 3:18 PM
> To: qemu-devel@nongnu.org
> Cc: Peter Maydell; Shlomo Pongratz; Shlomo Pongratz
> Subject: [Qemu-devel] [PATCH v2 0/2] cpu_arm: Implement irqchip property for ARM CPU
> 
> ARMv7m CPU needs a link to NVIC instance for processing interrupts.
> Similarly ARMv8 needs a link to GICv3 for its CPU interface.
> 
> This series builds upon existing mechanism for linking irqchip and
> CPU, bringing the code up to date and making it reusable. Another small
> step towards complete GICv3 implementation.
> 
> v1 => v2:
> - Set link to nvic after it has been initialized
> - Changed object type to "sys-bus-device" because GICv2 and GICv3 do not
>   share common ancestors above that.
> 
> Pavel Fedin (2):
>   cpu_arm: Rename 'nvic' to 'irqchip'
>   armv7m: Use irqchip property instead of direct assignment
> 
>  hw/arm/armv7m.c     |  5 ++---
>  target-arm/cpu.c    |  6 ++++++
>  target-arm/cpu.h    |  5 ++++-
>  target-arm/helper.c | 12 ++++++------
>  4 files changed, 18 insertions(+), 10 deletions(-)
> 
> --
> 1.9.5.msysgit.0

Re: [Qemu-devel] PING: [PATCH v2 0/2] cpu_arm: Implement irqchip property for ARM CPU

[Peter Maydell]

On 4 September 2015 at 07:49, Pavel Fedin <p.fedin@samsung.com> wrote:
>  Hi! This message is mainly for Peter. I say you reviewed my major sets, but looks like missed this
> one. If it is OK, we could apply it, and i could successfully bring back the missing part in
> vGICv3-enabled hw/arm/virt.c which attaches irqchip to CPUs. This would make us more ready for TCG
> version of GICv3.

It's not clear to me that the TCG version of GICv3
emulation should need to have such a link. The original
emulation patchset was definitely not handling the
GIC-to-CPU connection in the right way, and I haven't
seen anybody post an updated version of those patches
which fixes it. I'd rather not add this link until
we have the GIC emulation design sorted out and
we know that we need it.

thanks
-- PMM

Re: [Qemu-devel] PING: [PATCH v2 0/2] cpu_arm: Implement irqchip property for ARM CPU

[Pavel Fedin]

 Hello!

> It's not clear to me that the TCG version of GICv3
> emulation should need to have such a link.

 How can it be done otherwise? Should GIC code somehow register own system register handlers instead?

> The original
> emulation patchset was definitely not handling the
> GIC-to-CPU connection in the right way

 But it seems to be very convenient to reuse what already exists, isn't it? I remember that your main concern was about poking directly into ARMCPU structure, and i remember that one of us at least said the word "property". So, what is fundamentally bad here?
 Well, if you just don't want, then you don't want, i'm fine with this. My arguments end here.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [Qemu-devel] PING: [PATCH v2 0/2] cpu_arm: Implement irqchip property for ARM CPU

[Peter Maydell]

On 4 September 2015 at 15:47, Pavel Fedin <p.fedin@samsung.com> wrote:
>  Hello!
>
>> It's not clear to me that the TCG version of GICv3
>> emulation should need to have such a link.
>
>  How can it be done otherwise? Should GIC code somehow register
> own system register handlers instead?

Well, this comes down to how we want to design the part of the
GICv3 that deals with the CPU interface. In hardware this is
a clearly separated boundary with a defined protocol between the
"GIC proper" and the "CPU interface", which might well be
implementations from different companies. We could implement
it like that; or we could decide to merge the two into what
we call a single GIC device, where that GIC device registers
a set of system registers in each CPU.

I'm not sure what the right answer is; it depends a bit
on how the kernel ends up deciding to export the GIC
state which is in the CPU interface. Having a fused
single GIC device makes it easier to slot in gicv2 vs
gicv3 in our board models, though modelling it like
hardware would be cleaner in some ways.

>> The original
>> emulation patchset was definitely not handling the
>> GIC-to-CPU connection in the right way
>
>  But it seems to be very convenient to reuse what already exists,
> isn't it?

What already exists in QEMU is often legacy and not necessarily
a very good model for anything.

>  Well, if you just don't want, then you don't want, i'm fine with
>  this.

This series might be the right approach (I think if we do want
a property the CPU owns which points at the interrupt controller
it's good code to do that). But it needs to be part of
the patchset which adds the GICv3 emulation support that
requires it, so that we can review the whole design and
see if it makes sense as a whole.

thanks
-- PMM

Re: [Qemu-devel] PING: [PATCH v2 0/2] cpu_arm: Implement irqchip property for ARM CPU

[Shlomo Pongratz]

[-- Attachment #1: Type: text/plain, Size: 1587 bytes --]

On Friday, September 4, 2015, Peter Maydell <peter.maydell@linaro.org>
wrote:

> On 4 September 2015 at 07:49, Pavel Fedin <p.fedin@samsung.com
> <javascript:;>> wrote:
> >  Hi! This message is mainly for Peter. I say you reviewed my major sets,
> but looks like missed this
> > one. If it is OK, we could apply it, and i could successfully bring back
> the missing part in
> > vGICv3-enabled hw/arm/virt.c which attaches irqchip to CPUs. This would
> make us more ready for TCG
> > version of GICv3.
>
> It's not clear to me that the TCG version of GICv3
> emulation should need to have such a link. The original
> emulation patchset was definitely not handling the
> GIC-to-CPU connection in the right way, and I haven't
> seen anybody post an updated version of those patches
> which fixes it. I'd rather not add this link until
> we have the GIC emulation design sorted out and
> we know that we need it.
>
> thanks
> -- PMM
>

Hi,

First I want to apologize for been absent for such a long time.
As far as I remember Peter suggested that with the GICv3 support the
routine define_arm_cp_regs_with_opaque should be used, where the "opaque"
is the gic object.
This "opaque" is later accessed from the register info "opaque" member. I
assume the idea is to take hw/arm/pxa2xx.c as an example.

I also assume that the code that registers the system instructions should
be called from the GICv3 code as the GICv3 mode of operation determines if
the access to the GICv3 is via memory access or via system registers.
Am I right?

I hope to release a new version soon.

Best regards,

S.P.

[-- Attachment #2: Type: text/html, Size: 2121 bytes --]

Re: [Qemu-devel] PING: [PATCH v2 0/2] cpu_arm: Implement irqchip property for ARM CPU

[Pavel Fedin]

 Hello!

> As far as I remember Peter suggested that with the GICv3 support the routine define_arm_cp_regs_with_opaque should be used, where the "opaque" is the gic object.
> This "opaque" is later accessed from the register info "opaque" member. I assume the idea is to take hw/arm/pxa2xx.c as an example.

 Ok, well, my patch was just a suggestion. If you want to do in in other way, i do not insist.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia