qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-arm] [PATCH v5 08/20] intc/arm_gic: Refactor secure/ns access chec


From: Luc Michel
Subject: [Qemu-arm] [PATCH v5 08/20] intc/arm_gic: Refactor secure/ns access check in the CPU interface
Date: Fri, 27 Jul 2018 11:54:09 +0200

An access to the CPU interface is non-secure if the current GIC instance
implements the security extensions, and the memory access is actually
non-secure. Until then, it was checked with tests such as
  if (s->security_extn && !attrs.secure) { ... }
in various places of the CPU interface code.

With the implementation of the virtualization extensions, those tests
must be updated to take into account whether we are in a vCPU interface
or not. This is because the exposed vCPU interface does not implement
security extensions.

This commits replaces all those tests with a call to the
gic_cpu_ns_access() function to check if the current access to the CPU
interface is non-secure. This function takes into account whether the
current CPU is a vCPU or not.

Note that this function is used only in the (v)CPU interface code path.
The distributor code path is left unchanged, as the distributor is not
exposed to vCPUs at all.

Signed-off-by: Luc Michel <address@hidden>
Reviewed-by: Peter Maydell <address@hidden>
---
 hw/intc/arm_gic.c | 39 ++++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 41141fee53..94d5982e2a 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -72,10 +72,15 @@ static inline int gic_get_current_vcpu(GICState *s)
 static inline bool gic_has_groups(GICState *s)
 {
     return s->revision == 2 || s->security_extn;
 }
 
