| Message ID | 1414707132-24588-9-git-send-email-greg.bellows@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 30 October 2014 at 22:12, Greg Bellows <greg.bellows@linaro.org> wrote: > From: Fabian Aggeler <aggelerf@ethz.ch> > > ICCICR/GICC_CTLR is banked in GICv1 implementations with Security > Extensions or in GICv2 in independent from Security Extensions. > This makes it possible to enable forwarding of interrupts from > the CPU interfaces to the connected processors for Group0 and Group1. > > We also allow to set additional bits like AckCtl and FIQEn by changing > the type from bool to uint32. Since the field does not only store the > enable bit anymore and since we are touching the vmstate, we use the > opportunity to rename the field to cpu_control. > > Signed-off-by: Fabian Aggeler <aggelerf@ethz.ch> > > --- > > v1 -> v2 > - Rework gic_set_cpu_control() and gic_get_cpu_control() to close gap on > handling GICv1 wihtout security extensions. > - Fix use of incorrect control index in update. > --- > hw/intc/arm_gic.c | 82 +++++++++++++++++++++++++++++++++++++--- > hw/intc/arm_gic_common.c | 5 ++- > hw/intc/arm_gic_kvm.c | 8 ++-- > hw/intc/armv7m_nvic.c | 2 +- > hw/intc/gic_internal.h | 14 +++++++ > include/hw/intc/arm_gic_common.h | 2 +- > 6 files changed, 100 insertions(+), 13 deletions(-) > > diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c > index 1db15aa..3c0414f 100644 > --- a/hw/intc/arm_gic.c > +++ b/hw/intc/arm_gic.c > @@ -66,7 +66,7 @@ void gic_update(GICState *s) > for (cpu = 0; cpu < NUM_CPU(s); cpu++) { > cm = 1 << cpu; > s->current_pending[cpu] = 1023; > - if (!s->enabled || !s->cpu_enabled[cpu]) { > + if (!s->enabled || !(s->cpu_control[cpu][1] & 1)) { > qemu_irq_lower(s->parent_irq[cpu]); > return; > } > @@ -240,6 +240,80 @@ void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val) > } > } > > +uint32_t gic_get_cpu_control(GICState *s, int cpu) > +{ > + uint32_t ret; > + > + if (!s->security_extn) { > + if (s->revision == 1) { > + ret = s->cpu_control[cpu][1]; > + ret &= 0x1; /* Mask of reserved bits */ > + } else { > + ret = s->cpu_control[cpu][0]; > + ret &= GICC_CTLR_S_MASK; /* Mask of reserved bits */ > + } > + } else { > + if (ns_access()) { > + ret = s->cpu_control[cpu][1]; > + ret &= GICC_CTLR_NS_MASK; /* Mask of reserved bits */ > + if (s->revision == 1) { > + ret &= 0x1; /* Mask of reserved bits */ > + } > + } else { > + ret = s->cpu_control[cpu][0]; > + ret &= GICC_CTLR_S_MASK; /* Mask of reserved bits */ > + } > + } > + > + return ret; > +} > + > +void gic_set_cpu_control(GICState *s, int cpu, uint32_t value) > +{ > + if (!s->security_extn) { > + if (s->revision == 1) { > + s->cpu_control[cpu][1] = value & 0x1; > + DPRINTF("CPU Interface %d %sabled\n", cpu, > + s->cpu_control[cpu][1] ? "En" : "Dis"); > + } else { > + /* Write to Secure instance of the register */ > + s->cpu_control[cpu][0] = value & GICC_CTLR_S_MASK; > + /* Synchronize EnableGrp1 alias of Non-secure copy */ > + s->cpu_control[cpu][1] &= ~GICC_CTLR_NS_EN_GRP1; > + s->cpu_control[cpu][1] |= > + (value & GICC_CTLR_S_EN_GRP1) ? GICC_CTLR_NS_EN_GRP1 : 0; > + DPRINTF("CPU Interface %d: Group0 Interrupts %sabled, " > + "Group1 Interrupts %sabled\n", cpu, > + (s->cpu_control[cpu][0] & GICC_CTLR_S_EN_GRP0) ? "En" : "Dis", > + (s->cpu_control[cpu][0] & GICC_CTLR_S_EN_GRP1) ? "En" : "Dis"); > + } > + } else { > + if (ns_access()) { > + if (s->revision == 1) { > + s->cpu_control[cpu][1] = value & 0x1; > + DPRINTF("CPU Interface %d %sabled\n", cpu, > + s->cpu_control[cpu][1] ? "En" : "Dis"); > + } else { > + /* Write to Non-secure instance of the register */ > + s->cpu_control[cpu][1] = value & GICC_CTLR_NS_MASK; > + /* Synchronize EnableGrp1 alias of Secure copy */ > + s->cpu_control[cpu][0] &= ~GICC_CTLR_S_EN_GRP1; > + s->cpu_control[cpu][0] |= > + (value & GICC_CTLR_NS_EN_GRP1) ? GICC_CTLR_S_EN_GRP1 : 0; > + } > + DPRINTF("CPU Interface %d: Group1 Interrupts %sabled\n", cpu, > + (s->cpu_control[cpu][1] & GICC_CTLR_NS_EN_GRP1) ? "En" : "Dis"); > + } else { > + /* Write to Secure instance of the register */ > + s->cpu_control[cpu][0] = value & GICC_CTLR_S_MASK; > + /* Synchronize EnableGrp1 alias of Non-secure copy */ > + s->cpu_control[cpu][1] &= ~GICC_CTLR_NS_EN_GRP1; > + s->cpu_control[cpu][1] |= > + (value & GICC_CTLR_S_EN_GRP1) ? GICC_CTLR_NS_EN_GRP1 : 0; > + } > + } > +} These feel a bit verbose -- I shall look to see if there's any simplification possible here. > + > void gic_complete_irq(GICState *s, int cpu, int irq) > { > int update = 0; > @@ -762,7 +836,7 @@ static uint32_t gic_cpu_read(GICState *s, int cpu, int offset) > { > switch (offset) { > case 0x00: /* Control */ > - return s->cpu_enabled[cpu]; > + return gic_get_cpu_control(s, cpu); > case 0x04: /* Priority mask */ > return s->priority_mask[cpu]; > case 0x08: /* Binary Point */ > @@ -788,9 +862,7 @@ static void gic_cpu_write(GICState *s, int cpu, int offset, uint32_t value) > { > switch (offset) { > case 0x00: /* Control */ > - s->cpu_enabled[cpu] = (value & 1); > - DPRINTF("CPU %d %sabled\n", cpu, s->cpu_enabled[cpu] ? "En" : "Dis"); > - break; > + return gic_set_cpu_control(s, cpu, value); gic_set_cpu_control() has a 'void' return type, so you can't use it like this. > case 0x04: /* Priority mask */ > s->priority_mask[cpu] = (value & 0xff); > break; > diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c > index c44050d..e225f2b 100644 > --- a/hw/intc/arm_gic_common.c > +++ b/hw/intc/arm_gic_common.c > @@ -65,7 +65,7 @@ static const VMStateDescription vmstate_gic = { > .post_load = gic_post_load, > .fields = (VMStateField[]) { > VMSTATE_UINT8_ARRAY(enabled_grp, GICState, GIC_NR_GROUP), > - VMSTATE_BOOL_ARRAY(cpu_enabled, GICState, GIC_NCPU), > + VMSTATE_UINT32_2DARRAY(cpu_control, GICState, GIC_NCPU, GIC_NR_GROUP), > VMSTATE_STRUCT_ARRAY(irq_state, GICState, GIC_MAXIRQ, 1, > vmstate_gic_irq_state, gic_irq_state), > VMSTATE_UINT8_ARRAY(irq_target, GICState, GIC_MAXIRQ), > @@ -127,7 +127,8 @@ static void arm_gic_common_reset(DeviceState *dev) > s->current_pending[i] = 1023; > s->running_irq[i] = 1023; > s->running_priority[i] = 0x100; > - s->cpu_enabled[i] = false; > + s->cpu_control[i][0] = false; > + s->cpu_control[i][1] = false; The type has changed from bool to uint32_t, so we should be initializing these to 0, not false. > } > for (i = 0; i < GIC_NR_SGIS; i++) { > GIC_SET_ENABLED(i, ALL_CPU_MASK); > diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c > index 5038885..9a6a2bb 100644 > --- a/hw/intc/arm_gic_kvm.c > +++ b/hw/intc/arm_gic_kvm.c > @@ -379,8 +379,8 @@ static void kvm_arm_gic_put(GICState *s) > */ > > for (cpu = 0; cpu < s->num_cpu; cpu++) { > - /* s->cpu_enabled[cpu] -> GICC_CTLR */ > - reg = s->cpu_enabled[cpu]; > + /* s->cpu_enabled[cpu][0] -> GICC_CTLR */ > + reg = s->cpu_control[cpu]; This is missing the second array index. (Comment should have the same var name as the code, too.) > kvm_gicc_access(s, 0x00, cpu, ®, true); > > /* s->priority_mask[cpu] -> GICC_PMR */ > @@ -478,9 +478,9 @@ static void kvm_arm_gic_get(GICState *s) > */ > > for (cpu = 0; cpu < s->num_cpu; cpu++) { > - /* GICC_CTLR -> s->cpu_enabled[cpu] */ > + /* GICC_CTLR -> s->cpu_control[cpu][0] */ > kvm_gicc_access(s, 0x00, cpu, ®, false); > - s->cpu_enabled[cpu] = (reg & 1); > + s->cpu_control[cpu][0] = reg; > > /* GICC_PMR -> s->priority_mask[cpu] */ > kvm_gicc_access(s, 0x04, cpu, ®, false); > diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c > index d0543d4..7f4a55c 100644 > --- a/hw/intc/armv7m_nvic.c > +++ b/hw/intc/armv7m_nvic.c > @@ -465,7 +465,7 @@ static void armv7m_nvic_reset(DeviceState *dev) > * as enabled by default, and with a priority mask which allows > * all interrupts through. > */ > - s->gic.cpu_enabled[0] = true; > + s->gic.cpu_control[0][0] = true; > s->gic.priority_mask[0] = 0x100; > /* The NVIC as a whole is always enabled. */ > s->gic.enabled = true; > diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h > index f01955a..e360de6 100644 > --- a/hw/intc/gic_internal.h > +++ b/hw/intc/gic_internal.h > @@ -54,6 +54,17 @@ > #define GIC_SET_GROUP1(irq, cm) (s->irq_state[irq].group |= (cm)) > #define GIC_TEST_GROUP0(irq, cm) ((s->irq_state[irq].group & (cm)) == 0) > > +#define GICC_CTLR_S_EN_GRP0 (1U << 0) > +#define GICC_CTLR_S_EN_GRP1 (1U << 1) > +#define GICC_CTLR_S_ACK_CTL (1U << 2) > +#define GICC_CTLR_S_FIQ_EN (1U << 3) > +#define GICC_CTLR_S_CBPR (1U << 4) /* GICv1: SBPR */ Odd indent here. > + > +#define GICC_CTLR_S_MASK 0x7ff > + > +#define GICC_CTLR_NS_EN_GRP1 (1U << 0) > +#define GICC_CTLR_NS_MASK (1 | 3 << 5 | 1 << 9) More brackets in this expression would help clarity. > + > > /* The special cases for the revision property: */ > #define REV_11MPCORE 0 > @@ -65,6 +76,9 @@ void gic_complete_irq(GICState *s, int cpu, int irq); > void gic_update(GICState *s); > void gic_init_irqs_and_distributor(GICState *s); > void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val); > +uint32_t gic_get_cpu_control(GICState *s, int cpu); > +void gic_set_cpu_control(GICState *s, int cpu, uint32_t value); > + > > static inline bool gic_test_pending(GICState *s, int irq, int cm) > { > diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h > index 16e193d..1daa672 100644 > --- a/include/hw/intc/arm_gic_common.h > +++ b/include/hw/intc/arm_gic_common.h > @@ -58,7 +58,7 @@ typedef struct GICState { > uint8_t enabled; > uint8_t enabled_grp[GIC_NR_GROUP]; /* EnableGrp0 and EnableGrp1 */ > }; > - bool cpu_enabled[GIC_NCPU]; > + uint32_t cpu_control[GIC_NCPU][GIC_NR_GROUP]; > > gic_irq_state irq_state[GIC_MAXIRQ]; > uint8_t irq_target[GIC_MAXIRQ]; > -- > 1.8.3.2 -- PMM
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index 1db15aa..3c0414f 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -66,7 +66,7 @@ void gic_update(GICState *s) for (cpu = 0; cpu < NUM_CPU(s); cpu++) { cm = 1 << cpu; s->current_pending[cpu] = 1023; - if (!s->enabled || !s->cpu_enabled[cpu]) { + if (!s->enabled || !(s->cpu_control[cpu][1] & 1)) { qemu_irq_lower(s->parent_irq[cpu]); return; } @@ -240,6 +240,80 @@ void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val) } } +uint32_t gic_get_cpu_control(GICState *s, int cpu) +{ + uint32_t ret; + + if (!s->security_extn) { + if (s->revision == 1) { + ret = s->cpu_control[cpu][1]; + ret &= 0x1; /* Mask of reserved bits */ + } else { + ret = s->cpu_control[cpu][0]; + ret &= GICC_CTLR_S_MASK; /* Mask of reserved bits */ + } + } else { + if (ns_access()) { + ret = s->cpu_control[cpu][1]; + ret &= GICC_CTLR_NS_MASK; /* Mask of reserved bits */ + if (s->revision == 1) { + ret &= 0x1; /* Mask of reserved bits */ + } + } else { + ret = s->cpu_control[cpu][0]; + ret &= GICC_CTLR_S_MASK; /* Mask of reserved bits */ + } + } + + return ret; +} + +void gic_set_cpu_control(GICState *s, int cpu, uint32_t value) +{ + if (!s->security_extn) { + if (s->revision == 1) { + s->cpu_control[cpu][1] = value & 0x1; + DPRINTF("CPU Interface %d %sabled\n", cpu, + s->cpu_control[cpu][1] ? "En" : "Dis"); + } else { + /* Write to Secure instance of the register */ + s->cpu_control[cpu][0] = value & GICC_CTLR_S_MASK; + /* Synchronize EnableGrp1 alias of Non-secure copy */ + s->cpu_control[cpu][1] &= ~GICC_CTLR_NS_EN_GRP1; + s->cpu_control[cpu][1] |= + (value & GICC_CTLR_S_EN_GRP1) ? GICC_CTLR_NS_EN_GRP1 : 0; + DPRINTF("CPU Interface %d: Group0 Interrupts %sabled, " + "Group1 Interrupts %sabled\n", cpu, + (s->cpu_control[cpu][0] & GICC_CTLR_S_EN_GRP0) ? "En" : "Dis", + (s->cpu_control[cpu][0] & GICC_CTLR_S_EN_GRP1) ? "En" : "Dis"); + } + } else { + if (ns_access()) { + if (s->revision == 1) { + s->cpu_control[cpu][1] = value & 0x1; + DPRINTF("CPU Interface %d %sabled\n", cpu, + s->cpu_control[cpu][1] ? "En" : "Dis"); + } else { + /* Write to Non-secure instance of the register */ + s->cpu_control[cpu][1] = value & GICC_CTLR_NS_MASK; + /* Synchronize EnableGrp1 alias of Secure copy */ + s->cpu_control[cpu][0] &= ~GICC_CTLR_S_EN_GRP1; + s->cpu_control[cpu][0] |= + (value & GICC_CTLR_NS_EN_GRP1) ? GICC_CTLR_S_EN_GRP1 : 0; + } + DPRINTF("CPU Interface %d: Group1 Interrupts %sabled\n", cpu, + (s->cpu_control[cpu][1] & GICC_CTLR_NS_EN_GRP1) ? "En" : "Dis"); + } else { + /* Write to Secure instance of the register */ + s->cpu_control[cpu][0] = value & GICC_CTLR_S_MASK; + /* Synchronize EnableGrp1 alias of Non-secure copy */ + s->cpu_control[cpu][1] &= ~GICC_CTLR_NS_EN_GRP1; + s->cpu_control[cpu][1] |= + (value & GICC_CTLR_S_EN_GRP1) ? GICC_CTLR_NS_EN_GRP1 : 0; + } + } +} + void gic_complete_irq(GICState *s, int cpu, int irq) { int update = 0; @@ -762,7 +836,7 @@ static uint32_t gic_cpu_read(GICState *s, int cpu, int offset) { switch (offset) { case 0x00: /* Control */ - return s->cpu_enabled[cpu]; + return gic_get_cpu_control(s, cpu); case 0x04: /* Priority mask */ return s->priority_mask[cpu]; case 0x08: /* Binary Point */ @@ -788,9 +862,7 @@ static void gic_cpu_write(GICState *s, int cpu, int offset, uint32_t value) { switch (offset) { case 0x00: /* Control */ - s->cpu_enabled[cpu] = (value & 1); - DPRINTF("CPU %d %sabled\n", cpu, s->cpu_enabled[cpu] ? "En" : "Dis"); - break; + return gic_set_cpu_control(s, cpu, value); case 0x04: /* Priority mask */ s->priority_mask[cpu] = (value & 0xff); break; diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c index c44050d..e225f2b 100644 --- a/hw/intc/arm_gic_common.c +++ b/hw/intc/arm_gic_common.c @@ -65,7 +65,7 @@ static const VMStateDescription vmstate_gic = { .post_load = gic_post_load, .fields = (VMStateField[]) { VMSTATE_UINT8_ARRAY(enabled_grp, GICState, GIC_NR_GROUP), - VMSTATE_BOOL_ARRAY(cpu_enabled, GICState, GIC_NCPU), + VMSTATE_UINT32_2DARRAY(cpu_control, GICState, GIC_NCPU, GIC_NR_GROUP), VMSTATE_STRUCT_ARRAY(irq_state, GICState, GIC_MAXIRQ, 1, vmstate_gic_irq_state, gic_irq_state), VMSTATE_UINT8_ARRAY(irq_target, GICState, GIC_MAXIRQ), @@ -127,7 +127,8 @@ static void arm_gic_common_reset(DeviceState *dev) s->current_pending[i] = 1023; s->running_irq[i] = 1023; s->running_priority[i] = 0x100; - s->cpu_enabled[i] = false; + s->cpu_control[i][0] = false; + s->cpu_control[i][1] = false; } for (i = 0; i < GIC_NR_SGIS; i++) { GIC_SET_ENABLED(i, ALL_CPU_MASK); diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c index 5038885..9a6a2bb 100644 --- a/hw/intc/arm_gic_kvm.c +++ b/hw/intc/arm_gic_kvm.c @@ -379,8 +379,8 @@ static void kvm_arm_gic_put(GICState *s) */ for (cpu = 0; cpu < s->num_cpu; cpu++) { - /* s->cpu_enabled[cpu] -> GICC_CTLR */ - reg = s->cpu_enabled[cpu]; + /* s->cpu_enabled[cpu][0] -> GICC_CTLR */ + reg = s->cpu_control[cpu]; kvm_gicc_access(s, 0x00, cpu, ®, true); /* s->priority_mask[cpu] -> GICC_PMR */ @@ -478,9 +478,9 @@ static void kvm_arm_gic_get(GICState *s) */ for (cpu = 0; cpu < s->num_cpu; cpu++) { - /* GICC_CTLR -> s->cpu_enabled[cpu] */ + /* GICC_CTLR -> s->cpu_control[cpu][0] */ kvm_gicc_access(s, 0x00, cpu, ®, false); - s->cpu_enabled[cpu] = (reg & 1); + s->cpu_control[cpu][0] = reg; /* GICC_PMR -> s->priority_mask[cpu] */ kvm_gicc_access(s, 0x04, cpu, ®, false); diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index d0543d4..7f4a55c 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -465,7 +465,7 @@ static void armv7m_nvic_reset(DeviceState *dev) * as enabled by default, and with a priority mask which allows * all interrupts through. */ - s->gic.cpu_enabled[0] = true; + s->gic.cpu_control[0][0] = true; s->gic.priority_mask[0] = 0x100; /* The NVIC as a whole is always enabled. */ s->gic.enabled = true; diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h index f01955a..e360de6 100644 --- a/hw/intc/gic_internal.h +++ b/hw/intc/gic_internal.h @@ -54,6 +54,17 @@ #define GIC_SET_GROUP1(irq, cm) (s->irq_state[irq].group |= (cm)) #define GIC_TEST_GROUP0(irq, cm) ((s->irq_state[irq].group & (cm)) == 0) +#define GICC_CTLR_S_EN_GRP0 (1U << 0) +#define GICC_CTLR_S_EN_GRP1 (1U << 1) +#define GICC_CTLR_S_ACK_CTL (1U << 2) +#define GICC_CTLR_S_FIQ_EN (1U << 3) +#define GICC_CTLR_S_CBPR (1U << 4) /* GICv1: SBPR */ + +#define GICC_CTLR_S_MASK 0x7ff + +#define GICC_CTLR_NS_EN_GRP1 (1U << 0) +#define GICC_CTLR_NS_MASK (1 | 3 << 5 | 1 << 9) + /* The special cases for the revision property: */ #define REV_11MPCORE 0 @@ -65,6 +76,9 @@ void gic_complete_irq(GICState *s, int cpu, int irq); void gic_update(GICState *s); void gic_init_irqs_and_distributor(GICState *s); void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val); +uint32_t gic_get_cpu_control(GICState *s, int cpu); +void gic_set_cpu_control(GICState *s, int cpu, uint32_t value); + static inline bool gic_test_pending(GICState *s, int irq, int cm) { diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h index 16e193d..1daa672 100644 --- a/include/hw/intc/arm_gic_common.h +++ b/include/hw/intc/arm_gic_common.h @@ -58,7 +58,7 @@ typedef struct GICState { uint8_t enabled; uint8_t enabled_grp[GIC_NR_GROUP]; /* EnableGrp0 and EnableGrp1 */ }; - bool cpu_enabled[GIC_NCPU]; + uint32_t cpu_control[GIC_NCPU][GIC_NR_GROUP]; gic_irq_state irq_state[GIC_MAXIRQ]; uint8_t irq_target[GIC_MAXIRQ];