From 245422102c049cc744d695103ead1caa9d9870ca Mon Sep 17 00:00:00 2001
From: Gabe Black <gabeblack@google.com>
Date: Wed, 11 Sep 2019 16:11:11 -0700
Subject: [PATCH] x86: Use a std::function to handle MSI completion.

This removes the recvResponse callback from the IntMasterPort, and
makes it easier to handle the default case where we just need to clean
up the Packet.

Change-Id: I8bcbfee0aaf68b12310d773f925c399fc87ea65d
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/20828
Maintainer: Gabe Black <gabeblack@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Brandon Potter <Brandon.Potter@amd.com>
---
 src/arch/x86/interrupts.cc | 11 +++++------
 src/arch/x86/interrupts.hh |  2 +-
 src/dev/x86/i82094aa.cc    |  8 --------
 src/dev/x86/intdev.hh      | 20 +++++++++++++++-----
 4 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/src/arch/x86/interrupts.cc b/src/arch/x86/interrupts.cc
index 1b21835c5..14bdc0fc7 100644
--- a/src/arch/x86/interrupts.cc
+++ b/src/arch/x86/interrupts.cc
@@ -331,11 +331,9 @@ X86ISA::Interrupts::recvMessage(PacketPtr pkt)
 }
 
 
-bool
-X86ISA::Interrupts::recvResponse(PacketPtr pkt)
+void
+X86ISA::Interrupts::completeIPI(PacketPtr pkt)
 {
-    assert(!pkt->isError());
-    assert(pkt->cmd == MemCmd::WriteResp);
     if (--pendingIPIs == 0) {
         InterruptCommandRegLow low = regs[APIC_INTERRUPT_COMMAND_LOW];
         // Record that the ICR is now idle.
@@ -343,7 +341,7 @@ X86ISA::Interrupts::recvResponse(PacketPtr pkt)
         regs[APIC_INTERRUPT_COMMAND_LOW] = low;
     }
     DPRINTF(LocalApic, "ICR is now idle.\n");
-    return true;
+    delete pkt;
 }
 
 
@@ -548,7 +546,8 @@ X86ISA::Interrupts::setReg(ApicRegIndex reg, uint32_t val)
             regs[APIC_INTERRUPT_COMMAND_LOW] = low;
             for (auto id: apics) {
                 PacketPtr pkt = buildIntTriggerPacket(id, message);
-                intMasterPort.sendMessage(pkt, sys->isTimingMode());
+                intMasterPort.sendMessage(pkt, sys->isTimingMode(),
+                        [this](PacketPtr pkt) { completeIPI(pkt); });
             }
             newVal = regs[APIC_INTERRUPT_COMMAND_LOW];
         }
diff --git a/src/arch/x86/interrupts.hh b/src/arch/x86/interrupts.hh
index a6364c8b1..3d6ef1f82 100644
--- a/src/arch/x86/interrupts.hh
+++ b/src/arch/x86/interrupts.hh
@@ -206,7 +206,7 @@ class Interrupts : public PioDevice
     Tick read(PacketPtr pkt) override;
     Tick write(PacketPtr pkt) override;
     Tick recvMessage(PacketPtr pkt);
-    bool recvResponse(PacketPtr pkt);
+    void completeIPI(PacketPtr pkt);
 
     bool
     triggerTimerInterrupt()
diff --git a/src/dev/x86/i82094aa.cc b/src/dev/x86/i82094aa.cc
index 17ac65f96..d6742a780 100644
--- a/src/dev/x86/i82094aa.cc
+++ b/src/dev/x86/i82094aa.cc
@@ -89,14 +89,6 @@ X86ISA::I82094AA::getPort(const std::string &if_name, PortID idx)
         return BasicPioDevice::getPort(if_name, idx);
 }
 
-bool
-X86ISA::I82094AA::recvResponse(PacketPtr pkt)
-{
-    // Packet instantiated calling sendMessage() in signalInterrupt()
-    delete pkt;
-    return true;
-}
-
 Tick
 X86ISA::I82094AA::read(PacketPtr pkt)
 {
diff --git a/src/dev/x86/intdev.hh b/src/dev/x86/intdev.hh
index 48d32d771..7ba3e68e9 100644
--- a/src/dev/x86/intdev.hh
+++ b/src/dev/x86/intdev.hh
@@ -44,6 +44,7 @@
 #define __DEV_X86_INTDEV_HH__
 
 #include <cassert>
+#include <functional>
 #include <string>
 
 #include "mem/tport.hh"
@@ -96,12 +97,18 @@ buildIntPacket(Addr addr, T payload)
 template <class Device>
 class IntMasterPort : public QueuedMasterPort
 {
+  private:
     ReqPacketQueue reqQueue;
     SnoopRespPacketQueue snoopRespQueue;
 
     Device* device;
     Tick latency;
 
+    typedef std::function<void(PacketPtr)> OnCompletionFunc;
+    OnCompletionFunc onCompletion = nullptr;
+    // If nothing extra needs to happen, just clean up the packet.
+    static void defaultOnCompletion(PacketPtr pkt) { delete pkt; }
+
   public:
     IntMasterPort(const std::string& _name, SimObject* _parent,
                   Device* dev, Tick _latency) :
@@ -114,21 +121,24 @@ class IntMasterPort : public QueuedMasterPort
     bool
     recvTimingResp(PacketPtr pkt) override
     {
-        return device->recvResponse(pkt);
+        assert(pkt->isResponse());
+        onCompletion(pkt);
+        onCompletion = nullptr;
+        return true;
     }
 
     void
-    sendMessage(PacketPtr pkt, bool timing)
+    sendMessage(PacketPtr pkt, bool timing,
+            OnCompletionFunc func=defaultOnCompletion)
     {
         if (timing) {
+            onCompletion = func;
             schedTimingReq(pkt, curTick() + latency);
             // The target handles cleaning up the packet in timing mode.
         } else {
             // ignore the latency involved in the atomic transaction
             sendAtomic(pkt);
-            assert(pkt->isResponse());
-            // also ignore the latency in handling the response
-            device->recvResponse(pkt);
+            func(pkt);
         }
     }
 };
-- 
2.30.2