On Fri, May 01, 2015 at 06:50:30PM +0100, Peter Maydell wrote:
> From: Fabian Aggeler <aggel...@ethz.ch>
>
> The Interrupt Group Registers allow the guest to configure interrupts
> into one of two groups, where Group0 are higher priority and may
> be routed to IRQ or FIQ, and Group1 are lower priority and always
> routed to IRQ. (In a GIC with the security extensions Group0 is
> Secure interrupts and Group 1 is NonSecure.)
> The GICv2 always supports interrupt grouping; the GICv1 does only
> if it implements the security extensions.
>
> This patch implements the ability to read and write the registers;
> the actual functionality the bits control will be added in a
> subsequent patch.
>
> Signed-off-by: Fabian Aggeler <aggel...@ethz.ch>
> Signed-off-by: Greg Bellows <greg.bell...@linaro.org>
> Message-id: 1429113742-8371-7-git-send-email-greg.bell...@linaro.org
> [PMM: bring GIC_*_GROUP macros into line with the others, ie a
> simple SET/CLEAR/TEST rather than GROUP0/GROUP1;
> utility gic_has_groups() function;
> minor style fixes;
> bump vmstate version]
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
I see now why we are doing the mask thing on the group bits to support the
banking of private interrupts...
The comment,
/* Group bits are banked for private interrupts */
would have been more useful to me close to the definition of the GIC_SET_GROUP
macros but that may just be me...
Reviewed-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
> ---
> hw/intc/arm_gic.c | 50
> +++++++++++++++++++++++++++++++++++++---
> hw/intc/arm_gic_common.c | 5 ++--
> hw/intc/gic_internal.h | 4 ++++
> include/hw/intc/arm_gic_common.h | 1 +
> 4 files changed, 55 insertions(+), 5 deletions(-)
>
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> index 29015c2..1aa4520 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -45,6 +45,14 @@ static inline int gic_get_current_cpu(GICState *s)
> return 0;
> }
>
> +/* Return true if this GIC config has interrupt groups, which is
> + * true if we're a GICv2, or a GICv1 with the security extensions.
> + */
> +static inline bool gic_has_groups(GICState *s)
> +{
> + return s->revision == 2 || s->security_extn;
> +}
> +
> /* TODO: Many places that call this routine could be optimized. */
> /* Update interrupt status after enabled or pending bits have been changed.
> */
> void gic_update(GICState *s)
> @@ -305,8 +313,24 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr
> offset, MemTxAttrs attrs)
> if (offset < 0x08)
> return 0;
> if (offset >= 0x80) {
> - /* Interrupt Security , RAZ/WI */
> - return 0;
> + /* Interrupt Group Registers: these RAZ/WI if this is an NS
> + * access to a GIC with the security extensions, or if the GIC
> + * doesn't have groups at all.
> + */
> + res = 0;
> + if (!(s->security_extn && !attrs.secure) && gic_has_groups(s)) {
> + /* Every byte offset holds 8 group status bits */
> + irq = (offset - 0x080) * 8 + GIC_BASE_IRQ;
> + if (irq >= s->num_irq) {
> + goto bad_reg;
> + }
> + for (i = 0; i < 8; i++) {
> + if (GIC_TEST_GROUP(irq + i, cm)) {
> + res |= (1 << i);
> + }
> + }
> + }
> + return res;
> }
> goto bad_reg;
> } else if (offset < 0x200) {
> @@ -456,7 +480,27 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
> } else if (offset < 4) {
> /* ignored. */
> } else if (offset >= 0x80) {
> - /* Interrupt Security Registers, RAZ/WI */
> + /* Interrupt Group Registers: RAZ/WI for NS access to secure
> + * GIC, or for GICs without groups.
> + */
> + if (!(s->security_extn && !attrs.secure) && gic_has_groups(s)) {
> + /* Every byte offset holds 8 group status bits */
> + irq = (offset - 0x80) * 8 + GIC_BASE_IRQ;
> + if (irq >= s->num_irq) {
> + goto bad_reg;
> + }
> + for (i = 0; i < 8; i++) {
> + /* Group bits are banked for private interrupts */
> + int cm = (irq < GIC_INTERNAL) ? (1 << cpu) :
> ALL_CPU_MASK;
> + if (value & (1 << i)) {
> + /* Group1 (Non-secure) */
> + GIC_SET_GROUP(irq + i, cm);
> + } else {
> + /* Group0 (Secure) */
> + GIC_CLEAR_GROUP(irq + i, cm);
> + }
> + }
> + }
> } else {
> goto bad_reg;
> }
> diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
> index 5ed21f1..b5a85e5 100644
> --- a/hw/intc/arm_gic_common.c
> +++ b/hw/intc/arm_gic_common.c
> @@ -52,14 +52,15 @@ static const VMStateDescription vmstate_gic_irq_state = {
> VMSTATE_UINT8(level, gic_irq_state),
> VMSTATE_BOOL(model, gic_irq_state),
> VMSTATE_BOOL(edge_trigger, gic_irq_state),
> + VMSTATE_UINT8(group, gic_irq_state),
> VMSTATE_END_OF_LIST()
> }
> };
>
> static const VMStateDescription vmstate_gic = {
> .name = "arm_gic",
> - .version_id = 7,
> - .minimum_version_id = 7,
> + .version_id = 8,
> + .minimum_version_id = 8,
> .pre_save = gic_pre_save,
> .post_load = gic_post_load,
> .fields = (VMStateField[]) {
> diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
> index e87ef36..e8cf773 100644
> --- a/hw/intc/gic_internal.h
> +++ b/hw/intc/gic_internal.h
> @@ -50,6 +50,10 @@
> s->priority1[irq][cpu] : \
> s->priority2[(irq) - GIC_INTERNAL])
> #define GIC_TARGET(irq) s->irq_target[irq]
> +#define GIC_CLEAR_GROUP(irq, cm) (s->irq_state[irq].group &= ~(cm))
> +#define GIC_SET_GROUP(irq, cm) (s->irq_state[irq].group |= (cm))
> +#define GIC_TEST_GROUP(irq, cm) ((s->irq_state[irq].group & (cm)) != 0)
> +
>
> /* The special cases for the revision property: */
> #define REV_11MPCORE 0
> diff --git a/include/hw/intc/arm_gic_common.h
> b/include/hw/intc/arm_gic_common.h
> index 7825134..b78981e 100644
> --- a/include/hw/intc/arm_gic_common.h
> +++ b/include/hw/intc/arm_gic_common.h
> @@ -42,6 +42,7 @@ typedef struct gic_irq_state {
> uint8_t level;
> bool model; /* 0 = N:N, 1 = 1:N */
> bool edge_trigger; /* true: edge-triggered, false: level-triggered */
> + uint8_t group;
> } gic_irq_state;
>
> typedef struct GICState {
> --
> 1.9.1
>
