From 165cdaf857dc850f676fff0b5b873a51865baa9c Mon Sep 17 00:00:00 2001 From: Adrian Huang Date: Tue, 12 May 2015 11:57:16 +0100 Subject: [PATCH 01/19] armv7m_nvic: systick: Reload the RELOAD value and count down only if ENABLE bit is set Consider the following pseudo code to configure SYSTICK (The recommended programming sequence from "the definitive guide to the arm cortex-m3"): SYSTICK Reload Value Register = 0xffff SYSTICK Current Value Register = 0 SYSTICK Control and Status Register = 0x7 The pseudo code "SYSTICK Current Value Register = 0" leads to invoking systick_reload(). As a consequence, the systick.tick member is updated and the systick timer starts to count down when the ENABLE bit of SYSTICK Control and Status Register is cleared. The worst case is that: during the system initialization, the reset value of the SYSTICK Control and Status Register is 0x00000000. When the code "SYSTICK Current Value Register = 0" is executed, the systick.tick member is accumulated with "(s->systick.reload + 1) * systick_scale(s)". The systick_scale() gets the external_ref_clock scale because the CLKSOURCE bit of the SYSTICK Control and Status Register is cleared. This is the incorrect behavior because of the code "SYSTICK Control and Status Register = 0x7". Actually, we want the processor clock instead of the external reference clock. This incorrect behavior defers the generation of the first interrupt. The patch fixes the above-mentioned issue by setting the systick.tick member and modifying the systick timer only if the ENABLE bit of the SYSTICK Control and Status Register is set. In addition, the Cortex-M3 Devices Generic User Guide mentioned that "When ENABLE is set to 1, the counter loads the RELOAD value from the SYST RVR register and then counts down". This patch adheres to the statement of the user guide. Signed-off-by: Adrian Huang Reviewed-by: Jim Huang [PMM: minor tweak to comment text] Signed-off-by: Peter Maydell --- hw/intc/armv7m_nvic.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 6ff6c7f0cc..ca19157c1e 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -77,6 +77,15 @@ static inline int64_t systick_scale(nvic_state *s) static void systick_reload(nvic_state *s, int reset) { + /* The Cortex-M3 Devices Generic User Guide says that "When the + * ENABLE bit is set to 1, the counter loads the RELOAD value from the + * SYST RVR register and then counts down". So, we need to check the + * ENABLE bit before reloading the value. + */ + if ((s->systick.control & SYSTICK_ENABLE) == 0) { + return; + } + if (reset) s->systick.tick = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); s->systick.tick += (s->systick.reload + 1) * systick_scale(s); From 16b781aaef69c90d5f4f5456615f0c26a4f45740 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 12 May 2015 11:57:16 +0100 Subject: [PATCH 02/19] hw/sd: Don't pass BlockBackend to sd_reset() The only valid BlockBackend to pass to sd_reset() is the one for the SD card, which is sd->blk. Drop the second argument from this function in favour of having it just use sd->blk. Signed-off-by: Peter Maydell Reviewed-by: Markus Armbruster Message-id: 1430683444-9797-1-git-send-email-peter.maydell@linaro.org --- hw/sd/sd.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 8abf0c9e31..a1ff465a67 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -389,13 +389,13 @@ static inline uint64_t sd_addr_to_wpnum(uint64_t addr) return addr >> (HWBLOCK_SHIFT + SECTOR_SHIFT + WPGROUP_SHIFT); } -static void sd_reset(SDState *sd, BlockBackend *blk) +static void sd_reset(SDState *sd) { uint64_t size; uint64_t sect; - if (blk) { - blk_get_geometry(blk, §); + if (sd->blk) { + blk_get_geometry(sd->blk, §); } else { sect = 0; } @@ -412,11 +412,9 @@ static void sd_reset(SDState *sd, BlockBackend *blk) sd_set_cardstatus(sd); sd_set_sdstatus(sd); - sd->blk = blk; - if (sd->wp_groups) g_free(sd->wp_groups); - sd->wp_switch = blk ? blk_is_read_only(blk) : false; + sd->wp_switch = sd->blk ? blk_is_read_only(sd->blk) : false; sd->wpgrps_size = sect; sd->wp_groups = bitmap_new(sd->wpgrps_size); memset(sd->function_group, 0, sizeof(sd->function_group)); @@ -434,7 +432,7 @@ static void sd_cardchange(void *opaque, bool load) qemu_set_irq(sd->inserted_cb, blk_is_inserted(sd->blk)); if (blk_is_inserted(sd->blk)) { - sd_reset(sd, sd->blk); + sd_reset(sd); qemu_set_irq(sd->readonly_cb, sd->wp_switch); } } @@ -492,7 +490,8 @@ SDState *sd_init(BlockBackend *blk, bool is_spi) sd->buf = blk_blockalign(blk, 512); sd->spi = is_spi; sd->enable = true; - sd_reset(sd, blk); + sd->blk = blk; + sd_reset(sd); if (sd->blk) { blk_attach_dev_nofail(sd->blk, sd); blk_set_dev_ops(sd->blk, &sd_block_ops, sd); @@ -680,7 +679,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, default: sd->state = sd_idle_state; - sd_reset(sd, sd->blk); + sd_reset(sd); return sd->spi ? sd_r1 : sd_r0; } break; From 44f552964714a41ccd41b5e8ac4cbd2478249db1 Mon Sep 17 00:00:00 2001 From: Fabian Aggeler Date: Tue, 12 May 2015 11:57:16 +0100 Subject: [PATCH 03/19] hw/intc/arm_gic: Create outbound FIQ lines Create the outbound FIQ lines from the GIC to the CPUs; these are used if the GIC has security extensions or grouping support. Signed-off-by: Fabian Aggeler Signed-off-by: Greg Bellows Reviewed-by: Edgar E. Iglesias Signed-off-by: Peter Maydell Message-id: 1430502643-25909-2-git-send-email-peter.maydell@linaro.org Message-id: 1429113742-8371-2-git-send-email-greg.bellows@linaro.org [PMM: added FIQ lines to kvm-arm-gic so its interface is the same; tweaked commit message] Signed-off-by: Peter Maydell --- hw/intc/arm_gic.c | 3 +++ hw/intc/arm_gic_kvm.c | 5 ++++- include/hw/intc/arm_gic_common.h | 1 + 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index a04c822832..e9fb8b9400 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -790,6 +790,9 @@ void gic_init_irqs_and_distributor(GICState *s) for (i = 0; i < NUM_CPU(s); i++) { sysbus_init_irq(sbd, &s->parent_irq[i]); } + for (i = 0; i < NUM_CPU(s); i++) { + sysbus_init_irq(sbd, &s->parent_fiq[i]); + } memory_region_init_io(&s->iomem, OBJECT(s), &gic_dist_ops, s, "gic_dist", 0x1000); } diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c index e1952ad974..5aedae1660 100644 --- a/hw/intc/arm_gic_kvm.c +++ b/hw/intc/arm_gic_kvm.c @@ -554,12 +554,15 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp) */ i += (GIC_INTERNAL * s->num_cpu); qdev_init_gpio_in(dev, kvm_arm_gic_set_irq, i); - /* We never use our outbound IRQ lines but provide them so that + /* We never use our outbound IRQ/FIQ lines but provide them so that * we maintain the same interface as the non-KVM GIC. */ for (i = 0; i < s->num_cpu; i++) { sysbus_init_irq(sbd, &s->parent_irq[i]); } + for (i = 0; i < s->num_cpu; i++) { + sysbus_init_irq(sbd, &s->parent_fiq[i]); + } /* Try to create the device via the device control API */ s->dev_fd = -1; diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h index f6887ed92b..01c6f248ab 100644 --- a/include/hw/intc/arm_gic_common.h +++ b/include/hw/intc/arm_gic_common.h @@ -50,6 +50,7 @@ typedef struct GICState { /*< public >*/ qemu_irq parent_irq[GIC_NCPU]; + qemu_irq parent_fiq[GIC_NCPU]; bool enabled; bool cpu_enabled[GIC_NCPU]; From 5543d1abb6e218a9d3b8887b777fd3947c86c4cf Mon Sep 17 00:00:00 2001 From: Fabian Aggeler Date: Tue, 12 May 2015 11:57:16 +0100 Subject: [PATCH 04/19] hw/intc/arm_gic: Add Security Extensions property Add a QOM property which allows the GIC Security Extensions to be enabled. These are an optional part of the GICv1 and GICv2 architecture. This commit just adds the property and some sanity checks that it is only enabled on GIC revisions that support it. Signed-off-by: Fabian Aggeler Signed-off-by: Greg Bellows Reviewed-by: Edgar E. Iglesias Signed-off-by: Peter Maydell Message-id: 1430502643-25909-3-git-send-email-peter.maydell@linaro.org Message-id: 1429113742-8371-5-git-send-email-greg.bellows@linaro.org [PMM: changed property name, added checks that it isn't set for older GIC revisions or if using the KVM VGIC; reworded commit message] Signed-off-by: Peter Maydell --- hw/intc/arm_gic.c | 5 ++++- hw/intc/arm_gic_common.c | 9 +++++++++ hw/intc/arm_gic_kvm.c | 6 ++++++ include/hw/intc/arm_gic_common.h | 1 + 4 files changed, 20 insertions(+), 1 deletion(-) diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index e9fb8b9400..cdf7408774 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -298,7 +298,10 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset) if (offset == 0) return s->enabled; if (offset == 4) - return ((s->num_irq / 32) - 1) | ((NUM_CPU(s) - 1) << 5); + /* Interrupt Controller Type Register */ + return ((s->num_irq / 32) - 1) + | ((NUM_CPU(s) - 1) << 5) + | (s->security_extn << 10); if (offset < 0x08) return 0; if (offset >= 0x80) { diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c index 18b01ba0c7..5ed21f1b46 100644 --- a/hw/intc/arm_gic_common.c +++ b/hw/intc/arm_gic_common.c @@ -110,6 +110,13 @@ static void arm_gic_common_realize(DeviceState *dev, Error **errp) num_irq); return; } + + if (s->security_extn && + (s->revision == REV_11MPCORE || s->revision == REV_NVIC)) { + error_setg(errp, "this GIC revision does not implement " + "the security extensions"); + return; + } } static void arm_gic_common_reset(DeviceState *dev) @@ -149,6 +156,8 @@ static Property arm_gic_common_properties[] = { * (Internally, 0xffffffff also indicates "not a GIC but an NVIC".) */ DEFINE_PROP_UINT32("revision", GICState, revision, 1), + /* True if the GIC should implement the security extensions */ + DEFINE_PROP_BOOL("has-security-extensions", GICState, security_extn, 0), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c index 5aedae1660..cb47b12f0e 100644 --- a/hw/intc/arm_gic_kvm.c +++ b/hw/intc/arm_gic_kvm.c @@ -544,6 +544,12 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp) return; } + if (s->security_extn) { + error_setg(errp, "the in-kernel VGIC does not implement the " + "security extensions"); + return; + } + i = s->num_irq - GIC_INTERNAL; /* For the GIC, also expose incoming GPIO lines for PPIs for each CPU. * GPIO array layout is thus: diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h index 01c6f248ab..78251345f7 100644 --- a/include/hw/intc/arm_gic_common.h +++ b/include/hw/intc/arm_gic_common.h @@ -105,6 +105,7 @@ typedef struct GICState { MemoryRegion cpuiomem[GIC_NCPU + 1]; /* CPU interfaces */ uint32_t num_irq; uint32_t revision; + bool security_extn; int dev_fd; /* kvm device fd if backed by kvm vgic support */ } GICState; From a9d853533cc1a27dc09b10c7ab89677f9c5dd8f4 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 12 May 2015 11:57:16 +0100 Subject: [PATCH 05/19] hw/intc/arm_gic: Switch to read/write callbacks with tx attributes Switch the GIC's MMIO callback functions to the read_with_attrs and write_with_attrs functions which provide MemTxAttrs. This will allow the GIC to correctly handle secure and nonsecure register accesses. Signed-off-by: Peter Maydell Reviewed-by: Edgar E. Iglesias Message-id: 1430502643-25909-4-git-send-email-peter.maydell@linaro.org --- hw/intc/arm_gic.c | 174 ++++++++++++++++++++++++++++------------------ 1 file changed, 105 insertions(+), 69 deletions(-) diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index cdf7408774..29015c2e6d 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -282,7 +282,7 @@ void gic_complete_irq(GICState *s, int cpu, int irq) } } -static uint32_t gic_dist_readb(void *opaque, hwaddr offset) +static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs) { GICState *s = (GICState *)opaque; uint32_t res; @@ -418,24 +418,30 @@ bad_reg: return 0; } -static uint32_t gic_dist_readw(void *opaque, hwaddr offset) +static MemTxResult gic_dist_read(void *opaque, hwaddr offset, uint64_t *data, + unsigned size, MemTxAttrs attrs) { - uint32_t val; - val = gic_dist_readb(opaque, offset); - val |= gic_dist_readb(opaque, offset + 1) << 8; - return val; -} - -static uint32_t gic_dist_readl(void *opaque, hwaddr offset) -{ - uint32_t val; - val = gic_dist_readw(opaque, offset); - val |= gic_dist_readw(opaque, offset + 2) << 16; - return val; + switch (size) { + case 1: + *data = gic_dist_readb(opaque, offset, attrs); + return MEMTX_OK; + case 2: + *data = gic_dist_readb(opaque, offset, attrs); + *data |= gic_dist_readb(opaque, offset + 1, attrs) << 8; + return MEMTX_OK; + case 4: + *data = gic_dist_readb(opaque, offset, attrs); + *data |= gic_dist_readb(opaque, offset + 1, attrs) << 8; + *data |= gic_dist_readb(opaque, offset + 2, attrs) << 16; + *data |= gic_dist_readb(opaque, offset + 3, attrs) << 24; + return MEMTX_OK; + default: + return MEMTX_ERROR; + } } static void gic_dist_writeb(void *opaque, hwaddr offset, - uint32_t value) + uint32_t value, MemTxAttrs attrs) { GICState *s = (GICState *)opaque; int irq; @@ -612,14 +618,14 @@ bad_reg: } static void gic_dist_writew(void *opaque, hwaddr offset, - uint32_t value) + uint32_t value, MemTxAttrs attrs) { - gic_dist_writeb(opaque, offset, value & 0xff); - gic_dist_writeb(opaque, offset + 1, value >> 8); + gic_dist_writeb(opaque, offset, value & 0xff, attrs); + gic_dist_writeb(opaque, offset + 1, value >> 8, attrs); } static void gic_dist_writel(void *opaque, hwaddr offset, - uint32_t value) + uint32_t value, MemTxAttrs attrs) { GICState *s = (GICState *)opaque; if (offset == 0xf00) { @@ -655,45 +661,72 @@ static void gic_dist_writel(void *opaque, hwaddr offset, gic_update(s); return; } - gic_dist_writew(opaque, offset, value & 0xffff); - gic_dist_writew(opaque, offset + 2, value >> 16); + gic_dist_writew(opaque, offset, value & 0xffff, attrs); + gic_dist_writew(opaque, offset + 2, value >> 16, attrs); } -static const MemoryRegionOps gic_dist_ops = { - .old_mmio = { - .read = { gic_dist_readb, gic_dist_readw, gic_dist_readl, }, - .write = { gic_dist_writeb, gic_dist_writew, gic_dist_writel, }, - }, - .endianness = DEVICE_NATIVE_ENDIAN, -}; - -static uint32_t gic_cpu_read(GICState *s, int cpu, int offset) +static MemTxResult gic_dist_write(void *opaque, hwaddr offset, uint64_t data, + unsigned size, MemTxAttrs attrs) { - switch (offset) { - case 0x00: /* Control */ - return s->cpu_enabled[cpu]; - case 0x04: /* Priority mask */ - return s->priority_mask[cpu]; - case 0x08: /* Binary Point */ - return s->bpr[cpu]; - case 0x0c: /* Acknowledge */ - return gic_acknowledge_irq(s, cpu); - case 0x14: /* Running Priority */ - return s->running_priority[cpu]; - case 0x18: /* Highest Pending Interrupt */ - return s->current_pending[cpu]; - case 0x1c: /* Aliased Binary Point */ - return s->abpr[cpu]; - case 0xd0: case 0xd4: case 0xd8: case 0xdc: - return s->apr[(offset - 0xd0) / 4][cpu]; + switch (size) { + case 1: + gic_dist_writeb(opaque, offset, data, attrs); + return MEMTX_OK; + case 2: + gic_dist_writew(opaque, offset, data, attrs); + return MEMTX_OK; + case 4: + gic_dist_writel(opaque, offset, data, attrs); + return MEMTX_OK; default: - qemu_log_mask(LOG_GUEST_ERROR, - "gic_cpu_read: Bad offset %x\n", (int)offset); - return 0; + return MEMTX_ERROR; } } -static void gic_cpu_write(GICState *s, int cpu, int offset, uint32_t value) +static const MemoryRegionOps gic_dist_ops = { + .read_with_attrs = gic_dist_read, + .write_with_attrs = gic_dist_write, + .endianness = DEVICE_NATIVE_ENDIAN, +}; + +static MemTxResult gic_cpu_read(GICState *s, int cpu, int offset, + uint64_t *data, MemTxAttrs attrs) +{ + switch (offset) { + case 0x00: /* Control */ + *data = s->cpu_enabled[cpu]; + break; + case 0x04: /* Priority mask */ + *data = s->priority_mask[cpu]; + break; + case 0x08: /* Binary Point */ + *data = s->bpr[cpu]; + break; + case 0x0c: /* Acknowledge */ + *data = gic_acknowledge_irq(s, cpu); + break; + case 0x14: /* Running Priority */ + *data = s->running_priority[cpu]; + break; + case 0x18: /* Highest Pending Interrupt */ + *data = s->current_pending[cpu]; + break; + case 0x1c: /* Aliased Binary Point */ + *data = s->abpr[cpu]; + break; + case 0xd0: case 0xd4: case 0xd8: case 0xdc: + *data = s->apr[(offset - 0xd0) / 4][cpu]; + break; + default: + qemu_log_mask(LOG_GUEST_ERROR, + "gic_cpu_read: Bad offset %x\n", (int)offset); + return MEMTX_ERROR; + } + return MEMTX_OK; +} + +static MemTxResult gic_cpu_write(GICState *s, int cpu, int offset, + uint32_t value, MemTxAttrs attrs) { switch (offset) { case 0x00: /* Control */ @@ -708,7 +741,7 @@ static void gic_cpu_write(GICState *s, int cpu, int offset, uint32_t value) break; case 0x10: /* End Of Interrupt */ gic_complete_irq(s, cpu, value & 0x3ff); - return; + return MEMTX_OK; case 0x1c: /* Aliased Binary Point */ if (s->revision >= 2) { s->abpr[cpu] = (value & 0x7); @@ -720,56 +753,59 @@ static void gic_cpu_write(GICState *s, int cpu, int offset, uint32_t value) default: qemu_log_mask(LOG_GUEST_ERROR, "gic_cpu_write: Bad offset %x\n", (int)offset); - return; + return MEMTX_ERROR; } gic_update(s); + return MEMTX_OK; } /* Wrappers to read/write the GIC CPU interface for the current CPU */ -static uint64_t gic_thiscpu_read(void *opaque, hwaddr addr, - unsigned size) +static MemTxResult gic_thiscpu_read(void *opaque, hwaddr addr, uint64_t *data, + unsigned size, MemTxAttrs attrs) { GICState *s = (GICState *)opaque; - return gic_cpu_read(s, gic_get_current_cpu(s), addr); + return gic_cpu_read(s, gic_get_current_cpu(s), addr, data, attrs); } -static void gic_thiscpu_write(void *opaque, hwaddr addr, - uint64_t value, unsigned size) +static MemTxResult gic_thiscpu_write(void *opaque, hwaddr addr, + uint64_t value, unsigned size, + MemTxAttrs attrs) { GICState *s = (GICState *)opaque; - gic_cpu_write(s, gic_get_current_cpu(s), addr, value); + return gic_cpu_write(s, gic_get_current_cpu(s), addr, value, attrs); } /* Wrappers to read/write the GIC CPU interface for a specific CPU. * These just decode the opaque pointer into GICState* + cpu id. */ -static uint64_t gic_do_cpu_read(void *opaque, hwaddr addr, - unsigned size) +static MemTxResult gic_do_cpu_read(void *opaque, hwaddr addr, uint64_t *data, + unsigned size, MemTxAttrs attrs) { GICState **backref = (GICState **)opaque; GICState *s = *backref; int id = (backref - s->backref); - return gic_cpu_read(s, id, addr); + return gic_cpu_read(s, id, addr, data, attrs); } -static void gic_do_cpu_write(void *opaque, hwaddr addr, - uint64_t value, unsigned size) +static MemTxResult gic_do_cpu_write(void *opaque, hwaddr addr, + uint64_t value, unsigned size, + MemTxAttrs attrs) { GICState **backref = (GICState **)opaque; GICState *s = *backref; int id = (backref - s->backref); - gic_cpu_write(s, id, addr, value); + return gic_cpu_write(s, id, addr, value, attrs); } static const MemoryRegionOps gic_thiscpu_ops = { - .read = gic_thiscpu_read, - .write = gic_thiscpu_write, + .read_with_attrs = gic_thiscpu_read, + .write_with_attrs = gic_thiscpu_write, .endianness = DEVICE_NATIVE_ENDIAN, }; static const MemoryRegionOps gic_cpu_ops = { - .read = gic_do_cpu_read, - .write = gic_do_cpu_write, + .read_with_attrs = gic_do_cpu_read, + .write_with_attrs = gic_do_cpu_write, .endianness = DEVICE_NATIVE_ENDIAN, }; From c27a5ba94874cb3a29e21b3ad4bd5e504aea93b2 Mon Sep 17 00:00:00 2001 From: Fabian Aggeler Date: Tue, 12 May 2015 11:57:17 +0100 Subject: [PATCH 06/19] hw/intc/arm_gic: Add Interrupt Group Registers 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 Signed-off-by: Greg Bellows Reviewed-by: Edgar E. Iglesias Signed-off-by: Peter Maydell Message-id: 1430502643-25909-5-git-send-email-peter.maydell@linaro.org Message-id: 1429113742-8371-7-git-send-email-greg.bellows@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 --- 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 29015c2e6d..1aa45209ce 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 5ed21f1b46..b5a85e5673 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 e87ef36d0c..e8cf773e73 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 78251345f7..b78981ed58 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 { From eb8b9530b0c618d4f2e728eae10d89239d35b0c0 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 12 May 2015 11:57:17 +0100 Subject: [PATCH 07/19] hw/intc/arm_gic_kvm.c: Save and restore GICD_IGROUPRn state Now that the GIC base class has state fields for the GICD_IGROUPRn registers, make kvm_arm_gic_get() and kvm_arm_gic_put() write and read them. This allows us to remove the check that made us fail migration if the guest had set any of the group register bits. Signed-off-by: Peter Maydell Message-id: 1430502643-25909-6-git-send-email-peter.maydell@linaro.org --- hw/intc/arm_gic_kvm.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c index cb47b12f0e..3591ca7d5d 100644 --- a/hw/intc/arm_gic_kvm.c +++ b/hw/intc/arm_gic_kvm.c @@ -176,6 +176,20 @@ static void translate_clear(GICState *s, int irq, int cpu, } } +static void translate_group(GICState *s, int irq, int cpu, + uint32_t *field, bool to_kernel) +{ + int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK; + + if (to_kernel) { + *field = GIC_TEST_GROUP(irq, cm); + } else { + if (*field & 1) { + GIC_SET_GROUP(irq, cm); + } + } +} + static void translate_enabled(GICState *s, int irq, int cpu, uint32_t *field, bool to_kernel) { @@ -365,6 +379,9 @@ static void kvm_arm_gic_put(GICState *s) kvm_dist_put(s, 0x180, 1, s->num_irq, translate_clear); kvm_dist_put(s, 0x100, 1, s->num_irq, translate_enabled); + /* irq_state[n].group -> GICD_IGROUPRn */ + kvm_dist_put(s, 0x80, 1, s->num_irq, translate_group); + /* s->irq_target[irq] -> GICD_ITARGETSRn * (restore targets before pending to ensure the pending state is set on * the appropriate CPU interfaces in the kernel) */ @@ -454,21 +471,14 @@ static void kvm_arm_gic_get(GICState *s) /* GICD_IIDR -> ? */ kvm_gicd_access(s, 0x8, 0, ®, false); - /* Verify no GROUP 1 interrupts configured in the kernel */ - for_each_irq_reg(i, s->num_irq, 1) { - kvm_gicd_access(s, 0x80 + (i * 4), 0, ®, false); - if (reg != 0) { - fprintf(stderr, "Unsupported GICD_IGROUPRn value: %08x\n", - reg); - abort(); - } - } - /* Clear all the IRQ settings */ for (i = 0; i < s->num_irq; i++) { memset(&s->irq_state[i], 0, sizeof(s->irq_state[0])); } + /* GICD_IGROUPRn -> irq_state[n].group */ + kvm_dist_get(s, 0x80, 1, s->num_irq, translate_group); + /* GICD_ISENABLERn -> irq_state[n].enabled */ kvm_dist_get(s, 0x100, 1, s->num_irq, translate_enabled); From 679aa175e84f5f80b32b307fce5a6b92729e0e61 Mon Sep 17 00:00:00 2001 From: Fabian Aggeler Date: Tue, 12 May 2015 11:57:17 +0100 Subject: [PATCH 08/19] hw/intc/arm_gic: Make ICDDCR/GICD_CTLR banked ICDDCR/GICD_CTLR is banked if the GIC has the security extensions, and the S (or only) copy has separate enable bits for Group0 and Group1 enable if the GIC implements interrupt groups. EnableGroup0 (Bit [1]) in GICv1 is architecturally IMPDEF. Since this bit (Enable Non-secure) is present in the integrated GIC of the Cortex-A9 MPCore, we support this bit in our GICv1 implementation too. Signed-off-by: Fabian Aggeler Signed-off-by: Greg Bellows Reviewed-by: Edgar E. Iglesias Signed-off-by: Peter Maydell Message-id: 1430502643-25909-7-git-send-email-peter.maydell@linaro.org Message-id: 1429113742-8371-8-git-send-email-greg.bellows@linaro.org [PMM: rewritten to store the state in a single s->ctlr uint32, with the NS register handled as an alias of bit 1 in that value; added vmstate version bump] Signed-off-by: Peter Maydell --- hw/intc/arm_gic.c | 28 +++++++++++++++++++++++----- hw/intc/arm_gic_common.c | 8 ++++---- hw/intc/arm_gic_kvm.c | 8 ++++---- hw/intc/armv7m_nvic.c | 2 +- hw/intc/gic_internal.h | 2 ++ include/hw/intc/arm_gic_common.h | 5 ++++- 6 files changed, 38 insertions(+), 15 deletions(-) diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index 1aa45209ce..4f13ff2c90 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -67,7 +67,8 @@ 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->ctlr & (GICD_CTLR_EN_GRP0 | GICD_CTLR_EN_GRP1)) + || !s->cpu_enabled[cpu]) { qemu_irq_lower(s->parent_irq[cpu]); return; } @@ -303,8 +304,16 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs) cpu = gic_get_current_cpu(s); cm = 1 << cpu; if (offset < 0x100) { - if (offset == 0) - return s->enabled; + if (offset == 0) { /* GICD_CTLR */ + if (s->security_extn && !attrs.secure) { + /* The NS bank of this register is just an alias of the + * EnableGrp1 bit in the S bank version. + */ + return extract32(s->ctlr, 1, 1); + } else { + return s->ctlr; + } + } if (offset == 4) /* Interrupt Controller Type Register */ return ((s->num_irq / 32) - 1) @@ -475,8 +484,17 @@ static void gic_dist_writeb(void *opaque, hwaddr offset, cpu = gic_get_current_cpu(s); if (offset < 0x100) { if (offset == 0) { - s->enabled = (value & 1); - DPRINTF("Distribution %sabled\n", s->enabled ? "En" : "Dis"); + if (s->security_extn && !attrs.secure) { + /* NS version is just an alias of the S version's bit 1 */ + s->ctlr = deposit32(s->ctlr, 1, 1, value); + } else if (gic_has_groups(s)) { + s->ctlr = value & (GICD_CTLR_EN_GRP0 | GICD_CTLR_EN_GRP1); + } else { + s->ctlr = value & GICD_CTLR_EN_GRP0; + } + DPRINTF("Distributor: Group0 %sabled; Group 1 %sabled\n", + s->ctlr & GICD_CTLR_EN_GRP0 ? "En" : "Dis", + s->ctlr & GICD_CTLR_EN_GRP1 ? "En" : "Dis"); } else if (offset < 4) { /* ignored. */ } else if (offset >= 0x80) { diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c index b5a85e5673..bef76fc474 100644 --- a/hw/intc/arm_gic_common.c +++ b/hw/intc/arm_gic_common.c @@ -59,12 +59,12 @@ static const VMStateDescription vmstate_gic_irq_state = { static const VMStateDescription vmstate_gic = { .name = "arm_gic", - .version_id = 8, - .minimum_version_id = 8, + .version_id = 9, + .minimum_version_id = 9, .pre_save = gic_pre_save, .post_load = gic_post_load, .fields = (VMStateField[]) { - VMSTATE_BOOL(enabled, GICState), + VMSTATE_UINT32(ctlr, GICState), VMSTATE_BOOL_ARRAY(cpu_enabled, GICState, GIC_NCPU), VMSTATE_STRUCT_ARRAY(irq_state, GICState, GIC_MAXIRQ, 1, vmstate_gic_irq_state, gic_irq_state), @@ -146,7 +146,7 @@ static void arm_gic_common_reset(DeviceState *dev) s->irq_target[i] = 1; } } - s->enabled = false; + s->ctlr = 0; } static Property arm_gic_common_properties[] = { diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c index 3591ca7d5d..4260fd8f04 100644 --- a/hw/intc/arm_gic_kvm.c +++ b/hw/intc/arm_gic_kvm.c @@ -353,8 +353,8 @@ static void kvm_arm_gic_put(GICState *s) * Distributor State */ - /* s->enabled -> GICD_CTLR */ - reg = s->enabled; + /* s->ctlr -> GICD_CTLR */ + reg = s->ctlr; kvm_gicd_access(s, 0x0, 0, ®, true); /* Sanity checking on GICD_TYPER and s->num_irq, s->num_cpu */ @@ -453,9 +453,9 @@ static void kvm_arm_gic_get(GICState *s) * Distributor State */ - /* GICD_CTLR -> s->enabled */ + /* GICD_CTLR -> s->ctlr */ kvm_gicd_access(s, 0x0, 0, ®, false); - s->enabled = reg & 1; + s->ctlr = reg; /* Sanity checking on GICD_TYPER -> s->num_irq, s->num_cpu */ kvm_gicd_access(s, 0x4, 0, ®, false); diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index ca19157c1e..16f0dca2d4 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -477,7 +477,7 @@ static void armv7m_nvic_reset(DeviceState *dev) s->gic.cpu_enabled[0] = true; s->gic.priority_mask[0] = 0x100; /* The NVIC as a whole is always enabled. */ - s->gic.enabled = true; + s->gic.ctlr = 1; systick_reset(s); } diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h index e8cf773e73..3b4b3fbc0e 100644 --- a/hw/intc/gic_internal.h +++ b/hw/intc/gic_internal.h @@ -54,6 +54,8 @@ #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) +#define GICD_CTLR_EN_GRP0 (1U << 0) +#define GICD_CTLR_EN_GRP1 (1U << 1) /* 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 b78981ed58..d5d38772c2 100644 --- a/include/hw/intc/arm_gic_common.h +++ b/include/hw/intc/arm_gic_common.h @@ -52,7 +52,10 @@ typedef struct GICState { qemu_irq parent_irq[GIC_NCPU]; qemu_irq parent_fiq[GIC_NCPU]; - bool enabled; + /* GICD_CTLR; for a GIC with the security extensions the NS banked version + * of this register is just an alias of bit 1 of the S banked version. + */ + uint32_t ctlr; bool cpu_enabled[GIC_NCPU]; gic_irq_state irq_state[GIC_MAXIRQ]; From 822e9cc310484f77e0b1c16fbef763a5d0eec80a Mon Sep 17 00:00:00 2001 From: Fabian Aggeler Date: Tue, 12 May 2015 11:57:17 +0100 Subject: [PATCH 09/19] hw/intc/arm_gic: Make ICCBPR/GICC_BPR banked This register is banked in GICs with Security Extensions. Storing the non-secure copy of BPR in the abpr, which is an alias to the non-secure copy for secure access. ABPR itself is only accessible from secure state if the GIC implements Security Extensions. Signed-off-by: Fabian Aggeler Signed-off-by: Greg Bellows Reviewed-by: Edgar E. Iglesias Signed-off-by: Peter Maydell Message-id: 1430502643-25909-8-git-send-email-peter.maydell@linaro.org Message-id: 1429113742-8371-10-git-send-email-greg.bellows@linaro.org [PMM: rewrote to fix style issues and correct handling of GICv2 without security extensions] Signed-off-by: Peter Maydell --- hw/intc/arm_gic.c | 31 ++++++++++++++++++++++++++----- include/hw/intc/arm_gic_common.h | 11 ++++++++--- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index 4f13ff2c90..e6ad8dea72 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -762,7 +762,12 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, int offset, *data = s->priority_mask[cpu]; break; case 0x08: /* Binary Point */ - *data = s->bpr[cpu]; + if (s->security_extn && !attrs.secure) { + /* BPR is banked. Non-secure copy stored in ABPR. */ + *data = s->abpr[cpu]; + } else { + *data = s->bpr[cpu]; + } break; case 0x0c: /* Acknowledge */ *data = gic_acknowledge_irq(s, cpu); @@ -774,7 +779,16 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, int offset, *data = s->current_pending[cpu]; break; case 0x1c: /* Aliased Binary Point */ - *data = s->abpr[cpu]; + /* GIC v2, no security: ABPR + * GIC v1, no security: not implemented (RAZ/WI) + * With security extensions, secure access: ABPR (alias of NS BPR) + * With security extensions, nonsecure access: RAZ/WI + */ + if (!gic_has_groups(s) || (s->security_extn && !attrs.secure)) { + *data = 0; + } else { + *data = s->abpr[cpu]; + } break; case 0xd0: case 0xd4: case 0xd8: case 0xdc: *data = s->apr[(offset - 0xd0) / 4][cpu]; @@ -799,14 +813,21 @@ static MemTxResult gic_cpu_write(GICState *s, int cpu, int offset, s->priority_mask[cpu] = (value & 0xff); break; case 0x08: /* Binary Point */ - s->bpr[cpu] = (value & 0x7); + if (s->security_extn && !attrs.secure) { + s->abpr[cpu] = MAX(value & 0x7, GIC_MIN_ABPR); + } else { + s->bpr[cpu] = MAX(value & 0x7, GIC_MIN_BPR); + } break; case 0x10: /* End Of Interrupt */ gic_complete_irq(s, cpu, value & 0x3ff); return MEMTX_OK; case 0x1c: /* Aliased Binary Point */ - if (s->revision >= 2) { - s->abpr[cpu] = (value & 0x7); + if (!gic_has_groups(s) || (s->security_extn && !attrs.secure)) { + /* unimplemented, or NS access: RAZ/WI */ + return MEMTX_OK; + } else { + s->abpr[cpu] = MAX(value & 0x7, GIC_MIN_ABPR); } break; case 0xd0: case 0xd4: case 0xd8: case 0xdc: diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h index d5d38772c2..261402f86a 100644 --- a/include/hw/intc/arm_gic_common.h +++ b/include/hw/intc/arm_gic_common.h @@ -34,6 +34,9 @@ #define MAX_NR_GROUP_PRIO 128 #define GIC_NR_APRS (MAX_NR_GROUP_PRIO / 32) +#define GIC_MIN_BPR 0 +#define GIC_MIN_ABPR (GIC_MIN_BPR + 1) + typedef struct gic_irq_state { /* The enable bits are only banked for per-cpu interrupts. */ uint8_t enabled; @@ -76,9 +79,11 @@ typedef struct GICState { uint16_t running_priority[GIC_NCPU]; uint16_t current_pending[GIC_NCPU]; - /* We present the GICv2 without security extensions to a guest and - * therefore the guest can configure the GICC_CTLR to configure group 1 - * binary point in the abpr. + /* If we present the GICv2 without security extensions to a guest, + * the guest can configure the GICC_CTLR to configure group 1 binary point + * in the abpr. + * For a GIC with Security Extensions we use use bpr for the + * secure copy and abpr as storage for the non-secure copy of the register. */ uint8_t bpr[GIC_NCPU]; uint8_t abpr[GIC_NCPU]; From 32951860834f09d1c1a0b81d8d7d5529e2d0e074 Mon Sep 17 00:00:00 2001 From: Fabian Aggeler Date: Tue, 12 May 2015 11:57:17 +0100 Subject: [PATCH 10/19] hw/intc/arm_gic: Make ICCICR/GICC_CTLR banked 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_ctlr. Signed-off-by: Fabian Aggeler Signed-off-by: Greg Bellows Reviewed-by: Edgar E. Iglesias Signed-off-by: Peter Maydell Message-id: 1430502643-25909-9-git-send-email-peter.maydell@linaro.org Message-id: 1429113742-8371-9-git-send-email-greg.bellows@linaro.org [PMM: rewrote to store state in a single uint32_t rather than keeping the NS and S banked variants separate; this considerably simplifies the get/set functions] Signed-off-by: Peter Maydell --- hw/intc/arm_gic.c | 51 +++++++++++++++++++++++++++++--- hw/intc/arm_gic_common.c | 8 ++--- hw/intc/arm_gic_kvm.c | 8 ++--- hw/intc/armv7m_nvic.c | 2 +- hw/intc/gic_internal.h | 16 ++++++++++ include/hw/intc/arm_gic_common.h | 5 +++- 6 files changed, 76 insertions(+), 14 deletions(-) diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index e6ad8dea72..4aaaac2de3 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -68,7 +68,7 @@ void gic_update(GICState *s) cm = 1 << cpu; s->current_pending[cpu] = 1023; if (!(s->ctlr & (GICD_CTLR_EN_GRP0 | GICD_CTLR_EN_GRP1)) - || !s->cpu_enabled[cpu]) { + || !(s->cpu_ctlr[cpu] & (GICC_CTLR_EN_GRP0 | GICC_CTLR_EN_GRP1))) { qemu_irq_lower(s->parent_irq[cpu]); return; } @@ -242,6 +242,50 @@ void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val) } } +static uint32_t gic_get_cpu_control(GICState *s, int cpu, MemTxAttrs attrs) +{ + uint32_t ret = s->cpu_ctlr[cpu]; + + if (s->security_extn && !attrs.secure) { + /* Construct the NS banked view of GICC_CTLR from the correct + * bits of the S banked view. We don't need to move the bypass + * control bits because we don't implement that (IMPDEF) part + * of the GIC architecture. + */ + ret = (ret & (GICC_CTLR_EN_GRP1 | GICC_CTLR_EOIMODE_NS)) >> 1; + } + return ret; +} + +static void gic_set_cpu_control(GICState *s, int cpu, uint32_t value, + MemTxAttrs attrs) +{ + uint32_t mask; + + if (s->security_extn && !attrs.secure) { + /* The NS view can only write certain bits in the register; + * the rest are unchanged + */ + mask = GICC_CTLR_EN_GRP1; + if (s->revision == 2) { + mask |= GICC_CTLR_EOIMODE_NS; + } + s->cpu_ctlr[cpu] &= ~mask; + s->cpu_ctlr[cpu] |= (value << 1) & mask; + } else { + if (s->revision == 2) { + mask = s->security_extn ? GICC_CTLR_V2_S_MASK : GICC_CTLR_V2_MASK; + } else { + mask = s->security_extn ? GICC_CTLR_V1_S_MASK : GICC_CTLR_V1_MASK; + } + s->cpu_ctlr[cpu] = value & mask; + } + DPRINTF("CPU Interface %d: Group0 Interrupts %sabled, " + "Group1 Interrupts %sabled\n", cpu, + (s->cpu_ctlr[cpu] & GICC_CTLR_EN_GRP0) ? "En" : "Dis", + (s->cpu_ctlr[cpu] & GICC_CTLR_EN_GRP1) ? "En" : "Dis"); +} + void gic_complete_irq(GICState *s, int cpu, int irq) { int update = 0; @@ -756,7 +800,7 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, int offset, { switch (offset) { case 0x00: /* Control */ - *data = s->cpu_enabled[cpu]; + *data = gic_get_cpu_control(s, cpu, attrs); break; case 0x04: /* Priority mask */ *data = s->priority_mask[cpu]; @@ -806,8 +850,7 @@ static MemTxResult gic_cpu_write(GICState *s, int cpu, int offset, { switch (offset) { case 0x00: /* Control */ - s->cpu_enabled[cpu] = (value & 1); - DPRINTF("CPU %d %sabled\n", cpu, s->cpu_enabled[cpu] ? "En" : "Dis"); + gic_set_cpu_control(s, cpu, value, attrs); break; case 0x04: /* Priority mask */ s->priority_mask[cpu] = (value & 0xff); diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c index bef76fc474..044ad66730 100644 --- a/hw/intc/arm_gic_common.c +++ b/hw/intc/arm_gic_common.c @@ -59,13 +59,13 @@ static const VMStateDescription vmstate_gic_irq_state = { static const VMStateDescription vmstate_gic = { .name = "arm_gic", - .version_id = 9, - .minimum_version_id = 9, + .version_id = 10, + .minimum_version_id = 10, .pre_save = gic_pre_save, .post_load = gic_post_load, .fields = (VMStateField[]) { VMSTATE_UINT32(ctlr, GICState), - VMSTATE_BOOL_ARRAY(cpu_enabled, GICState, GIC_NCPU), + VMSTATE_UINT32_ARRAY(cpu_ctlr, GICState, GIC_NCPU), VMSTATE_STRUCT_ARRAY(irq_state, GICState, GIC_MAXIRQ, 1, vmstate_gic_irq_state, gic_irq_state), VMSTATE_UINT8_ARRAY(irq_target, GICState, GIC_MAXIRQ), @@ -134,7 +134,7 @@ 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_ctlr[i] = 0; } 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 4260fd8f04..33e6a87b3c 100644 --- a/hw/intc/arm_gic_kvm.c +++ b/hw/intc/arm_gic_kvm.c @@ -414,8 +414,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_ctlr[cpu] -> GICC_CTLR */ + reg = s->cpu_ctlr[cpu]; kvm_gicc_access(s, 0x00, cpu, ®, true); /* s->priority_mask[cpu] -> GICC_PMR */ @@ -506,9 +506,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_ctlr[cpu] */ kvm_gicc_access(s, 0x00, cpu, ®, false); - s->cpu_enabled[cpu] = (reg & 1); + s->cpu_ctlr[cpu] = 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 16f0dca2d4..a6567a378d 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -474,7 +474,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_ctlr[0] = GICC_CTLR_EN_GRP0; s->gic.priority_mask[0] = 0x100; /* The NVIC as a whole is always enabled. */ s->gic.ctlr = 1; diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h index 3b4b3fbc0e..81c764c100 100644 --- a/hw/intc/gic_internal.h +++ b/hw/intc/gic_internal.h @@ -57,6 +57,22 @@ #define GICD_CTLR_EN_GRP0 (1U << 0) #define GICD_CTLR_EN_GRP1 (1U << 1) +#define GICC_CTLR_EN_GRP0 (1U << 0) +#define GICC_CTLR_EN_GRP1 (1U << 1) +#define GICC_CTLR_ACK_CTL (1U << 2) +#define GICC_CTLR_FIQ_EN (1U << 3) +#define GICC_CTLR_CBPR (1U << 4) /* GICv1: SBPR */ +#define GICC_CTLR_EOIMODE (1U << 9) +#define GICC_CTLR_EOIMODE_NS (1U << 10) + +/* Valid bits for GICC_CTLR for GICv1, v1 with security extensions, + * GICv2 and GICv2 with security extensions: + */ +#define GICC_CTLR_V1_MASK 0x1 +#define GICC_CTLR_V1_S_MASK 0x1f +#define GICC_CTLR_V2_MASK 0x21f +#define GICC_CTLR_V2_S_MASK 0x61f + /* The special cases for the revision property: */ #define REV_11MPCORE 0 #define REV_NVIC 0xffffffff diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h index 261402f86a..899db3d7a0 100644 --- a/include/hw/intc/arm_gic_common.h +++ b/include/hw/intc/arm_gic_common.h @@ -59,7 +59,10 @@ typedef struct GICState { * of this register is just an alias of bit 1 of the S banked version. */ uint32_t ctlr; - bool cpu_enabled[GIC_NCPU]; + /* GICC_CTLR; again, the NS banked version is just aliases of bits of + * the S banked register, so our state only needs to store the S version. + */ + uint32_t cpu_ctlr[GIC_NCPU]; gic_irq_state irq_state[GIC_MAXIRQ]; uint8_t irq_target[GIC_MAXIRQ]; From 08efa9f2d1bb27d64fbedcc2879ca45ae6832c20 Mon Sep 17 00:00:00 2001 From: Fabian Aggeler Date: Tue, 12 May 2015 11:57:17 +0100 Subject: [PATCH 11/19] hw/intc/arm_gic: Implement Non-secure view of RPR For GICs with Security Extensions Non-secure reads have a restricted view on the current running priority. Signed-off-by: Fabian Aggeler Signed-off-by: Greg Bellows Reviewed-by: Edgar E. Iglesias Signed-off-by: Peter Maydell Message-id: 1430502643-25909-10-git-send-email-peter.maydell@linaro.org Message-id: 1429113742-8371-11-git-send-email-greg.bellows@linaro.org [PMM: make function static, minor comment tweak] Signed-off-by: Peter Maydell --- hw/intc/arm_gic.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index 4aaaac2de3..e3bbe9e9e0 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -286,6 +286,23 @@ static void gic_set_cpu_control(GICState *s, int cpu, uint32_t value, (s->cpu_ctlr[cpu] & GICC_CTLR_EN_GRP1) ? "En" : "Dis"); } +static uint8_t gic_get_running_priority(GICState *s, int cpu, MemTxAttrs attrs) +{ + if (s->security_extn && !attrs.secure) { + if (s->running_priority[cpu] & 0x80) { + /* Running priority in upper half of range: return the Non-secure + * view of the priority. + */ + return s->running_priority[cpu] << 1; + } else { + /* Running priority in lower half of range: RAZ */ + return 0; + } + } else { + return s->running_priority[cpu]; + } +} + void gic_complete_irq(GICState *s, int cpu, int irq) { int update = 0; @@ -817,7 +834,7 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, int offset, *data = gic_acknowledge_irq(s, cpu); break; case 0x14: /* Running Priority */ - *data = s->running_priority[cpu]; + *data = gic_get_running_priority(s, cpu, attrs); break; case 0x18: /* Highest Pending Interrupt */ *data = s->current_pending[cpu]; From 8150847061f8d2606101bfff77cc6ec86b081ab0 Mon Sep 17 00:00:00 2001 From: Fabian Aggeler Date: Tue, 12 May 2015 11:57:17 +0100 Subject: [PATCH 12/19] hw/intc/arm_gic: Restrict priority view GICs with Security Extensions restrict the non-secure view of the interrupt priority and priority mask registers. Signed-off-by: Fabian Aggeler Signed-off-by: Greg Bellows Reviewed-by: Edgar E. Iglesias Signed-off-by: Peter Maydell Message-id: 1430502643-25909-11-git-send-email-peter.maydell@linaro.org Message-id: 1429113742-8371-15-git-send-email-greg.bellows@linaro.org [PMM: minor code tweaks; fixed missing masking in gic_set_priority_mask and gic_set_priority] Signed-off-by: Peter Maydell --- hw/intc/arm_gic.c | 63 ++++++++++++++++++++++++++++++++++++++---- hw/intc/arm_gic_kvm.c | 2 +- hw/intc/gic_internal.h | 3 +- 3 files changed, 61 insertions(+), 7 deletions(-) diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index e3bbe9e9e0..7c0ddc85cf 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -233,8 +233,16 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu) return ret; } -void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val) +void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val, + MemTxAttrs attrs) { + if (s->security_extn && !attrs.secure) { + if (!GIC_TEST_GROUP(irq, (1 << cpu))) { + return; /* Ignore Non-secure access of Group0 IRQ */ + } + val = 0x80 | (val >> 1); /* Non-secure view */ + } + if (irq < GIC_INTERNAL) { s->priority1[irq][cpu] = val; } else { @@ -242,6 +250,51 @@ void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val) } } +static uint32_t gic_get_priority(GICState *s, int cpu, int irq, + MemTxAttrs attrs) +{ + uint32_t prio = GIC_GET_PRIORITY(irq, cpu); + + if (s->security_extn && !attrs.secure) { + if (!GIC_TEST_GROUP(irq, (1 << cpu))) { + return 0; /* Non-secure access cannot read priority of Group0 IRQ */ + } + prio = (prio << 1) & 0xff; /* Non-secure view */ + } + return prio; +} + +static void gic_set_priority_mask(GICState *s, int cpu, uint8_t pmask, + MemTxAttrs attrs) +{ + if (s->security_extn && !attrs.secure) { + if (s->priority_mask[cpu] & 0x80) { + /* Priority Mask in upper half */ + pmask = 0x80 | (pmask >> 1); + } else { + /* Non-secure write ignored if priority mask is in lower half */ + return; + } + } + s->priority_mask[cpu] = pmask; +} + +static uint32_t gic_get_priority_mask(GICState *s, int cpu, MemTxAttrs attrs) +{ + uint32_t pmask = s->priority_mask[cpu]; + + if (s->security_extn && !attrs.secure) { + if (pmask & 0x80) { + /* Priority Mask in upper half, return Non-secure view */ + pmask = (pmask << 1) & 0xff; + } else { + /* Priority Mask in lower half, RAZ */ + pmask = 0; + } + } + return pmask; +} + static uint32_t gic_get_cpu_control(GICState *s, int cpu, MemTxAttrs attrs) { uint32_t ret = s->cpu_ctlr[cpu]; @@ -451,7 +504,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs) irq = (offset - 0x400) + GIC_BASE_IRQ; if (irq >= s->num_irq) goto bad_reg; - res = GIC_GET_PRIORITY(irq, cpu); + res = gic_get_priority(s, cpu, irq, attrs); } else if (offset < 0xc00) { /* Interrupt CPU Target. */ if (s->num_cpu == 1 && s->revision != REV_11MPCORE) { @@ -669,7 +722,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset, irq = (offset - 0x400) + GIC_BASE_IRQ; if (irq >= s->num_irq) goto bad_reg; - gic_set_priority(s, cpu, irq, value); + gic_set_priority(s, cpu, irq, value, attrs); } else if (offset < 0xc00) { /* Interrupt CPU Target. RAZ/WI on uniprocessor GICs, with the * annoying exception of the 11MPCore's GIC. @@ -820,7 +873,7 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, int offset, *data = gic_get_cpu_control(s, cpu, attrs); break; case 0x04: /* Priority mask */ - *data = s->priority_mask[cpu]; + *data = gic_get_priority_mask(s, cpu, attrs); break; case 0x08: /* Binary Point */ if (s->security_extn && !attrs.secure) { @@ -870,7 +923,7 @@ static MemTxResult gic_cpu_write(GICState *s, int cpu, int offset, gic_set_cpu_control(s, cpu, value, attrs); break; case 0x04: /* Priority mask */ - s->priority_mask[cpu] = (value & 0xff); + gic_set_priority_mask(s, cpu, value, attrs); break; case 0x08: /* Binary Point */ if (s->security_extn && !attrs.secure) { diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c index 33e6a87b3c..2cb7d255d2 100644 --- a/hw/intc/arm_gic_kvm.c +++ b/hw/intc/arm_gic_kvm.c @@ -251,7 +251,7 @@ static void translate_priority(GICState *s, int irq, int cpu, if (to_kernel) { *field = GIC_GET_PRIORITY(irq, cpu) & 0xff; } else { - gic_set_priority(s, cpu, irq, *field & 0xff); + gic_set_priority(s, cpu, irq, *field & 0xff, MEMTXATTRS_UNSPECIFIED); } } diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h index 81c764c100..119fb8147e 100644 --- a/hw/intc/gic_internal.h +++ b/hw/intc/gic_internal.h @@ -82,7 +82,8 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu); 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); +void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val, + MemTxAttrs attrs); static inline bool gic_test_pending(GICState *s, int irq, int cm) { From 7c0fa108d918ab818e49c4588ab290004d6b532e Mon Sep 17 00:00:00 2001 From: Fabian Aggeler Date: Tue, 12 May 2015 11:57:18 +0100 Subject: [PATCH 13/19] hw/intc/arm_gic: Handle grouping for GICC_HPPIR Grouping (GICv2) and Security Extensions change the behaviour of reads of the highest priority pending interrupt register (ICCHPIR/GICC_HPPIR). Signed-off-by: Fabian Aggeler Signed-off-by: Greg Bellows Reviewed-by: Edgar E. Iglesias Signed-off-by: Peter Maydell Message-id: 1430502643-25909-12-git-send-email-peter.maydell@linaro.org Message-id: 1429113742-8371-12-git-send-email-greg.bellows@linaro.org [PMM: make utility fn static; coding style fixes; AckCtl has an effect for GICv2 without security extensions as well; removed checks on enable bits because these are done when we set current_pending[cpu]] Signed-off-by: Peter Maydell --- hw/intc/arm_gic.c | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index 7c0ddc85cf..75c69b3fa3 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -176,6 +176,32 @@ static void gic_set_irq(void *opaque, int irq, int level) gic_update(s); } +static uint16_t gic_get_current_pending_irq(GICState *s, int cpu, + MemTxAttrs attrs) +{ + uint16_t pending_irq = s->current_pending[cpu]; + + if (pending_irq < GIC_MAXIRQ && gic_has_groups(s)) { + int group = GIC_TEST_GROUP(pending_irq, (1 << cpu)); + /* On a GIC without the security extensions, reading this register + * behaves in the same way as a secure access to a GIC with them. + */ + bool secure = !s->security_extn || attrs.secure; + + if (group == 0 && !secure) { + /* Group0 interrupts hidden from Non-secure access */ + return 1023; + } + if (group == 1 && secure && !(s->cpu_ctlr[cpu] & GICC_CTLR_ACK_CTL)) { + /* Group1 interrupts only seen by Secure access if + * AckCtl bit set. + */ + return 1022; + } + } + return pending_irq; +} + static void gic_set_running_irq(GICState *s, int cpu, int irq) { s->running_irq[cpu] = irq; @@ -890,7 +916,7 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, int offset, *data = gic_get_running_priority(s, cpu, attrs); break; case 0x18: /* Highest Pending Interrupt */ - *data = s->current_pending[cpu]; + *data = gic_get_current_pending_irq(s, cpu, attrs); break; case 0x1c: /* Aliased Binary Point */ /* GIC v2, no security: ABPR From f9c6a7f1395c6d88a3bb1a0cb48811994709966e Mon Sep 17 00:00:00 2001 From: Fabian Aggeler Date: Tue, 12 May 2015 11:57:18 +0100 Subject: [PATCH 14/19] hw/intc/arm_gic: Change behavior of EOIR writes Grouping (GICv2) and Security Extensions change the behavior of EOIR writes. Completing Group0 interrupts is only allowed from Secure state. Signed-off-by: Fabian Aggeler Signed-off-by: Greg Bellows Reviewed-by: Edgar E. Iglesias Signed-off-by: Peter Maydell Message-id: 1430502643-25909-13-git-send-email-peter.maydell@linaro.org Message-id: 1429113742-8371-13-git-send-email-greg.bellows@linaro.org [PMM: Rather than go to great lengths to ignore the UNPREDICTABLE case of a Secure EOI of a Group1 (NS) irq with AckCtl == 0, we just let it fall through; add a comment about it.] Signed-off-by: Peter Maydell --- hw/intc/arm_gic.c | 14 ++++++++++++-- hw/intc/armv7m_nvic.c | 2 +- hw/intc/gic_internal.h | 2 +- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index 75c69b3fa3..4ad80e77bb 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -382,7 +382,7 @@ static uint8_t gic_get_running_priority(GICState *s, int cpu, MemTxAttrs attrs) } } -void gic_complete_irq(GICState *s, int cpu, int irq) +void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs) { int update = 0; int cm = 1 << cpu; @@ -412,6 +412,16 @@ void gic_complete_irq(GICState *s, int cpu, int irq) } } + if (s->security_extn && !attrs.secure && !GIC_TEST_GROUP(irq, cm)) { + DPRINTF("Non-secure EOI for Group0 interrupt %d ignored\n", irq); + return; + } + + /* Secure EOI with GICC_CTLR.AckCtl == 0 when the IRQ is a Group 1 + * interrupt is UNPREDICTABLE. We choose to handle it as if AckCtl == 1, + * i.e. go ahead and complete the irq anyway. + */ + if (irq != s->running_irq[cpu]) { /* Complete an IRQ that is not currently running. */ int tmp = s->running_irq[cpu]; @@ -959,7 +969,7 @@ static MemTxResult gic_cpu_write(GICState *s, int cpu, int offset, } break; case 0x10: /* End Of Interrupt */ - gic_complete_irq(s, cpu, value & 0x3ff); + gic_complete_irq(s, cpu, value & 0x3ff, attrs); return MEMTX_OK; case 0x1c: /* Aliased Binary Point */ if (!gic_has_groups(s) || (s->security_extn && !attrs.secure)) { diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index a6567a378d..1bfff7530a 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -144,7 +144,7 @@ void armv7m_nvic_complete_irq(void *opaque, int irq) nvic_state *s = (nvic_state *)opaque; if (irq >= 16) irq += 16; - gic_complete_irq(&s->gic, 0, irq); + gic_complete_irq(&s->gic, 0, irq, MEMTXATTRS_UNSPECIFIED); } static uint32_t nvic_readl(nvic_state *s, uint32_t offset) diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h index 119fb8147e..d3cebeff3b 100644 --- a/hw/intc/gic_internal.h +++ b/hw/intc/gic_internal.h @@ -79,7 +79,7 @@ void gic_set_pending_private(GICState *s, int cpu, int irq); uint32_t gic_acknowledge_irq(GICState *s, int cpu); -void gic_complete_irq(GICState *s, int cpu, int irq); +void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs); 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, From c5619bf9e8935aeb972c0bd935549e9ee0a739f2 Mon Sep 17 00:00:00 2001 From: Fabian Aggeler Date: Tue, 12 May 2015 11:57:18 +0100 Subject: [PATCH 15/19] hw/intc/arm_gic: Change behavior of IAR writes Grouping (GICv2) and Security Extensions change the behavior of IAR reads. Acknowledging Group0 interrupts is only allowed from Secure state and acknowledging Group1 interrupts from Secure state is only allowed if AckCtl bit is set. Signed-off-by: Fabian Aggeler Signed-off-by: Greg Bellows Reviewed-by: Edgar E. Iglesias Signed-off-by: Peter Maydell Message-id: 1430502643-25909-14-git-send-email-peter.maydell@linaro.org Message-id: 1429113742-8371-14-git-send-email-greg.bellows@linaro.org [PMM: simplify significantly by reusing the existing gic_get_current_pending_irq() rather than reimplementing the same logic here] Signed-off-by: Peter Maydell --- hw/intc/arm_gic.c | 22 ++++++++++++++++------ hw/intc/armv7m_nvic.c | 2 +- hw/intc/gic_internal.h | 2 +- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index 4ad80e77bb..6abdb14379 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -213,14 +213,24 @@ static void gic_set_running_irq(GICState *s, int cpu, int irq) gic_update(s); } -uint32_t gic_acknowledge_irq(GICState *s, int cpu) +uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs) { int ret, irq, src; int cm = 1 << cpu; - irq = s->current_pending[cpu]; - if (irq == 1023 - || GIC_GET_PRIORITY(irq, cpu) >= s->running_priority[cpu]) { - DPRINTF("ACK no pending IRQ\n"); + + /* gic_get_current_pending_irq() will return 1022 or 1023 appropriately + * for the case where this GIC supports grouping and the pending interrupt + * is in the wrong group. + */ + irq = gic_get_current_pending_irq(s, cpu, attrs);; + + if (irq >= GIC_MAXIRQ) { + DPRINTF("ACK, no pending interrupt or it is hidden: %d\n", irq); + return irq; + } + + if (GIC_GET_PRIORITY(irq, cpu) >= s->running_priority[cpu]) { + DPRINTF("ACK, pending interrupt (%d) has insufficient priority\n", irq); return 1023; } s->last_active[irq][cpu] = s->running_irq[cpu]; @@ -920,7 +930,7 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, int offset, } break; case 0x0c: /* Acknowledge */ - *data = gic_acknowledge_irq(s, cpu); + *data = gic_acknowledge_irq(s, cpu, attrs); break; case 0x14: /* Running Priority */ *data = gic_get_running_priority(s, cpu, attrs); diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 1bfff7530a..e13b729e1d 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -131,7 +131,7 @@ int armv7m_nvic_acknowledge_irq(void *opaque) nvic_state *s = (nvic_state *)opaque; uint32_t irq; - irq = gic_acknowledge_irq(&s->gic, 0); + irq = gic_acknowledge_irq(&s->gic, 0, MEMTXATTRS_UNSPECIFIED); if (irq == 1023) hw_error("Interrupt but no vector\n"); if (irq >= 32) diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h index d3cebeff3b..20c1e8a242 100644 --- a/hw/intc/gic_internal.h +++ b/hw/intc/gic_internal.h @@ -78,7 +78,7 @@ #define REV_NVIC 0xffffffff void gic_set_pending_private(GICState *s, int cpu, int irq); -uint32_t gic_acknowledge_irq(GICState *s, int cpu); +uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs); void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs); void gic_update(GICState *s); void gic_init_irqs_and_distributor(GICState *s); From dadbb58f5955053c5f5dc2252a4b183f90d7bfce Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 12 May 2015 11:57:18 +0100 Subject: [PATCH 16/19] hw/intc/arm_gic: Add grouping support to gic_update() Add support to gic_update() for determining the current IRQ and FIQ status when interrupt grouping is supported. This simply requires that instead of always raising IRQ we check the group of the highest priority pending interrupt and the GICC_CTLR.FIQEn bit to see whether we should raise IRQ or FIQ. Signed-off-by: Peter Maydell Reviewed-by: Edgar E. Iglesias Message-id: 1430502643-25909-15-git-send-email-peter.maydell@linaro.org --- hw/intc/arm_gic.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index 6abdb14379..c1d2e704ec 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -60,7 +60,7 @@ void gic_update(GICState *s) int best_irq; int best_prio; int irq; - int level; + int irq_level, fiq_level; int cpu; int cm; @@ -70,6 +70,7 @@ void gic_update(GICState *s) if (!(s->ctlr & (GICD_CTLR_EN_GRP0 | GICD_CTLR_EN_GRP1)) || !(s->cpu_ctlr[cpu] & (GICC_CTLR_EN_GRP0 | GICC_CTLR_EN_GRP1))) { qemu_irq_lower(s->parent_irq[cpu]); + qemu_irq_lower(s->parent_fiq[cpu]); return; } best_prio = 0x100; @@ -83,15 +84,31 @@ void gic_update(GICState *s) } } } - level = 0; + + irq_level = fiq_level = 0; + if (best_prio < s->priority_mask[cpu]) { s->current_pending[cpu] = best_irq; if (best_prio < s->running_priority[cpu]) { - DPRINTF("Raised pending IRQ %d (cpu %d)\n", best_irq, cpu); - level = 1; + int group = GIC_TEST_GROUP(best_irq, cm); + + if (extract32(s->ctlr, group, 1) && + extract32(s->cpu_ctlr[cpu], group, 1)) { + if (group == 0 && s->cpu_ctlr[cpu] & GICC_CTLR_FIQ_EN) { + DPRINTF("Raised pending FIQ %d (cpu %d)\n", + best_irq, cpu); + fiq_level = 1; + } else { + DPRINTF("Raised pending IRQ %d (cpu %d)\n", + best_irq, cpu); + irq_level = 1; + } + } } } - qemu_set_irq(s->parent_irq[cpu], level); + + qemu_set_irq(s->parent_irq[cpu], irq_level); + qemu_set_irq(s->parent_fiq[cpu], fiq_level); } } From 8e7b4ca08b968c9e195bcae9c6cb827c8564871a Mon Sep 17 00:00:00 2001 From: Greg Bellows Date: Tue, 12 May 2015 11:57:18 +0100 Subject: [PATCH 17/19] hw/arm/virt.c: Wire FIQ between CPU <> GIC Connect FIQ output of the GIC CPU interfaces to the CPUs. Signed-off-by: Greg Bellows Reviewed-by: Edgar E. Iglesias Signed-off-by: Peter Maydell Message-id: 1430502643-25909-16-git-send-email-peter.maydell@linaro.org Message-id: 1429113742-8371-4-git-send-email-greg.bellows@linaro.org [PMM: minor format tweak] Signed-off-by: Peter Maydell --- hw/arm/virt.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 565f573137..a7f9a10eca 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -386,6 +386,8 @@ static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic) qdev_get_gpio_in(gicdev, ppibase + 27)); sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ)); + sysbus_connect_irq(gicbusdev, i + smp_cpus, + qdev_get_gpio_in(cpudev, ARM_CPU_FIQ)); } for (i = 0; i < NUM_IRQS; i++) { From 27192e390d064489dcb23d5fcceb21cabf86d789 Mon Sep 17 00:00:00 2001 From: Fabian Aggeler Date: Tue, 12 May 2015 11:57:18 +0100 Subject: [PATCH 18/19] hw/arm/vexpress.c: Wire FIQ between CPU <> GIC Connect FIQ output of the GIC CPU interfaces to the CPUs. Signed-off-by: Fabian Aggeler Signed-off-by: Greg Bellows Reviewed-by: Edgar E. Iglesias Signed-off-by: Peter Maydell Message-id: 1430502643-25909-17-git-send-email-peter.maydell@linaro.org Message-id: 1429113742-8371-3-git-send-email-greg.bellows@linaro.org [PMM: minor format tweak] Signed-off-by: Peter Maydell --- hw/arm/vexpress.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c index 3989bc52b7..8f1a5ea992 100644 --- a/hw/arm/vexpress.c +++ b/hw/arm/vexpress.c @@ -253,6 +253,8 @@ static void init_cpus(const char *cpu_model, const char *privdev, DeviceState *cpudev = DEVICE(qemu_get_cpu(n)); sysbus_connect_irq(busdev, n, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ)); + sysbus_connect_irq(busdev, n + smp_cpus, + qdev_get_gpio_in(cpudev, ARM_CPU_FIQ)); } } From 5ae79fe825bedc89db8b6bde9d0ed0bb5d59558c Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 12 May 2015 11:57:19 +0100 Subject: [PATCH 19/19] hw/arm/highbank.c: Wire FIQ between CPU <> GIC Connect FIQ output of the GIC CPU interfaces to the CPUs. Signed-off-by: Peter Maydell Message-id: 1430502643-25909-18-git-send-email-peter.maydell@linaro.org --- hw/arm/highbank.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c index b2d048b911..f8353a7874 100644 --- a/hw/arm/highbank.c +++ b/hw/arm/highbank.c @@ -217,6 +217,7 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id) qemu_irq pic[128]; int n; qemu_irq cpu_irq[4]; + qemu_irq cpu_fiq[4]; MemoryRegion *sysram; MemoryRegion *dram; MemoryRegion *sysmem; @@ -269,6 +270,7 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id) exit(1); } cpu_irq[n] = qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ); + cpu_fiq[n] = qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_FIQ); } sysmem = get_system_memory(); @@ -313,6 +315,7 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id) sysbus_mmio_map(busdev, 0, MPCORE_PERIPHBASE); for (n = 0; n < smp_cpus; n++) { sysbus_connect_irq(busdev, n, cpu_irq[n]); + sysbus_connect_irq(busdev, n + smp_cpus, cpu_fiq[n]); } for (n = 0; n < 128; n++) {