[PATCH v6 15/15] virt: arm: support hip04 gic

[christoffer.dall@linaro.org (Christoffer Dall)]

On Thu, May 15, 2014 at 09:09:05PM +0800, Haojian Zhuang wrote:
> On 15 May 2014 17:42, Marc Zyngier <marc.zyngier@arm.com> wrote:
> > On 11/05/14 09:06, Haojian Zhuang wrote:
> >> In ARM standard GIC, GICH_APR offset is 0xf0 & GICH_LR0 offset is 0x100.
> >> In HiP04 GIC, GICH_APR offset is 0x70 & GICH_LR0 offset is 0x80.
> >>
> >> Now reuse the nr_lr field in struct vgic_cpu. Bit[31:16] is used to store
> >> GICH_APR offset in HiP04, and bit[15:0] is used to store real nr_lr
> >> variable. In ARM standard GIC, don't set bit[31:16]. So we could avoid
> >> to change the VGIC implementation in arm64.
> >>
> >> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> >> ---
> >>  arch/arm/kvm/interrupts_head.S  | 32 ++++++++++++++++++++++------
> >>  include/kvm/arm_vgic.h          |  5 ++++-
> >>  include/linux/irqchip/arm-gic.h |  6 ++++++
> >>  virt/kvm/arm/vgic.c             | 47 +++++++++++++++++++++++++++++------------
> >>  4 files changed, 69 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S
> >> index 76af9302..7aacaff 100644
> >> --- a/arch/arm/kvm/interrupts_head.S
> >> +++ b/arch/arm/kvm/interrupts_head.S
> >> @@ -419,7 +419,10 @@ vcpu     .req    r0              @ vcpu pointer always in r0
> >>       ldr     r7, [r2, #GICH_EISR1]
> >>       ldr     r8, [r2, #GICH_ELRSR0]
> >>       ldr     r9, [r2, #GICH_ELRSR1]
> >> -     ldr     r10, [r2, #GICH_APR]
> >> +     ldr     r10, [r11, #VGIC_CPU_NR_LR]
> >
> > Please rename this field to something else, now that it contains more
> > than the number of LRs.
> 
> Since vgic driver is shared between arm & arm64, I only use high word
> in VGIC_CPU_NR_LR register if SoC is HiP04. Then I could avoid to
> change the vgic implementation in arm64.
> 
> Do you want to me change arm64 implementation at the same time?
> 
> >
> >> +     movs    r10, r10, lsr #HWCFG_APR_SHIFT
> >> +     ldrne   r10, [r2, r10]
> >> +     ldreq   r10, [r2, #GICH_APR]
> >
> > No. Just encode the offset there always. No need for a test.
> >

We don't want all these conditionals in the critical path and it makes
the code impossible to read, please follow Marc's advice.

Thanks,
-Christoffer