Hello Andreas Sandberg,

I'd like you to do a code review. Please visit

    https://gem5-review.googlesource.com/c/public/gem5/+/18593

to review the following change.


Change subject: dev-arm: Disable LPI Configuration Table caching
......................................................................

dev-arm: Disable LPI Configuration Table caching

This is done since caching is not done correctly, and we don't care for
now about performance degradations since the redistributor is using
PhysProxy ports.
Caching will make sense once the magical accesses will be replaced by
real atomic/timing transactions.

Change-Id: Iafe2a7843210111efc82c265bd0d5ec3cd9abb5a
Signed-off-by: Giacomo Travaglini <giacomo.travagl...@arm.com>
Reviewed-by: Andreas Sandberg <andreas.sandb...@arm.com>
---
M src/dev/arm/gic_v3_redistributor.cc
M src/dev/arm/gic_v3_redistributor.hh
2 files changed, 20 insertions(+), 38 deletions(-)



diff --git a/src/dev/arm/gic_v3_redistributor.cc b/src/dev/arm/gic_v3_redistributor.cc
index 2b73c57..be28d3a 100644
--- a/src/dev/arm/gic_v3_redistributor.cc
+++ b/src/dev/arm/gic_v3_redistributor.cc
@@ -675,9 +675,6 @@
               lpiIDBits = 0xf;
           }

-          uint32_t largest_lpi_id = 1 << (lpiIDBits + 1);
-          uint32_t number_lpis = largest_lpi_id - SMALLEST_LPI_ID + 1;
-          lpiConfigurationTable.resize(number_lpis);
           break;
       }

@@ -698,25 +695,12 @@
         break;

       case GICR_INVLPIR: { // Redistributor Invalidate LPI Register
-          uint32_t lpi_id = data & 0xffffffff;
-          uint32_t largest_lpi_id = 1 << (lpiIDBits + 1);
-
-          if (lpi_id > largest_lpi_id) {
-              return;
-          }
-
-          uint32_t lpi_table_entry_index = lpi_id - SMALLEST_LPI_ID;
-          invalLpiConfig(lpi_table_entry_index);
+          // Do nothing: no caching supported
           break;
       }

       case GICR_INVALLR: { // Redistributor Invalidate All Register
-          for (int lpi_table_entry_index = 0;
-               lpi_table_entry_index < lpiConfigurationTable.size();
-               lpi_table_entry_index++) {
-              invalLpiConfig(lpi_table_entry_index);
-          }
-
+          // Do nothing: no caching supported
           break;
       }

@@ -727,17 +711,6 @@
 }

 void
-Gicv3Redistributor::invalLpiConfig(uint32_t lpi_entry_index)
-{
-    Addr lpi_table_entry_ptr = lpiConfigurationTablePtr +
-        lpi_entry_index * sizeof(LPIConfigurationTableEntry);
-    ThreadContext * tc = gic->getSystem()->getThreadContext(cpuId);
-    tc->getPhysProxy().readBlob(lpi_table_entry_ptr,
-            (uint8_t*) &lpiConfigurationTable[lpi_entry_index],
-            sizeof(LPIConfigurationTableEntry));
-}
-
-void
 Gicv3Redistributor::sendPPInt(uint32_t int_id)
 {
     assert((int_id >= Gicv3::SGI_MAX) &&
@@ -831,13 +804,22 @@

     // Check LPIs
     if (EnableLPIs) {
-        const uint32_t largest_lpi_id = 1 << (lpiIDBits + 1);
-        char lpi_pending_table[largest_lpi_id / 8];
         ThreadContext * tc = gic->getSystem()->getThreadContext(cpuId);
+
+        const uint32_t largest_lpi_id = 1 << (lpiIDBits + 1);
+        const uint32_t number_lpis = largest_lpi_id - SMALLEST_LPI_ID + 1;
+
+        uint8_t lpi_pending_table[largest_lpi_id / 8];
+        uint8_t lpi_config_table[number_lpis];
+
         tc->getPhysProxy().readBlob(lpiPendingTablePtr,
                                     (uint8_t *) lpi_pending_table,
                                     sizeof(lpi_pending_table));

+        tc->getPhysProxy().readBlob(lpiConfigurationTablePtr,
+                (uint8_t*) lpi_config_table,
+                sizeof(lpi_config_table));
+
         for (int lpi_id = SMALLEST_LPI_ID; lpi_id < largest_lpi_id;
              lpi_id++) {
             uint32_t lpi_pending_entry_byte = lpi_id / 8;
@@ -845,16 +827,19 @@
bool lpi_is_pending = lpi_pending_table[lpi_pending_entry_byte] &
                                   1 << lpi_pending_entry_bit_position;
uint32_t lpi_configuration_entry_index = lpi_id - SMALLEST_LPI_ID;
-            bool lpi_is_enable =
- lpiConfigurationTable[lpi_configuration_entry_index].enable;
+
+            LPIConfigurationTableEntry config_entry =
+                lpi_config_table[lpi_configuration_entry_index];
+
+            bool lpi_is_enable = config_entry.enable;
+
             // LPIs are always Non-secure Group 1 interrupts,
             // in a system where two Security states are enabled.
             Gicv3::GroupId lpi_group = Gicv3::G1NS;
             bool group_enabled = distributor->groupEnabled(lpi_group);

             if (lpi_is_pending && lpi_is_enable && group_enabled) {
-                uint8_t lpi_priority =
- lpiConfigurationTable[lpi_configuration_entry_index].priority;
+                uint8_t lpi_priority =config_entry.priority;

                 if ((lpi_priority < cpuInterface->hppi.prio) ||
                     (lpi_priority == cpuInterface->hppi.prio &&
diff --git a/src/dev/arm/gic_v3_redistributor.hh b/src/dev/arm/gic_v3_redistributor.hh
index 36504be..0e99b74 100644
--- a/src/dev/arm/gic_v3_redistributor.hh
+++ b/src/dev/arm/gic_v3_redistributor.hh
@@ -161,8 +161,6 @@
         Bitfield<0> enable;
     EndBitUnion(LPIConfigurationTableEntry)

-    std::vector<LPIConfigurationTableEntry> lpiConfigurationTable;
-
     static const uint32_t GICR_CTLR_ENABLE_LPIS = 1 << 0;
     static const uint32_t GICR_CTLR_DPG0   = 1 << 24;
     static const uint32_t GICR_CTLR_DPG1NS = 1 << 25;
@@ -205,7 +203,6 @@
   public:

     Gicv3Redistributor(Gicv3 * gic, uint32_t cpu_id);
-    void invalLpiConfig(uint32_t lpi_entry_index);
     uint32_t getAffinity() const;
     void init();
     void initState();

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/18593
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-Change-Id: Iafe2a7843210111efc82c265bd0d5ec3cd9abb5a
Gerrit-Change-Number: 18593
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini <giacomo.travagl...@arm.com>
Gerrit-Reviewer: Andreas Sandberg <andreas.sandb...@arm.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to