From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39703) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XRUYr-0001WL-SF for qemu-devel@nongnu.org; Tue, 09 Sep 2014 19:11:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XRUYm-00043N-TP for qemu-devel@nongnu.org; Tue, 09 Sep 2014 19:10:57 -0400 Received: from mail-qc0-f170.google.com ([209.85.216.170]:59543) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XRUYm-00043I-NK for qemu-devel@nongnu.org; Tue, 09 Sep 2014 19:10:52 -0400 Received: by mail-qc0-f170.google.com with SMTP id l6so268550qcy.15 for ; Tue, 09 Sep 2014 16:10:52 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1408703392-23893-14-git-send-email-aggelerf@ethz.ch> References: <1408703392-23893-1-git-send-email-aggelerf@ethz.ch> <1408703392-23893-14-git-send-email-aggelerf@ethz.ch> Date: Tue, 9 Sep 2014 18:10:52 -0500 Message-ID: From: Greg Bellows Content-Type: multipart/alternative; boundary=001a1134497afe054a0502aa0bac Subject: Re: [Qemu-devel] [PATCH 13/15] hw/intc/arm_gic: Restrict priority view List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fabian Aggeler Cc: Peter Maydell , QEMU Developers , Christoffer Dall , "Edgar E. Iglesias" --001a1134497afe054a0502aa0bac Content-Type: text/plain; charset=UTF-8 On 22 August 2014 05:29, Fabian Aggeler wrote: > GICs with Security Extensions restrict the non-secure view of the > interrupt priority and priority mask registers. > > Signed-off-by: Fabian Aggeler > --- > hw/intc/arm_gic.c | 66 > +++++++++++++++++++++++++++++++++++++++++++++----- > hw/intc/gic_internal.h | 3 +++ > 2 files changed, 63 insertions(+), 6 deletions(-) > > diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c > index cddad45..3fe5f09 100644 > --- a/hw/intc/arm_gic.c > +++ b/hw/intc/arm_gic.c > @@ -256,11 +256,66 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu) > > void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val) > { > + uint8_t prio = val; > + > + if (s->security_extn && ns_access()) { > + if (GIC_TEST_GROUP0(irq, (1 << cpu))) { > + return; /* Ignore Non-secure access of Group0 IRQ */ > + } > + prio = 0x80 | (prio >> 1); /* Non-secure view */ > + } > + > if (irq < GIC_INTERNAL) { > - s->priority1[irq][cpu] = val; > + s->priority1[irq][cpu] = prio; > } else { > - s->priority2[(irq) - GIC_INTERNAL] = val; > + s->priority2[(irq) - GIC_INTERNAL] = prio; > + } > +} > + > +uint32_t gic_get_priority(GICState *s, int cpu, int irq) > +{ > + uint32_t prio = GIC_GET_PRIORITY(irq, cpu); > + > + if (s->security_extn && ns_access()) { > + if (GIC_TEST_GROUP0(irq, (1 << cpu))) { > + return 0; /* Non-secure access cannot read priority of Group0 > IRQ */ > + } > + prio = (prio << 1); /* Non-secure view */ > This should probably be masked to avoid a return containing reserved bits or only return a uint8_t. > } > + return prio; > +} > + > +void gic_set_priority_mask(GICState *s, int cpu, uint8_t val) > +{ > + uint8_t pmask = (val & 0xff); > + > + if (s->security_extn && ns_access()) { > + 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; > +} > + > +uint32_t gic_get_priority_mask(GICState *s, int cpu) > +{ > + uint32_t pmask = s->priority_mask[cpu]; > + > + if (s->security_extn && ns_access()) { > + if (pmask & 0x80) { > + /* Priority Mask in upper half, return Non-secure view */ > + pmask = (pmask << 1); > + } else { > + /* Priority Mask in lower half, RAZ */ > + pmask = 0; > Again this should probably be masked to prevent returning a value with reserved bits set. > + } > + } > + return pmask; > + > } > > uint32_t gic_get_cpu_control(GICState *s, int cpu) > @@ -518,7 +573,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr > offset) > 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); > } else if (offset < 0xc00) { > /* Interrupt CPU Target. */ > if (s->num_cpu == 1 && s->revision != REV_11MPCORE) { > @@ -871,7 +926,7 @@ static uint32_t gic_cpu_read(GICState *s, int cpu, int > offset) > case 0x00: /* Control */ > return gic_get_cpu_control(s, cpu); > case 0x04: /* Priority mask */ > - return s->priority_mask[cpu]; > + return gic_get_priority_mask(s, cpu); > case 0x08: /* Binary Point */ > if (s->security_extn && ns_access()) { > /* BPR is banked. Non-secure copy stored in ABPR. */ > @@ -909,8 +964,7 @@ static void gic_cpu_write(GICState *s, int cpu, int > offset, uint32_t value) > case 0x00: /* Control */ > return gic_set_cpu_control(s, cpu, value); > case 0x04: /* Priority mask */ > - s->priority_mask[cpu] = (value & 0xff); > - break; > + return gic_set_priority_mask(s, cpu, value); > case 0x08: /* Binary Point */ > if (s->security_extn && ns_access()) { > /* BPR is banked. Non-secure copy stored in ABPR. */ > diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h > index 17632c1..8d951cc 100644 > --- a/hw/intc/gic_internal.h > +++ b/hw/intc/gic_internal.h > @@ -76,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, int num_irq); > void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val); > +uint32_t gic_get_priority(GICState *s, int cpu, int irq); > +void gic_set_priority_mask(GICState *s, int cpu, uint8_t val); > +uint32_t gic_get_priority_mask(GICState *s, int cpu); > uint32_t gic_get_cpu_control(GICState *s, int cpu); > void gic_set_cpu_control(GICState *s, int cpu, uint32_t value); > uint8_t gic_get_running_priority(GICState *s, int cpu); > -- > 1.8.3.2 > > --001a1134497afe054a0502aa0bac Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On 22 August 2014 05:29, Fabian Aggeler <aggelerf@ethz.ch> wrote:
GICs with Security Extensions = restrict the non-secure view of the
interrupt priority and priority mask registers.

Signed-off-by: Fabian Aggeler <aggelerf@ethz.ch>
---
=C2=A0hw/intc/arm_gic.c=C2=A0 =C2=A0 =C2=A0 | 66 ++++++++++++++++++++++++++= +++++++++++++++++++-----
=C2=A0hw/intc/gic_internal.h |=C2=A0 3 +++
=C2=A02 files changed, 63 insertions(+), 6 deletions(-)

diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index cddad45..3fe5f09 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -256,11 +256,66 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu)
=C2=A0void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val)
=C2=A0{
+=C2=A0 =C2=A0 uint8_t prio =3D val;
+
+=C2=A0 =C2=A0 if (s->security_extn && ns_access()) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (GIC_TEST_GROUP0(irq, (1 << cpu))) {<= br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return; /* Ignore Non-secure acc= ess of Group0 IRQ */
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 prio =3D 0x80 | (prio >> 1); /* Non-secu= re view */
+=C2=A0 =C2=A0 }
+
=C2=A0 =C2=A0 =C2=A0if (irq < GIC_INTERNAL) {
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->priority1[irq][cpu] =3D val;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->priority1[irq][cpu] =3D prio;
=C2=A0 =C2=A0 =C2=A0} else {
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->priority2[(irq) - GIC_INTERNAL] =3D val;=
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->priority2[(irq) - GIC_INTERNAL] =3D prio= ;
+=C2=A0 =C2=A0 }
+}
+
+uint32_t gic_get_priority(GICState *s, int cpu, int irq)
+{
+=C2=A0 =C2=A0 uint32_t prio =3D GIC_GET_PRIORITY(irq, cpu);
+
+=C2=A0 =C2=A0 if (s->security_extn && ns_access()) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (GIC_TEST_GROUP0(irq, (1 << cpu))) {<= br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return 0; /* Non-secure access c= annot read priority of Group0 IRQ */
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 prio =3D (prio << 1); /* Non-secure view= */

This should probably be masked to a= void a return containing reserved bits or only return a uint8_t.
= =C2=A0
=C2=A0 =C2=A0 =C2=A0}
+=C2=A0 =C2=A0 return prio;
+}
+
+void gic_set_priority_mask(GICState *s, int cpu, uint8_t val)
+{
+=C2=A0 =C2=A0 uint8_t pmask =3D (val & 0xff);
+
+=C2=A0 =C2=A0 if (s->security_extn && ns_access()) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (s->priority_mask[cpu] & 0x80) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Priority Mask in upper half *= /
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pmask =3D 0x80 | (pmask >>= 1);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 } else {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Non-secure write ignored if p= riority mask is in lower half */
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
+=C2=A0 =C2=A0 }
+=C2=A0 =C2=A0 s->priority_mask[cpu] =3D pmask;
+}
+
+uint32_t gic_get_priority_mask(GICState *s, int cpu)
+{
+=C2=A0 =C2=A0 uint32_t pmask =3D s->priority_mask[cpu];
+
+=C2=A0 =C2=A0 if (s->security_extn && ns_access()) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (pmask & 0x80) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Priority Mask in upper half, = return Non-secure view */
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pmask =3D (pmask << 1); +=C2=A0 =C2=A0 =C2=A0 =C2=A0 } else {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Priority Mask in lower half, = RAZ */
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pmask =3D 0;

Again this should probably be masked to prevent returning = a value with reserved bits set.
=C2=A0
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
+=C2=A0 =C2=A0 }
+=C2=A0 =C2=A0 return pmask;
+
=C2=A0}