+static inline bool gic_cpu_ns_access(GICState *s, int cpu, MemTxAttrs attrs)
+{
+    return !gic_is_vcpu(cpu) && s->security_extn && !attrs.secure;
+}
+
 /* TODO: Many places that call this routine could be optimized.  */
 /* Update interrupt status after enabled or pending bits have been changed.  */
 static void gic_update(GICState *s)
 {
     int best_irq;
@@ -219,11 +224,11 @@ static uint16_t gic_get_current_pending_irq(GICState *s, 
int cpu,
     if (pending_irq < GIC_MAXIRQ && gic_has_groups(s)) {
         int group = GIC_DIST_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;
+        bool secure = !gic_cpu_ns_access(s, cpu, attrs);
 
         if (group == 0 && !secure) {
             /* Group0 interrupts hidden from Non-secure access */
             return 1023;
         }
@@ -426,11 +431,11 @@ static uint32_t gic_dist_get_priority(GICState *s, int 
cpu, int irq,
 }
 
 static void gic_set_priority_mask(GICState *s, int cpu, uint8_t pmask,
                                   MemTxAttrs attrs)
 {
-    if (s->security_extn && !attrs.secure) {
+    if (gic_cpu_ns_access(s, cpu, attrs)) {
         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 */
@@ -442,11 +447,11 @@ static void gic_set_priority_mask(GICState *s, int cpu, 
uint8_t 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 (gic_cpu_ns_access(s, cpu, attrs)) {
         if (pmask & 0x80) {
             /* Priority Mask in upper half, return Non-secure view */
             pmask = (pmask << 1) & 0xff;
         } else {
             /* Priority Mask in lower half, RAZ */
@@ -458,11 +463,11 @@ static uint32_t gic_get_priority_mask(GICState *s, int 
cpu, MemTxAttrs attrs)
 
 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) {
+    if (gic_cpu_ns_access(s, cpu, attrs)) {
         /* 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.
          */
@@ -474,11 +479,11 @@ static uint32_t gic_get_cpu_control(GICState *s, int cpu, 
MemTxAttrs attrs)
 static void gic_set_cpu_control(GICState *s, int cpu, uint32_t value,
                                 MemTxAttrs attrs)
 {
     uint32_t mask;
 
-    if (s->security_extn && !attrs.secure) {
+    if (gic_cpu_ns_access(s, cpu, attrs)) {
         /* The NS view can only write certain bits in the register;
          * the rest are unchanged
          */
         mask = GICC_CTLR_EN_GRP1;
         if (s->revision == 2) {
@@ -505,11 +510,11 @@ static uint8_t gic_get_running_priority(GICState *s, int 
cpu, MemTxAttrs attrs)
     if ((s->revision != REV_11MPCORE) && (s->running_priority[cpu] > 0xff)) {
         /* Idle priority */
         return 0xff;
     }
 
-    if (s->security_extn && !attrs.secure) {
+    if (gic_cpu_ns_access(s, cpu, attrs)) {
         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;
@@ -529,11 +534,11 @@ static bool gic_eoi_split(GICState *s, int cpu, 
MemTxAttrs attrs)
 {
     if (s->revision != 2) {
         /* Before GICv2 prio-drop and deactivate are not separable */
         return false;
     }
-    if (s->security_extn && !attrs.secure) {
+    if (gic_cpu_ns_access(s, cpu, attrs)) {
         return s->cpu_ctlr[cpu] & GICC_CTLR_EOIMODE_NS;
     }
     return s->cpu_ctlr[cpu] & GICC_CTLR_EOIMODE;
 }
 
@@ -561,11 +566,11 @@ static void gic_deactivate_irq(GICState *s, int cpu, int 
irq, MemTxAttrs attrs)
         qemu_log_mask(LOG_GUEST_ERROR,
                       "gic_deactivate_irq: GICC_DIR write when EOIMode clear");
         return;
     }
 
-    if (s->security_extn && !attrs.secure && !group) {
+    if (gic_cpu_ns_access(s, cpu, attrs) && !group) {
         DPRINTF("Non-secure DI for Group0 interrupt %d ignored\n", irq);
         return;
     }
 
     GIC_DIST_CLEAR_ACTIVE(irq, cm);
@@ -603,11 +608,11 @@ static void gic_complete_irq(GICState *s, int cpu, int 
irq, MemTxAttrs attrs)
         }
     }
 
     group = gic_has_groups(s) && GIC_DIST_TEST_GROUP(irq, cm);
 
-    if (s->security_extn && !attrs.secure && !group) {
+    if (gic_cpu_ns_access(s, cpu, attrs) && !group) {
         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
@@ -1279,11 +1284,11 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, 
int offset,
         break;
     case 0x04: /* Priority mask */
         *data = gic_get_priority_mask(s, cpu, attrs);
         break;
     case 0x08: /* Binary Point */
-        if (s->security_extn && !attrs.secure) {
+        if (gic_cpu_ns_access(s, cpu, attrs)) {
             if (s->cpu_ctlr[cpu] & GICC_CTLR_CBPR) {
                 /* NS view of BPR when CBPR is 1 */
                 *data = MIN(s->bpr[cpu] + 1, 7);
             } else {
                 /* BPR is banked. Non-secure copy stored in ABPR. */
@@ -1306,11 +1311,11 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, 
int offset,
         /* 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)) {
+        if (!gic_has_groups(s) || (gic_cpu_ns_access(s, cpu, attrs))) {
             *data = 0;
         } else {
             *data = s->abpr[cpu];
         }
         break;
@@ -1318,11 +1323,11 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, 
int offset,
     {
         int regno = (offset - 0xd0) / 4;
 
         if (regno >= GIC_NR_APRS || s->revision != 2) {
             *data = 0;
-        } else if (s->security_extn && !attrs.secure) {
+        } else if (gic_cpu_ns_access(s, cpu, attrs)) {
             /* NS view of GICC_APR<n> is the top half of GIC_NSAPR<n> */
             *data = gic_apr_ns_view(s, regno, cpu);
         } else {
             *data = s->apr[regno][cpu];
         }
@@ -1331,11 +1336,11 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, 
int offset,
     case 0xe0: case 0xe4: case 0xe8: case 0xec:
     {
         int regno = (offset - 0xe0) / 4;
 
         if (regno >= GIC_NR_APRS || s->revision != 2 || !gic_has_groups(s) ||
-            (s->security_extn && !attrs.secure)) {
+            gic_cpu_ns_access(s, cpu, attrs)) {
             *data = 0;
         } else {
             *data = s->nsapr[regno][cpu];
         }
         break;
@@ -1358,11 +1363,11 @@ static MemTxResult gic_cpu_write(GICState *s, int cpu, 
int offset,
         break;
     case 0x04: /* Priority mask */
         gic_set_priority_mask(s, cpu, value, attrs);
         break;
     case 0x08: /* Binary Point */
-        if (s->security_extn && !attrs.secure) {
+        if (gic_cpu_ns_access(s, cpu, attrs)) {
             if (s->cpu_ctlr[cpu] & GICC_CTLR_CBPR) {
                 /* WI when CBPR is 1 */
                 return MEMTX_OK;
             } else {
                 s->abpr[cpu] = MAX(value & 0x7, GIC_MIN_ABPR);
@@ -1373,11 +1378,11 @@ static MemTxResult gic_cpu_write(GICState *s, int cpu, 
int offset,
         break;
     case 0x10: /* End Of Interrupt */
         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)) {
+        if (!gic_has_groups(s) || (gic_cpu_ns_access(s, cpu, attrs))) {
             /* unimplemented, or NS access: RAZ/WI */
             return MEMTX_OK;
         } else {
             s->abpr[cpu] = MAX(value & 0x7, GIC_MIN_ABPR);
         }
@@ -1387,11 +1392,11 @@ static MemTxResult gic_cpu_write(GICState *s, int cpu, 
int offset,
         int regno = (offset - 0xd0) / 4;
 
         if (regno >= GIC_NR_APRS || s->revision != 2) {
             return MEMTX_OK;
         }
-        if (s->security_extn && !attrs.secure) {
+        if (gic_cpu_ns_access(s, cpu, attrs)) {
             /* NS view of GICC_APR<n> is the top half of GIC_NSAPR<n> */
             gic_apr_write_ns_view(s, regno, cpu, value);
         } else {
             s->apr[regno][cpu] = value;
         }
@@ -1402,11 +1407,11 @@ static MemTxResult gic_cpu_write(GICState *s, int cpu, 
int offset,
         int regno = (offset - 0xe0) / 4;
 
         if (regno >= GIC_NR_APRS || s->revision != 2) {
             return MEMTX_OK;
         }
-        if (!gic_has_groups(s) || (s->security_extn && !attrs.secure)) {
+        if (!gic_has_groups(s) || (gic_cpu_ns_access(s, cpu, attrs))) {
             return MEMTX_OK;
         }
         s->nsapr[regno][cpu] = value;
         break;
     }
-- 
2.18.0




reply via email to

[Prev in Thread] Current Thread [Next in Thread]