Hello Nikos Nikoleris,

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

    https://gem5-review.googlesource.com/4288

to review the following change.


Change subject: kvm, arm: Switch to the device EQ when accessing ISA devices
......................................................................

kvm, arm: Switch to the device EQ when accessing ISA devices

ISA devices typically run in the device event queue. Previously, we
assumed that devices would perform their own EQ migrations as
needed. This isn't ideal since it means we have different conventions
for IO devices and ISA devices. Switch to doing migrations in the KVM
CPU instead to make the behavior consistent.

Change-Id: I33b74480fb2126b0786dbdbfdcfa86083384250c
Signed-off-by: Andreas Sandberg <[email protected]>
Reviewed-by: Nikos Nikoleris <[email protected]>
---
M src/arch/arm/kvm/armv8_cpu.cc
M src/dev/arm/generic_timer.cc
2 files changed, 18 insertions(+), 13 deletions(-)



diff --git a/src/arch/arm/kvm/armv8_cpu.cc b/src/arch/arm/kvm/armv8_cpu.cc
index db2b9c0..209f49e 100644
--- a/src/arch/arm/kvm/armv8_cpu.cc
+++ b/src/arch/arm/kvm/armv8_cpu.cc
@@ -260,7 +260,17 @@
     }

     for (const auto &ri : getSysRegMap()) {
-        const uint64_t value(tc->readMiscReg(ri.idx));
+        uint64_t value;
+        if (ri.is_device) {
+            // This system register is backed by a device. This means
+            // we need to lock the device event queue.
+            EventQueue::ScopedMigration migrate(deviceEventQueue());
+
+            value = tc->readMiscReg(ri.idx);
+        } else {
+            value = tc->readMiscReg(ri.idx);
+        }
+
         DPRINTF(KvmContext, "  %s := 0x%x\n", ri.name, value);
         setOneReg(ri.kvm, value);
     }
@@ -323,10 +333,15 @@
     for (const auto &ri : getSysRegMap()) {
         const auto value(getOneRegU64(ri.kvm));
         DPRINTF(KvmContext, "  %s := 0x%x\n", ri.name, value);
-        if (ri.is_device)
+        if (ri.is_device) {
+            // This system register is backed by a device. This means
+            // we need to lock the device event queue.
+            EventQueue::ScopedMigration migrate(deviceEventQueue());
+
             tc->setMiscReg(ri.idx, value);
-        else
+        } else {
             tc->setMiscRegNoEffect(ri.idx, value);
+        }
     }

     PCState pc(getOneRegU64(INT_REG(regs.pc)));
diff --git a/src/dev/arm/generic_timer.cc b/src/dev/arm/generic_timer.cc
index 6332b8f..3508674 100644
--- a/src/dev/arm/generic_timer.cc
+++ b/src/dev/arm/generic_timer.cc
@@ -318,11 +318,6 @@
 void
 GenericTimer::setMiscReg(int reg, unsigned cpu, MiscReg val)
 {
-    // This method might have been called from another context if we
-    // are running in multi-core KVM. Migrate to the SimObject's event
-    // queue to prevent surprising race conditions.
-    EventQueue::ScopedMigration migrate(eventQueue());
-
     CoreTimers &core(getTimers(cpu));

     switch (reg) {
@@ -415,11 +410,6 @@
 MiscReg
 GenericTimer::readMiscReg(int reg, unsigned cpu)
 {
-    // This method might have been called from another context if we
-    // are running in multi-core KVM. Migrate to the SimObject's event
-    // queue to prevent surprising race conditions.
-    EventQueue::ScopedMigration migrate(eventQueue());
-
     CoreTimers &core(getTimers(cpu));

     switch (reg) {

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

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I33b74480fb2126b0786dbdbfdcfa86083384250c
Gerrit-Change-Number: 4288
Gerrit-PatchSet: 1
Gerrit-Owner: Andreas Sandberg <[email protected]>
Gerrit-Reviewer: Nikos Nikoleris <[email protected]>
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to