=C2=A0uint32_t gic_get_cpu_control(GICState *s, int cpu)
@@ -518,7 +573,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr off= set)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0irq =3D (offset - 0x400) + GIC_BASE_IRQ;<= br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (irq >=3D s->num_irq)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0goto bad_reg;
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 res =3D GIC_GET_PRIORITY(irq, cpu);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 res =3D gic_get_priority(s, cpu, irq);
=C2=A0 =C2=A0 =C2=A0} else if (offset < 0xc00) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Interrupt CPU Target.=C2=A0 */
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (s->num_cpu =3D=3D 1 && s-&= gt;revision !=3D REV_11MPCORE) {
@@ -871,7 +926,7 @@ static uint32_t gic_cpu_read(GICState *s, int cpu, int = offset)
=C2=A0 =C2=A0 =C2=A0case 0x00: /* Control */
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return gic_get_cpu_control(s, cpu);
=C2=A0 =C2=A0 =C2=A0case 0x04: /* Priority mask */
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 return s->priority_mask[cpu];
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 return gic_get_priority_mask(s, cpu);
=C2=A0 =C2=A0 =C2=A0case 0x08: /* Binary Point */
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (s->security_extn && ns_acc= ess()) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* BPR is banked. Non-secur= e copy stored in ABPR. */
@@ -909,8 +964,7 @@ static void gic_cpu_write(GICState *s, int cpu, int off= set, uint32_t value)
=C2=A0 =C2=A0 =C2=A0case 0x00: /* Control */
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return gic_set_cpu_control(s, cpu, value)= ;
=C2=A0 =C2=A0 =C2=A0case 0x04: /* Priority mask */
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->priority_mask[cpu] =3D (value & 0xff= );
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 return gic_set_priority_mask(s, cpu, value); =C2=A0 =C2=A0 =C2=A0case 0x08: /* Binary Point */
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (s->security_extn && ns_acc= ess()) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* BPR is banked. Non-secur= e copy stored in ABPR. */
diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
index 17632c1..8d951cc 100644
--- a/hw/intc/gic_internal.h
+++ b/hw/intc/gic_internal.h
@@ -76,6 +76,9 @@ void gic_complete_irq(GICState *s, int cpu, int irq);
=C2=A0void gic_update(GICState *s);
=C2=A0void gic_init_irqs_and_distributor(GICState *s, int num_irq);
=C2=A0void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val); +uint32_t gic_get_priority(GICState *s, int cpu, int irq);
+void gic_set_priority_mask(GICState *s, int cpu, uint8_t val);
+uint32_t gic_get_priority_mask(GICState *s, int cpu);
=C2=A0uint32_t gic_get_cpu_control(GICState *s, int cpu);
=C2=A0void gic_set_cpu_control(GICState *s, int cpu, uint32_t value);
=C2=A0uint8_t gic_get_running_priority(GICState *s, int cpu);
--
1.8.3.2


--001a1134497afe054a0502aa0bac--