| Message ID | 1478636499-14339-10-git-send-email-drjones@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi, On 08/11/16 20:21, Andrew Jones wrote: > Signed-off-by: Andrew Jones <drjones@redhat.com> > > --- > v4: > - only take defines from kernel we need now [Andre] > - simplify enable by not caring if we reinit the distributor [drew] > v2: > - configure irqs as NS GRP1 > --- > lib/arm/asm/arch_gicv3.h | 42 +++++++++++++++++++++ > lib/arm/asm/gic-v3.h | 92 ++++++++++++++++++++++++++++++++++++++++++++++ > lib/arm/asm/gic.h | 1 + > lib/arm/gic.c | 56 ++++++++++++++++++++++++++++ > lib/arm64/asm/arch_gicv3.h | 44 ++++++++++++++++++++++ > lib/arm64/asm/gic-v3.h | 1 + > lib/arm64/asm/sysreg.h | 44 ++++++++++++++++++++++ > 7 files changed, 280 insertions(+) > create mode 100644 lib/arm/asm/arch_gicv3.h > create mode 100644 lib/arm/asm/gic-v3.h > create mode 100644 lib/arm64/asm/arch_gicv3.h > create mode 100644 lib/arm64/asm/gic-v3.h > create mode 100644 lib/arm64/asm/sysreg.h > > diff --git a/lib/arm/asm/arch_gicv3.h b/lib/arm/asm/arch_gicv3.h > new file mode 100644 > index 000000000000..81a1e5f6c29c > --- /dev/null > +++ b/lib/arm/asm/arch_gicv3.h > @@ -0,0 +1,42 @@ > +/* > + * All ripped off from arch/arm/include/asm/arch_gicv3.h > + * > + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com> > + * > + * This work is licensed under the terms of the GNU LGPL, version 2. > + */ > +#ifndef _ASMARM_ARCH_GICV3_H_ > +#define _ASMARM_ARCH_GICV3_H_ > + > +#ifndef __ASSEMBLY__ > +#include <libcflat.h> > +#include <asm/barrier.h> > +#include <asm/io.h> > + > +#define __stringify xstr > + > +#define __ACCESS_CP15(CRn, Op1, CRm, Op2) p15, Op1, %0, CRn, CRm, Op2 > + > +#define ICC_PMR __ACCESS_CP15(c4, 0, c6, 0) > +#define ICC_IGRPEN1 __ACCESS_CP15(c12, 0, c12, 7) > + > +static inline void gicv3_write_pmr(u32 val) > +{ > + asm volatile("mcr " __stringify(ICC_PMR) : : "r" (val)); > +} > + > +static inline void gicv3_write_grpen1(u32 val) > +{ > + asm volatile("mcr " __stringify(ICC_IGRPEN1) : : "r" (val)); > + isb(); > +} > + > +static inline u64 gicv3_read_typer(const volatile void __iomem *addr) > +{ > + u64 val = readl(addr); > + val |= (u64)readl(addr + 4) << 32; > + return val; > +} > + > +#endif /* !__ASSEMBLY__ */ > +#endif /* _ASMARM_ARCH_GICV3_H_ */ > diff --git a/lib/arm/asm/gic-v3.h b/lib/arm/asm/gic-v3.h > new file mode 100644 > index 000000000000..03321f8c860f > --- /dev/null > +++ b/lib/arm/asm/gic-v3.h > @@ -0,0 +1,92 @@ > +/* > + * All GIC* defines are lifted from include/linux/irqchip/arm-gic-v3.h > + * > + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com> > + * > + * This work is licensed under the terms of the GNU LGPL, version 2. > + */ > +#ifndef _ASMARM_GIC_V3_H_ > +#define _ASMARM_GIC_V3_H_ > + > +#ifndef _ASMARM_GIC_H_ > +#error Do not directly include <asm/gic-v3.h>. Include <asm/gic.h> > +#endif > + > +#define GICD_CTLR 0x0000 > +#define GICD_TYPER 0x0004 So if we share the distributor register definition with GICv2, these shouldn't be here, but in gic.h. But this is the right naming scheme we should use (instead of GIC_DIST_xxx). Now this gets interesting with your wish to both share the definitions for the GICv2 and GICv3 distributors, but also stick to the names the kernel uses (because they differ between the two) ;-) So now you loose the greppability for either GIC_DIST_CTR or GICD_TYPER, for instance. > +#define GICD_IGROUPR 0x0080 > + > +#define GICD_CTLR_RWP (1U << 31) > +#define GICD_CTLR_ARE_NS (1U << 4) > +#define GICD_CTLR_ENABLE_G1A (1U << 1) > +#define GICD_CTLR_ENABLE_G1 (1U << 0) > + > +#define GICR_TYPER 0x0008 > +#define GICR_IGROUPR0 GICD_IGROUPR > +#define GICR_TYPER_LAST (1U << 4) > + > + > +#include <asm/arch_gicv3.h> > + > +#ifndef __ASSEMBLY__ > +#include <asm/setup.h> > +#include <asm/smp.h> > +#include <asm/processor.h> > +#include <asm/io.h> > + > +struct gicv3_data { > + void *dist_base; > + void *redist_base[NR_CPUS]; > + unsigned int irq_nr; > +}; > +extern struct gicv3_data gicv3_data; > + > +#define gicv3_dist_base() (gicv3_data.dist_base) > +#define gicv3_redist_base() (gicv3_data.redist_base[smp_processor_id()]) > +#define gicv3_sgi_base() (gicv3_data.redist_base[smp_processor_id()] + SZ_64K) > + > +extern int gicv3_init(void); > +extern void gicv3_enable_defaults(void); > + > +static inline void gicv3_do_wait_for_rwp(void *base) > +{ > + int count = 100000; /* 1s */ > + > + while (readl(base + GICD_CTLR) & GICD_CTLR_RWP) { > + if (!--count) { > + printf("GICv3: RWP timeout!\n"); > + abort(); > + } > + cpu_relax(); > + udelay(10); > + }; > +} > + > +static inline void gicv3_dist_wait_for_rwp(void) > +{ > + gicv3_do_wait_for_rwp(gicv3_dist_base()); > +} > + > +static inline void gicv3_redist_wait_for_rwp(void) > +{ > + gicv3_do_wait_for_rwp(gicv3_redist_base()); > +} > + > +static inline u32 mpidr_compress(u64 mpidr) > +{ > + u64 compressed = mpidr & MPIDR_HWID_BITMASK; > + > + compressed = (((compressed >> 32) & 0xff) << 24) | compressed; > + return compressed; > +} > + > +static inline u64 mpidr_uncompress(u32 compressed) > +{ > + u64 mpidr = ((u64)compressed >> 24) << 32; > + > + mpidr |= compressed & MPIDR_HWID_BITMASK; > + return mpidr; > +} > + > +#endif /* !__ASSEMBLY__ */ > +#endif /* _ASMARM_GIC_V3_H_ */ > diff --git a/lib/arm/asm/gic.h b/lib/arm/asm/gic.h > index 328e078a9ae1..4897bc592cdd 100644 > --- a/lib/arm/asm/gic.h > +++ b/lib/arm/asm/gic.h > @@ -7,6 +7,7 @@ > #define _ASMARM_GIC_H_ > > #include <asm/gic-v2.h> > +#include <asm/gic-v3.h> > > #define GIC_CPU_CTRL 0x00 > #define GIC_CPU_PRIMASK 0x04 > diff --git a/lib/arm/gic.c b/lib/arm/gic.c > index 91d78c9a0cc2..af58a11ea13e 100644 > --- a/lib/arm/gic.c > +++ b/lib/arm/gic.c > @@ -8,9 +8,11 @@ > #include <asm/io.h> > > struct gicv2_data gicv2_data; > +struct gicv3_data gicv3_data; > > /* > * Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt > + * Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt > */ > static bool > gic_get_dt_bases(const char *compatible, void **base1, void **base2) > @@ -48,10 +50,18 @@ int gicv2_init(void) > &gicv2_data.dist_base, &gicv2_data.cpu_base); > } > > +int gicv3_init(void) > +{ > + return gic_get_dt_bases("arm,gic-v3", &gicv3_data.dist_base, > + &gicv3_data.redist_base[0]); > +} > + > int gic_init(void) > { > if (gicv2_init()) > return 2; > + else if (gicv3_init()) > + return 3; > return 0; > } > > @@ -73,3 +83,49 @@ void gicv2_enable_defaults(void) > writel(GICC_INT_PRI_THRESHOLD, cpu_base + GIC_CPU_PRIMASK); > writel(GICC_ENABLE, cpu_base + GIC_CPU_CTRL); > } > + > +void gicv3_set_redist_base(void) > +{ > + u32 aff = mpidr_compress(get_mpidr()); > + void *ptr = gicv3_data.redist_base[0]; > + u64 typer; > + > + do { > + typer = gicv3_read_typer(ptr + GICR_TYPER); > + if ((typer >> 32) == aff) { > + gicv3_redist_base() = ptr; > + return; > + } > + ptr += SZ_64K * 2; /* skip RD_base and SGI_base */ > + } while (!(typer & GICR_TYPER_LAST)); > + assert(0); > +} > + > +void gicv3_enable_defaults(void) > +{ > + void *dist = gicv3_dist_base(); > + void *sgi_base; > + unsigned int i; > + > + gicv3_data.irq_nr = GICD_TYPER_IRQS(readl(dist + GICD_TYPER)); > + if (gicv3_data.irq_nr > 1020) > + gicv3_data.irq_nr = 1020; > + > + writel(GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A | GICD_CTLR_ENABLE_G1, > + dist + GICD_CTLR); > + gicv3_dist_wait_for_rwp(); > + > + if (!gicv3_redist_base()) > + gicv3_set_redist_base(); > + sgi_base = gicv3_sgi_base(); > + > + writel(~0, sgi_base + GICR_IGROUPR0); This is mixing redist setup with distributor setup. Is it that what you meant with: " - simplify enable by not caring if we reinit the distributor [drew]"? Also if you set the group for the SGIs, you should set it for SPIs as well (like the kernel does). This was done in v3 of the series. What about you finish the per-CPU setup first, then bail out with: if (smp_processor_id() != 0) return; and then do the distributor setup (only on the first core). Cheers, Andre. > + writel(GICD_INT_EN_SET_SGI, sgi_base + GIC_DIST_ENABLE_SET); > + > + for (i = 0; i < 32; i += 4) > + writel(GICD_INT_DEF_PRI_X4, sgi_base + GIC_DIST_PRI + i); > + gicv3_redist_wait_for_rwp(); > + > + gicv3_write_pmr(0xf0); > + gicv3_write_grpen1(1); > +} > diff --git a/lib/arm64/asm/arch_gicv3.h b/lib/arm64/asm/arch_gicv3.h > new file mode 100644 > index 000000000000..6d353567f56a > --- /dev/null > +++ b/lib/arm64/asm/arch_gicv3.h > @@ -0,0 +1,44 @@ > +/* > + * All ripped off from arch/arm64/include/asm/arch_gicv3.h > + * > + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com> > + * > + * This work is licensed under the terms of the GNU LGPL, version 2. > + */ > +#ifndef _ASMARM64_ARCH_GICV3_H_ > +#define _ASMARM64_ARCH_GICV3_H_ > + > +#include <asm/sysreg.h> > + > +#define ICC_PMR_EL1 sys_reg(3, 0, 4, 6, 0) > +#define ICC_GRPEN1_EL1 sys_reg(3, 0, 12, 12, 7) > + > +#ifndef __ASSEMBLY__ > + > +#include <libcflat.h> > +#include <asm/barrier.h> > + > +#define __stringify xstr > + > +/* > + * Low-level accessors > + * > + * These system registers are 32 bits, but we make sure that the compiler > + * sets the GP register's most significant bits to 0 with an explicit cast. > + */ > + > +static inline void gicv3_write_pmr(u32 val) > +{ > + asm volatile("msr_s " __stringify(ICC_PMR_EL1) ", %0" : : "r" ((u64)val)); > +} > + > +static inline void gicv3_write_grpen1(u32 val) > +{ > + asm volatile("msr_s " __stringify(ICC_GRPEN1_EL1) ", %0" : : "r" ((u64)val)); > + isb(); > +} > + > +#define gicv3_read_typer(c) readq(c) > + > +#endif /* __ASSEMBLY__ */ > +#endif /* _ASMARM64_ARCH_GICV3_H_ */ > diff --git a/lib/arm64/asm/gic-v3.h b/lib/arm64/asm/gic-v3.h > new file mode 100644 > index 000000000000..8ee5d4d9c181 > --- /dev/null > +++ b/lib/arm64/asm/gic-v3.h > @@ -0,0 +1 @@ > +#include "../../arm/asm/gic-v3.h" > diff --git a/lib/arm64/asm/sysreg.h b/lib/arm64/asm/sysreg.h > new file mode 100644 > index 000000000000..544a46cb8cc5 > --- /dev/null > +++ b/lib/arm64/asm/sysreg.h > @@ -0,0 +1,44 @@ > +/* > + * Ripped off from arch/arm64/include/asm/sysreg.h > + * > + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com> > + * > + * This work is licensed under the terms of the GNU LGPL, version 2. > + */ > +#ifndef _ASMARM64_SYSREG_H_ > +#define _ASMARM64_SYSREG_H_ > + > +#define sys_reg(op0, op1, crn, crm, op2) \ > + ((((op0)&3)<<19)|((op1)<<16)|((crn)<<12)|((crm)<<8)|((op2)<<5)) > + > +#ifdef __ASSEMBLY__ > + .irp num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30 > + .equ .L__reg_num_x\num, \num > + .endr > + .equ .L__reg_num_xzr, 31 > + > + .macro mrs_s, rt, sreg > + .inst 0xd5200000|(\sreg)|(.L__reg_num_\rt) > + .endm > + > + .macro msr_s, sreg, rt > + .inst 0xd5000000|(\sreg)|(.L__reg_num_\rt) > + .endm > +#else > +asm( > +" .irp num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30\n" > +" .equ .L__reg_num_x\\num, \\num\n" > +" .endr\n" > +" .equ .L__reg_num_xzr, 31\n" > +"\n" > +" .macro mrs_s, rt, sreg\n" > +" .inst 0xd5200000|(\\sreg)|(.L__reg_num_\\rt)\n" > +" .endm\n" > +"\n" > +" .macro msr_s, sreg, rt\n" > +" .inst 0xd5000000|(\\sreg)|(.L__reg_num_\\rt)\n" > +" .endm\n" > +); > +#endif > + > +#endif /* _ASMARM64_SYSREG_H_ */ >
On Wed, Nov 09, 2016 at 12:35:48PM +0000, Andre Przywara wrote: [...] > > diff --git a/lib/arm/asm/gic-v3.h b/lib/arm/asm/gic-v3.h > > new file mode 100644 > > index 000000000000..03321f8c860f > > --- /dev/null > > +++ b/lib/arm/asm/gic-v3.h > > @@ -0,0 +1,92 @@ > > +/* > > + * All GIC* defines are lifted from include/linux/irqchip/arm-gic-v3.h > > + * > > + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com> > > + * > > + * This work is licensed under the terms of the GNU LGPL, version 2. > > + */ > > +#ifndef _ASMARM_GIC_V3_H_ > > +#define _ASMARM_GIC_V3_H_ > > + > > +#ifndef _ASMARM_GIC_H_ > > +#error Do not directly include <asm/gic-v3.h>. Include <asm/gic.h> > > +#endif > > + > > +#define GICD_CTLR 0x0000 > > +#define GICD_TYPER 0x0004 > > So if we share the distributor register definition with GICv2, these > shouldn't be here, but in gic.h. > But this is the right naming scheme we should use (instead of GIC_DIST_xxx). > > Now this gets interesting with your wish to both share the definitions > for the GICv2 and GICv3 distributors, but also stick to the names the > kernel uses (because they differ between the two) ;-) > So now you loose the greppability for either GIC_DIST_CTR or GICD_TYPER, > for instance. Well, we just have the same offset with two names (giving us two symbols to grep). I put them here, vs. asm/gic.h, because the kernel only uses theses symbols for gicv3. Now, nothing stops a unit test from using them with gicv2 tests, though, because unit tests include gic.h, which includes both gic-v2.h and gic-v3.h, and thus it gets both. I know, it's sounding messy... Shouldn't we post some churn to the kernel? :-) Note, I tried to only add defines to asm/gic.h that are actually shared in the kernel between v2 and v3, e.g. GIC_DIST_ENABLE_SET. Actually, GIC_DIST_CTRL and GIC_DIST_CTR may be the only exceptions we have so far. > > > +#define GICD_IGROUPR 0x0080 > > + > > +#define GICD_CTLR_RWP (1U << 31) > > +#define GICD_CTLR_ARE_NS (1U << 4) > > +#define GICD_CTLR_ENABLE_G1A (1U << 1) > > +#define GICD_CTLR_ENABLE_G1 (1U << 0) > > + > > +#define GICR_TYPER 0x0008 > > +#define GICR_IGROUPR0 GICD_IGROUPR > > +#define GICR_TYPER_LAST (1U << 4) > > + > > + > > +#include <asm/arch_gicv3.h> > > + > > +#ifndef __ASSEMBLY__ > > +#include <asm/setup.h> > > +#include <asm/smp.h> > > +#include <asm/processor.h> > > +#include <asm/io.h> > > + > > +struct gicv3_data { > > + void *dist_base; > > + void *redist_base[NR_CPUS]; > > + unsigned int irq_nr; > > +}; > > +extern struct gicv3_data gicv3_data; > > + > > +#define gicv3_dist_base() (gicv3_data.dist_base) > > +#define gicv3_redist_base() (gicv3_data.redist_base[smp_processor_id()]) > > +#define gicv3_sgi_base() (gicv3_data.redist_base[smp_processor_id()] + SZ_64K) > > + > > +extern int gicv3_init(void); > > +extern void gicv3_enable_defaults(void); > > + > > +static inline void gicv3_do_wait_for_rwp(void *base) > > +{ > > + int count = 100000; /* 1s */ > > + > > + while (readl(base + GICD_CTLR) & GICD_CTLR_RWP) { > > + if (!--count) { > > + printf("GICv3: RWP timeout!\n"); > > + abort(); > > + } > > + cpu_relax(); > > + udelay(10); > > + }; > > +} > > + > > +static inline void gicv3_dist_wait_for_rwp(void) > > +{ > > + gicv3_do_wait_for_rwp(gicv3_dist_base()); > > +} > > + > > +static inline void gicv3_redist_wait_for_rwp(void) > > +{ > > + gicv3_do_wait_for_rwp(gicv3_redist_base()); > > +} > > + > > +static inline u32 mpidr_compress(u64 mpidr) > > +{ > > + u64 compressed = mpidr & MPIDR_HWID_BITMASK; > > + > > + compressed = (((compressed >> 32) & 0xff) << 24) | compressed; > > + return compressed; > > +} > > + > > +static inline u64 mpidr_uncompress(u32 compressed) > > +{ > > + u64 mpidr = ((u64)compressed >> 24) << 32; > > + > > + mpidr |= compressed & MPIDR_HWID_BITMASK; > > + return mpidr; > > +} > > + > > +#endif /* !__ASSEMBLY__ */ > > +#endif /* _ASMARM_GIC_V3_H_ */ > > diff --git a/lib/arm/asm/gic.h b/lib/arm/asm/gic.h > > index 328e078a9ae1..4897bc592cdd 100644 > > --- a/lib/arm/asm/gic.h > > +++ b/lib/arm/asm/gic.h > > @@ -7,6 +7,7 @@ > > #define _ASMARM_GIC_H_ > > > > #include <asm/gic-v2.h> > > +#include <asm/gic-v3.h> > > > > #define GIC_CPU_CTRL 0x00 > > #define GIC_CPU_PRIMASK 0x04 > > diff --git a/lib/arm/gic.c b/lib/arm/gic.c > > index 91d78c9a0cc2..af58a11ea13e 100644 > > --- a/lib/arm/gic.c > > +++ b/lib/arm/gic.c > > @@ -8,9 +8,11 @@ > > #include <asm/io.h> > > > > struct gicv2_data gicv2_data; > > +struct gicv3_data gicv3_data; > > > > /* > > * Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt > > + * Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt > > */ > > static bool > > gic_get_dt_bases(const char *compatible, void **base1, void **base2) > > @@ -48,10 +50,18 @@ int gicv2_init(void) > > &gicv2_data.dist_base, &gicv2_data.cpu_base); > > } > > > > +int gicv3_init(void) > > +{ > > + return gic_get_dt_bases("arm,gic-v3", &gicv3_data.dist_base, > > + &gicv3_data.redist_base[0]); > > +} > > + > > int gic_init(void) > > { > > if (gicv2_init()) > > return 2; > > + else if (gicv3_init()) > > + return 3; > > return 0; > > } > > > > @@ -73,3 +83,49 @@ void gicv2_enable_defaults(void) > > writel(GICC_INT_PRI_THRESHOLD, cpu_base + GIC_CPU_PRIMASK); > > writel(GICC_ENABLE, cpu_base + GIC_CPU_CTRL); > > } > > + > > +void gicv3_set_redist_base(void) > > +{ > > + u32 aff = mpidr_compress(get_mpidr()); > > + void *ptr = gicv3_data.redist_base[0]; > > + u64 typer; > > + > > + do { > > + typer = gicv3_read_typer(ptr + GICR_TYPER); > > + if ((typer >> 32) == aff) { > > + gicv3_redist_base() = ptr; > > + return; > > + } > > + ptr += SZ_64K * 2; /* skip RD_base and SGI_base */ > > + } while (!(typer & GICR_TYPER_LAST)); > > + assert(0); > > +} > > + > > +void gicv3_enable_defaults(void) > > +{ > > + void *dist = gicv3_dist_base(); > > + void *sgi_base; > > + unsigned int i; > > + > > + gicv3_data.irq_nr = GICD_TYPER_IRQS(readl(dist + GICD_TYPER)); > > + if (gicv3_data.irq_nr > 1020) > > + gicv3_data.irq_nr = 1020; > > + > > + writel(GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A | GICD_CTLR_ENABLE_G1, > > + dist + GICD_CTLR); > > + gicv3_dist_wait_for_rwp(); > > + > > + if (!gicv3_redist_base()) > > + gicv3_set_redist_base(); > > + sgi_base = gicv3_sgi_base(); > > + > > + writel(~0, sgi_base + GICR_IGROUPR0); > > This is mixing redist setup with distributor setup. Is it that what you > meant with: > " - simplify enable by not caring if we reinit the distributor [drew]"? Yes, but, TBH, I wasn't sure I could get away with it. I tested and it worked, and I figured you'd yell at me if I was wrong :-) > > Also if you set the group for the SGIs, you should set it for SPIs as > well (like the kernel does). This was done in v3 of the series. OK, I was also simplifying by removing everything and then adding stuff back until it worked :-) I can certainly add this back for completeness though. > > What about you finish the per-CPU setup first, then bail out with: > > if (smp_processor_id() != 0) > return; > > and then do the distributor setup (only on the first core). Sure, if it's necessary. I actually like not having to worry about a particular core or a particular order/number of times this enable gets called. Does it hurt to just do it each time? Thanks, drew
Hi, On 09/11/16 13:08, Andrew Jones wrote: > On Wed, Nov 09, 2016 at 12:35:48PM +0000, Andre Przywara wrote: > [...] >>> diff --git a/lib/arm/asm/gic-v3.h b/lib/arm/asm/gic-v3.h >>> new file mode 100644 >>> index 000000000000..03321f8c860f >>> --- /dev/null >>> +++ b/lib/arm/asm/gic-v3.h >>> @@ -0,0 +1,92 @@ >>> +/* >>> + * All GIC* defines are lifted from include/linux/irqchip/arm-gic-v3.h >>> + * >>> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com> >>> + * >>> + * This work is licensed under the terms of the GNU LGPL, version 2. >>> + */ >>> +#ifndef _ASMARM_GIC_V3_H_ >>> +#define _ASMARM_GIC_V3_H_ >>> + >>> +#ifndef _ASMARM_GIC_H_ >>> +#error Do not directly include <asm/gic-v3.h>. Include <asm/gic.h> >>> +#endif >>> + >>> +#define GICD_CTLR 0x0000 >>> +#define GICD_TYPER 0x0004 >> >> So if we share the distributor register definition with GICv2, these >> shouldn't be here, but in gic.h. >> But this is the right naming scheme we should use (instead of GIC_DIST_xxx). >> >> Now this gets interesting with your wish to both share the definitions >> for the GICv2 and GICv3 distributors, but also stick to the names the >> kernel uses (because they differ between the two) ;-) >> So now you loose the greppability for either GIC_DIST_CTR or GICD_TYPER, >> for instance. > > Well, we just have the same offset with two names (giving us two > symbols to grep). I put them here, vs. asm/gic.h, because the kernel > only uses theses symbols for gicv3. Now, nothing stops a unit test > from using them with gicv2 tests, though, because unit tests include > gic.h, which includes both gic-v2.h and gic-v3.h, and thus it gets > both. I know, it's sounding messy... Shouldn't we post some churn to > the kernel? :-) Well, on top of that the distributor registers are slightly different (check CTLR and TYPER, for instance). So it's churn plus a stretch, I guess Marc won't like that. So if greppability is important, should we revert to separate definitions in separate header files then, like in v3? I don't think we actually share _code_ between the two GIC revisions, do we? > Note, I tried to only add defines to asm/gic.h that are actually > shared in the kernel between v2 and v3, e.g. GIC_DIST_ENABLE_SET. Huh? GICv3 uses GICD_ISENABLER for that register. > Actually, GIC_DIST_CTRL and GIC_DIST_CTR may be the only exceptions > we have so far. Note that it's GIC_DIST_CTLR (L and R swapped), one reason more to dump _CTR ;-) >> >>> +#define GICD_IGROUPR 0x0080 >>> + >>> +#define GICD_CTLR_RWP (1U << 31) >>> +#define GICD_CTLR_ARE_NS (1U << 4) >>> +#define GICD_CTLR_ENABLE_G1A (1U << 1) >>> +#define GICD_CTLR_ENABLE_G1 (1U << 0) >>> + >>> +#define GICR_TYPER 0x0008 >>> +#define GICR_IGROUPR0 GICD_IGROUPR >>> +#define GICR_TYPER_LAST (1U << 4) >>> + >>> + >>> +#include <asm/arch_gicv3.h> >>> + >>> +#ifndef __ASSEMBLY__ >>> +#include <asm/setup.h> >>> +#include <asm/smp.h> >>> +#include <asm/processor.h> >>> +#include <asm/io.h> >>> + >>> +struct gicv3_data { >>> + void *dist_base; >>> + void *redist_base[NR_CPUS]; >>> + unsigned int irq_nr; >>> +}; >>> +extern struct gicv3_data gicv3_data; >>> + >>> +#define gicv3_dist_base() (gicv3_data.dist_base) >>> +#define gicv3_redist_base() (gicv3_data.redist_base[smp_processor_id()]) >>> +#define gicv3_sgi_base() (gicv3_data.redist_base[smp_processor_id()] + SZ_64K) >>> + >>> +extern int gicv3_init(void); >>> +extern void gicv3_enable_defaults(void); >>> + >>> +static inline void gicv3_do_wait_for_rwp(void *base) >>> +{ >>> + int count = 100000; /* 1s */ >>> + >>> + while (readl(base + GICD_CTLR) & GICD_CTLR_RWP) { >>> + if (!--count) { >>> + printf("GICv3: RWP timeout!\n"); >>> + abort(); >>> + } >>> + cpu_relax(); >>> + udelay(10); >>> + }; >>> +} >>> + >>> +static inline void gicv3_dist_wait_for_rwp(void) >>> +{ >>> + gicv3_do_wait_for_rwp(gicv3_dist_base()); >>> +} >>> + >>> +static inline void gicv3_redist_wait_for_rwp(void) >>> +{ >>> + gicv3_do_wait_for_rwp(gicv3_redist_base()); >>> +} >>> + >>> +static inline u32 mpidr_compress(u64 mpidr) >>> +{ >>> + u64 compressed = mpidr & MPIDR_HWID_BITMASK; >>> + >>> + compressed = (((compressed >> 32) & 0xff) << 24) | compressed; >>> + return compressed; >>> +} >>> + >>> +static inline u64 mpidr_uncompress(u32 compressed) >>> +{ >>> + u64 mpidr = ((u64)compressed >> 24) << 32; >>> + >>> + mpidr |= compressed & MPIDR_HWID_BITMASK; >>> + return mpidr; >>> +} >>> + >>> +#endif /* !__ASSEMBLY__ */ >>> +#endif /* _ASMARM_GIC_V3_H_ */ >>> diff --git a/lib/arm/asm/gic.h b/lib/arm/asm/gic.h >>> index 328e078a9ae1..4897bc592cdd 100644 >>> --- a/lib/arm/asm/gic.h >>> +++ b/lib/arm/asm/gic.h >>> @@ -7,6 +7,7 @@ >>> #define _ASMARM_GIC_H_ >>> >>> #include <asm/gic-v2.h> >>> +#include <asm/gic-v3.h> >>> >>> #define GIC_CPU_CTRL 0x00 >>> #define GIC_CPU_PRIMASK 0x04 >>> diff --git a/lib/arm/gic.c b/lib/arm/gic.c >>> index 91d78c9a0cc2..af58a11ea13e 100644 >>> --- a/lib/arm/gic.c >>> +++ b/lib/arm/gic.c >>> @@ -8,9 +8,11 @@ >>> #include <asm/io.h> >>> >>> struct gicv2_data gicv2_data; >>> +struct gicv3_data gicv3_data; >>> >>> /* >>> * Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt >>> + * Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt >>> */ >>> static bool >>> gic_get_dt_bases(const char *compatible, void **base1, void **base2) >>> @@ -48,10 +50,18 @@ int gicv2_init(void) >>> &gicv2_data.dist_base, &gicv2_data.cpu_base); >>> } >>> >>> +int gicv3_init(void) >>> +{ >>> + return gic_get_dt_bases("arm,gic-v3", &gicv3_data.dist_base, >>> + &gicv3_data.redist_base[0]); >>> +} >>> + >>> int gic_init(void) >>> { >>> if (gicv2_init()) >>> return 2; >>> + else if (gicv3_init()) >>> + return 3; >>> return 0; >>> } >>> >>> @@ -73,3 +83,49 @@ void gicv2_enable_defaults(void) >>> writel(GICC_INT_PRI_THRESHOLD, cpu_base + GIC_CPU_PRIMASK); >>> writel(GICC_ENABLE, cpu_base + GIC_CPU_CTRL); >>> } >>> + >>> +void gicv3_set_redist_base(void) >>> +{ >>> + u32 aff = mpidr_compress(get_mpidr()); >>> + void *ptr = gicv3_data.redist_base[0]; >>> + u64 typer; >>> + >>> + do { >>> + typer = gicv3_read_typer(ptr + GICR_TYPER); >>> + if ((typer >> 32) == aff) { >>> + gicv3_redist_base() = ptr; >>> + return; >>> + } >>> + ptr += SZ_64K * 2; /* skip RD_base and SGI_base */ >>> + } while (!(typer & GICR_TYPER_LAST)); >>> + assert(0); >>> +} >>> + >>> +void gicv3_enable_defaults(void) >>> +{ >>> + void *dist = gicv3_dist_base(); >>> + void *sgi_base; >>> + unsigned int i; >>> + >>> + gicv3_data.irq_nr = GICD_TYPER_IRQS(readl(dist + GICD_TYPER)); >>> + if (gicv3_data.irq_nr > 1020) >>> + gicv3_data.irq_nr = 1020; >>> + >>> + writel(GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A | GICD_CTLR_ENABLE_G1, >>> + dist + GICD_CTLR); >>> + gicv3_dist_wait_for_rwp(); >>> + >>> + if (!gicv3_redist_base()) >>> + gicv3_set_redist_base(); >>> + sgi_base = gicv3_sgi_base(); >>> + >>> + writel(~0, sgi_base + GICR_IGROUPR0); >> >> This is mixing redist setup with distributor setup. Is it that what you >> meant with: >> " - simplify enable by not caring if we reinit the distributor [drew]"? > > Yes, but, TBH, I wasn't sure I could get away with it. I tested and it > worked, and I figured you'd yell at me if I was wrong :-) > >> >> Also if you set the group for the SGIs, you should set it for SPIs as >> well (like the kernel does). This was done in v3 of the series. > > OK, I was also simplifying by removing everything and then adding stuff > back until it worked :-) I can certainly add this back for completeness > though. So you did need IGROUP0? Actually the VGIC implements a single security state, where those registers are supposed to be RAZ/WI, if I get the spec correctly. And KVM implements them as RAO/WI, both for GICR_IGROUPR0 and GICD_IGROUPRn. But the kernel sets both of them up (because it drives real hardware), so I'd trust Marc's wisdom more here ;-) If we don't need this GROUPR setup for proper functionality, we could move it from the generic setup into an actual test. >> What about you finish the per-CPU setup first, then bail out with: >> >> if (smp_processor_id() != 0) >> return; >> >> and then do the distributor setup (only on the first core). > > Sure, if it's necessary. I actually like not having to worry about > a particular core or a particular order/number of times this enable > gets called. Does it hurt to just do it each time? Shouldn't really, so we could let it stay in there until someone complains ... Cheers, Andre.
On Wed, Nov 09, 2016 at 02:43:53PM +0000, Andre Przywara wrote: > Hi, > > On 09/11/16 13:08, Andrew Jones wrote: > > On Wed, Nov 09, 2016 at 12:35:48PM +0000, Andre Przywara wrote: > > [...] > >>> diff --git a/lib/arm/asm/gic-v3.h b/lib/arm/asm/gic-v3.h > >>> new file mode 100644 > >>> index 000000000000..03321f8c860f > >>> --- /dev/null > >>> +++ b/lib/arm/asm/gic-v3.h > >>> @@ -0,0 +1,92 @@ > >>> +/* > >>> + * All GIC* defines are lifted from include/linux/irqchip/arm-gic-v3.h > >>> + * > >>> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com> > >>> + * > >>> + * This work is licensed under the terms of the GNU LGPL, version 2. > >>> + */ > >>> +#ifndef _ASMARM_GIC_V3_H_ > >>> +#define _ASMARM_GIC_V3_H_ > >>> + > >>> +#ifndef _ASMARM_GIC_H_ > >>> +#error Do not directly include <asm/gic-v3.h>. Include <asm/gic.h> > >>> +#endif > >>> + > >>> +#define GICD_CTLR 0x0000 > >>> +#define GICD_TYPER 0x0004 > >> > >> So if we share the distributor register definition with GICv2, these > >> shouldn't be here, but in gic.h. > >> But this is the right naming scheme we should use (instead of GIC_DIST_xxx). > >> > >> Now this gets interesting with your wish to both share the definitions > >> for the GICv2 and GICv3 distributors, but also stick to the names the > >> kernel uses (because they differ between the two) ;-) > >> So now you loose the greppability for either GIC_DIST_CTR or GICD_TYPER, > >> for instance. > > > > Well, we just have the same offset with two names (giving us two > > symbols to grep). I put them here, vs. asm/gic.h, because the kernel > > only uses theses symbols for gicv3. Now, nothing stops a unit test > > from using them with gicv2 tests, though, because unit tests include > > gic.h, which includes both gic-v2.h and gic-v3.h, and thus it gets > > both. I know, it's sounding messy... Shouldn't we post some churn to > > the kernel? :-) > > Well, on top of that the distributor registers are slightly different > (check CTLR and TYPER, for instance). So it's churn plus a stretch, I > guess Marc won't like that. > > So if greppability is important, should we revert to separate > definitions in separate header files then, like in v3? > I don't think we actually share _code_ between the two GIC revisions, do we? > > > Note, I tried to only add defines to asm/gic.h that are actually > > shared in the kernel between v2 and v3, e.g. GIC_DIST_ENABLE_SET. > > Huh? GICv3 uses GICD_ISENABLER for that register. drivers/irqchip/irq-gic-common.c:gic_cpu_config uses it, along with GICD_INT_DEF_PRI_X4 and GIC_DIST_PRI. But I guess those are the only shared ones duplicated here so far, so I was wrong to say the two below were the only two not shared. > > > Actually, GIC_DIST_CTRL and GIC_DIST_CTR may be the only exceptions > > we have so far. > > Note that it's GIC_DIST_CTLR (L and R swapped), one reason more to dump > _CTR ;-) Yeah, I noticed that too, craziness. OK, I won't fight for the greppability argument too hard. Actually, you'll likely be the one doing the grepping when you go fix the driver :-) If you'd prefer we only use one set of defines (the better, modern ones), then for v5 that's what I'll do. > > >> > >>> +#define GICD_IGROUPR 0x0080 > >>> + > >>> +#define GICD_CTLR_RWP (1U << 31) > >>> +#define GICD_CTLR_ARE_NS (1U << 4) > >>> +#define GICD_CTLR_ENABLE_G1A (1U << 1) > >>> +#define GICD_CTLR_ENABLE_G1 (1U << 0) > >>> + > >>> +#define GICR_TYPER 0x0008 > >>> +#define GICR_IGROUPR0 GICD_IGROUPR > >>> +#define GICR_TYPER_LAST (1U << 4) > >>> + > >>> + > >>> +#include <asm/arch_gicv3.h> > >>> + > >>> +#ifndef __ASSEMBLY__ > >>> +#include <asm/setup.h> > >>> +#include <asm/smp.h> > >>> +#include <asm/processor.h> > >>> +#include <asm/io.h> > >>> + > >>> +struct gicv3_data { > >>> + void *dist_base; > >>> + void *redist_base[NR_CPUS]; > >>> + unsigned int irq_nr; > >>> +}; > >>> +extern struct gicv3_data gicv3_data; > >>> + > >>> +#define gicv3_dist_base() (gicv3_data.dist_base) > >>> +#define gicv3_redist_base() (gicv3_data.redist_base[smp_processor_id()]) > >>> +#define gicv3_sgi_base() (gicv3_data.redist_base[smp_processor_id()] + SZ_64K) > >>> + > >>> +extern int gicv3_init(void); > >>> +extern void gicv3_enable_defaults(void); > >>> + > >>> +static inline void gicv3_do_wait_for_rwp(void *base) > >>> +{ > >>> + int count = 100000; /* 1s */ > >>> + > >>> + while (readl(base + GICD_CTLR) & GICD_CTLR_RWP) { > >>> + if (!--count) { > >>> + printf("GICv3: RWP timeout!\n"); > >>> + abort(); > >>> + } > >>> + cpu_relax(); > >>> + udelay(10); > >>> + }; > >>> +} > >>> + > >>> +static inline void gicv3_dist_wait_for_rwp(void) > >>> +{ > >>> + gicv3_do_wait_for_rwp(gicv3_dist_base()); > >>> +} > >>> + > >>> +static inline void gicv3_redist_wait_for_rwp(void) > >>> +{ > >>> + gicv3_do_wait_for_rwp(gicv3_redist_base()); > >>> +} > >>> + > >>> +static inline u32 mpidr_compress(u64 mpidr) > >>> +{ > >>> + u64 compressed = mpidr & MPIDR_HWID_BITMASK; > >>> + > >>> + compressed = (((compressed >> 32) & 0xff) << 24) | compressed; > >>> + return compressed; > >>> +} > >>> + > >>> +static inline u64 mpidr_uncompress(u32 compressed) > >>> +{ > >>> + u64 mpidr = ((u64)compressed >> 24) << 32; > >>> + > >>> + mpidr |= compressed & MPIDR_HWID_BITMASK; > >>> + return mpidr; > >>> +} > >>> + > >>> +#endif /* !__ASSEMBLY__ */ > >>> +#endif /* _ASMARM_GIC_V3_H_ */ > >>> diff --git a/lib/arm/asm/gic.h b/lib/arm/asm/gic.h > >>> index 328e078a9ae1..4897bc592cdd 100644 > >>> --- a/lib/arm/asm/gic.h > >>> +++ b/lib/arm/asm/gic.h > >>> @@ -7,6 +7,7 @@ > >>> #define _ASMARM_GIC_H_ > >>> > >>> #include <asm/gic-v2.h> > >>> +#include <asm/gic-v3.h> > >>> > >>> #define GIC_CPU_CTRL 0x00 > >>> #define GIC_CPU_PRIMASK 0x04 > >>> diff --git a/lib/arm/gic.c b/lib/arm/gic.c > >>> index 91d78c9a0cc2..af58a11ea13e 100644 > >>> --- a/lib/arm/gic.c > >>> +++ b/lib/arm/gic.c > >>> @@ -8,9 +8,11 @@ > >>> #include <asm/io.h> > >>> > >>> struct gicv2_data gicv2_data; > >>> +struct gicv3_data gicv3_data; > >>> > >>> /* > >>> * Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt > >>> + * Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt > >>> */ > >>> static bool > >>> gic_get_dt_bases(const char *compatible, void **base1, void **base2) > >>> @@ -48,10 +50,18 @@ int gicv2_init(void) > >>> &gicv2_data.dist_base, &gicv2_data.cpu_base); > >>> } > >>> > >>> +int gicv3_init(void) > >>> +{ > >>> + return gic_get_dt_bases("arm,gic-v3", &gicv3_data.dist_base, > >>> + &gicv3_data.redist_base[0]); > >>> +} > >>> + > >>> int gic_init(void) > >>> { > >>> if (gicv2_init()) > >>> return 2; > >>> + else if (gicv3_init()) > >>> + return 3; > >>> return 0; > >>> } > >>> > >>> @@ -73,3 +83,49 @@ void gicv2_enable_defaults(void) > >>> writel(GICC_INT_PRI_THRESHOLD, cpu_base + GIC_CPU_PRIMASK); > >>> writel(GICC_ENABLE, cpu_base + GIC_CPU_CTRL); > >>> } > >>> + > >>> +void gicv3_set_redist_base(void) > >>> +{ > >>> + u32 aff = mpidr_compress(get_mpidr()); > >>> + void *ptr = gicv3_data.redist_base[0]; > >>> + u64 typer; > >>> + > >>> + do { > >>> + typer = gicv3_read_typer(ptr + GICR_TYPER); > >>> + if ((typer >> 32) == aff) { > >>> + gicv3_redist_base() = ptr; > >>> + return; > >>> + } > >>> + ptr += SZ_64K * 2; /* skip RD_base and SGI_base */ > >>> + } while (!(typer & GICR_TYPER_LAST)); > >>> + assert(0); > >>> +} > >>> + > >>> +void gicv3_enable_defaults(void) > >>> +{ > >>> + void *dist = gicv3_dist_base(); > >>> + void *sgi_base; > >>> + unsigned int i; > >>> + > >>> + gicv3_data.irq_nr = GICD_TYPER_IRQS(readl(dist + GICD_TYPER)); > >>> + if (gicv3_data.irq_nr > 1020) > >>> + gicv3_data.irq_nr = 1020; > >>> + > >>> + writel(GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A | GICD_CTLR_ENABLE_G1, > >>> + dist + GICD_CTLR); > >>> + gicv3_dist_wait_for_rwp(); > >>> + > >>> + if (!gicv3_redist_base()) > >>> + gicv3_set_redist_base(); > >>> + sgi_base = gicv3_sgi_base(); > >>> + > >>> + writel(~0, sgi_base + GICR_IGROUPR0); > >> > >> This is mixing redist setup with distributor setup. Is it that what you > >> meant with: > >> " - simplify enable by not caring if we reinit the distributor [drew]"? > > > > Yes, but, TBH, I wasn't sure I could get away with it. I tested and it > > worked, and I figured you'd yell at me if I was wrong :-) > > > >> > >> Also if you set the group for the SGIs, you should set it for SPIs as > >> well (like the kernel does). This was done in v3 of the series. > > > > OK, I was also simplifying by removing everything and then adding stuff > > back until it worked :-) I can certainly add this back for completeness > > though. > > So you did need IGROUP0? At least with TCG, yes. When I removed it and quick tested on my x86 notebook the gic test hung. I didn't try to debug, I just added stuff until it worked... > > Actually the VGIC implements a single security state, where those > registers are supposed to be RAZ/WI, if I get the spec correctly. > And KVM implements them as RAO/WI, both for GICR_IGROUPR0 and GICD_IGROUPRn. > But the kernel sets both of them up (because it drives real hardware), > so I'd trust Marc's wisdom more here ;-) > If we don't need this GROUPR setup for proper functionality, we could > move it from the generic setup into an actual test. As I need GICR_IGROUP0, I'll bring GICD_IGROUPRn back too. > > >> What about you finish the per-CPU setup first, then bail out with: > >> > >> if (smp_processor_id() != 0) > >> return; > >> > >> and then do the distributor setup (only on the first core). > > > > Sure, if it's necessary. I actually like not having to worry about > > a particular core or a particular order/number of times this enable > > gets called. Does it hurt to just do it each time? > > Shouldn't really, so we could let it stay in there until someone > complains ... Thanks, drew
On 09/11/16 15:23, Andrew Jones wrote: > On Wed, Nov 09, 2016 at 02:43:53PM +0000, Andre Przywara wrote: >> Hi, >> >> On 09/11/16 13:08, Andrew Jones wrote: >>> On Wed, Nov 09, 2016 at 12:35:48PM +0000, Andre Przywara wrote: >>> [...] >>>>> diff --git a/lib/arm/asm/gic-v3.h b/lib/arm/asm/gic-v3.h >>>>> new file mode 100644 >>>>> index 000000000000..03321f8c860f >>>>> --- /dev/null >>>>> +++ b/lib/arm/asm/gic-v3.h >>>>> @@ -0,0 +1,92 @@ >>>>> +/* >>>>> + * All GIC* defines are lifted from include/linux/irqchip/arm-gic-v3.h >>>>> + * >>>>> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com> >>>>> + * >>>>> + * This work is licensed under the terms of the GNU LGPL, version 2. >>>>> + */ >>>>> +#ifndef _ASMARM_GIC_V3_H_ >>>>> +#define _ASMARM_GIC_V3_H_ >>>>> + >>>>> +#ifndef _ASMARM_GIC_H_ >>>>> +#error Do not directly include <asm/gic-v3.h>. Include <asm/gic.h> >>>>> +#endif >>>>> + >>>>> +#define GICD_CTLR 0x0000 >>>>> +#define GICD_TYPER 0x0004 >>>> >>>> So if we share the distributor register definition with GICv2, these >>>> shouldn't be here, but in gic.h. >>>> But this is the right naming scheme we should use (instead of GIC_DIST_xxx). >>>> >>>> Now this gets interesting with your wish to both share the definitions >>>> for the GICv2 and GICv3 distributors, but also stick to the names the >>>> kernel uses (because they differ between the two) ;-) >>>> So now you loose the greppability for either GIC_DIST_CTR or GICD_TYPER, >>>> for instance. >>> >>> Well, we just have the same offset with two names (giving us two >>> symbols to grep). I put them here, vs. asm/gic.h, because the kernel >>> only uses theses symbols for gicv3. Now, nothing stops a unit test >>> from using them with gicv2 tests, though, because unit tests include >>> gic.h, which includes both gic-v2.h and gic-v3.h, and thus it gets >>> both. I know, it's sounding messy... Shouldn't we post some churn to >>> the kernel? :-) >> >> Well, on top of that the distributor registers are slightly different >> (check CTLR and TYPER, for instance). So it's churn plus a stretch, I >> guess Marc won't like that. >> >> So if greppability is important, should we revert to separate >> definitions in separate header files then, like in v3? >> I don't think we actually share _code_ between the two GIC revisions, do we? >> >>> Note, I tried to only add defines to asm/gic.h that are actually >>> shared in the kernel between v2 and v3, e.g. GIC_DIST_ENABLE_SET. >> >> Huh? GICv3 uses GICD_ISENABLER for that register. > > drivers/irqchip/irq-gic-common.c:gic_cpu_config uses it, along with > GICD_INT_DEF_PRI_X4 and GIC_DIST_PRI. But I guess those are the only > shared ones duplicated here so far, so I was wrong to say the two > below were the only two not shared. > >> >>> Actually, GIC_DIST_CTRL and GIC_DIST_CTR may be the only exceptions >>> we have so far. >> >> Note that it's GIC_DIST_CTLR (L and R swapped), one reason more to dump >> _CTR ;-) > > Yeah, I noticed that too, craziness. OK, I won't fight for the > greppability argument too hard. Actually, you'll likely be the > one doing the grepping when you go fix the driver :-) If you'd > prefer we only use one set of defines (the better, modern ones), > then for v5 that's what I'll do. I am fine with either of them (grep vs. same names), just not both at the same time ;-) So it's your call at the end, but I lean more toward modern names. And yes, I can deal with both naming schemes when debugging ;-) >>>> >>>>> +#define GICD_IGROUPR 0x0080 >>>>> + >>>>> +#define GICD_CTLR_RWP (1U << 31) >>>>> +#define GICD_CTLR_ARE_NS (1U << 4) >>>>> +#define GICD_CTLR_ENABLE_G1A (1U << 1) >>>>> +#define GICD_CTLR_ENABLE_G1 (1U << 0) >>>>> + >>>>> +#define GICR_TYPER 0x0008 >>>>> +#define GICR_IGROUPR0 GICD_IGROUPR >>>>> +#define GICR_TYPER_LAST (1U << 4) >>>>> + >>>>> + >>>>> +#include <asm/arch_gicv3.h> >>>>> + >>>>> +#ifndef __ASSEMBLY__ >>>>> +#include <asm/setup.h> >>>>> +#include <asm/smp.h> >>>>> +#include <asm/processor.h> >>>>> +#include <asm/io.h> >>>>> + >>>>> +struct gicv3_data { >>>>> + void *dist_base; >>>>> + void *redist_base[NR_CPUS]; >>>>> + unsigned int irq_nr; >>>>> +}; >>>>> +extern struct gicv3_data gicv3_data; >>>>> + >>>>> +#define gicv3_dist_base() (gicv3_data.dist_base) >>>>> +#define gicv3_redist_base() (gicv3_data.redist_base[smp_processor_id()]) >>>>> +#define gicv3_sgi_base() (gicv3_data.redist_base[smp_processor_id()] + SZ_64K) >>>>> + >>>>> +extern int gicv3_init(void); >>>>> +extern void gicv3_enable_defaults(void); >>>>> + >>>>> +static inline void gicv3_do_wait_for_rwp(void *base) >>>>> +{ >>>>> + int count = 100000; /* 1s */ >>>>> + >>>>> + while (readl(base + GICD_CTLR) & GICD_CTLR_RWP) { >>>>> + if (!--count) { >>>>> + printf("GICv3: RWP timeout!\n"); >>>>> + abort(); >>>>> + } >>>>> + cpu_relax(); >>>>> + udelay(10); >>>>> + }; >>>>> +} >>>>> + >>>>> +static inline void gicv3_dist_wait_for_rwp(void) >>>>> +{ >>>>> + gicv3_do_wait_for_rwp(gicv3_dist_base()); >>>>> +} >>>>> + >>>>> +static inline void gicv3_redist_wait_for_rwp(void) >>>>> +{ >>>>> + gicv3_do_wait_for_rwp(gicv3_redist_base()); >>>>> +} >>>>> + >>>>> +static inline u32 mpidr_compress(u64 mpidr) >>>>> +{ >>>>> + u64 compressed = mpidr & MPIDR_HWID_BITMASK; >>>>> + >>>>> + compressed = (((compressed >> 32) & 0xff) << 24) | compressed; >>>>> + return compressed; >>>>> +} >>>>> + >>>>> +static inline u64 mpidr_uncompress(u32 compressed) >>>>> +{ >>>>> + u64 mpidr = ((u64)compressed >> 24) << 32; >>>>> + >>>>> + mpidr |= compressed & MPIDR_HWID_BITMASK; >>>>> + return mpidr; >>>>> +} >>>>> + >>>>> +#endif /* !__ASSEMBLY__ */ >>>>> +#endif /* _ASMARM_GIC_V3_H_ */ >>>>> diff --git a/lib/arm/asm/gic.h b/lib/arm/asm/gic.h >>>>> index 328e078a9ae1..4897bc592cdd 100644 >>>>> --- a/lib/arm/asm/gic.h >>>>> +++ b/lib/arm/asm/gic.h >>>>> @@ -7,6 +7,7 @@ >>>>> #define _ASMARM_GIC_H_ >>>>> >>>>> #include <asm/gic-v2.h> >>>>> +#include <asm/gic-v3.h> >>>>> >>>>> #define GIC_CPU_CTRL 0x00 >>>>> #define GIC_CPU_PRIMASK 0x04 >>>>> diff --git a/lib/arm/gic.c b/lib/arm/gic.c >>>>> index 91d78c9a0cc2..af58a11ea13e 100644 >>>>> --- a/lib/arm/gic.c >>>>> +++ b/lib/arm/gic.c >>>>> @@ -8,9 +8,11 @@ >>>>> #include <asm/io.h> >>>>> >>>>> struct gicv2_data gicv2_data; >>>>> +struct gicv3_data gicv3_data; >>>>> >>>>> /* >>>>> * Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt >>>>> + * Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt >>>>> */ >>>>> static bool >>>>> gic_get_dt_bases(const char *compatible, void **base1, void **base2) >>>>> @@ -48,10 +50,18 @@ int gicv2_init(void) >>>>> &gicv2_data.dist_base, &gicv2_data.cpu_base); >>>>> } >>>>> >>>>> +int gicv3_init(void) >>>>> +{ >>>>> + return gic_get_dt_bases("arm,gic-v3", &gicv3_data.dist_base, >>>>> + &gicv3_data.redist_base[0]); >>>>> +} >>>>> + >>>>> int gic_init(void) >>>>> { >>>>> if (gicv2_init()) >>>>> return 2; >>>>> + else if (gicv3_init()) >>>>> + return 3; >>>>> return 0; >>>>> } >>>>> >>>>> @@ -73,3 +83,49 @@ void gicv2_enable_defaults(void) >>>>> writel(GICC_INT_PRI_THRESHOLD, cpu_base + GIC_CPU_PRIMASK); >>>>> writel(GICC_ENABLE, cpu_base + GIC_CPU_CTRL); >>>>> } >>>>> + >>>>> +void gicv3_set_redist_base(void) >>>>> +{ >>>>> + u32 aff = mpidr_compress(get_mpidr()); >>>>> + void *ptr = gicv3_data.redist_base[0]; >>>>> + u64 typer; >>>>> + >>>>> + do { >>>>> + typer = gicv3_read_typer(ptr + GICR_TYPER); >>>>> + if ((typer >> 32) == aff) { >>>>> + gicv3_redist_base() = ptr; >>>>> + return; >>>>> + } >>>>> + ptr += SZ_64K * 2; /* skip RD_base and SGI_base */ >>>>> + } while (!(typer & GICR_TYPER_LAST)); >>>>> + assert(0); >>>>> +} >>>>> + >>>>> +void gicv3_enable_defaults(void) >>>>> +{ >>>>> + void *dist = gicv3_dist_base(); >>>>> + void *sgi_base; >>>>> + unsigned int i; >>>>> + >>>>> + gicv3_data.irq_nr = GICD_TYPER_IRQS(readl(dist + GICD_TYPER)); >>>>> + if (gicv3_data.irq_nr > 1020) >>>>> + gicv3_data.irq_nr = 1020; >>>>> + >>>>> + writel(GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A | GICD_CTLR_ENABLE_G1, >>>>> + dist + GICD_CTLR); >>>>> + gicv3_dist_wait_for_rwp(); >>>>> + >>>>> + if (!gicv3_redist_base()) >>>>> + gicv3_set_redist_base(); >>>>> + sgi_base = gicv3_sgi_base(); >>>>> + >>>>> + writel(~0, sgi_base + GICR_IGROUPR0); >>>> >>>> This is mixing redist setup with distributor setup. Is it that what you >>>> meant with: >>>> " - simplify enable by not caring if we reinit the distributor [drew]"? >>> >>> Yes, but, TBH, I wasn't sure I could get away with it. I tested and it >>> worked, and I figured you'd yell at me if I was wrong :-) >>> >>>> >>>> Also if you set the group for the SGIs, you should set it for SPIs as >>>> well (like the kernel does). This was done in v3 of the series. >>> >>> OK, I was also simplifying by removing everything and then adding stuff >>> back until it worked :-) I can certainly add this back for completeness >>> though. >> >> So you did need IGROUP0? > > At least with TCG, yes. When I removed it and quick tested on my x86 > notebook the gic test hung. I didn't try to debug, I just added stuff > until it worked... Ah, TCG might be different, because it also aims at emulating EL2 & 3, AFAIK. So the implementation in there is probably including the secure side as well (haven't checked, though). Thanks! Andre. >> Actually the VGIC implements a single security state, where those >> registers are supposed to be RAZ/WI, if I get the spec correctly. >> And KVM implements them as RAO/WI, both for GICR_IGROUPR0 and GICD_IGROUPRn. >> But the kernel sets both of them up (because it drives real hardware), >> so I'd trust Marc's wisdom more here ;-) >> If we don't need this GROUPR setup for proper functionality, we could >> move it from the generic setup into an actual test. > > As I need GICR_IGROUP0, I'll bring GICD_IGROUPRn back too. > >> >>>> What about you finish the per-CPU setup first, then bail out with: >>>> >>>> if (smp_processor_id() != 0) >>>> return; >>>> >>>> and then do the distributor setup (only on the first core). >>> >>> Sure, if it's necessary. I actually like not having to worry about >>> a particular core or a particular order/number of times this enable >>> gets called. Does it hurt to just do it each time? >> >> Shouldn't really, so we could let it stay in there until someone >> complains ... > > Thanks, > drew >
diff --git a/lib/arm/asm/arch_gicv3.h b/lib/arm/asm/arch_gicv3.h new file mode 100644 index 000000000000..81a1e5f6c29c --- /dev/null +++ b/lib/arm/asm/arch_gicv3.h @@ -0,0 +1,42 @@ +/* + * All ripped off from arch/arm/include/asm/arch_gicv3.h + * + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com> + * + * This work is licensed under the terms of the GNU LGPL, version 2. + */ +#ifndef _ASMARM_ARCH_GICV3_H_ +#define _ASMARM_ARCH_GICV3_H_ + +#ifndef __ASSEMBLY__ +#include <libcflat.h> +#include <asm/barrier.h> +#include <asm/io.h> + +#define __stringify xstr + +#define __ACCESS_CP15(CRn, Op1, CRm, Op2) p15, Op1, %0, CRn, CRm, Op2 + +#define ICC_PMR __ACCESS_CP15(c4, 0, c6, 0) +#define ICC_IGRPEN1 __ACCESS_CP15(c12, 0, c12, 7) + +static inline void gicv3_write_pmr(u32 val) +{ + asm volatile("mcr " __stringify(ICC_PMR) : : "r" (val)); +} + +static inline void gicv3_write_grpen1(u32 val) +{ + asm volatile("mcr " __stringify(ICC_IGRPEN1) : : "r" (val)); + isb(); +} + +static inline u64 gicv3_read_typer(const volatile void __iomem *addr) +{ + u64 val = readl(addr); + val |= (u64)readl(addr + 4) << 32; + return val; +} + +#endif /* !__ASSEMBLY__ */ +#endif /* _ASMARM_ARCH_GICV3_H_ */ diff --git a/lib/arm/asm/gic-v3.h b/lib/arm/asm/gic-v3.h new file mode 100644 index 000000000000..03321f8c860f --- /dev/null +++ b/lib/arm/asm/gic-v3.h @@ -0,0 +1,92 @@ +/* + * All GIC* defines are lifted from include/linux/irqchip/arm-gic-v3.h + * + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com> + * + * This work is licensed under the terms of the GNU LGPL, version 2. + */ +#ifndef _ASMARM_GIC_V3_H_ +#define _ASMARM_GIC_V3_H_ + +#ifndef _ASMARM_GIC_H_ +#error Do not directly include <asm/gic-v3.h>. Include <asm/gic.h> +#endif + +#define GICD_CTLR 0x0000 +#define GICD_TYPER 0x0004 +#define GICD_IGROUPR 0x0080 + +#define GICD_CTLR_RWP (1U << 31) +#define GICD_CTLR_ARE_NS (1U << 4) +#define GICD_CTLR_ENABLE_G1A (1U << 1) +#define GICD_CTLR_ENABLE_G1 (1U << 0) + +#define GICR_TYPER 0x0008 +#define GICR_IGROUPR0 GICD_IGROUPR +#define GICR_TYPER_LAST (1U << 4) + + +#include <asm/arch_gicv3.h> + +#ifndef __ASSEMBLY__ +#include <asm/setup.h> +#include <asm/smp.h> +#include <asm/processor.h> +#include <asm/io.h> + +struct gicv3_data { + void *dist_base; + void *redist_base[NR_CPUS]; + unsigned int irq_nr; +}; +extern struct gicv3_data gicv3_data; + +#define gicv3_dist_base() (gicv3_data.dist_base) +#define gicv3_redist_base() (gicv3_data.redist_base[smp_processor_id()]) +#define gicv3_sgi_base() (gicv3_data.redist_base[smp_processor_id()] + SZ_64K) + +extern int gicv3_init(void); +extern void gicv3_enable_defaults(void); + +static inline void gicv3_do_wait_for_rwp(void *base) +{ + int count = 100000; /* 1s */ + + while (readl(base + GICD_CTLR) & GICD_CTLR_RWP) { + if (!--count) { + printf("GICv3: RWP timeout!\n"); + abort(); + } + cpu_relax(); + udelay(10); + }; +} + +static inline void gicv3_dist_wait_for_rwp(void) +{ + gicv3_do_wait_for_rwp(gicv3_dist_base()); +} + +static inline void gicv3_redist_wait_for_rwp(void) +{ + gicv3_do_wait_for_rwp(gicv3_redist_base()); +} + +static inline u32 mpidr_compress(u64 mpidr) +{ + u64 compressed = mpidr & MPIDR_HWID_BITMASK; + + compressed = (((compressed >> 32) & 0xff) << 24) | compressed; + return compressed; +} + +static inline u64 mpidr_uncompress(u32 compressed) +{ + u64 mpidr = ((u64)compressed >> 24) << 32; + + mpidr |= compressed & MPIDR_HWID_BITMASK; + return mpidr; +} + +#endif /* !__ASSEMBLY__ */ +#endif /* _ASMARM_GIC_V3_H_ */ diff --git a/lib/arm/asm/gic.h b/lib/arm/asm/gic.h index 328e078a9ae1..4897bc592cdd 100644 --- a/lib/arm/asm/gic.h +++ b/lib/arm/asm/gic.h @@ -7,6 +7,7 @@ #define _ASMARM_GIC_H_ #include <asm/gic-v2.h> +#include <asm/gic-v3.h> #define GIC_CPU_CTRL 0x00 #define GIC_CPU_PRIMASK 0x04 diff --git a/lib/arm/gic.c b/lib/arm/gic.c index 91d78c9a0cc2..af58a11ea13e 100644 --- a/lib/arm/gic.c +++ b/lib/arm/gic.c @@ -8,9 +8,11 @@ #include <asm/io.h> struct gicv2_data gicv2_data; +struct gicv3_data gicv3_data; /* * Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt + * Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt */ static bool gic_get_dt_bases(const char *compatible, void **base1, void **base2) @@ -48,10 +50,18 @@ int gicv2_init(void) &gicv2_data.dist_base, &gicv2_data.cpu_base); } +int gicv3_init(void) +{ + return gic_get_dt_bases("arm,gic-v3", &gicv3_data.dist_base, + &gicv3_data.redist_base[0]); +} + int gic_init(void) { if (gicv2_init()) return 2; + else if (gicv3_init()) + return 3; return 0; } @@ -73,3 +83,49 @@ void gicv2_enable_defaults(void) writel(GICC_INT_PRI_THRESHOLD, cpu_base + GIC_CPU_PRIMASK); writel(GICC_ENABLE, cpu_base + GIC_CPU_CTRL); } + +void gicv3_set_redist_base(void) +{ + u32 aff = mpidr_compress(get_mpidr()); + void *ptr = gicv3_data.redist_base[0]; + u64 typer; + + do { + typer = gicv3_read_typer(ptr + GICR_TYPER); + if ((typer >> 32) == aff) { + gicv3_redist_base() = ptr; + return; + } + ptr += SZ_64K * 2; /* skip RD_base and SGI_base */ + } while (!(typer & GICR_TYPER_LAST)); + assert(0); +} + +void gicv3_enable_defaults(void) +{ + void *dist = gicv3_dist_base(); + void *sgi_base; + unsigned int i; + + gicv3_data.irq_nr = GICD_TYPER_IRQS(readl(dist + GICD_TYPER)); + if (gicv3_data.irq_nr > 1020) + gicv3_data.irq_nr = 1020; + + writel(GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A | GICD_CTLR_ENABLE_G1, + dist + GICD_CTLR); + gicv3_dist_wait_for_rwp(); + + if (!gicv3_redist_base()) + gicv3_set_redist_base(); + sgi_base = gicv3_sgi_base(); + + writel(~0, sgi_base + GICR_IGROUPR0); + writel(GICD_INT_EN_SET_SGI, sgi_base + GIC_DIST_ENABLE_SET); + + for (i = 0; i < 32; i += 4) + writel(GICD_INT_DEF_PRI_X4, sgi_base + GIC_DIST_PRI + i); + gicv3_redist_wait_for_rwp(); + + gicv3_write_pmr(0xf0); + gicv3_write_grpen1(1); +} diff --git a/lib/arm64/asm/arch_gicv3.h b/lib/arm64/asm/arch_gicv3.h new file mode 100644 index 000000000000..6d353567f56a --- /dev/null +++ b/lib/arm64/asm/arch_gicv3.h @@ -0,0 +1,44 @@ +/* + * All ripped off from arch/arm64/include/asm/arch_gicv3.h + * + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com> + * + * This work is licensed under the terms of the GNU LGPL, version 2. + */ +#ifndef _ASMARM64_ARCH_GICV3_H_ +#define _ASMARM64_ARCH_GICV3_H_ + +#include <asm/sysreg.h> + +#define ICC_PMR_EL1 sys_reg(3, 0, 4, 6, 0) +#define ICC_GRPEN1_EL1 sys_reg(3, 0, 12, 12, 7) + +#ifndef __ASSEMBLY__ + +#include <libcflat.h> +#include <asm/barrier.h> + +#define __stringify xstr + +/* + * Low-level accessors + * + * These system registers are 32 bits, but we make sure that the compiler + * sets the GP register's most significant bits to 0 with an explicit cast. + */ + +static inline void gicv3_write_pmr(u32 val) +{ + asm volatile("msr_s " __stringify(ICC_PMR_EL1) ", %0" : : "r" ((u64)val)); +} + +static inline void gicv3_write_grpen1(u32 val) +{ + asm volatile("msr_s " __stringify(ICC_GRPEN1_EL1) ", %0" : : "r" ((u64)val)); + isb(); +} + +#define gicv3_read_typer(c) readq(c) + +#endif /* __ASSEMBLY__ */ +#endif /* _ASMARM64_ARCH_GICV3_H_ */ diff --git a/lib/arm64/asm/gic-v3.h b/lib/arm64/asm/gic-v3.h new file mode 100644 index 000000000000..8ee5d4d9c181 --- /dev/null +++ b/lib/arm64/asm/gic-v3.h @@ -0,0 +1 @@ +#include "../../arm/asm/gic-v3.h" diff --git a/lib/arm64/asm/sysreg.h b/lib/arm64/asm/sysreg.h new file mode 100644 index 000000000000..544a46cb8cc5 --- /dev/null +++ b/lib/arm64/asm/sysreg.h @@ -0,0 +1,44 @@ +/* + * Ripped off from arch/arm64/include/asm/sysreg.h + * + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com> + * + * This work is licensed under the terms of the GNU LGPL, version 2. + */ +#ifndef _ASMARM64_SYSREG_H_ +#define _ASMARM64_SYSREG_H_ + +#define sys_reg(op0, op1, crn, crm, op2) \ + ((((op0)&3)<<19)|((op1)<<16)|((crn)<<12)|((crm)<<8)|((op2)<<5)) + +#ifdef __ASSEMBLY__ + .irp num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30 + .equ .L__reg_num_x\num, \num + .endr + .equ .L__reg_num_xzr, 31 + + .macro mrs_s, rt, sreg + .inst 0xd5200000|(\sreg)|(.L__reg_num_\rt) + .endm + + .macro msr_s, sreg, rt + .inst 0xd5000000|(\sreg)|(.L__reg_num_\rt) + .endm +#else +asm( +" .irp num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30\n" +" .equ .L__reg_num_x\\num, \\num\n" +" .endr\n" +" .equ .L__reg_num_xzr, 31\n" +"\n" +" .macro mrs_s, rt, sreg\n" +" .inst 0xd5200000|(\\sreg)|(.L__reg_num_\\rt)\n" +" .endm\n" +"\n" +" .macro msr_s, sreg, rt\n" +" .inst 0xd5000000|(\\sreg)|(.L__reg_num_\\rt)\n" +" .endm\n" +); +#endif + +#endif /* _ASMARM64_SYSREG_H_ */
Signed-off-by: Andrew Jones <drjones@redhat.com> --- v4: - only take defines from kernel we need now [Andre] - simplify enable by not caring if we reinit the distributor [drew] v2: - configure irqs as NS GRP1 --- lib/arm/asm/arch_gicv3.h | 42 +++++++++++++++++++++ lib/arm/asm/gic-v3.h | 92 ++++++++++++++++++++++++++++++++++++++++++++++ lib/arm/asm/gic.h | 1 + lib/arm/gic.c | 56 ++++++++++++++++++++++++++++ lib/arm64/asm/arch_gicv3.h | 44 ++++++++++++++++++++++ lib/arm64/asm/gic-v3.h | 1 + lib/arm64/asm/sysreg.h | 44 ++++++++++++++++++++++ 7 files changed, 280 insertions(+) create mode 100644 lib/arm/asm/arch_gicv3.h create mode 100644 lib/arm/asm/gic-v3.h create mode 100644 lib/arm64/asm/arch_gicv3.h create mode 100644 lib/arm64/asm/gic-v3.h create mode 100644 lib/arm64/asm/sysreg.h -- 2.7.4