| Message ID | 1397801156-25682-14-git-send-email-haojian.zhuang@linaro.org |
|---|---|
| State | Changes Requested |
| Headers | show |
On Fri, Apr 18 2014 at 7:05:56 am BST, Haojian Zhuang <haojian.zhuang@linaro.org> wrote: > In HiP04 SoC, the address of GICH_APR & GICH_LR0 registers are different > from ARM standard SoC. So add the support of HiP04 SoC in VGIC. Please explain the differences in the commit message. > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org> > --- > arch/arm/kvm/interrupts_head.S | 23 +++++++++++++++++++---- > include/linux/irqchip/arm-gic.h | 3 +++ > virt/kvm/arm/vgic.c | 10 ++++++++-- > 3 files changed, 30 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S > index 76af9302..13e4144 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, =gich_apr > + ldr r10, [r10] > + ldr r10, [r2, r10] > > str r3, [r11, #VGIC_CPU_HCR] > str r4, [r11, #VGIC_CPU_VMCR] > @@ -435,7 +437,9 @@ vcpu .req r0 @ vcpu pointer always in r0 > str r5, [r2, #GICH_HCR] > > /* Save list registers */ > - add r2, r2, #GICH_LR0 > + ldr r10, =gich_lr0 > + ldr r10, [r10] > + add r2, r2, r10 > add r3, r11, #VGIC_CPU_LR > ldr r4, [r11, #VGIC_CPU_NR_LR] > 1: ldr r6, [r2], #4 > @@ -469,10 +473,14 @@ 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, =gich_apr > + ldr r6, [r6] > + str r8, [r2, r6] > > /* Restore list registers */ > - add r2, r2, #GICH_LR0 > + ldr r6, =gich_lr0 > + ldr r6, [r6] > + add r2, r2, r6 > add r3, r11, #VGIC_CPU_LR > ldr r4, [r11, #VGIC_CPU_NR_LR] > 1: ldr r6, [r3], #4 So we get four extra memory accesses per world switch, just to find out about the new fancy memory map. Let's just say that I'm not exactly pleased. How about encoding that in the field containing the number of LRs? the distance between GICH_APR and GICH_LR0 is constant (0x10), so we only need to encode the offset of GICH_APR. This way, we only have to read one single word from memory. Not pretty, but I'm not really willing to impact > @@ -618,3 +626,10 @@ vcpu .req r0 @ vcpu pointer always in r0 > .macro load_vcpu > mrc p15, 4, vcpu, c13, c0, 2 @ HTPIDR > .endm > + > + .global gich_apr > +gich_apr: > + .long GICH_APR > + .global gich_lr0 > +gich_lr0: > + .long GICH_LR0 > diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h > index 55933aa..653525b 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 > +#define HIP04_GICH_LR0 0x80 > > #define GICH_HCR_EN (1 << 0) > #define GICH_HCR_UIE (1 << 1) > @@ -78,6 +80,7 @@ > struct device_node; > > extern struct irq_chip gic_arch_extn; > +extern unsigned int gich_apr, gich_lr0; > > void gic_init_bases(unsigned int, int, void __iomem *, void __iomem *, > u32 offset, struct device_node *); > diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c > index 47b2983..010e491 100644 > --- a/virt/kvm/arm/vgic.c > +++ b/virt/kvm/arm/vgic.c > @@ -1478,8 +1478,14 @@ int kvm_vgic_hyp_init(void) > > vgic_node = of_find_compatible_node(NULL, NULL, "arm,cortex-a15-gic"); > if (!vgic_node) { > - kvm_err("error: no compatible vgic node in DT\n"); > - return -ENODEV; > + vgic_node = of_find_compatible_node(NULL, NULL, > + "hisilicon,hip04-gic"); > + if (!vgic_node) { > + kvm_err("error: no compatible vgic node in DT\n"); > + return -ENODEV; > + } > + gich_apr = HIP04_GICH_APR; > + gich_lr0 = HIP04_GICH_LR0; Consider using of_find_matching_node_and_match instead, with a set of flags for this particular case. > } > > vgic_maint_irq = irq_of_parse_and_map(vgic_node, 0); Thanks, M.
On 22 April 2014 20:15, Marc Zyngier <marc.zyngier@arm.com> wrote: > On Fri, Apr 18 2014 at 7:05:56 am BST, Haojian Zhuang <haojian.zhuang@linaro.org> wrote: >> In HiP04 SoC, the address of GICH_APR & GICH_LR0 registers are different >> from ARM standard SoC. So add the support of HiP04 SoC in VGIC. > > Please explain the differences in the commit message. > OK. I'll append it. >> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org> >> --- >> arch/arm/kvm/interrupts_head.S | 23 +++++++++++++++++++---- >> include/linux/irqchip/arm-gic.h | 3 +++ >> virt/kvm/arm/vgic.c | 10 ++++++++-- >> 3 files changed, 30 insertions(+), 6 deletions(-) >> >> diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S >> index 76af9302..13e4144 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, =gich_apr >> + ldr r10, [r10] >> + ldr r10, [r2, r10] >> >> str r3, [r11, #VGIC_CPU_HCR] >> str r4, [r11, #VGIC_CPU_VMCR] >> @@ -435,7 +437,9 @@ vcpu .req r0 @ vcpu pointer always in r0 >> str r5, [r2, #GICH_HCR] >> >> /* Save list registers */ >> - add r2, r2, #GICH_LR0 >> + ldr r10, =gich_lr0 >> + ldr r10, [r10] >> + add r2, r2, r10 >> add r3, r11, #VGIC_CPU_LR >> ldr r4, [r11, #VGIC_CPU_NR_LR] >> 1: ldr r6, [r2], #4 >> @@ -469,10 +473,14 @@ 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, =gich_apr >> + ldr r6, [r6] >> + str r8, [r2, r6] >> >> /* Restore list registers */ >> - add r2, r2, #GICH_LR0 >> + ldr r6, =gich_lr0 >> + ldr r6, [r6] >> + add r2, r2, r6 >> add r3, r11, #VGIC_CPU_LR >> ldr r4, [r11, #VGIC_CPU_NR_LR] >> 1: ldr r6, [r3], #4 > > So we get four extra memory accesses per world switch, just to find out > about the new fancy memory map. Let's just say that I'm not exactly > pleased. > > How about encoding that in the field containing the number of LRs? the > distance between GICH_APR and GICH_LR0 is constant (0x10), so we only > need to encode the offset of GICH_APR. This way, we only have to read > one single word from memory. Not pretty, but I'm not really willing to > impact > Yes, it's better. > >> @@ -618,3 +626,10 @@ vcpu .req r0 @ vcpu pointer always in r0 >> .macro load_vcpu >> mrc p15, 4, vcpu, c13, c0, 2 @ HTPIDR >> .endm >> + >> + .global gich_apr >> +gich_apr: >> + .long GICH_APR >> + .global gich_lr0 >> +gich_lr0: >> + .long GICH_LR0 >> diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h >> index 55933aa..653525b 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 >> +#define HIP04_GICH_LR0 0x80 >> >> #define GICH_HCR_EN (1 << 0) >> #define GICH_HCR_UIE (1 << 1) >> @@ -78,6 +80,7 @@ >> struct device_node; >> >> extern struct irq_chip gic_arch_extn; >> +extern unsigned int gich_apr, gich_lr0; >> >> void gic_init_bases(unsigned int, int, void __iomem *, void __iomem *, >> u32 offset, struct device_node *); >> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c >> index 47b2983..010e491 100644 >> --- a/virt/kvm/arm/vgic.c >> +++ b/virt/kvm/arm/vgic.c >> @@ -1478,8 +1478,14 @@ int kvm_vgic_hyp_init(void) >> >> vgic_node = of_find_compatible_node(NULL, NULL, "arm,cortex-a15-gic"); >> if (!vgic_node) { >> - kvm_err("error: no compatible vgic node in DT\n"); >> - return -ENODEV; >> + vgic_node = of_find_compatible_node(NULL, NULL, >> + "hisilicon,hip04-gic"); >> + if (!vgic_node) { >> + kvm_err("error: no compatible vgic node in DT\n"); >> + return -ENODEV; >> + } >> + gich_apr = HIP04_GICH_APR; >> + gich_lr0 = HIP04_GICH_LR0; > > Consider using of_find_matching_node_and_match instead, with a set of > flags for this particular case. > OK. I'll use it. Thanks Haojian
diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S index 76af9302..13e4144 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, =gich_apr + ldr r10, [r10] + ldr r10, [r2, r10] str r3, [r11, #VGIC_CPU_HCR] str r4, [r11, #VGIC_CPU_VMCR] @@ -435,7 +437,9 @@ vcpu .req r0 @ vcpu pointer always in r0 str r5, [r2, #GICH_HCR] /* Save list registers */ - add r2, r2, #GICH_LR0 + ldr r10, =gich_lr0 + ldr r10, [r10] + add r2, r2, r10 add r3, r11, #VGIC_CPU_LR ldr r4, [r11, #VGIC_CPU_NR_LR] 1: ldr r6, [r2], #4 @@ -469,10 +473,14 @@ 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, =gich_apr + ldr r6, [r6] + str r8, [r2, r6] /* Restore list registers */ - add r2, r2, #GICH_LR0 + ldr r6, =gich_lr0 + ldr r6, [r6] + add r2, r2, r6 add r3, r11, #VGIC_CPU_LR ldr r4, [r11, #VGIC_CPU_NR_LR] 1: ldr r6, [r3], #4 @@ -618,3 +626,10 @@ vcpu .req r0 @ vcpu pointer always in r0 .macro load_vcpu mrc p15, 4, vcpu, c13, c0, 2 @ HTPIDR .endm + + .global gich_apr +gich_apr: + .long GICH_APR + .global gich_lr0 +gich_lr0: + .long GICH_LR0 diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h index 55933aa..653525b 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 +#define HIP04_GICH_LR0 0x80 #define GICH_HCR_EN (1 << 0) #define GICH_HCR_UIE (1 << 1) @@ -78,6 +80,7 @@ struct device_node; extern struct irq_chip gic_arch_extn; +extern unsigned int gich_apr, gich_lr0; void gic_init_bases(unsigned int, int, void __iomem *, void __iomem *, u32 offset, struct device_node *); diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c index 47b2983..010e491 100644 --- a/virt/kvm/arm/vgic.c +++ b/virt/kvm/arm/vgic.c @@ -1478,8 +1478,14 @@ int kvm_vgic_hyp_init(void) vgic_node = of_find_compatible_node(NULL, NULL, "arm,cortex-a15-gic"); if (!vgic_node) { - kvm_err("error: no compatible vgic node in DT\n"); - return -ENODEV; + vgic_node = of_find_compatible_node(NULL, NULL, + "hisilicon,hip04-gic"); + if (!vgic_node) { + kvm_err("error: no compatible vgic node in DT\n"); + return -ENODEV; + } + gich_apr = HIP04_GICH_APR; + gich_lr0 = HIP04_GICH_LR0; } vgic_maint_irq = irq_of_parse_and_map(vgic_node, 0);
In HiP04 SoC, the address of GICH_APR & GICH_LR0 registers are different from ARM standard SoC. So add the support of HiP04 SoC in VGIC. Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org> --- arch/arm/kvm/interrupts_head.S | 23 +++++++++++++++++++---- include/linux/irqchip/arm-gic.h | 3 +++ virt/kvm/arm/vgic.c | 10 ++++++++-- 3 files changed, 30 insertions(+), 6 deletions(-)