| Message ID | 1400591427-21922-15-git-send-email-haojian.zhuang@linaro.org |
|---|---|
| State | Changes Requested |
| Headers | show |
On 20 May 2014 21:10, 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 | 28 ++++++++++++++++++------ > include/kvm/arm_vgic.h | 7 ++++-- > include/linux/irqchip/arm-gic.h | 6 ++++++ > virt/kvm/arm/vgic.c | 48 +++++++++++++++++++++++++++++------------ > 7 files changed, 92 insertions(+), 30 deletions(-) > > diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c > index 85598b5..166cc98 100644 > diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S > index 2c56012..a4a8b3d 100644 > --- a/arch/arm64/kvm/hyp.S > +++ b/arch/arm64/kvm/hyp.S > @@ -402,7 +402,9 @@ __kvm_hyp_code_start: > ldr w8, [x2, #GICH_EISR1] > ldr w9, [x2, #GICH_ELRSR0] > ldr w10, [x2, #GICH_ELRSR1] > - ldr w11, [x2, #GICH_APR] > + ldr w11, [x3, #VGIC_CPU_HW_CFG] > + mov w11, w11, lsr #HWCFG_APR_SHIFT > + ldr w11, [x2, w10] > CPU_BE( rev w4, w4 ) > CPU_BE( rev w5, w5 ) > CPU_BE( rev w6, w6 ) > @@ -425,8 +427,13 @@ CPU_BE( rev w11, w11 ) > str wzr, [x2, #GICH_HCR] > > /* Save list registers */ > - add x2, x2, #GICH_LR0 > - ldr w4, [x3, #VGIC_CPU_NR_LR] > + ldr w4, [x3, #VGIC_CPU_HW_CFG] > + mov w6, w4, lsr #HWCFG_APR_SHIFT > + ldr w7, =HWCFG_NR_LR_MASK > + and w4, w4, w7 > + /* the offset between GICH_APR and GICH_LR0 is 0x10 */ > + add w6, w6, 0x10 > + add x2, x2, w6 > add x3, x3, #VGIC_CPU_LR > 1: ldr w5, [x2], #4 > CPU_BE( rev w5, w5 ) > @@ -461,11 +468,20 @@ CPU_BE( rev w6, w6 ) > > str w4, [x2, #GICH_HCR] > str w5, [x2, #GICH_VMCR] > - str w6, [x2, #GICH_APR] > + ldr w4, [x3, #VGIC_CPU_HW_CFG] > + mov w4, w4, #HWCFG_APR_SHIFT > + str w6, [x2, w4] Oh, I just found it's wrong. Marc, How to handle this case? Do I need to use another x{n} register at here? If so, how to convert data from 32-bit register to 64-bit register? Regards Haojian
On Tue, May 20, 2014 at 09:10:27PM +0800, 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/kernel/asm-offsets.c | 2 +- > arch/arm/kvm/interrupts_head.S | 29 +++++++++++++++++++------ > arch/arm64/kernel/asm-offsets.c | 2 +- > arch/arm64/kvm/hyp.S | 28 ++++++++++++++++++------ > include/kvm/arm_vgic.h | 7 ++++-- > include/linux/irqchip/arm-gic.h | 6 ++++++ > virt/kvm/arm/vgic.c | 48 +++++++++++++++++++++++++++++------------ > 7 files changed, 92 insertions(+), 30 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 > 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 2c56012..a4a8b3d 100644 > --- a/arch/arm64/kvm/hyp.S > +++ b/arch/arm64/kvm/hyp.S > @@ -402,7 +402,9 @@ __kvm_hyp_code_start: > ldr w8, [x2, #GICH_EISR1] > ldr w9, [x2, #GICH_ELRSR0] > ldr w10, [x2, #GICH_ELRSR1] > - ldr w11, [x2, #GICH_APR] > + ldr w11, [x3, #VGIC_CPU_HW_CFG] > + mov w11, w11, lsr #HWCFG_APR_SHIFT > + ldr w11, [x2, w10] > CPU_BE( rev w4, w4 ) > CPU_BE( rev w5, w5 ) > CPU_BE( rev w6, w6 ) > @@ -425,8 +427,13 @@ CPU_BE( rev w11, w11 ) > str wzr, [x2, #GICH_HCR] > > /* Save list registers */ > - add x2, x2, #GICH_LR0 > - ldr w4, [x3, #VGIC_CPU_NR_LR] > + ldr w4, [x3, #VGIC_CPU_HW_CFG] > + mov w6, w4, lsr #HWCFG_APR_SHIFT > + ldr w7, =HWCFG_NR_LR_MASK > + and w4, w4, w7 > + /* the offset between GICH_APR and GICH_LR0 is 0x10 */ > + add w6, w6, 0x10 > + add x2, x2, w6 > add x3, x3, #VGIC_CPU_LR > 1: ldr w5, [x2], #4 > CPU_BE( rev w5, w5 ) > @@ -461,11 +468,20 @@ CPU_BE( rev w6, w6 ) > > str w4, [x2, #GICH_HCR] > str w5, [x2, #GICH_VMCR] > - str w6, [x2, #GICH_APR] > + ldr w4, [x3, #VGIC_CPU_HW_CFG] > + mov w4, w4, #HWCFG_APR_SHIFT > + str w6, [x2, w4] > > /* Restore list registers */ > - add x2, x2, #GICH_LR0 > - ldr w4, [x3, #VGIC_CPU_NR_LR] > + ldr w4, [x3, #VGIC_CPU_HW_CFG] > + mov w6, w4, #HWCFG_APR_SHIFT > + /* the offset between GICH_APR and GICH_LR0 is 0x10 */ > + add w6, w6, #0x10 > + /* get offset of GICH_LR0 */ > + add x2, x2, w6 > + /* get NR_LR from VGIC_CPU_HW_CFG */ > + ldr w6, =HWCFG_NR_LR_MASK > + and w4, w4, w6 > 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 47b2983..4c0c1e9 100644 > --- a/virt/kvm/arm/vgic.c > +++ b/virt/kvm/arm/vgic.c > @@ -76,6 +76,8 @@ > #define IMPLEMENTER_ARM 0x43b > #define GICC_ARCH_VERSION_V2 0x2 > > +#define vgic_nr_lr(vcpu) (vcpu->hw_cfg & HWCFG_NR_LR_MASK) > + > /* Physical address of vgic virtual cpu interface */ > static phys_addr_t vgic_vcpu_base; > > @@ -97,7 +99,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; > > static unsigned int vgic_maint_irq; > > @@ -624,9 +626,9 @@ static void vgic_unqueue_irqs(struct kvm_vcpu *vcpu) > struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu; > int vcpu_id = vcpu->vcpu_id; > int i, irq, source_cpu; > - u32 *lr; > + u32 *lr, nr_lr = vgic_nr_lr(vgic_cpu); This is static for any system post-boot, right? Can't we set this global variable once like we did before instead of having to define these extra variables and do the bit manipulation all over the place? -Christoffer
On 20 May 2014 21:44, Christoffer Dall <christoffer.dall@linaro.org> wrote: > On Tue, May 20, 2014 at 09:10:27PM +0800, 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/kernel/asm-offsets.c | 2 +- >> arch/arm/kvm/interrupts_head.S | 29 +++++++++++++++++++------ >> arch/arm64/kernel/asm-offsets.c | 2 +- >> arch/arm64/kvm/hyp.S | 28 ++++++++++++++++++------ >> include/kvm/arm_vgic.h | 7 ++++-- >> include/linux/irqchip/arm-gic.h | 6 ++++++ >> virt/kvm/arm/vgic.c | 48 +++++++++++++++++++++++++++++------------ >> 7 files changed, 92 insertions(+), 30 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 >> 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 2c56012..a4a8b3d 100644 >> --- a/arch/arm64/kvm/hyp.S >> +++ b/arch/arm64/kvm/hyp.S >> @@ -402,7 +402,9 @@ __kvm_hyp_code_start: >> ldr w8, [x2, #GICH_EISR1] >> ldr w9, [x2, #GICH_ELRSR0] >> ldr w10, [x2, #GICH_ELRSR1] >> - ldr w11, [x2, #GICH_APR] >> + ldr w11, [x3, #VGIC_CPU_HW_CFG] >> + mov w11, w11, lsr #HWCFG_APR_SHIFT >> + ldr w11, [x2, w10] >> CPU_BE( rev w4, w4 ) >> CPU_BE( rev w5, w5 ) >> CPU_BE( rev w6, w6 ) >> @@ -425,8 +427,13 @@ CPU_BE( rev w11, w11 ) >> str wzr, [x2, #GICH_HCR] >> >> /* Save list registers */ >> - add x2, x2, #GICH_LR0 >> - ldr w4, [x3, #VGIC_CPU_NR_LR] >> + ldr w4, [x3, #VGIC_CPU_HW_CFG] >> + mov w6, w4, lsr #HWCFG_APR_SHIFT >> + ldr w7, =HWCFG_NR_LR_MASK >> + and w4, w4, w7 >> + /* the offset between GICH_APR and GICH_LR0 is 0x10 */ >> + add w6, w6, 0x10 >> + add x2, x2, w6 >> add x3, x3, #VGIC_CPU_LR >> 1: ldr w5, [x2], #4 >> CPU_BE( rev w5, w5 ) >> @@ -461,11 +468,20 @@ CPU_BE( rev w6, w6 ) >> >> str w4, [x2, #GICH_HCR] >> str w5, [x2, #GICH_VMCR] >> - str w6, [x2, #GICH_APR] >> + ldr w4, [x3, #VGIC_CPU_HW_CFG] >> + mov w4, w4, #HWCFG_APR_SHIFT >> + str w6, [x2, w4] >> >> /* Restore list registers */ >> - add x2, x2, #GICH_LR0 >> - ldr w4, [x3, #VGIC_CPU_NR_LR] >> + ldr w4, [x3, #VGIC_CPU_HW_CFG] >> + mov w6, w4, #HWCFG_APR_SHIFT >> + /* the offset between GICH_APR and GICH_LR0 is 0x10 */ >> + add w6, w6, #0x10 >> + /* get offset of GICH_LR0 */ >> + add x2, x2, w6 >> + /* get NR_LR from VGIC_CPU_HW_CFG */ >> + ldr w6, =HWCFG_NR_LR_MASK >> + and w4, w4, w6 >> 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 47b2983..4c0c1e9 100644 >> --- a/virt/kvm/arm/vgic.c >> +++ b/virt/kvm/arm/vgic.c >> @@ -76,6 +76,8 @@ >> #define IMPLEMENTER_ARM 0x43b >> #define GICC_ARCH_VERSION_V2 0x2 >> >> +#define vgic_nr_lr(vcpu) (vcpu->hw_cfg & HWCFG_NR_LR_MASK) >> + >> /* Physical address of vgic virtual cpu interface */ >> static phys_addr_t vgic_vcpu_base; >> >> @@ -97,7 +99,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; >> >> static unsigned int vgic_maint_irq; >> >> @@ -624,9 +626,9 @@ static void vgic_unqueue_irqs(struct kvm_vcpu *vcpu) >> struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu; >> int vcpu_id = vcpu->vcpu_id; >> int i, irq, source_cpu; >> - u32 *lr; >> + u32 *lr, nr_lr = vgic_nr_lr(vgic_cpu); > > This is static for any system post-boot, right? Can't we set this > global variable once like we did before instead of having to define > these extra variables and do the bit manipulation all over the place? > > -Christoffer I tried to define a global gich_apr variable before. But Marc didn't agree on that. He suggested to use vgic_cpu_nr_lr to save both GICH_APR offset and nr_lr. Adding gich_apr variable should be the simpler implementation. Regards Haojian
On Tue, May 20, 2014 at 09:52:53PM +0800, Haojian Zhuang wrote: > On 20 May 2014 21:44, Christoffer Dall <christoffer.dall@linaro.org> wrote: > > On Tue, May 20, 2014 at 09:10:27PM +0800, Haojian Zhuang wrote: [...] > >> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c > >> index 47b2983..4c0c1e9 100644 > >> --- a/virt/kvm/arm/vgic.c > >> +++ b/virt/kvm/arm/vgic.c > >> @@ -76,6 +76,8 @@ > >> #define IMPLEMENTER_ARM 0x43b > >> #define GICC_ARCH_VERSION_V2 0x2 > >> > >> +#define vgic_nr_lr(vcpu) (vcpu->hw_cfg & HWCFG_NR_LR_MASK) > >> + > >> /* Physical address of vgic virtual cpu interface */ > >> static phys_addr_t vgic_vcpu_base; > >> > >> @@ -97,7 +99,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; > >> > >> static unsigned int vgic_maint_irq; > >> > >> @@ -624,9 +626,9 @@ static void vgic_unqueue_irqs(struct kvm_vcpu *vcpu) > >> struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu; > >> int vcpu_id = vcpu->vcpu_id; > >> int i, irq, source_cpu; > >> - u32 *lr; > >> + u32 *lr, nr_lr = vgic_nr_lr(vgic_cpu); > > > > This is static for any system post-boot, right? Can't we set this > > global variable once like we did before instead of having to define > > these extra variables and do the bit manipulation all over the place? > > > > -Christoffer > > I tried to define a global gich_apr variable before. But Marc didn't agree on > that. He suggested to use vgic_cpu_nr_lr to save both GICH_APR offset > and nr_lr. > > Adding gich_apr variable should be the simpler implementation. > You're talking about storing this information on the vgic_cpu struct, which is accessed on every world-switch patch. There, you don't want two memory accesses. Here, on the other hand, you're in host kernel land, and you can do your bit-shuffling once, and always access a single static variable like we did before, which will simplify the C-code. Makes sense? -Christoffer
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 2c56012..a4a8b3d 100644 --- a/arch/arm64/kvm/hyp.S +++ b/arch/arm64/kvm/hyp.S @@ -402,7 +402,9 @@ __kvm_hyp_code_start: ldr w8, [x2, #GICH_EISR1] ldr w9, [x2, #GICH_ELRSR0] ldr w10, [x2, #GICH_ELRSR1] - ldr w11, [x2, #GICH_APR] + ldr w11, [x3, #VGIC_CPU_HW_CFG] + mov w11, w11, lsr #HWCFG_APR_SHIFT + ldr w11, [x2, w10] CPU_BE( rev w4, w4 ) CPU_BE( rev w5, w5 ) CPU_BE( rev w6, w6 ) @@ -425,8 +427,13 @@ CPU_BE( rev w11, w11 ) str wzr, [x2, #GICH_HCR] /* Save list registers */ - add x2, x2, #GICH_LR0 - ldr w4, [x3, #VGIC_CPU_NR_LR] + ldr w4, [x3, #VGIC_CPU_HW_CFG] + mov w6, w4, lsr #HWCFG_APR_SHIFT + ldr w7, =HWCFG_NR_LR_MASK + and w4, w4, w7 + /* the offset between GICH_APR and GICH_LR0 is 0x10 */ + add w6, w6, 0x10 + add x2, x2, w6 add x3, x3, #VGIC_CPU_LR 1: ldr w5, [x2], #4 CPU_BE( rev w5, w5 ) @@ -461,11 +468,20 @@ CPU_BE( rev w6, w6 ) str w4, [x2, #GICH_HCR] str w5, [x2, #GICH_VMCR] - str w6, [x2, #GICH_APR] + ldr w4, [x3, #VGIC_CPU_HW_CFG] + mov w4, w4, #HWCFG_APR_SHIFT + str w6, [x2, w4] /* Restore list registers */ - add x2, x2, #GICH_LR0 - ldr w4, [x3, #VGIC_CPU_NR_LR] + ldr w4, [x3, #VGIC_CPU_HW_CFG] + mov w6, w4, #HWCFG_APR_SHIFT + /* the offset between GICH_APR and GICH_LR0 is 0x10 */ + add w6, w6, #0x10 + /* get offset of GICH_LR0 */ + add x2, x2, w6 + /* get NR_LR from VGIC_CPU_HW_CFG */ + ldr w6, =HWCFG_NR_LR_MASK + and w4, w4, w6 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 47b2983..4c0c1e9 100644 --- a/virt/kvm/arm/vgic.c +++ b/virt/kvm/arm/vgic.c @@ -76,6 +76,8 @@ #define IMPLEMENTER_ARM 0x43b #define GICC_ARCH_VERSION_V2 0x2 +#define vgic_nr_lr(vcpu) (vcpu->hw_cfg & HWCFG_NR_LR_MASK) + /* Physical address of vgic virtual cpu interface */ static phys_addr_t vgic_vcpu_base; @@ -97,7 +99,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; static unsigned int vgic_maint_irq; @@ -624,9 +626,9 @@ static void vgic_unqueue_irqs(struct kvm_vcpu *vcpu) struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu; int vcpu_id = vcpu->vcpu_id; int i, irq, source_cpu; - u32 *lr; + u32 *lr, nr_lr = vgic_nr_lr(vgic_cpu); - for_each_set_bit(i, vgic_cpu->lr_used, vgic_cpu->nr_lr) { + for_each_set_bit(i, vgic_cpu->lr_used, nr_lr) { lr = &vgic_cpu->vgic_lr[i]; irq = LR_IRQID(*lr); source_cpu = LR_CPUID(*lr); @@ -1005,8 +1007,9 @@ static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu) { struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu; int lr; + int nr_lr = vgic_nr_lr(vgic_cpu); - for_each_set_bit(lr, vgic_cpu->lr_used, vgic_cpu->nr_lr) { + for_each_set_bit(lr, vgic_cpu->lr_used, nr_lr) { int irq = vgic_cpu->vgic_lr[lr] & GICH_LR_VIRTUALID; if (!vgic_irq_is_enabled(vcpu, irq)) { @@ -1025,6 +1028,7 @@ static bool vgic_queue_irq(struct kvm_vcpu *vcpu, u8 sgi_source_id, int irq) { struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu; int lr; + int nr_lr = vgic_nr_lr(vgic_cpu); /* Sanitize the input... */ BUG_ON(sgi_source_id & ~7); @@ -1046,9 +1050,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) + lr = find_first_zero_bit((unsigned long *)vgic_cpu->lr_used, nr_lr); + if (lr >= nr_lr) return false; kvm_debug("LR%d allocated for IRQ%d %x\n", lr, irq, sgi_source_id); @@ -1181,9 +1184,10 @@ static bool vgic_process_maintenance(struct kvm_vcpu *vcpu) * active bit. */ int lr, irq; + int nr_lr = vgic_nr_lr(vgic_cpu); for_each_set_bit(lr, (unsigned long *)vgic_cpu->vgic_eisr, - vgic_cpu->nr_lr) { + nr_lr) { irq = vgic_cpu->vgic_lr[lr] & GICH_LR_VIRTUALID; vgic_irq_clear_active(vcpu, irq); @@ -1221,13 +1225,13 @@ static void __kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu) struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu; struct vgic_dist *dist = &vcpu->kvm->arch.vgic; int lr, pending; + int nr_lr = vgic_nr_lr(vgic_cpu); bool level_pending; level_pending = vgic_process_maintenance(vcpu); /* Clear mappings for empty LRs */ - for_each_set_bit(lr, (unsigned long *)vgic_cpu->vgic_elrsr, - vgic_cpu->nr_lr) { + for_each_set_bit(lr, (unsigned long *)vgic_cpu->vgic_elrsr, nr_lr) { int irq; if (!test_and_clear_bit(lr, vgic_cpu->lr_used)) @@ -1241,8 +1245,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) + nr_lr); + if (level_pending || pending < nr_lr) set_bit(vcpu->vcpu_id, &dist->irq_pending_on_cpu); } @@ -1438,7 +1442,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 +1474,32 @@ 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; + u32 vgic_nr_lr; - 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 int)match->data << HWCFG_APR_SHIFT; vgic_maint_irq = irq_of_parse_and_map(vgic_node, 0); if (!vgic_maint_irq) { @@ -1517,6 +1536,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),
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 | 28 ++++++++++++++++++------ include/kvm/arm_vgic.h | 7 ++++-- include/linux/irqchip/arm-gic.h | 6 ++++++ virt/kvm/arm/vgic.c | 48 +++++++++++++++++++++++++++++------------ 7 files changed, 92 insertions(+), 30 deletions(-)