[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 09/16] hw/intc/arm_gic: Make ICCBPR/GICC_BPR
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH v2 09/16] hw/intc/arm_gic: Make ICCBPR/GICC_BPR banked |
Date: |
Tue, 14 Apr 2015 20:23:57 +0100 |
On 30 October 2014 at 22:12, Greg Bellows <address@hidden> wrote:
> From: Fabian Aggeler <address@hidden>
>
> 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 <address@hidden>
>
> ---
>
> v1 -> v2
> - Fix ABPR read handling when security extensions are not present
> - Fix BPR write to take into consideration the minimum value written to ABPR
> and restrict BPR->ABPR mirroring to GICv2 and up.
> - Fix ABPR write to take into consideration the minumum value written
> - Fix ABPR write condition break-down to include mirroring of ABPR writes to
> BPR.
> ---
> hw/intc/arm_gic.c | 54
> ++++++++++++++++++++++++++++++++++++----
> include/hw/intc/arm_gic_common.h | 11 +++++---
> 2 files changed, 57 insertions(+), 8 deletions(-)
>
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> index 3c0414f..3761d12 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -840,7 +840,12 @@ static uint32_t gic_cpu_read(GICState *s, int cpu, int
> offset)
> case 0x04: /* Priority mask */
> return s->priority_mask[cpu];
> case 0x08: /* Binary Point */
> - return s->bpr[cpu];
> + if (s->security_extn && ns_access()) {
> + /* BPR is banked. Non-secure copy stored in ABPR. */
> + return s->abpr[cpu];
> + } else {
> + return s->bpr[cpu];
> + }
> case 0x0c: /* Acknowledge */
> return gic_acknowledge_irq(s, cpu);
> case 0x14: /* Running Priority */
> @@ -848,7 +853,14 @@ static uint32_t gic_cpu_read(GICState *s, int cpu, int
> offset)
> case 0x18: /* Highest Pending Interrupt */
> return s->current_pending[cpu];
> case 0x1c: /* Aliased Binary Point */
> - return s->abpr[cpu];
> + if (!s->security_extn || (s->security_extn && ns_access())) {
> + /* If Security Extensions are present ABPR is a secure register,
> + * only accessible from secure state.
> + */
> + return 0;
> + } else {
> + return s->abpr[cpu];
> + }
> case 0xd0: case 0xd4: case 0xd8: case 0xdc:
> return s->apr[(offset - 0xd0) / 4][cpu];
> default:
> @@ -867,13 +879,45 @@ static void gic_cpu_write(GICState *s, int cpu, int
> offset, uint32_t value)
> s->priority_mask[cpu] = (value & 0xff);
> break;
> case 0x08: /* Binary Point */
> - s->bpr[cpu] = (value & 0x7);
> + if (s->security_extn && ns_access()) {
> + /* BPR is banked. Non-secure copy stored in ABPR. */
> + /* The non-secure (ABPR) must not be below an implementation
> + * defined minimum value between 1-4.
> + * NOTE: BPR_MIN is currently set to 0, which is always true
> given
> + * the value is unsigned, so no check is necessary.
> + */
> + s->abpr[cpu] = (GIC_MIN_ABPR <= (value & 0x7))
> + ? (value & 0x7) : GIC_MIN_ABPR;
> + } else {
> + s->bpr[cpu] = (value & 0x7);
> + if (s->revision >= 2) {
> + /* On GICv2 without sec ext, GICC_ABPR is an alias of
> GICC_BPR
> + * so mirror the write.
> + */
> + s->abpr[cpu] = s->bpr[cpu];
My reading of the spec says that GICv2 without Security extensions
should not alias these two registers.
-- PMM
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v2 09/16] hw/intc/arm_gic: Make ICCBPR/GICC_BPR banked,
Peter Maydell <=