On Thu, Sep 01, 2016 at 12:20:06PM +0200, Auger Eric wrote:
>
>
> On 15/07/2016 15:00, Andrew Jones wrote:
> > Add some gicv2 support. This just adds init and enable
> > functions, allowing unit tests to start messing with it.
> >
> > Signed-off-by: Andrew Jones <drjones@xxxxxxxxxx>
> > ---
> > arm/Makefile.common | 1 +
> > lib/arm/asm/gic-v2.h | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > lib/arm/asm/gic.h | 20 ++++++++++++++
> > lib/arm/gic.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++
> > lib/arm64/asm/gic-v2.h | 1 +
> > lib/arm64/asm/gic.h | 1 +
> > 6 files changed, 166 insertions(+)
> > create mode 100644 lib/arm/asm/gic-v2.h
> > create mode 100644 lib/arm/asm/gic.h
> > create mode 100644 lib/arm/gic.c
> > create mode 100644 lib/arm64/asm/gic-v2.h
> > create mode 100644 lib/arm64/asm/gic.h
> >
> > diff --git a/arm/Makefile.common b/arm/Makefile.common
> > index ccb554d9251a4..41239c37e0920 100644
> > --- a/arm/Makefile.common
> > +++ b/arm/Makefile.common
> > @@ -42,6 +42,7 @@ cflatobjs += lib/arm/mmu.o
> > cflatobjs += lib/arm/bitops.o
> > cflatobjs += lib/arm/psci.o
> > cflatobjs += lib/arm/smp.o
> > +cflatobjs += lib/arm/gic.o
> >
> > libeabi = lib/arm/libeabi.a
> > eabiobjs = lib/arm/eabi_compat.o
> > diff --git a/lib/arm/asm/gic-v2.h b/lib/arm/asm/gic-v2.h
> > new file mode 100644
> > index 0000000000000..973c2bf3cc796
> > --- /dev/null
> > +++ b/lib/arm/asm/gic-v2.h
> > @@ -0,0 +1,74 @@
> > +/*
> > + * All GIC* defines are lifted from include/linux/irqchip/arm-gic.h
> > + *
> > + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@xxxxxxxxxx>
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +#ifndef _ASMARM_GIC_V2_H_
> > +#define _ASMARM_GIC_V2_H_
> > +
> > +#define GIC_CPU_CTRL 0x00
> > +#define GIC_CPU_PRIMASK 0x04
> > +#define GIC_CPU_BINPOINT 0x08
> > +#define GIC_CPU_INTACK 0x0c
> > +#define GIC_CPU_EOI 0x10
> > +#define GIC_CPU_RUNNINGPRI 0x14
> > +#define GIC_CPU_HIGHPRI 0x18
> > +#define GIC_CPU_ALIAS_BINPOINT 0x1c
> > +#define GIC_CPU_ACTIVEPRIO 0xd0
> > +#define GIC_CPU_IDENT 0xfc
> > +#define GIC_CPU_DEACTIVATE 0x1000
> > +
> > +#define GICC_ENABLE 0x1
> > +#define GICC_INT_PRI_THRESHOLD 0xf0
> > +
> > +#define GIC_CPU_CTRL_EOImodeNS (1 << 9)
> > +
> > +#define GICC_IAR_INT_ID_MASK 0x3ff
> > +#define GICC_INT_SPURIOUS 1023
> > +#define GICC_DIS_BYPASS_MASK 0x1e0
> > +
> > +#define GIC_DIST_CTRL 0x000
> > +#define GIC_DIST_CTR 0x004
> you could add #define GIC_DIST_IIDR 0x008
> which can be found in arm-gic.h
I'll resync with the latest kernel headers.
> > +#define GIC_DIST_IGROUP 0x080
> > +#define GIC_DIST_ENABLE_SET 0x100
> > +#define GIC_DIST_ENABLE_CLEAR 0x180
> > +#define GIC_DIST_PENDING_SET 0x200
> > +#define GIC_DIST_PENDING_CLEAR 0x280
> > +#define GIC_DIST_ACTIVE_SET 0x300
> > +#define GIC_DIST_ACTIVE_CLEAR 0x380
> > +#define GIC_DIST_PRI 0x400
> > +#define GIC_DIST_TARGET 0x800
> > +#define GIC_DIST_CONFIG 0xc00
> > +#define GIC_DIST_SOFTINT 0xf00
> > +#define GIC_DIST_SGI_PENDING_CLEAR 0xf10
> > +#define GIC_DIST_SGI_PENDING_SET 0xf20
> > +
> > +#define GICD_ENABLE 0x1
> > +#define GICD_DISABLE 0x0
> > +#define GICD_INT_ACTLOW_LVLTRIG 0x0
> > +#define GICD_INT_EN_CLR_X32 0xffffffff
> > +#define GICD_INT_EN_SET_SGI 0x0000ffff
> > +#define GICD_INT_EN_CLR_PPI 0xffff0000
> > +#define GICD_INT_DEF_PRI 0xa0
> > +#define GICD_INT_DEF_PRI_X4 ((GICD_INT_DEF_PRI << 24) |\
> > + (GICD_INT_DEF_PRI << 16) |\
> > + (GICD_INT_DEF_PRI << 8) |\
> > + GICD_INT_DEF_PRI)
> > +#ifndef __ASSEMBLY__
> > +
> > +struct gicv2_data {
> > + void *dist_base;
> > + void *cpu_base;
> > +};
> > +extern struct gicv2_data gicv2_data;
> > +
> > +#define gicv2_dist_base() (gicv2_data.dist_base)
> > +#define gicv2_cpu_base() (gicv2_data.cpu_base)
> > +
> > +extern int gicv2_init(void);
> > +extern void gicv2_enable_defaults(void);
> > +
> > +#endif /* !__ASSEMBLY__ */
> > +#endif /* _ASMARM_GIC_V2_H_ */
> > diff --git a/lib/arm/asm/gic.h b/lib/arm/asm/gic.h
> > new file mode 100644
> > index 0000000000000..b1237d1c5ef22
> > --- /dev/null
> > +++ b/lib/arm/asm/gic.h
> > @@ -0,0 +1,20 @@
> > +/*
> > + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@xxxxxxxxxx>
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +#ifndef _ASMARM_GIC_H_
> > +#define _ASMARM_GIC_H_
> > +
> > +#include <asm/gic-v2.h>
> to be moved in gic.c?
The idea is that we can strive to create a common interface for basic
gic features, as some unit tests will use such a minimal feature set
of the gic that it won't matter if it's a v2 or v3. Those unit tests
would then just include the generic 'asm/gic.h' and use generic
functions only. This series doesn't introduce any generic functions,
except gic_init, but going forward we may.
> > +
> > +/*
> > + * gic_init will try to find all known gics, and then
> > + * initialize the gic data for the one found.
> > + * returns
> > + * 0 : no gic was found
> > + * > 0 : the gic version of the gic found
> > + */
> > +extern int gic_init(void);
> > +
> > +#endif /* _ASMARM_GIC_H_ */
> > diff --git a/lib/arm/gic.c b/lib/arm/gic.c
> > new file mode 100644
> > index 0000000000000..64a3049c9e8ce
> > --- /dev/null
> > +++ b/lib/arm/gic.c
> > @@ -0,0 +1,69 @@
> > +/*
> > + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@xxxxxxxxxx>
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +#include <libcflat.h>
> > +#include <devicetree.h>
> > +#include <asm/gic.h>
> gic-v2.h?
We'll need both -v2 and -v3, and gic.h does that for us. In the future
we may need gic.h too, so just gic.h seems like the better choice from
the start.
> > +#include <asm/smp.h>
> > +#include <asm/io.h>
> > +
> > +struct gicv2_data gicv2_data;
> > +
> > +/*
> > + * Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt
> > + */
> > +static bool
> > +gic_get_dt_bases(const char *compatible, void **base1, void **base2)
> > +{
> > + struct dt_pbus_reg reg;
> > + struct dt_device gic;
> > + struct dt_bus bus;
> > + int node, ret;
> > +
> > + dt_bus_init_defaults(&bus);
> > + dt_device_init(&gic, &bus, NULL);
> > +
> > + node = dt_device_find_compatible(&gic, compatible);
> > + assert(node >= 0 || node == -FDT_ERR_NOTFOUND);
> > +
> > + if (node == -FDT_ERR_NOTFOUND)
> > + return false;
> > +
> > + dt_device_bind_node(&gic, node);
> > +
> > + ret = dt_pbus_translate(&gic, 0, ®);
> > + assert(ret == 0);
> > + *base1 = ioremap(reg.addr, reg.size);
> > +
> > + ret = dt_pbus_translate(&gic, 1, ®);
> > + assert(ret == 0);
> > + *base2 = ioremap(reg.addr, reg.size);
> > +
> > + return true;
> > +}
> > +
> > +int gicv2_init(void)
> > +{
> > + return gic_get_dt_bases("arm,cortex-a15-gic",
> > + &gicv2_data.dist_base, &gicv2_data.cpu_base);
> what about the other possible compat strings. Is it safe to look only
> for that one?
Currently, yes. Currently we only care about mach-virt, and thus far
mach-virt uses only this. Let's only add new compat strings as we test
models that use them.
> > +}
> > +
> > +int gic_init(void)
> > +{
> > + if (gicv2_init())
> > + return 2;
> > + return 0;
> > +}
> > +
> > +void gicv2_enable_defaults(void)
> > +{
> > + if (smp_processor_id() == 0) {
> > + writel(GICD_INT_DEF_PRI_X4, gicv2_dist_base() + GIC_DIST_PRI);
> I don't get this. There are (8*(GICD_TYPER.ITLinesNumber+1)) such
> registers. See irq-gic-common.c/gic_dist_config
>
> More generally shouldn't we implement something like gic_dist_config and
> gic_cpu_config
So the gic*_enable_defaults functions are certainly places I could have
messed up. I appreciate the review trying to get it right. In the least
I should have commented my intentions better, because I hardly remember
them now myself... Here's what I was trying to achieve
1) Create an enable function for unit tests which don't really care about
what's enabled, i.e. provide a functioning, but minimally configured,
gic.
2) While using gic_dist_config and gic_cpu_config as inspiration, cut
it down to just a handful of register writes in order to make it
easy for unit test writers to understand on a quick read.
3) Only one cpu sets up the distributor.
I think you're right that I screwed up on gicv2's enable. I should have
an irq_nr, like I do for gicv3, in order to set all registers.
Also, in both the v2 and v3 cases I should probably improve how I
determine by whom and when the distributor is set up. Rather than
requiring only cpu0 to do the setup, and the secondaries to not even
try until it's done, I can probably let any cpu do it, and force the
others to wait until it's ready to setup the cpu interfaces.
I'll give this another pass...
Thanks,
drew
>
> Thanks
>
> Eric
>
> > + writel(GICD_INT_EN_SET_SGI, gicv2_dist_base() + GIC_DIST_ENABLE_SET);
> > + writel(GICD_ENABLE, gicv2_dist_base() + GIC_DIST_CTRL);
> > + }
> > + writel(GICC_INT_PRI_THRESHOLD, gicv2_cpu_base() + GIC_CPU_PRIMASK);
> > + writel(GICC_ENABLE, gicv2_cpu_base() + GIC_CPU_CTRL);
> > +}
> > diff --git a/lib/arm64/asm/gic-v2.h b/lib/arm64/asm/gic-v2.h
> > new file mode 100644
> > index 0000000000000..52226624a2092
> > --- /dev/null
> > +++ b/lib/arm64/asm/gic-v2.h
> > @@ -0,0 +1 @@
> > +#include "../../arm/asm/gic-v2.h"
> > diff --git a/lib/arm64/asm/gic.h b/lib/arm64/asm/gic.h
> > new file mode 100644
> > index 0000000000000..e5eb302a31b4d
> > --- /dev/null
> > +++ b/lib/arm64/asm/gic.h
> > @@ -0,0 +1 @@
> > +#include "../../arm/asm/gic.h"
> >
>
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
