diff mbox

[v15,11/12] virt: arm: support hip04 gic

Message ID 1406555876-11989-12-git-send-email-haojian.zhuang@linaro.org
State Changes Requested
Headers show

Commit Message

Haojian Zhuang July 28, 2014, 1:57 p.m. UTC
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/kernel/asm-offsets.c   |  2 +-
 arch/arm/kvm/interrupts_head.S  | 29 +++++++++++++++++++++++------
 arch/arm64/kernel/asm-offsets.c |  2 +-
 arch/arm64/kvm/hyp.S            |  4 ++--
 include/kvm/arm_vgic.h          |  7 +++++--
 include/linux/irqchip/arm-gic.h |  6 ++++++
 virt/kvm/arm/vgic.c             | 37 ++++++++++++++++++++++++++-----------
 7 files changed, 64 insertions(+), 23 deletions(-)

Comments

Marc Zyngier July 28, 2014, 6 p.m. UTC | #1
On Mon, Jul 28 2014 at  2:57:55 pm BST, Haojian Zhuang <haojian.zhuang@linaro.org> 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/kernel/asm-offsets.c   |  2 +-
>  arch/arm/kvm/interrupts_head.S  | 29 +++++++++++++++++++++++------
>  arch/arm64/kernel/asm-offsets.c |  2 +-
>  arch/arm64/kvm/hyp.S            |  4 ++--
>  include/kvm/arm_vgic.h          |  7 +++++--
>  include/linux/irqchip/arm-gic.h |  6 ++++++
>  virt/kvm/arm/vgic.c             | 37 ++++++++++++++++++++++++++-----------
>  7 files changed, 64 insertions(+), 23 deletions(-)
>
> diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
> index 85598b5..166cc98 100644
> --- a/arch/arm/kernel/asm-offsets.c
> +++ b/arch/arm/kernel/asm-offsets.c
> @@ -189,7 +189,7 @@ int main(void)
>    DEFINE(VGIC_CPU_ELRSR,	offsetof(struct vgic_cpu, vgic_elrsr));
>    DEFINE(VGIC_CPU_APR,		offsetof(struct vgic_cpu, vgic_apr));
>    DEFINE(VGIC_CPU_LR,		offsetof(struct vgic_cpu, vgic_lr));
> -  DEFINE(VGIC_CPU_NR_LR,	offsetof(struct vgic_cpu, nr_lr));
> +  DEFINE(VGIC_CPU_HW_CFG,	offsetof(struct vgic_cpu, hw_cfg));
>  #ifdef CONFIG_KVM_ARM_TIMER
>    DEFINE(VCPU_TIMER_CNTV_CTL,	offsetof(struct kvm_vcpu, arch.timer_cpu.cntv_ctl));
>    DEFINE(VCPU_TIMER_CNTV_CVAL,	offsetof(struct kvm_vcpu, arch.timer_cpu.cntv_cval));
> diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S
> index 76af9302..9fbbf99 100644
> --- a/arch/arm/kvm/interrupts_head.S
> +++ b/arch/arm/kvm/interrupts_head.S
> @@ -419,7 +419,9 @@ 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_HW_CFG]
> +	mov	r10, r10, lsr #HWCFG_APR_SHIFT
> +	ldr	r10, [r2, r10]
>  
>  	str	r3, [r11, #VGIC_CPU_HCR]
>  	str	r4, [r11, #VGIC_CPU_VMCR]
> @@ -435,9 +437,15 @@ vcpu	.req	r0		@ vcpu pointer always in r0
>  	str	r5, [r2, #GICH_HCR]
>  
>  	/* Save list registers */
> -	add	r2, r2, #GICH_LR0
> +	ldr	r4, [r11, #VGIC_CPU_HW_CFG]
> +	mov	r10, r4, lsr #HWCFG_APR_SHIFT
> +	/* the offset between GICH_APR & GICH_LR0 is 0x10 */
> +	add	r10, r10, #0x10
> +	add	r2, r2, r10
>  	add	r3, r11, #VGIC_CPU_LR
> -	ldr	r4, [r11, #VGIC_CPU_NR_LR]
> +	/* Get NR_LR from VGIC_CPU_HW_CFG */
> +	ldr	r6, =HWCFG_NR_LR_MASK
> +	and	r4, r4, r6

Use "ubfx r4, r4, #0, #16" instead (with the proper symbolic constants,
of course). It will save loading this mask from the constant pool. For
consistency, you can also replace the above mov+lsr with the same
mechanism.

>  1:	ldr	r6, [r2], #4
>  	str	r6, [r3], #4
>  	subs	r4, r4, #1
> @@ -469,12 +477,21 @@ vcpu	.req	r0		@ vcpu pointer always in r0
>  
>  	str	r3, [r2, #GICH_HCR]
>  	str	r4, [r2, #GICH_VMCR]
> -	str	r8, [r2, #GICH_APR]
> +	ldr	r6, [r11, #VGIC_CPU_HW_CFG]
> +	mov	r6, r6, lsr #HWCFG_APR_SHIFT
> +	str	r8, [r2, r6]
>  
>  	/* Restore list registers */
> -	add	r2, r2, #GICH_LR0
> +	ldr	r4, [r11, #VGIC_CPU_HW_CFG]
> +	mov	r6, r4, lsr #HWCFG_APR_SHIFT
> +	/* the offset between GICH_APR & GICH_LR0 is 0x10 */
> +	add	r6, r6, #0x10
> +	/* get offset of GICH_LR0 */
> +	add	r2, r2, r6
> +	/* Get NR_LR from VGIC_CPU_HW_CFG */
>  	add	r3, r11, #VGIC_CPU_LR
> -	ldr	r4, [r11, #VGIC_CPU_NR_LR]
> +	ldr	r6, =HWCFG_NR_LR_MASK
> +	and	r4, r4, r6

Same here.

>  1:	ldr	r6, [r3], #4
>  	str	r6, [r2], #4
>  	subs	r4, r4, #1
> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
> index 646f888..2422358 100644
> --- a/arch/arm64/kernel/asm-offsets.c
> +++ b/arch/arm64/kernel/asm-offsets.c
> @@ -136,7 +136,7 @@ int main(void)
>    DEFINE(VGIC_CPU_ELRSR,	offsetof(struct vgic_cpu, vgic_elrsr));
>    DEFINE(VGIC_CPU_APR,		offsetof(struct vgic_cpu, vgic_apr));
>    DEFINE(VGIC_CPU_LR,		offsetof(struct vgic_cpu, vgic_lr));
> -  DEFINE(VGIC_CPU_NR_LR,	offsetof(struct vgic_cpu, nr_lr));
> +  DEFINE(VGIC_CPU_HW_CFG,	offsetof(struct vgic_cpu, hw_cfg));
>    DEFINE(KVM_VTTBR,		offsetof(struct kvm, arch.vttbr));
>    DEFINE(KVM_VGIC_VCTRL,	offsetof(struct kvm, arch.vgic.vctrl_base));
>  #endif
> diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
> index b0d1512..2dbe337 100644
> --- a/arch/arm64/kvm/hyp.S
> +++ b/arch/arm64/kvm/hyp.S
> @@ -426,7 +426,7 @@ CPU_BE(	rev	w11, w11 )
>  
>  	/* Save list registers */
>  	add	x2, x2, #GICH_LR0
> -	ldr	w4, [x3, #VGIC_CPU_NR_LR]
> +	ldr	w4, [x3, #VGIC_CPU_HW_CFG]

Have you tested this? It looks to me that you're not dropping the top
bits, so you're going to end-up with 0x1000004 list registers instead of
the expected 4 with GIC-400...

>  	add	x3, x3, #VGIC_CPU_LR
>  1:	ldr	w5, [x2], #4
>  CPU_BE(	rev	w5, w5 )
> @@ -465,7 +465,7 @@ CPU_BE(	rev	w6, w6 )
>  
>  	/* Restore list registers */
>  	add	x2, x2, #GICH_LR0
> -	ldr	w4, [x3, #VGIC_CPU_NR_LR]
> +	ldr	w4, [x3, #VGIC_CPU_HW_CFG]

Same here.

>  	add	x3, x3, #VGIC_CPU_LR
>  1:	ldr	w5, [x3], #4
>  CPU_BE(	rev	w5, w5 )
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index f27000f..eba4b51 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -122,8 +122,11 @@ struct vgic_cpu {
>  	/* Bitmap of used/free list registers */
>  	DECLARE_BITMAP(	lr_used, VGIC_MAX_LRS);
>  
> -	/* Number of list registers on this CPU */
> -	int		nr_lr;
> +	/*
> +	 * bit[31:16]: GICH_APR offset
> +	 * bit[15:0]:  Number of list registers on this CPU
> +	 */
> +	u32		hw_cfg;
>  
>  	/* CPU vif control registers for world switch */
>  	u32		vgic_hcr;
> diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
> index 45e2d8c..b055f92 100644
> --- a/include/linux/irqchip/arm-gic.h
> +++ b/include/linux/irqchip/arm-gic.h
> @@ -49,6 +49,8 @@
>  #define GICH_ELRSR1 			0x34
>  #define GICH_APR			0xf0
>  #define GICH_LR0			0x100
> +#define HIP04_GICH_APR			0x70
> +/* GICH_LR0 offset in HiP04 is 0x80 */
>  
>  #define GICH_HCR_EN			(1 << 0)
>  #define GICH_HCR_UIE			(1 << 1)
> @@ -73,6 +75,10 @@
>  #define GICH_MISR_EOI			(1 << 0)
>  #define GICH_MISR_U			(1 << 1)
>  
> +#define HWCFG_NR_LR_MASK	0xffff
> +#define HWCFG_APR_SHIFT		16
> +#define HWCFG_APR_MASK		(0xffff << HWCFG_APR_SHIFT)
> +
>  #ifndef __ASSEMBLY__
>  
>  struct device_node;
> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> index 56ff9be..6c0ee89 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -97,7 +97,7 @@ static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu);
>  static void vgic_update_state(struct kvm *kvm);
>  static void vgic_kick_vcpus(struct kvm *kvm);
>  static void vgic_dispatch_sgi(struct kvm_vcpu *vcpu, u32 reg);
> -static u32 vgic_nr_lr;
> +static u32 vgic_hw_cfg, vgic_nr_lr;
>  
>  static unsigned int vgic_maint_irq;
>  
> @@ -625,7 +625,7 @@ static void vgic_unqueue_irqs(struct kvm_vcpu *vcpu)
>  	int i, irq, source_cpu;
>  	u32 *lr;
>  
> -	for_each_set_bit(i, vgic_cpu->lr_used, vgic_cpu->nr_lr) {
> +	for_each_set_bit(i, vgic_cpu->lr_used, vgic_nr_lr) {
>  		lr = &vgic_cpu->vgic_lr[i];
>  		irq = LR_IRQID(*lr);
>  		source_cpu = LR_CPUID(*lr);
> @@ -1006,7 +1006,7 @@ static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu)
>  	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
>  	int lr;
>  
> -	for_each_set_bit(lr, vgic_cpu->lr_used, vgic_cpu->nr_lr) {
> +	for_each_set_bit(lr, vgic_cpu->lr_used, vgic_nr_lr) {
>  		int irq = vgic_cpu->vgic_lr[lr] & GICH_LR_VIRTUALID;
>  
>  		if (!vgic_irq_is_enabled(vcpu, irq)) {
> @@ -1047,8 +1047,8 @@ static bool vgic_queue_irq(struct kvm_vcpu *vcpu, u8 sgi_source_id, int irq)
>  
>  	/* Try to use another LR for this interrupt */
>  	lr = find_first_zero_bit((unsigned long *)vgic_cpu->lr_used,
> -			       vgic_cpu->nr_lr);
> -	if (lr >= vgic_cpu->nr_lr)
> +				 vgic_nr_lr);
> +	if (lr >= vgic_nr_lr)
>  		return false;
>  
>  	kvm_debug("LR%d allocated for IRQ%d %x\n", lr, irq, sgi_source_id);
> @@ -1183,7 +1183,7 @@ static bool vgic_process_maintenance(struct kvm_vcpu *vcpu)
>  		int lr, irq;
>  
>  		for_each_set_bit(lr, (unsigned long *)vgic_cpu->vgic_eisr,
> -				 vgic_cpu->nr_lr) {
> +				 vgic_nr_lr) {
>  			irq = vgic_cpu->vgic_lr[lr] & GICH_LR_VIRTUALID;
>  
>  			vgic_irq_clear_active(vcpu, irq);
> @@ -1227,7 +1227,7 @@ static void __kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
>  
>  	/* Clear mappings for empty LRs */
>  	for_each_set_bit(lr, (unsigned long *)vgic_cpu->vgic_elrsr,
> -			 vgic_cpu->nr_lr) {
> +			 vgic_nr_lr) {
>  		int irq;
>  
>  		if (!test_and_clear_bit(lr, vgic_cpu->lr_used))
> @@ -1241,8 +1241,8 @@ static void __kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
>  
>  	/* Check if we still have something up our sleeve... */
>  	pending = find_first_zero_bit((unsigned long *)vgic_cpu->vgic_elrsr,
> -				      vgic_cpu->nr_lr);
> -	if (level_pending || pending < vgic_cpu->nr_lr)
> +				      vgic_nr_lr);
> +	if (level_pending || pending < vgic_nr_lr)
>  		set_bit(vcpu->vcpu_id, &dist->irq_pending_on_cpu);
>  }
>  
> @@ -1438,7 +1438,7 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
>  	 */
>  	vgic_cpu->vgic_vmcr = 0;
>  
> -	vgic_cpu->nr_lr = vgic_nr_lr;
> +	vgic_cpu->hw_cfg = vgic_hw_cfg;
>  	vgic_cpu->vgic_hcr = GICH_HCR_EN; /* Get the show on the road... */
>  
>  	return 0;
> @@ -1470,17 +1470,31 @@ static struct notifier_block vgic_cpu_nb = {
>  	.notifier_call = vgic_cpu_notify,
>  };
>  
> +static const struct of_device_id of_vgic_ids[] = {
> +	{
> +		.compatible = "arm,cortex-a15-gic",
> +		.data = (void *)GICH_APR,
> +	}, {
> +		.compatible = "hisilicon,hip04-gic",
> +		.data = (void *)HIP04_GICH_APR,
> +	}, {
> +	},
> +};
> +
>  int kvm_vgic_hyp_init(void)
>  {
>  	int ret;
>  	struct resource vctrl_res;
>  	struct resource vcpu_res;
> +	const struct of_device_id *match;
>  
> -	vgic_node = of_find_compatible_node(NULL, NULL, "arm,cortex-a15-gic");
> +	vgic_node = of_find_matching_node_and_match(NULL, of_vgic_ids, &match);
>  	if (!vgic_node) {
>  		kvm_err("error: no compatible vgic node in DT\n");
>  		return -ENODEV;
>  	}
> +	/* High word of vgic_hw_cfg is the offset of GICH_APR. */
> +	vgic_hw_cfg = (unsigned long)match->data << HWCFG_APR_SHIFT;
>  
>  	vgic_maint_irq = irq_of_parse_and_map(vgic_node, 0);
>  	if (!vgic_maint_irq) {
> @@ -1517,6 +1531,7 @@ int kvm_vgic_hyp_init(void)
>  
>  	vgic_nr_lr = readl_relaxed(vgic_vctrl_base + GICH_VTR);
>  	vgic_nr_lr = (vgic_nr_lr & 0x3f) + 1;
> +	vgic_hw_cfg |= vgic_nr_lr;
>  
>  	ret = create_hyp_io_mappings(vgic_vctrl_base,
>  				     vgic_vctrl_base + resource_size(&vctrl_res),

Most of this is going to conflict very badly with the rest of the GICv3
patch series which is already queued for 3.17. You'll have to rebase it
on top of the current kvmarm/queue branch.

Thanks,

	M.
Haojian Zhuang July 29, 2014, 2:31 a.m. UTC | #2
On 29 July 2014 02:00, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On Mon, Jul 28 2014 at  2:57:55 pm BST, Haojian Zhuang <haojian.zhuang@linaro.org> 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/kernel/asm-offsets.c   |  2 +-
>>  arch/arm/kvm/interrupts_head.S  | 29 +++++++++++++++++++++++------
>>  arch/arm64/kernel/asm-offsets.c |  2 +-
>>  arch/arm64/kvm/hyp.S            |  4 ++--
>>  include/kvm/arm_vgic.h          |  7 +++++--
>>  include/linux/irqchip/arm-gic.h |  6 ++++++
>>  virt/kvm/arm/vgic.c             | 37 ++++++++++++++++++++++++++-----------
>>  7 files changed, 64 insertions(+), 23 deletions(-)
>>
>> diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
>> index 85598b5..166cc98 100644
>> --- a/arch/arm/kernel/asm-offsets.c
>> +++ b/arch/arm/kernel/asm-offsets.c
>> @@ -189,7 +189,7 @@ int main(void)
>>    DEFINE(VGIC_CPU_ELRSR,     offsetof(struct vgic_cpu, vgic_elrsr));
>>    DEFINE(VGIC_CPU_APR,               offsetof(struct vgic_cpu, vgic_apr));
>>    DEFINE(VGIC_CPU_LR,                offsetof(struct vgic_cpu, vgic_lr));
>> -  DEFINE(VGIC_CPU_NR_LR,     offsetof(struct vgic_cpu, nr_lr));
>> +  DEFINE(VGIC_CPU_HW_CFG,    offsetof(struct vgic_cpu, hw_cfg));
>>  #ifdef CONFIG_KVM_ARM_TIMER
>>    DEFINE(VCPU_TIMER_CNTV_CTL,        offsetof(struct kvm_vcpu, arch.timer_cpu.cntv_ctl));
>>    DEFINE(VCPU_TIMER_CNTV_CVAL,       offsetof(struct kvm_vcpu, arch.timer_cpu.cntv_cval));
>> diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S
>> index 76af9302..9fbbf99 100644
>> --- a/arch/arm/kvm/interrupts_head.S
>> +++ b/arch/arm/kvm/interrupts_head.S
>> @@ -419,7 +419,9 @@ 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_HW_CFG]
>> +     mov     r10, r10, lsr #HWCFG_APR_SHIFT
>> +     ldr     r10, [r2, r10]
>>
>>       str     r3, [r11, #VGIC_CPU_HCR]
>>       str     r4, [r11, #VGIC_CPU_VMCR]
>> @@ -435,9 +437,15 @@ vcpu     .req    r0              @ vcpu pointer always in r0
>>       str     r5, [r2, #GICH_HCR]
>>
>>       /* Save list registers */
>> -     add     r2, r2, #GICH_LR0
>> +     ldr     r4, [r11, #VGIC_CPU_HW_CFG]
>> +     mov     r10, r4, lsr #HWCFG_APR_SHIFT
>> +     /* the offset between GICH_APR & GICH_LR0 is 0x10 */
>> +     add     r10, r10, #0x10
>> +     add     r2, r2, r10
>>       add     r3, r11, #VGIC_CPU_LR
>> -     ldr     r4, [r11, #VGIC_CPU_NR_LR]
>> +     /* Get NR_LR from VGIC_CPU_HW_CFG */
>> +     ldr     r6, =HWCFG_NR_LR_MASK
>> +     and     r4, r4, r6
>
> Use "ubfx r4, r4, #0, #16" instead (with the proper symbolic constants,
> of course). It will save loading this mask from the constant pool. For
> consistency, you can also replace the above mov+lsr with the same
> mechanism.
>
OK

>>  1:   ldr     r6, [r2], #4
>>       str     r6, [r3], #4
>>       subs    r4, r4, #1
>> @@ -469,12 +477,21 @@ vcpu    .req    r0              @ vcpu pointer always in r0
>>
>>       str     r3, [r2, #GICH_HCR]
>>       str     r4, [r2, #GICH_VMCR]
>> -     str     r8, [r2, #GICH_APR]
>> +     ldr     r6, [r11, #VGIC_CPU_HW_CFG]
>> +     mov     r6, r6, lsr #HWCFG_APR_SHIFT
>> +     str     r8, [r2, r6]
>>
>>       /* Restore list registers */
>> -     add     r2, r2, #GICH_LR0
>> +     ldr     r4, [r11, #VGIC_CPU_HW_CFG]
>> +     mov     r6, r4, lsr #HWCFG_APR_SHIFT
>> +     /* the offset between GICH_APR & GICH_LR0 is 0x10 */
>> +     add     r6, r6, #0x10
>> +     /* get offset of GICH_LR0 */
>> +     add     r2, r2, r6
>> +     /* Get NR_LR from VGIC_CPU_HW_CFG */
>>       add     r3, r11, #VGIC_CPU_LR
>> -     ldr     r4, [r11, #VGIC_CPU_NR_LR]
>> +     ldr     r6, =HWCFG_NR_LR_MASK
>> +     and     r4, r4, r6
>
> Same here.
>

OK
>>  1:   ldr     r6, [r3], #4
>>       str     r6, [r2], #4
>>       subs    r4, r4, #1
>> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
>> index 646f888..2422358 100644
>> --- a/arch/arm64/kernel/asm-offsets.c
>> +++ b/arch/arm64/kernel/asm-offsets.c
>> @@ -136,7 +136,7 @@ int main(void)
>>    DEFINE(VGIC_CPU_ELRSR,     offsetof(struct vgic_cpu, vgic_elrsr));
>>    DEFINE(VGIC_CPU_APR,               offsetof(struct vgic_cpu, vgic_apr));
>>    DEFINE(VGIC_CPU_LR,                offsetof(struct vgic_cpu, vgic_lr));
>> -  DEFINE(VGIC_CPU_NR_LR,     offsetof(struct vgic_cpu, nr_lr));
>> +  DEFINE(VGIC_CPU_HW_CFG,    offsetof(struct vgic_cpu, hw_cfg));
>>    DEFINE(KVM_VTTBR,          offsetof(struct kvm, arch.vttbr));
>>    DEFINE(KVM_VGIC_VCTRL,     offsetof(struct kvm, arch.vgic.vctrl_base));
>>  #endif
>> diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
>> index b0d1512..2dbe337 100644
>> --- a/arch/arm64/kvm/hyp.S
>> +++ b/arch/arm64/kvm/hyp.S
>> @@ -426,7 +426,7 @@ CPU_BE(   rev     w11, w11 )
>>
>>       /* Save list registers */
>>       add     x2, x2, #GICH_LR0
>> -     ldr     w4, [x3, #VGIC_CPU_NR_LR]
>> +     ldr     w4, [x3, #VGIC_CPU_HW_CFG]
>
> Have you tested this? It looks to me that you're not dropping the top
> bits, so you're going to end-up with 0x1000004 list registers instead of
> the expected 4 with GIC-400...
>

As you mentioned, GICv2 isn't used in arm64. And the HIP04 GIC isn't
used in arm64 also. I renamed to VGIC_CPU_NR_LR to VGIC_CPU_HW_CFG. It
seems that I needn't drop it as APR offset isn't recorded in
VGIC_CPU_HW_CFG.

>>       add     x3, x3, #VGIC_CPU_LR
>>  1:   ldr     w5, [x2], #4
>>  CPU_BE(      rev     w5, w5 )
>> @@ -465,7 +465,7 @@ CPU_BE(   rev     w6, w6 )
>>
>>       /* Restore list registers */
>>       add     x2, x2, #GICH_LR0
>> -     ldr     w4, [x3, #VGIC_CPU_NR_LR]
>> +     ldr     w4, [x3, #VGIC_CPU_HW_CFG]
>
> Same here.
>
>>       add     x3, x3, #VGIC_CPU_LR
>>  1:   ldr     w5, [x3], #4
>>  CPU_BE(      rev     w5, w5 )
>> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
>> index f27000f..eba4b51 100644
>> --- a/include/kvm/arm_vgic.h
>> +++ b/include/kvm/arm_vgic.h
>> @@ -122,8 +122,11 @@ struct vgic_cpu {
>>       /* Bitmap of used/free list registers */
>>       DECLARE_BITMAP( lr_used, VGIC_MAX_LRS);
>>
>> -     /* Number of list registers on this CPU */
>> -     int             nr_lr;
>> +     /*
>> +      * bit[31:16]: GICH_APR offset
>> +      * bit[15:0]:  Number of list registers on this CPU
>> +      */
>> +     u32             hw_cfg;
>>
>>       /* CPU vif control registers for world switch */
>>       u32             vgic_hcr;
>> diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
>> index 45e2d8c..b055f92 100644
>> --- a/include/linux/irqchip/arm-gic.h
>> +++ b/include/linux/irqchip/arm-gic.h
>> @@ -49,6 +49,8 @@
>>  #define GICH_ELRSR1                  0x34
>>  #define GICH_APR                     0xf0
>>  #define GICH_LR0                     0x100
>> +#define HIP04_GICH_APR                       0x70
>> +/* GICH_LR0 offset in HiP04 is 0x80 */
>>
>>  #define GICH_HCR_EN                  (1 << 0)
>>  #define GICH_HCR_UIE                 (1 << 1)
>> @@ -73,6 +75,10 @@
>>  #define GICH_MISR_EOI                        (1 << 0)
>>  #define GICH_MISR_U                  (1 << 1)
>>
>> +#define HWCFG_NR_LR_MASK     0xffff
>> +#define HWCFG_APR_SHIFT              16
>> +#define HWCFG_APR_MASK               (0xffff << HWCFG_APR_SHIFT)
>> +
>>  #ifndef __ASSEMBLY__
>>
>>  struct device_node;
>> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
>> index 56ff9be..6c0ee89 100644
>> --- a/virt/kvm/arm/vgic.c
>> +++ b/virt/kvm/arm/vgic.c
>> @@ -97,7 +97,7 @@ static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu);
>>  static void vgic_update_state(struct kvm *kvm);
>>  static void vgic_kick_vcpus(struct kvm *kvm);
>>  static void vgic_dispatch_sgi(struct kvm_vcpu *vcpu, u32 reg);
>> -static u32 vgic_nr_lr;
>> +static u32 vgic_hw_cfg, vgic_nr_lr;
>>
>>  static unsigned int vgic_maint_irq;
>>
>> @@ -625,7 +625,7 @@ static void vgic_unqueue_irqs(struct kvm_vcpu *vcpu)
>>       int i, irq, source_cpu;
>>       u32 *lr;
>>
>> -     for_each_set_bit(i, vgic_cpu->lr_used, vgic_cpu->nr_lr) {
>> +     for_each_set_bit(i, vgic_cpu->lr_used, vgic_nr_lr) {
>>               lr = &vgic_cpu->vgic_lr[i];
>>               irq = LR_IRQID(*lr);
>>               source_cpu = LR_CPUID(*lr);
>> @@ -1006,7 +1006,7 @@ static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu)
>>       struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
>>       int lr;
>>
>> -     for_each_set_bit(lr, vgic_cpu->lr_used, vgic_cpu->nr_lr) {
>> +     for_each_set_bit(lr, vgic_cpu->lr_used, vgic_nr_lr) {
>>               int irq = vgic_cpu->vgic_lr[lr] & GICH_LR_VIRTUALID;
>>
>>               if (!vgic_irq_is_enabled(vcpu, irq)) {
>> @@ -1047,8 +1047,8 @@ static bool vgic_queue_irq(struct kvm_vcpu *vcpu, u8 sgi_source_id, int irq)
>>
>>       /* Try to use another LR for this interrupt */
>>       lr = find_first_zero_bit((unsigned long *)vgic_cpu->lr_used,
>> -                            vgic_cpu->nr_lr);
>> -     if (lr >= vgic_cpu->nr_lr)
>> +                              vgic_nr_lr);
>> +     if (lr >= vgic_nr_lr)
>>               return false;
>>
>>       kvm_debug("LR%d allocated for IRQ%d %x\n", lr, irq, sgi_source_id);
>> @@ -1183,7 +1183,7 @@ static bool vgic_process_maintenance(struct kvm_vcpu *vcpu)
>>               int lr, irq;
>>
>>               for_each_set_bit(lr, (unsigned long *)vgic_cpu->vgic_eisr,
>> -                              vgic_cpu->nr_lr) {
>> +                              vgic_nr_lr) {
>>                       irq = vgic_cpu->vgic_lr[lr] & GICH_LR_VIRTUALID;
>>
>>                       vgic_irq_clear_active(vcpu, irq);
>> @@ -1227,7 +1227,7 @@ static void __kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
>>
>>       /* Clear mappings for empty LRs */
>>       for_each_set_bit(lr, (unsigned long *)vgic_cpu->vgic_elrsr,
>> -                      vgic_cpu->nr_lr) {
>> +                      vgic_nr_lr) {
>>               int irq;
>>
>>               if (!test_and_clear_bit(lr, vgic_cpu->lr_used))
>> @@ -1241,8 +1241,8 @@ static void __kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
>>
>>       /* Check if we still have something up our sleeve... */
>>       pending = find_first_zero_bit((unsigned long *)vgic_cpu->vgic_elrsr,
>> -                                   vgic_cpu->nr_lr);
>> -     if (level_pending || pending < vgic_cpu->nr_lr)
>> +                                   vgic_nr_lr);
>> +     if (level_pending || pending < vgic_nr_lr)
>>               set_bit(vcpu->vcpu_id, &dist->irq_pending_on_cpu);
>>  }
>>
>> @@ -1438,7 +1438,7 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
>>        */
>>       vgic_cpu->vgic_vmcr = 0;
>>
>> -     vgic_cpu->nr_lr = vgic_nr_lr;
>> +     vgic_cpu->hw_cfg = vgic_hw_cfg;
>>       vgic_cpu->vgic_hcr = GICH_HCR_EN; /* Get the show on the road... */
>>
>>       return 0;
>> @@ -1470,17 +1470,31 @@ static struct notifier_block vgic_cpu_nb = {
>>       .notifier_call = vgic_cpu_notify,
>>  };
>>
>> +static const struct of_device_id of_vgic_ids[] = {
>> +     {
>> +             .compatible = "arm,cortex-a15-gic",
>> +             .data = (void *)GICH_APR,
>> +     }, {
>> +             .compatible = "hisilicon,hip04-gic",
>> +             .data = (void *)HIP04_GICH_APR,
>> +     }, {
>> +     },
>> +};
>> +
>>  int kvm_vgic_hyp_init(void)
>>  {
>>       int ret;
>>       struct resource vctrl_res;
>>       struct resource vcpu_res;
>> +     const struct of_device_id *match;
>>
>> -     vgic_node = of_find_compatible_node(NULL, NULL, "arm,cortex-a15-gic");
>> +     vgic_node = of_find_matching_node_and_match(NULL, of_vgic_ids, &match);
>>       if (!vgic_node) {
>>               kvm_err("error: no compatible vgic node in DT\n");
>>               return -ENODEV;
>>       }
>> +     /* High word of vgic_hw_cfg is the offset of GICH_APR. */
>> +     vgic_hw_cfg = (unsigned long)match->data << HWCFG_APR_SHIFT;
>>
>>       vgic_maint_irq = irq_of_parse_and_map(vgic_node, 0);
>>       if (!vgic_maint_irq) {
>> @@ -1517,6 +1531,7 @@ int kvm_vgic_hyp_init(void)
>>
>>       vgic_nr_lr = readl_relaxed(vgic_vctrl_base + GICH_VTR);
>>       vgic_nr_lr = (vgic_nr_lr & 0x3f) + 1;
>> +     vgic_hw_cfg |= vgic_nr_lr;
>>
>>       ret = create_hyp_io_mappings(vgic_vctrl_base,
>>                                    vgic_vctrl_base + resource_size(&vctrl_res),
>
> Most of this is going to conflict very badly with the rest of the GICv3
> patch series which is already queued for 3.17. You'll have to rebase it
> on top of the current kvmarm/queue branch.
>

OK
Marc Zyngier July 29, 2014, 8:47 a.m. UTC | #3
On Tue, Jul 29 2014 at  3:31:52 am BST, Haojian Zhuang <haojian.zhuang@linaro.org> wrote:
> On 29 July 2014 02:00, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On Mon, Jul 28 2014 at 2:57:55 pm BST, Haojian Zhuang
>> <haojian.zhuang@linaro.org> 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/kernel/asm-offsets.c   |  2 +-
>>>  arch/arm/kvm/interrupts_head.S  | 29 +++++++++++++++++++++++------
>>>  arch/arm64/kernel/asm-offsets.c |  2 +-
>>>  arch/arm64/kvm/hyp.S            |  4 ++--
>>>  include/kvm/arm_vgic.h          |  7 +++++--
>>>  include/linux/irqchip/arm-gic.h |  6 ++++++
>>>  virt/kvm/arm/vgic.c             | 37 ++++++++++++++++++++++++++-----------
>>>  7 files changed, 64 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
>>> index 85598b5..166cc98 100644
>>> --- a/arch/arm/kernel/asm-offsets.c
>>> +++ b/arch/arm/kernel/asm-offsets.c
>>> @@ -189,7 +189,7 @@ int main(void)
>>>    DEFINE(VGIC_CPU_ELRSR,     offsetof(struct vgic_cpu, vgic_elrsr));
>>>    DEFINE(VGIC_CPU_APR,               offsetof(struct vgic_cpu, vgic_apr));
>>>    DEFINE(VGIC_CPU_LR,                offsetof(struct vgic_cpu, vgic_lr));
>>> -  DEFINE(VGIC_CPU_NR_LR,     offsetof(struct vgic_cpu, nr_lr));
>>> +  DEFINE(VGIC_CPU_HW_CFG,    offsetof(struct vgic_cpu, hw_cfg));
>>>  #ifdef CONFIG_KVM_ARM_TIMER
>>>    DEFINE(VCPU_TIMER_CNTV_CTL,        offsetof(struct kvm_vcpu, arch.timer_cpu.cntv_ctl));
>>>    DEFINE(VCPU_TIMER_CNTV_CVAL,       offsetof(struct kvm_vcpu, arch.timer_cpu.cntv_cval));
>>> diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S
>>> index 76af9302..9fbbf99 100644
>>> --- a/arch/arm/kvm/interrupts_head.S
>>> +++ b/arch/arm/kvm/interrupts_head.S
>>> @@ -419,7 +419,9 @@ 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_HW_CFG]
>>> +     mov     r10, r10, lsr #HWCFG_APR_SHIFT
>>> +     ldr     r10, [r2, r10]
>>>
>>>       str     r3, [r11, #VGIC_CPU_HCR]
>>>       str     r4, [r11, #VGIC_CPU_VMCR]
>>> @@ -435,9 +437,15 @@ vcpu     .req    r0              @ vcpu pointer always in r0
>>>       str     r5, [r2, #GICH_HCR]
>>>
>>>       /* Save list registers */
>>> -     add     r2, r2, #GICH_LR0
>>> +     ldr     r4, [r11, #VGIC_CPU_HW_CFG]
>>> +     mov     r10, r4, lsr #HWCFG_APR_SHIFT
>>> +     /* the offset between GICH_APR & GICH_LR0 is 0x10 */
>>> +     add     r10, r10, #0x10
>>> +     add     r2, r2, r10
>>>       add     r3, r11, #VGIC_CPU_LR
>>> -     ldr     r4, [r11, #VGIC_CPU_NR_LR]
>>> +     /* Get NR_LR from VGIC_CPU_HW_CFG */
>>> +     ldr     r6, =HWCFG_NR_LR_MASK
>>> +     and     r4, r4, r6
>>
>> Use "ubfx r4, r4, #0, #16" instead (with the proper symbolic constants,
>> of course). It will save loading this mask from the constant pool. For
>> consistency, you can also replace the above mov+lsr with the same
>> mechanism.
>>
> OK
>
>>>  1:   ldr     r6, [r2], #4
>>>       str     r6, [r3], #4
>>>       subs    r4, r4, #1
>>> @@ -469,12 +477,21 @@ vcpu    .req    r0              @ vcpu pointer always in r0
>>>
>>>       str     r3, [r2, #GICH_HCR]
>>>       str     r4, [r2, #GICH_VMCR]
>>> -     str     r8, [r2, #GICH_APR]
>>> +     ldr     r6, [r11, #VGIC_CPU_HW_CFG]
>>> +     mov     r6, r6, lsr #HWCFG_APR_SHIFT
>>> +     str     r8, [r2, r6]
>>>
>>>       /* Restore list registers */
>>> -     add     r2, r2, #GICH_LR0
>>> +     ldr     r4, [r11, #VGIC_CPU_HW_CFG]
>>> +     mov     r6, r4, lsr #HWCFG_APR_SHIFT
>>> +     /* the offset between GICH_APR & GICH_LR0 is 0x10 */
>>> +     add     r6, r6, #0x10
>>> +     /* get offset of GICH_LR0 */
>>> +     add     r2, r2, r6
>>> +     /* Get NR_LR from VGIC_CPU_HW_CFG */
>>>       add     r3, r11, #VGIC_CPU_LR
>>> -     ldr     r4, [r11, #VGIC_CPU_NR_LR]
>>> +     ldr     r6, =HWCFG_NR_LR_MASK
>>> +     and     r4, r4, r6
>>
>> Same here.
>>
>
> OK
>>>  1:   ldr     r6, [r3], #4
>>>       str     r6, [r2], #4
>>>       subs    r4, r4, #1
>>> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
>>> index 646f888..2422358 100644
>>> --- a/arch/arm64/kernel/asm-offsets.c
>>> +++ b/arch/arm64/kernel/asm-offsets.c
>>> @@ -136,7 +136,7 @@ int main(void)
>>>    DEFINE(VGIC_CPU_ELRSR,     offsetof(struct vgic_cpu, vgic_elrsr));
>>>    DEFINE(VGIC_CPU_APR,               offsetof(struct vgic_cpu, vgic_apr));
>>>    DEFINE(VGIC_CPU_LR,                offsetof(struct vgic_cpu, vgic_lr));
>>> -  DEFINE(VGIC_CPU_NR_LR,     offsetof(struct vgic_cpu, nr_lr));
>>> +  DEFINE(VGIC_CPU_HW_CFG,    offsetof(struct vgic_cpu, hw_cfg));
>>>    DEFINE(KVM_VTTBR,          offsetof(struct kvm, arch.vttbr));
>>>    DEFINE(KVM_VGIC_VCTRL,     offsetof(struct kvm, arch.vgic.vctrl_base));
>>>  #endif
>>> diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
>>> index b0d1512..2dbe337 100644
>>> --- a/arch/arm64/kvm/hyp.S
>>> +++ b/arch/arm64/kvm/hyp.S
>>> @@ -426,7 +426,7 @@ CPU_BE(   rev     w11, w11 )
>>>
>>>       /* Save list registers */
>>>       add     x2, x2, #GICH_LR0
>>> -     ldr     w4, [x3, #VGIC_CPU_NR_LR]
>>> +     ldr     w4, [x3, #VGIC_CPU_HW_CFG]
>>
>> Have you tested this? It looks to me that you're not dropping the top
>> bits, so you're going to end-up with 0x1000004 list registers instead of
>> the expected 4 with GIC-400...
>>
>
> As you mentioned, GICv2 isn't used in arm64. And the HIP04 GIC isn't
> used in arm64 also. I renamed to VGIC_CPU_NR_LR to VGIC_CPU_HW_CFG. It
> seems that I needn't drop it as APR offset isn't recorded in
> VGIC_CPU_HW_CFG.

If you understood that arm64 didn't use GICv2, I must have expressed
myself extremely badly. What do you think this code does? The only
saving grace is that the is no FrankenGIC involved here, and we just
have to trim the offset.

Thanks,

	M.
diff mbox

Patch

diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
index 85598b5..166cc98 100644
--- a/arch/arm/kernel/asm-offsets.c
+++ b/arch/arm/kernel/asm-offsets.c
@@ -189,7 +189,7 @@  int main(void)
   DEFINE(VGIC_CPU_ELRSR,	offsetof(struct vgic_cpu, vgic_elrsr));
   DEFINE(VGIC_CPU_APR,		offsetof(struct vgic_cpu, vgic_apr));
   DEFINE(VGIC_CPU_LR,		offsetof(struct vgic_cpu, vgic_lr));
-  DEFINE(VGIC_CPU_NR_LR,	offsetof(struct vgic_cpu, nr_lr));
+  DEFINE(VGIC_CPU_HW_CFG,	offsetof(struct vgic_cpu, hw_cfg));
 #ifdef CONFIG_KVM_ARM_TIMER
   DEFINE(VCPU_TIMER_CNTV_CTL,	offsetof(struct kvm_vcpu, arch.timer_cpu.cntv_ctl));
   DEFINE(VCPU_TIMER_CNTV_CVAL,	offsetof(struct kvm_vcpu, arch.timer_cpu.cntv_cval));
diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S
index 76af9302..9fbbf99 100644
--- a/arch/arm/kvm/interrupts_head.S
+++ b/arch/arm/kvm/interrupts_head.S
@@ -419,7 +419,9 @@  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_HW_CFG]
+	mov	r10, r10, lsr #HWCFG_APR_SHIFT
+	ldr	r10, [r2, r10]
 
 	str	r3, [r11, #VGIC_CPU_HCR]
 	str	r4, [r11, #VGIC_CPU_VMCR]
@@ -435,9 +437,15 @@  vcpu	.req	r0		@ vcpu pointer always in r0
 	str	r5, [r2, #GICH_HCR]
 
 	/* Save list registers */
-	add	r2, r2, #GICH_LR0
+	ldr	r4, [r11, #VGIC_CPU_HW_CFG]
+	mov	r10, r4, lsr #HWCFG_APR_SHIFT
+	/* the offset between GICH_APR & GICH_LR0 is 0x10 */
+	add	r10, r10, #0x10
+	add	r2, r2, r10
 	add	r3, r11, #VGIC_CPU_LR
-	ldr	r4, [r11, #VGIC_CPU_NR_LR]
+	/* Get NR_LR from VGIC_CPU_HW_CFG */
+	ldr	r6, =HWCFG_NR_LR_MASK
+	and	r4, r4, r6
 1:	ldr	r6, [r2], #4
 	str	r6, [r3], #4
 	subs	r4, r4, #1
@@ -469,12 +477,21 @@  vcpu	.req	r0		@ vcpu pointer always in r0
 
 	str	r3, [r2, #GICH_HCR]
 	str	r4, [r2, #GICH_VMCR]
-	str	r8, [r2, #GICH_APR]
+	ldr	r6, [r11, #VGIC_CPU_HW_CFG]
+	mov	r6, r6, lsr #HWCFG_APR_SHIFT
+	str	r8, [r2, r6]
 
 	/* Restore list registers */
-	add	r2, r2, #GICH_LR0
+	ldr	r4, [r11, #VGIC_CPU_HW_CFG]
+	mov	r6, r4, lsr #HWCFG_APR_SHIFT
+	/* the offset between GICH_APR & GICH_LR0 is 0x10 */
+	add	r6, r6, #0x10
+	/* get offset of GICH_LR0 */
+	add	r2, r2, r6
+	/* Get NR_LR from VGIC_CPU_HW_CFG */
 	add	r3, r11, #VGIC_CPU_LR
-	ldr	r4, [r11, #VGIC_CPU_NR_LR]
+	ldr	r6, =HWCFG_NR_LR_MASK
+	and	r4, r4, r6
 1:	ldr	r6, [r3], #4
 	str	r6, [r2], #4
 	subs	r4, r4, #1
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 646f888..2422358 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -136,7 +136,7 @@  int main(void)
   DEFINE(VGIC_CPU_ELRSR,	offsetof(struct vgic_cpu, vgic_elrsr));
   DEFINE(VGIC_CPU_APR,		offsetof(struct vgic_cpu, vgic_apr));
   DEFINE(VGIC_CPU_LR,		offsetof(struct vgic_cpu, vgic_lr));
-  DEFINE(VGIC_CPU_NR_LR,	offsetof(struct vgic_cpu, nr_lr));
+  DEFINE(VGIC_CPU_HW_CFG,	offsetof(struct vgic_cpu, hw_cfg));
   DEFINE(KVM_VTTBR,		offsetof(struct kvm, arch.vttbr));
   DEFINE(KVM_VGIC_VCTRL,	offsetof(struct kvm, arch.vgic.vctrl_base));
 #endif
diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
index b0d1512..2dbe337 100644
--- a/arch/arm64/kvm/hyp.S
+++ b/arch/arm64/kvm/hyp.S
@@ -426,7 +426,7 @@  CPU_BE(	rev	w11, w11 )
 
 	/* Save list registers */
 	add	x2, x2, #GICH_LR0
-	ldr	w4, [x3, #VGIC_CPU_NR_LR]
+	ldr	w4, [x3, #VGIC_CPU_HW_CFG]
 	add	x3, x3, #VGIC_CPU_LR
 1:	ldr	w5, [x2], #4
 CPU_BE(	rev	w5, w5 )
@@ -465,7 +465,7 @@  CPU_BE(	rev	w6, w6 )
 
 	/* Restore list registers */
 	add	x2, x2, #GICH_LR0
-	ldr	w4, [x3, #VGIC_CPU_NR_LR]
+	ldr	w4, [x3, #VGIC_CPU_HW_CFG]
 	add	x3, x3, #VGIC_CPU_LR
 1:	ldr	w5, [x3], #4
 CPU_BE(	rev	w5, w5 )
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index f27000f..eba4b51 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -122,8 +122,11 @@  struct vgic_cpu {
 	/* Bitmap of used/free list registers */
 	DECLARE_BITMAP(	lr_used, VGIC_MAX_LRS);
 
-	/* Number of list registers on this CPU */
-	int		nr_lr;
+	/*
+	 * bit[31:16]: GICH_APR offset
+	 * bit[15:0]:  Number of list registers on this CPU
+	 */
+	u32		hw_cfg;
 
 	/* CPU vif control registers for world switch */
 	u32		vgic_hcr;
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 45e2d8c..b055f92 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -49,6 +49,8 @@ 
 #define GICH_ELRSR1 			0x34
 #define GICH_APR			0xf0
 #define GICH_LR0			0x100
+#define HIP04_GICH_APR			0x70
+/* GICH_LR0 offset in HiP04 is 0x80 */
 
 #define GICH_HCR_EN			(1 << 0)
 #define GICH_HCR_UIE			(1 << 1)
@@ -73,6 +75,10 @@ 
 #define GICH_MISR_EOI			(1 << 0)
 #define GICH_MISR_U			(1 << 1)
 
+#define HWCFG_NR_LR_MASK	0xffff
+#define HWCFG_APR_SHIFT		16
+#define HWCFG_APR_MASK		(0xffff << HWCFG_APR_SHIFT)
+
 #ifndef __ASSEMBLY__
 
 struct device_node;
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 56ff9be..6c0ee89 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -97,7 +97,7 @@  static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu);
 static void vgic_update_state(struct kvm *kvm);
 static void vgic_kick_vcpus(struct kvm *kvm);
 static void vgic_dispatch_sgi(struct kvm_vcpu *vcpu, u32 reg);
-static u32 vgic_nr_lr;
+static u32 vgic_hw_cfg, vgic_nr_lr;
 
 static unsigned int vgic_maint_irq;
 
@@ -625,7 +625,7 @@  static void vgic_unqueue_irqs(struct kvm_vcpu *vcpu)
 	int i, irq, source_cpu;
 	u32 *lr;
 
-	for_each_set_bit(i, vgic_cpu->lr_used, vgic_cpu->nr_lr) {
+	for_each_set_bit(i, vgic_cpu->lr_used, vgic_nr_lr) {
 		lr = &vgic_cpu->vgic_lr[i];
 		irq = LR_IRQID(*lr);
 		source_cpu = LR_CPUID(*lr);
@@ -1006,7 +1006,7 @@  static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu)
 	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
 	int lr;
 
-	for_each_set_bit(lr, vgic_cpu->lr_used, vgic_cpu->nr_lr) {
+	for_each_set_bit(lr, vgic_cpu->lr_used, vgic_nr_lr) {
 		int irq = vgic_cpu->vgic_lr[lr] & GICH_LR_VIRTUALID;
 
 		if (!vgic_irq_is_enabled(vcpu, irq)) {
@@ -1047,8 +1047,8 @@  static bool vgic_queue_irq(struct kvm_vcpu *vcpu, u8 sgi_source_id, int irq)
 
 	/* Try to use another LR for this interrupt */
 	lr = find_first_zero_bit((unsigned long *)vgic_cpu->lr_used,
-			       vgic_cpu->nr_lr);
-	if (lr >= vgic_cpu->nr_lr)
+				 vgic_nr_lr);
+	if (lr >= vgic_nr_lr)
 		return false;
 
 	kvm_debug("LR%d allocated for IRQ%d %x\n", lr, irq, sgi_source_id);
@@ -1183,7 +1183,7 @@  static bool vgic_process_maintenance(struct kvm_vcpu *vcpu)
 		int lr, irq;
 
 		for_each_set_bit(lr, (unsigned long *)vgic_cpu->vgic_eisr,
-				 vgic_cpu->nr_lr) {
+				 vgic_nr_lr) {
 			irq = vgic_cpu->vgic_lr[lr] & GICH_LR_VIRTUALID;
 
 			vgic_irq_clear_active(vcpu, irq);
@@ -1227,7 +1227,7 @@  static void __kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
 
 	/* Clear mappings for empty LRs */
 	for_each_set_bit(lr, (unsigned long *)vgic_cpu->vgic_elrsr,
-			 vgic_cpu->nr_lr) {
+			 vgic_nr_lr) {
 		int irq;
 
 		if (!test_and_clear_bit(lr, vgic_cpu->lr_used))
@@ -1241,8 +1241,8 @@  static void __kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
 
 	/* Check if we still have something up our sleeve... */
 	pending = find_first_zero_bit((unsigned long *)vgic_cpu->vgic_elrsr,
-				      vgic_cpu->nr_lr);
-	if (level_pending || pending < vgic_cpu->nr_lr)
+				      vgic_nr_lr);
+	if (level_pending || pending < vgic_nr_lr)
 		set_bit(vcpu->vcpu_id, &dist->irq_pending_on_cpu);
 }
 
@@ -1438,7 +1438,7 @@  int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 	 */
 	vgic_cpu->vgic_vmcr = 0;
 
-	vgic_cpu->nr_lr = vgic_nr_lr;
+	vgic_cpu->hw_cfg = vgic_hw_cfg;
 	vgic_cpu->vgic_hcr = GICH_HCR_EN; /* Get the show on the road... */
 
 	return 0;
@@ -1470,17 +1470,31 @@  static struct notifier_block vgic_cpu_nb = {
 	.notifier_call = vgic_cpu_notify,
 };
 
+static const struct of_device_id of_vgic_ids[] = {
+	{
+		.compatible = "arm,cortex-a15-gic",
+		.data = (void *)GICH_APR,
+	}, {
+		.compatible = "hisilicon,hip04-gic",
+		.data = (void *)HIP04_GICH_APR,
+	}, {
+	},
+};
+
 int kvm_vgic_hyp_init(void)
 {
 	int ret;
 	struct resource vctrl_res;
 	struct resource vcpu_res;
+	const struct of_device_id *match;
 
-	vgic_node = of_find_compatible_node(NULL, NULL, "arm,cortex-a15-gic");
+	vgic_node = of_find_matching_node_and_match(NULL, of_vgic_ids, &match);
 	if (!vgic_node) {
 		kvm_err("error: no compatible vgic node in DT\n");
 		return -ENODEV;
 	}
+	/* High word of vgic_hw_cfg is the offset of GICH_APR. */
+	vgic_hw_cfg = (unsigned long)match->data << HWCFG_APR_SHIFT;
 
 	vgic_maint_irq = irq_of_parse_and_map(vgic_node, 0);
 	if (!vgic_maint_irq) {
@@ -1517,6 +1531,7 @@  int kvm_vgic_hyp_init(void)
 
 	vgic_nr_lr = readl_relaxed(vgic_vctrl_base + GICH_VTR);
 	vgic_nr_lr = (vgic_nr_lr & 0x3f) + 1;
+	vgic_hw_cfg |= vgic_nr_lr;
 
 	ret = create_hyp_io_mappings(vgic_vctrl_base,
 				     vgic_vctrl_base + resource_size(&vctrl_res),