Hello Andreas Hansson,
I'd like you to do a code review. Please visit
https://gem5-review.googlesource.com/2740
to review the following change.
Change subject: dev, x86: Use the generic InterruptLine interface
......................................................................
dev, x86: Use the generic InterruptLine interface
Refactor x86 interrupt handling to use the generic InterruptLine
interface instead of the x86-specific
X86IntSourcePin/X86IntSinkPin/X86IntLine interfaces.
Change-Id: Id77d99c879d69d605e28326d35b8d90c8ef06d56
Signed-off-by: Andreas Sandberg <[email protected]>
Reviewed-by: Andreas Hansson <[email protected]>
---
M src/dev/x86/Cmos.py
M src/dev/x86/I8042.py
M src/dev/x86/I82094AA.py
M src/dev/x86/I8254.py
M src/dev/x86/I8259.py
M src/dev/x86/SouthBridge.py
M src/dev/x86/X86IntPin.py
M src/dev/x86/cmos.cc
M src/dev/x86/cmos.hh
M src/dev/x86/i8042.cc
M src/dev/x86/i8042.hh
M src/dev/x86/i82094aa.cc
M src/dev/x86/i82094aa.hh
M src/dev/x86/i8254.cc
M src/dev/x86/i8254.hh
M src/dev/x86/i8259.cc
M src/dev/x86/i8259.hh
M src/dev/x86/intdev.cc
M src/dev/x86/intdev.hh
19 files changed, 89 insertions(+), 170 deletions(-)
diff --git a/src/dev/x86/Cmos.py b/src/dev/x86/Cmos.py
index c0b2be5..043b134 100644
--- a/src/dev/x86/Cmos.py
+++ b/src/dev/x86/Cmos.py
@@ -28,8 +28,7 @@
from m5.params import *
from m5.proxy import *
-from Device import BasicPioDevice
-from X86IntPin import X86IntSourcePin
+from Device import BasicPioDevice, InterruptLine
class Cmos(BasicPioDevice):
type = 'Cmos'
@@ -37,5 +36,4 @@
cxx_header = "dev/x86/cmos.hh"
time = Param.Time('01/01/2012',
"System time to use ('Now' for actual time)")
- int_pin = Param.X86IntSourcePin(X86IntSourcePin(),
- 'Pin to signal RTC alarm interrupts to')
+ int_pin = Param.InterruptLine('Interrupt for the RTC alarm signal')
diff --git a/src/dev/x86/I8042.py b/src/dev/x86/I8042.py
index d13a133..6771f23 100644
--- a/src/dev/x86/I8042.py
+++ b/src/dev/x86/I8042.py
@@ -28,8 +28,7 @@
from m5.params import *
from m5.proxy import *
-from Device import BasicPioDevice
-from X86IntPin import X86IntSourcePin
+from Device import BasicPioDevice, InterruptLine
class I8042(BasicPioDevice):
type = 'I8042'
@@ -39,7 +38,6 @@
pio_addr = 0x0
data_port = Param.Addr('Data port address')
command_port = Param.Addr('Command/status port address')
- mouse_int_pin = Param.X86IntSourcePin(X86IntSourcePin(),
- 'Pin to signal the mouse has data')
- keyboard_int_pin = Param.X86IntSourcePin(X86IntSourcePin(),
- 'Pin to signal the keyboard has data')
+ mouse_int_pin = Param.InterruptLine('Pin to signal the mouse has data')
+ keyboard_int_pin = Param.InterruptLine(
+ 'Pin to signal the keyboard has data')
diff --git a/src/dev/x86/I82094AA.py b/src/dev/x86/I82094AA.py
index 7e71cdf..644c091 100644
--- a/src/dev/x86/I82094AA.py
+++ b/src/dev/x86/I82094AA.py
@@ -29,7 +29,7 @@
from m5.params import *
from m5.proxy import *
from Device import BasicPioDevice
-from X86IntPin import X86IntSinkPin
+from X86IntPin import X86IntLine
class I82094AA(BasicPioDevice):
type = 'I82094AA'
@@ -42,4 +42,4 @@
external_int_pic = Param.I8259(NULL, "External PIC, if any")
def pin(self, line):
- return X86IntSinkPin(device=self, number=line)
+ return X86IntLine(device=self, number=line)
diff --git a/src/dev/x86/I8254.py b/src/dev/x86/I8254.py
index 574dd81..03f8f25 100644
--- a/src/dev/x86/I8254.py
+++ b/src/dev/x86/I8254.py
@@ -28,12 +28,10 @@
from m5.params import *
from m5.proxy import *
-from Device import BasicPioDevice
-from X86IntPin import X86IntSourcePin
+from Device import BasicPioDevice, InterruptLine
class I8254(BasicPioDevice):
type = 'I8254'
cxx_class = 'X86ISA::I8254'
cxx_header = "dev/x86/i8254.hh"
- int_pin = Param.X86IntSourcePin(X86IntSourcePin(),
- 'Pin to signal timer interrupts to')
+ int_pin = Param.InterruptLine('Pin to signal timer interrupts to')
diff --git a/src/dev/x86/I8259.py b/src/dev/x86/I8259.py
index 4403c3f..e663cc0 100644
--- a/src/dev/x86/I8259.py
+++ b/src/dev/x86/I8259.py
@@ -29,7 +29,7 @@
from m5.params import *
from m5.proxy import *
from Device import BasicPioDevice
-from X86IntPin import X86IntSourcePin, X86IntSinkPin
+from X86IntPin import X86IntLine
class X86I8259CascadeMode(Enum):
map = {'I8259Master' : 0,
@@ -41,10 +41,8 @@
type = 'I8259'
cxx_class='X86ISA::I8259'
cxx_header = "dev/x86/i8259.hh"
- output = Param.X86IntSourcePin(X86IntSourcePin(),
- 'The pin this I8259 drives')
+ output = Param.X86IntLine('The pin this I8259 drives')
mode = Param.X86I8259CascadeMode('How this I8259 is cascaded')
- slave = Param.I8259(NULL, 'Slave I8259, if any')
def pin(self, line):
- return X86IntSinkPin(device=self, number=line)
+ return X86IntLine(device=self, number=line)
diff --git a/src/dev/x86/SouthBridge.py b/src/dev/x86/SouthBridge.py
index 7046565..d6cace2 100644
--- a/src/dev/x86/SouthBridge.py
+++ b/src/dev/x86/SouthBridge.py
@@ -36,7 +36,6 @@
from I8259 import I8259
from Ide import IdeController
from PcSpeaker import PcSpeaker
-from X86IntPin import X86IntLine
from m5.SimObject import SimObject
def x86IOAddress(port):
@@ -88,20 +87,16 @@
def attachIO(self, bus, dma_ports):
# Route interupt signals
- self.int_lines = \
- [X86IntLine(source=self.pic1.output, sink=self.io_apic.pin(0)),
- X86IntLine(source=self.pic2.output, sink=self.pic1.pin(2)),
- X86IntLine(source=self.cmos.int_pin, sink=self.pic2.pin(0)),
- X86IntLine(source=self.pit.int_pin, sink=self.pic1.pin(0)),
- X86IntLine(source=self.pit.int_pin, sink=self.io_apic.pin(2)),
- X86IntLine(source=self.keyboard.keyboard_int_pin,
- sink=self.io_apic.pin(1)),
- X86IntLine(source=self.keyboard.mouse_int_pin,
- sink=self.io_apic.pin(12))]
+ self.pic1.output = self.io_apic.pin(0)
+ self.pic2.output = self.pic1.pin(2)
+ self.cmos.int_pin = self.pic2.pin(0)
+ self.pit.int_pin = self.pic1.pin(0)
+ self.pit.int_pin = self.io_apic.pin(2)
+ self.keyboard.keyboard_int_pin = self.io_apic.pin(1)
+ self.keyboard.mouse_int_pin = self.io_apic.pin(12)
+
# Tell the devices about each other
- self.pic1.slave = self.pic2
self.speaker.i8254 = self.pit
- self.io_apic.external_int_pic = self.pic1
# Connect to the bus
self.cmos.pio = bus.master
self.dma1.pio = bus.master
diff --git a/src/dev/x86/X86IntPin.py b/src/dev/x86/X86IntPin.py
index 53760b4..b7b18cc 100644
--- a/src/dev/x86/X86IntPin.py
+++ b/src/dev/x86/X86IntPin.py
@@ -27,28 +27,13 @@
# Authors: Gabe Black
from m5.params import *
-from m5.SimObject import SimObject
-
-# A generic pin to drive an interrupt signal generated by a device.
-class X86IntSourcePin(SimObject):
- type = 'X86IntSourcePin'
- cxx_class = 'X86ISA::IntSourcePin'
- cxx_header = "dev/x86/intdev.hh"
-
-# A generic pin to receive an interrupt signal generated by another device.
-class X86IntSinkPin(SimObject):
- type = 'X86IntSinkPin'
- cxx_class = 'X86ISA::IntSinkPin'
- cxx_header = "dev/x86/intdev.hh"
-
- device = Param.SimObject("Device this pin belongs to")
- number = Param.Int("The pin number on the device")
+from Device import InterruptLine
# An interrupt line which is driven by a source pin and drives a sink pin.
-class X86IntLine(SimObject):
+class X86IntLine(InterruptLine):
type = 'X86IntLine'
cxx_class = 'X86ISA::IntLine'
cxx_header = "dev/x86/intdev.hh"
- source = Param.X86IntSourcePin("Pin driving this line")
- sink = Param.X86IntSinkPin("Pin driven by this line")
+ device = Param.SimObject("Device this pin belongs to")
+ number = Param.Int("The pin number on the device")
diff --git a/src/dev/x86/cmos.cc b/src/dev/x86/cmos.cc
index 16286f0..e79db3a 100644
--- a/src/dev/x86/cmos.cc
+++ b/src/dev/x86/cmos.cc
@@ -31,7 +31,6 @@
#include "dev/x86/cmos.hh"
#include "debug/CMOS.hh"
-#include "dev/x86/intdev.hh"
#include "mem/packet_access.hh"
void
@@ -40,7 +39,7 @@
assert(intPin);
intPin->raise();
//XXX This is a hack.
- intPin->lower();
+ intPin->clear();
}
Tick
diff --git a/src/dev/x86/cmos.hh b/src/dev/x86/cmos.hh
index 1a755be..8262578 100644
--- a/src/dev/x86/cmos.hh
+++ b/src/dev/x86/cmos.hh
@@ -38,8 +38,6 @@
namespace X86ISA
{
-class IntSourcePin;
-
class Cmos : public BasicPioDevice
{
protected:
@@ -57,10 +55,10 @@
class X86RTC : public MC146818
{
protected:
- IntSourcePin * intPin;
+ InterruptLine * intPin;
public:
X86RTC(EventManager *em, const std::string &n, const struct tm
time,
- bool bcd, Tick frequency, IntSourcePin * _intPin) :
+ bool bcd, Tick frequency, InterruptLine * _intPin) :
MC146818(em, n, time, bcd, frequency), intPin(_intPin)
{
}
diff --git a/src/dev/x86/i8042.cc b/src/dev/x86/i8042.cc
index 39b0205..9920e37 100644
--- a/src/dev/x86/i8042.cc
+++ b/src/dev/x86/i8042.cc
@@ -83,12 +83,12 @@
DPRINTF(I8042, "Sending keyboard interrupt.\n");
keyboardIntPin->raise();
//This is a hack
- keyboardIntPin->lower();
+ keyboardIntPin->clear();
} else if (mouse && commandByte.mouseFullInt) {
DPRINTF(I8042, "Sending mouse interrupt.\n");
mouseIntPin->raise();
//This is a hack
- mouseIntPin->lower();
+ mouseIntPin->clear();
}
}
diff --git a/src/dev/x86/i8042.hh b/src/dev/x86/i8042.hh
index 82d07e9..35e0548 100644
--- a/src/dev/x86/i8042.hh
+++ b/src/dev/x86/i8042.hh
@@ -33,7 +33,6 @@
#include <deque>
-#include "dev/x86/intdev.hh"
#include "dev/io_device.hh"
#include "params/I8042.hh"
@@ -221,8 +220,8 @@
static const uint16_t NoCommand = (uint16_t)(-1);
uint16_t lastCommand;
- IntSourcePin *mouseIntPin;
- IntSourcePin *keyboardIntPin;
+ InterruptLine *mouseIntPin;
+ InterruptLine *keyboardIntPin;
PS2Mouse mouse;
PS2Keyboard keyboard;
diff --git a/src/dev/x86/i82094aa.cc b/src/dev/x86/i82094aa.cc
index a4b05b0..061916f 100644
--- a/src/dev/x86/i82094aa.cc
+++ b/src/dev/x86/i82094aa.cc
@@ -41,7 +41,7 @@
X86ISA::I82094AA::I82094AA(Params *p)
: BasicPioDevice(p, 20), IntDevice(this, p->int_latency),
- extIntPic(p->external_int_pic), lowestPriorityOffset(0)
+ lowestPriorityOffset(0)
{
// This assumes there's only one I/O APIC in the system and since the
apic
// id is stored in a 8-bit field with 0xff meaning broadcast, the id
must
@@ -198,8 +198,8 @@
TriggerIntMessage message = 0;
message.destination = entry.dest;
if (entry.deliveryMode == DeliveryMode::ExtInt) {
- assert(extIntPic);
- message.vector = extIntPic->getVector();
+ assert(extPic);
+ message.vector = extPic->getVector();
} else {
message.vector = entry.vector;
}
diff --git a/src/dev/x86/i82094aa.hh b/src/dev/x86/i82094aa.hh
index c9e2f1c..1ef557c 100644
--- a/src/dev/x86/i82094aa.hh
+++ b/src/dev/x86/i82094aa.hh
@@ -64,8 +64,6 @@
EndBitUnion(RedirTableEntry)
protected:
- I8259 * extIntPic;
-
uint8_t regSel;
uint8_t initialApicId;
uint8_t id;
diff --git a/src/dev/x86/i8254.cc b/src/dev/x86/i8254.cc
index 457db13..7343075 100644
--- a/src/dev/x86/i8254.cc
+++ b/src/dev/x86/i8254.cc
@@ -42,7 +42,7 @@
if (num == 0) {
intPin->raise();
//XXX This is a hack.
- intPin->lower();
+ intPin->clear();
}
}
diff --git a/src/dev/x86/i8254.hh b/src/dev/x86/i8254.hh
index e1de8a1..5ac92b4 100644
--- a/src/dev/x86/i8254.hh
+++ b/src/dev/x86/i8254.hh
@@ -38,8 +38,6 @@
namespace X86ISA
{
-class IntSourcePin;
-
class I8254 : public BasicPioDevice
{
protected:
@@ -64,7 +62,7 @@
X86Intel8254Timer pit;
- IntSourcePin *intPin;
+ InterruptLine *intPin;
void counterInterrupt(unsigned int num);
diff --git a/src/dev/x86/i8259.cc b/src/dev/x86/i8259.cc
index 03c5cb9..35e4e29 100644
--- a/src/dev/x86/i8259.cc
+++ b/src/dev/x86/i8259.cc
@@ -39,10 +39,14 @@
X86ISA::I8259::I8259(Params * p)
: BasicPioDevice(p, 2), IntDevice(this),
latency(p->pio_latency), output(p->output),
- mode(p->mode), slave(p->slave),
+ mode(p->mode),
IRR(0), ISR(0), IMR(0),
readIRR(true), initControlWord(0), autoEOI(false)
{
+ IntDevice *master = output ? output->device : nullptr;
+ if (master)
+ master->setExtPic(this);
+
for (int i = 0; i < NumLines; i++)
pinStates[i] = false;
}
@@ -235,7 +239,7 @@
DPRINTF(I8259, "Propogating interrupt.\n");
output->raise();
//XXX This is a hack.
- output->lower();
+ output->clear();
} else {
warn("Received interrupt but didn't have "
"anyone to tell about it.\n");
@@ -297,10 +301,10 @@
} else {
ISR |= 1 << line;
}
- if (slave && bits(cascadeBits, line)) {
+ if (extPic && bits(cascadeBits, line)) {
DPRINTF(I8259, "Interrupt was from slave who will "
"provide the vector.\n");
- return slave->getVector();
+ return extPic->getVector();
}
return line | vectorOffset;
}
diff --git a/src/dev/x86/i8259.hh b/src/dev/x86/i8259.hh
index c443f78..4721ed8 100644
--- a/src/dev/x86/i8259.hh
+++ b/src/dev/x86/i8259.hh
@@ -46,9 +46,8 @@
bool pinStates[NumLines];
Tick latency;
- IntSourcePin *output;
+ IntLine *output;
Enums::X86I8259CascadeMode mode;
- I8259 * slave;
// Interrupt Request Register
uint8_t IRR;
diff --git a/src/dev/x86/intdev.cc b/src/dev/x86/intdev.cc
index a35f76b..d6f0e96 100644
--- a/src/dev/x86/intdev.cc
+++ b/src/dev/x86/intdev.cc
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2012 ARM Limited
+ * Copyright (c) 2012, 2017 ARM Limited
* All rights reserved
*
* The license below extends only to copyright in the software and shall
@@ -42,6 +42,8 @@
#include "dev/x86/intdev.hh"
+#include "params/X86IntLine.hh"
+
void
X86ISA::IntDevice::IntMasterPort::sendMessage(ApicList apics,
TriggerIntMessage message,
@@ -71,18 +73,30 @@
}
}
-X86ISA::IntSourcePin *
-X86IntSourcePinParams::create()
+
+X86ISA::IntLine::IntLine(const X86IntLineParams *p)
+ : InterruptLine(p),
+ device(dynamic_cast<X86ISA::IntDevice *>(p->device)),
number(p->number)
{
- return new X86ISA::IntSourcePin(this);
+ fatal_if(!p->device, "No interrupt controller specified");
+ fatal_if(!device, "Interrupt controller doesn't implement the " \
+ " X86ISA::Intdevice interface");
}
-X86ISA::IntSinkPin *
-X86IntSinkPinParams::create()
+void
+X86ISA::IntLine::raise()
{
- return new X86ISA::IntSinkPin(this);
+ device->raiseInterruptPin(number);
}
+void
+X86ISA::IntLine::clear()
+{
+ device->lowerInterruptPin(number);
+}
+
+
+
X86ISA::IntLine *
X86IntLineParams::create()
{
diff --git a/src/dev/x86/intdev.hh b/src/dev/x86/intdev.hh
index 1a49bb3..260912c 100644
--- a/src/dev/x86/intdev.hh
+++ b/src/dev/x86/intdev.hh
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2012 ARM Limited
+ * Copyright (c) 2012, 2017 ARM Limited
* All rights reserved
*
* The license below extends only to copyright in the software and shall
@@ -46,18 +46,19 @@
#include <cassert>
#include <list>
#include <string>
-
#include "arch/x86/intmessage.hh"
#include "arch/x86/x86_traits.hh"
+#include "dev/io_device.hh"
#include "mem/mem_object.hh"
#include "mem/mport.hh"
-#include "params/X86IntLine.hh"
-#include "params/X86IntSinkPin.hh"
-#include "params/X86IntSourcePin.hh"
#include "sim/sim_object.hh"
+struct X86IntLineParams;
+
namespace X86ISA {
+class I8259;
+
typedef std::list<int> ApicList;
class IntDevice
@@ -110,10 +111,12 @@
};
IntMasterPort intMasterPort;
+ I8259 *extPic;
public:
IntDevice(MemObject * parent, Tick latency = 0) :
- intMasterPort(parent->name() + ".int_master", parent, this,
latency)
+ intMasterPort(parent->name() + ".int_master", parent, this,
latency),
+ extPic(nullptr)
{
}
@@ -122,6 +125,13 @@
virtual void init();
+ void
+ setExtPic(struct I8259 *pic)
+ {
+ fatal_if(extPic, "IntDevice with multiple slaves is unsupported");
+ extPic = pic;
+ }
+
virtual void
signalInterrupt(int line)
{
@@ -161,88 +171,16 @@
}
};
-class IntSinkPin : public SimObject
+class IntLine : public ::InterruptLine
{
public:
- IntDevice * device;
- int number;
+ IntLine(const X86IntLineParams *p);
- typedef X86IntSinkPinParams Params;
+ void raise() override;
+ void clear() override;
- const Params *
- params() const
- {
- return dynamic_cast<const Params *>(_params);
- }
-
- IntSinkPin(Params *p) : SimObject(p),
- device(dynamic_cast<IntDevice *>(p->device)), number(p->number)
- {
- assert(device);
- }
-};
-
-class IntSourcePin : public SimObject
-{
- protected:
- std::vector<IntSinkPin *> sinks;
-
- public:
- typedef X86IntSourcePinParams Params;
-
- const Params *
- params() const
- {
- return dynamic_cast<const Params *>(_params);
- }
-
- void
- addSink(IntSinkPin *sink)
- {
- sinks.push_back(sink);
- }
-
- void
- raise()
- {
- for (int i = 0; i < sinks.size(); i++) {
- const IntSinkPin &pin = *sinks[i];
- pin.device->raiseInterruptPin(pin.number);
- }
- }
-
- void
- lower()
- {
- for (int i = 0; i < sinks.size(); i++) {
- const IntSinkPin &pin = *sinks[i];
- pin.device->lowerInterruptPin(pin.number);
- }
- }
-
- IntSourcePin(Params *p) : SimObject(p)
- {}
-};
-
-class IntLine : public SimObject
-{
- protected:
- IntSourcePin *source;
- IntSinkPin *sink;
-
- public:
- typedef X86IntLineParams Params;
-
- const Params *
- params() const
- {
- return dynamic_cast<const Params *>(_params);
- }
-
- IntLine(Params *p) : SimObject(p), source(p->source), sink(p->sink)
- {
- source->addSink(sink);
- }
+ IntDevice *const device;
+ const int number;
};
} // namespace X86ISA
--
To view, visit https://gem5-review.googlesource.com/2740
To unsubscribe, visit https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id77d99c879d69d605e28326d35b8d90c8ef06d56
Gerrit-Change-Number: 2740
Gerrit-PatchSet: 1
Gerrit-Owner: Andreas Sandberg <[email protected]>
Gerrit-Reviewer: Andreas Hansson <[email protected]>
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
