Hello Andreas Sandberg,

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

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

to review the following change.


Change subject: kvm, arm: Don't forward IRQ/FIQ when using the kernel's GIC
......................................................................

kvm, arm: Don't forward IRQ/FIQ when using the kernel's GIC

The BaseArmKvmCPU is responsible for forwarding the IRQ and FIQ
signals from gem5's simulated GIC to KVM. However, these signals
shouldn't be used when the in-kernel GIC emulator is used.

Instead of delivering the interrupts to the guest, we should just
ignore them since any such pending interrupts are likely to be an
artifact of CPU switching or incorrect draining.

Change-Id: I083b72639384272157f92f44a6606bdf0be7413c
Signed-off-by: Andreas Sandberg <[email protected]>
Reviewed-by: Sudhanshu Jha <[email protected]>
Reviewed-by: Curtis Dunham <[email protected]>
---
M src/arch/arm/kvm/base_cpu.cc
M src/arch/arm/kvm/gic.cc
M src/cpu/kvm/vm.hh
3 files changed, 36 insertions(+), 12 deletions(-)



diff --git a/src/arch/arm/kvm/base_cpu.cc b/src/arch/arm/kvm/base_cpu.cc
index e25112c..7659650 100644
--- a/src/arch/arm/kvm/base_cpu.cc
+++ b/src/arch/arm/kvm/base_cpu.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2015 ARM Limited
+ * Copyright (c) 2012, 2015, 2017 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -88,19 +88,30 @@
 Tick
 BaseArmKvmCPU::kvmRun(Tick ticks)
 {
-    bool simFIQ(interrupts[0]->checkRaw(INT_FIQ));
-    bool simIRQ(interrupts[0]->checkRaw(INT_IRQ));
+    const bool simFIQ(interrupts[0]->checkRaw(INT_FIQ));
+    const bool simIRQ(interrupts[0]->checkRaw(INT_IRQ));

-    if (fiqAsserted != simFIQ) {
-        fiqAsserted = simFIQ;
-        DPRINTF(KvmInt, "KVM: Update FIQ state: %i\n", simFIQ);
-        vm.setIRQLine(INTERRUPT_VCPU_FIQ(vcpuID), simFIQ);
+    if (!vm.hasKernelIRQChip()) {
+        if (fiqAsserted != simFIQ) {
+            DPRINTF(KvmInt, "KVM: Update FIQ state: %i\n", simFIQ);
+            vm.setIRQLine(INTERRUPT_VCPU_FIQ(vcpuID), simFIQ);
+        }
+        if (irqAsserted != simIRQ) {
+            DPRINTF(KvmInt, "KVM: Update IRQ state: %i\n", simIRQ);
+            vm.setIRQLine(INTERRUPT_VCPU_IRQ(vcpuID), simIRQ);
+        }
+    } else {
+        warn_if(simFIQ && !fiqAsserted,
+                "FIQ raised by the simulated interrupt controller " \
+ "despite in-kernel GIC emulation. This is probably a bug.");
+
+        warn_if(simIRQ && !irqAsserted,
+                "IRQ raised by the simulated interrupt controller " \
+ "despite in-kernel GIC emulation. This is probably a bug.");
     }
-    if (irqAsserted != simIRQ) {
-        irqAsserted = simIRQ;
-        DPRINTF(KvmInt, "KVM: Update IRQ state: %i\n", simIRQ);
-        vm.setIRQLine(INTERRUPT_VCPU_IRQ(vcpuID), simIRQ);
-    }
+
+    irqAsserted = simIRQ;
+    fiqAsserted = simFIQ;

     return BaseKvmCPU::kvmRun(ticks);
 }
diff --git a/src/arch/arm/kvm/gic.cc b/src/arch/arm/kvm/gic.cc
index 7cf4d07..463fbaa 100644
--- a/src/arch/arm/kvm/gic.cc
+++ b/src/arch/arm/kvm/gic.cc
@@ -54,6 +54,10 @@
       vm(_vm),
       kdev(vm.createDevice(KVM_DEV_TYPE_ARM_VGIC_V2))
 {
+    // Tell the VM that we will emulate the GIC in the kernel. This
+    // disables IRQ and FIQ handling in the KVM CPU model.
+    vm.enableKernelIRQChip();
+
     kdev.setAttr<uint64_t>(
         KVM_DEV_ARM_VGIC_GRP_ADDR, KVM_VGIC_V2_ADDR_TYPE_DIST, dist_addr);
     kdev.setAttr<uint64_t>(
diff --git a/src/cpu/kvm/vm.hh b/src/cpu/kvm/vm.hh
index df2e411..e122bbf 100644
--- a/src/cpu/kvm/vm.hh
+++ b/src/cpu/kvm/vm.hh
@@ -351,6 +351,15 @@
      * Is in-kernel IRQ chip emulation enabled?
      */
     bool hasKernelIRQChip() const { return _hasKernelIRQChip; }
+
+    /**
+     * Tell the VM and VCPUs to use an in-kernel IRQ chip for
+     * interrupt delivery.
+     *
+     * @note This is set automatically if the IRQ chip is created
+     * using the KvmVM::createIRQChip() API.
+     */
+    void enableKernelIRQChip() { _hasKernelIRQChip = true; }
     /** @} */

     struct MemSlot

--
To view, visit https://gem5-review.googlesource.com/3660
To unsubscribe, visit https://gem5-review.googlesource.com/settings

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

Reply via email to