[gem5-dev] changeset in gem5: config: Fix typo in Float param

2015-02-03 Thread Geoffrey Blake via gem5-dev
changeset cefb03a42760 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=cefb03a42760
description:
config: Fix typo in Float param

The Float param was not settable on the command line
due to a typo in the class definition in
python/m5/params.py.  This corrects the typo and allows
floats to be set on the command line as intended.

diffstat:

 src/python/m5/params.py |  2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diffs (12 lines):

diff -r e17949745150 -r cefb03a42760 src/python/m5/params.py
--- a/src/python/m5/params.py   Fri Jan 30 15:49:34 2015 -0600
+++ b/src/python/m5/params.py   Tue Feb 03 14:25:07 2015 -0500
@@ -638,7 +638,7 @@
 
 class Float(ParamValue, float):
 cxx_type = 'double'
-cmdLineSettable = True
+cmd_line_settable = True
 
 def __init__(self, value):
 if isinstance(value, (int, long, float, NumericParamValue, Float, 
str)):
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev


[gem5-dev] changeset in gem5: dev: refactor pci config space for sysfs scan...

2014-10-16 Thread Geoffrey Blake via gem5-dev
changeset 60b4471a8181 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=60b4471a8181
description:
dev: refactor pci config space for sysfs scanning

Sysfs on ubuntu scrapes the entire PCI config space
when it discovers a device using 4 byte accesses.
This was not supported by our devices, in particular the NIC
that implemented the extended PCI config space.  This change
allows the extended PCI config space to be accessed by
sysfs properly.

diffstat:

 src/dev/pcidev.cc |   84 ++--
 src/dev/pcidev.hh |   11 +
 src/dev/pcireg.h  |  377 +++--
 3 files changed, 245 insertions(+), 227 deletions(-)

diffs (truncated from 585 to 300 lines):

diff -r 7135f938ff28 -r 60b4471a8181 src/dev/pcidev.cc
--- a/src/dev/pcidev.cc Thu Oct 16 05:49:56 2014 -0400
+++ b/src/dev/pcidev.cc Thu Oct 16 05:49:57 2014 -0400
@@ -97,8 +97,15 @@
 PciDevice::PciDevice(const Params *p)
 : DmaDevice(p),
   PMCAP_BASE(p-PMCAPBaseOffset),
+  PMCAP_ID_OFFSET(p-PMCAPBaseOffset+PMCAP_ID),
+  PMCAP_PC_OFFSET(p-PMCAPBaseOffset+PMCAP_PC),
+  PMCAP_PMCS_OFFSET(p-PMCAPBaseOffset+PMCAP_PMCS),
   MSICAP_BASE(p-MSICAPBaseOffset),
   MSIXCAP_BASE(p-MSIXCAPBaseOffset),
+  MSIXCAP_ID_OFFSET(p-MSIXCAPBaseOffset+MSIXCAP_ID),
+  MSIXCAP_MXC_OFFSET(p-MSIXCAPBaseOffset+MSIXCAP_MXC),
+  MSIXCAP_MTAB_OFFSET(p-MSIXCAPBaseOffset+MSIXCAP_MTAB),
+  MSIXCAP_MPBA_OFFSET(p-MSIXCAPBaseOffset+MSIXCAP_MPBA),
   PXCAP_BASE(p-PXCAPBaseOffset),
   platform(p-platform),
   pioDelay(p-pio_latency),
@@ -142,14 +149,14 @@
 // endianess and must be converted to Little Endian when accessed
 // by the guest
 // PMCAP
-pmcap.pid.cid = p-PMCAPCapId;
-pmcap.pid.next = p-PMCAPNextCapability;
+pmcap.pid = (uint16_t)p-PMCAPCapId; // pid.cid
+pmcap.pid |= (uint16_t)p-PMCAPNextCapability  8; //pid.next
 pmcap.pc = p-PMCAPCapabilities;
 pmcap.pmcs = p-PMCAPCtrlStatus;
 
 // MSICAP
-msicap.mid.cid = p-MSICAPCapId;
-msicap.mid.next = p-MSICAPNextCapability;
+msicap.mid = (uint16_t)p-MSICAPCapId; //mid.cid
+msicap.mid |= (uint16_t)p-MSICAPNextCapability  8; //mid.next
 msicap.mc = p-MSICAPMsgCtrl;
 msicap.ma = p-MSICAPMsgAddr;
 msicap.mua = p-MSICAPMsgUpperAddr;
@@ -158,8 +165,8 @@
 msicap.mpend = p-MSICAPPendingBits;
 
 // MSIXCAP
-msixcap.mxid.cid = p-MSIXCAPCapId;
-msixcap.mxid.next = p-MSIXCAPNextCapability;
+msixcap.mxid = (uint16_t)p-MSIXCAPCapId; //mxid.cid
+msixcap.mxid |= (uint16_t)p-MSIXCAPNextCapability  8; //mxid.next
 msixcap.mxc = p-MSIXMsgCtrl;
 msixcap.mtab = p-MSIXTableOffset;
 msixcap.mpba = p-MSIXPbaOffset;
@@ -171,8 +178,9 @@
 // little endian byte-order as according the
 // PCIe specification.  Make sure to take the proper
 // actions when manipulating these tables on the host
+uint16_t msixcap_mxc_ts = msixcap.mxc  0x07ff;
 if (MSIXCAP_BASE != 0x0) {
-int msix_vecs = msixcap.mxc.ts + 1;
+int msix_vecs = msixcap_mxc_ts + 1;
 MSIXTable tmp1 = {{0UL,0UL,0UL,0UL}};
 msix_table.resize(msix_vecs, tmp1);
 
@@ -183,10 +191,20 @@
 }
 msix_pba.resize(pba_size, tmp2);
 }
+MSIX_TABLE_OFFSET = msixcap.mtab  0xfffc;
+MSIX_TABLE_END = MSIX_TABLE_OFFSET +
+ (msixcap_mxc_ts + 1) * sizeof(MSIXTable);
+MSIX_PBA_OFFSET = msixcap.mpba  0xfffc;
+MSIX_PBA_END = MSIX_PBA_OFFSET +
+   ((msixcap_mxc_ts + 1) / MSIXVECS_PER_PBA)
+   * sizeof(MSIXPbaEntry);
+if (((msixcap_mxc_ts + 1) % MSIXVECS_PER_PBA)  0) {
+MSIX_PBA_END += sizeof(MSIXPbaEntry);
+}
 
 // PXCAP
-pxcap.pxid.cid = p-PXCAPCapId;
-pxcap.pxid.next = p-PXCAPNextCapability;
+pxcap.pxid = (uint16_t)p-PXCAPCapId; //pxid.cid
+pxcap.pxid |= (uint16_t)p-PXCAPNextCapability  8; //pxid.next
 pxcap.pxcap = p-PXCAPCapabilities;
 pxcap.pxdcap = p-PXCAPDevCapabilities;
 pxcap.pxdc = p-PXCAPDevCtrl;
@@ -253,8 +271,30 @@
 PciDevice::readConfig(PacketPtr pkt)
 {
 int offset = pkt-getAddr()  PCI_CONFIG_SIZE;
-if (offset = PCI_DEVICE_SPECIFIC)
-panic(Device specific PCI config space not implemented!\n);
+
+/* Return 0 for accesses to unimplemented PCI configspace areas */
+if (offset = PCI_DEVICE_SPECIFIC 
+offset  PCI_CONFIG_SIZE) {
+warn_once(Device specific PCI config space 
+  not implemented for %s!\n, this-name());
+switch (pkt-getSize()) {
+case sizeof(uint8_t):
+pkt-setuint8_t(0);
+break;
+case sizeof(uint16_t):
+pkt-setuint16_t(0);
+break;
+case sizeof(uint32_t):
+pkt-setuint32_t(0);
+break;
+default:
+panic(invalid access size(?) for PCI 

[gem5-dev] changeset in gem5: config: Fix vectorparam command line parsing

2014-09-09 Thread Geoffrey Blake via gem5-dev
changeset c12ec2a0de52 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=c12ec2a0de52
description:
config: Fix vectorparam command line parsing

Parsing vectorparams from the command was slightly broken
in that it wouldn't accept the input that the help message
provided to the user and it didn't do the conversion
on the second code path used to convert the string input
to the actual internal representation.  This patch fixes these bugs.

diffstat:

 src/python/m5/params.py |  7 ++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diffs (24 lines):

diff -r c870b43d2ba6 -r c12ec2a0de52 src/python/m5/params.py
--- a/src/python/m5/params.py   Tue Sep 09 04:36:34 2014 -0400
+++ b/src/python/m5/params.py   Tue Sep 09 04:36:34 2014 -0400
@@ -311,6 +311,10 @@
 if isinstance(value, (list, tuple)):
 # list: coerce each element into new list
 tmp_list = [ ParamDesc.convert(self, v) for v in value ]
+elif isinstance(value, str):
+# If input is a csv string
+tmp_list = [ ParamDesc.convert(self, v) \
+ for v in value.strip('[').strip(']').split(',') ]
 else:
 # singleton: coerce to a single-element list
 tmp_list = [ ParamDesc.convert(self, value) ]
@@ -346,7 +350,8 @@
 tmp_list = [ ParamDesc.convert(self, v) for v in value ]
 elif isinstance(value, str):
 # If input is a csv string
-tmp_list = [ ParamDesc.convert(self, v) for v in value.split(',') ]
+tmp_list = [ ParamDesc.convert(self, v) \
+ for v in value.strip('[').strip(']').split(',') ]
 else:
 # singleton: coerce to a single-element list
 tmp_list = [ ParamDesc.convert(self, value) ]
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev


[gem5-dev] changeset in gem5: cache: Fix handling of LL/SC requests under c...

2014-09-03 Thread Geoffrey Blake via gem5-dev
changeset 7aacec2a247d in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=7aacec2a247d
description:
cache: Fix handling of LL/SC requests under contention

If a set of LL/SC requests contend on the same cache block we
can get into a situation where CPUs will deadlock if they expect
a failed SC to supply them data.  This case happens where 3 or
more cores are contending for a cache block using LL/SC and the system
is configured where 2 cores are connected to a local bus and the
third is connected to a remote bus.  If a core on the local bus
sends an SCUpgrade and the core on the remote bus sends and SCUpgrade
they will race to see who will win the SC access.  In the meantime
if the other core appends a read to one of the SCUpgrades it will expect
to be supplied data by that SCUpgrade transaction.  If it happens that
the SCUpgrade that was picked to supply the data is failed, it will
drop the appended request for data and never respond, leaving the 
requesting
core to deadlock.  This patch makes all SC's behave as normal stores to
prevent this case but still makes sure to check whether it can perform
the update.

diffstat:

 src/mem/cache/cache_impl.hh |  28 ++--
 src/mem/packet.cc   |  15 ---
 2 files changed, 22 insertions(+), 21 deletions(-)

diffs (96 lines):

diff -r f40134eb3f85 -r 7aacec2a247d src/mem/cache/cache_impl.hh
--- a/src/mem/cache/cache_impl.hh   Tue May 27 11:00:56 2014 -0500
+++ b/src/mem/cache/cache_impl.hh   Wed Sep 03 07:42:31 2014 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2013 ARM Limited
+ * Copyright (c) 2010-2014 ARM Limited
  * All rights reserved.
  *
  * The license below extends only to copyright in the software and shall
@@ -166,8 +166,12 @@
 } else if (pkt-isWrite()) {
 if (blk-checkWrite(pkt)) {
 pkt-writeDataToBlock(blk-data, blkSize);
-blk-status |= BlkDirty;
 }
+// Always mark the line as dirty even if we are a failed
+// StoreCond so we supply data to any snoops that have
+// appended themselves to this cache before knowing the store
+// will fail.
+blk-status |= BlkDirty;
 } else if (pkt-isRead()) {
 if (pkt-isLLSC()) {
 blk-trackLoadLocked(pkt);
@@ -658,6 +662,13 @@
 // (read-only) and we need exclusive
 assert(needsExclusive  !blk-isWritable());
 cmd = cpu_pkt-isLLSC() ? MemCmd::SCUpgradeReq : MemCmd::UpgradeReq;
+} else if (cpu_pkt-cmd == MemCmd::SCUpgradeFailReq ||
+   cpu_pkt-cmd == MemCmd::StoreCondFailReq) {
+// Even though this SC will fail, we still need to send out the
+// request and get the data to supply it to other snoopers in the case
+// where the determination the StoreCond fails is delayed due to
+// all caches not being on the same local bus.
+cmd = MemCmd::SCUpgradeFailReq;
 } else {
 // block is invalid
 cmd = needsExclusive ? MemCmd::ReadExReq : MemCmd::ReadReq;
@@ -1724,18 +1735,7 @@
 DPRINTF(CachePort, %s %s for address %x size %d\n, __func__,
 tgt_pkt-cmdString(), tgt_pkt-getAddr(), tgt_pkt-getSize());
 
-if (tgt_pkt-cmd == MemCmd::SCUpgradeFailReq ||
-tgt_pkt-cmd == MemCmd::StoreCondFailReq) {
-// SCUpgradeReq or StoreCondReq saw invalidation while queued
-// in MSHR, so now that we are getting around to processing
-// it, just treat it as if we got a failure response
-pkt = new Packet(tgt_pkt);
-pkt-cmd = MemCmd::UpgradeFailResp;
-pkt-senderState = mshr;
-pkt-busFirstWordDelay = pkt-busLastWordDelay = 0;
-recvTimingResp(pkt);
-return NULL;
-} else if (mshr-isForwardNoResponse()) {
+if (mshr-isForwardNoResponse()) {
 // no response expected, just forward packet as it is
 assert(tags-findBlock(mshr-addr, mshr-isSecure) == NULL);
 pkt = tgt_pkt;
diff -r f40134eb3f85 -r 7aacec2a247d src/mem/packet.cc
--- a/src/mem/packet.cc Tue May 27 11:00:56 2014 -0500
+++ b/src/mem/packet.cc Wed Sep 03 07:42:31 2014 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011-2013 ARM Limited
+ * Copyright (c) 2011-2014 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -115,12 +115,13 @@
 /* UpgradeResp */
 { SET3(NeedsExclusive, IsUpgrade, IsResponse),
 InvalidCmd, UpgradeResp },
-/* SCUpgradeFailReq: generates UpgradeFailResp ASAP */
-{ SET5(IsInvalidate, NeedsExclusive, IsLlsc,
-   IsRequest, NeedsResponse),
+/* SCUpgradeFailReq: generates UpgradeFailResp but still gets the data */
+{ SET6(IsRead, NeedsExclusive, IsInvalidate,
+   IsLlsc, IsRequest, NeedsResponse),
 UpgradeFailResp, SCUpgradeFailReq },
-   

[gem5-dev] changeset in gem5: config: Refactor RealviewEMM to fit into new ...

2014-09-03 Thread Geoffrey Blake via gem5-dev
changeset dfebd39c48a7 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=dfebd39c48a7
description:
config: Refactor RealviewEMM to fit into new config system

This eliminates some default devices and adds in helper functions
to connect the devices defined here to associate with the proper
clock domains.

diffstat:

 configs/common/FSConfig.py |3 +
 src/dev/arm/RealView.py|  169 +++-
 2 files changed, 152 insertions(+), 20 deletions(-)

diffs (283 lines):

diff -r 5f1f92bf76ee -r dfebd39c48a7 configs/common/FSConfig.py
--- a/configs/common/FSConfig.pyWed Sep 03 07:42:59 2014 -0400
+++ b/configs/common/FSConfig.pyWed Sep 03 07:43:01 2014 -0400
@@ -221,6 +221,9 @@
 
 self.cf0 = CowIdeDisk(driveID='master')
 self.cf0.childImage(mdesc.disk())
+
+# Attach any PCI devices this platform supports
+self.realview.attachPciDevices()
 # default to an IDE controller rather than a CF one
 # assuming we've got one; EMM64 is an exception for the moment
 if machine_type != VExpress_EMM64:
diff -r 5f1f92bf76ee -r dfebd39c48a7 src/dev/arm/RealView.py
--- a/src/dev/arm/RealView.py   Wed Sep 03 07:42:59 2014 -0400
+++ b/src/dev/arm/RealView.py   Wed Sep 03 07:43:01 2014 -0400
@@ -1,4 +1,4 @@
-# Copyright (c) 2009-2013 ARM Limited
+# Copyright (c) 2009-2014 ARM Limited
 # All rights reserved.
 #
 # The license below extends only to copyright in the software and shall
@@ -44,7 +44,7 @@
 from m5.proxy import *
 from Device import BasicPioDevice, PioDevice, IsaFake, BadAddr, DmaDevice
 from Pci import PciConfigAll
-from Ethernet import NSGigE, IGbE_e1000, IGbE_igb
+from Ethernet import NSGigE, IGbE_igb, IGbE_e1000
 from Ide import *
 from Platform import Platform
 from Terminal import Terminal
@@ -184,6 +184,18 @@
 mem_start_addr = Param.Addr(0, Start address of main memory)
 max_mem_size = Param.Addr('256MB', Maximum amount of RAM supported by 
platform)
 
+def attachPciDevices(self):
+pass
+
+def enableMSIX(self):
+pass
+
+def onChipIOClkDomain(self, clkdomain):
+pass
+
+def offChipIOClkDomain(self, clkdomain):
+pass
+
 def setupBootLoader(self, mem_bus, cur_sys, loc):
 self.nvmem = SimpleMemory(range = AddrRange('2GB', size = '64MB'),
   conf_table_reported = False)
@@ -250,6 +262,14 @@
   self.flash_fake.pio_addr + \
   self.flash_fake.pio_size - 1)]
 
+# Set the clock domain for IO objects that are considered
+# to be close to the cores.
+def onChipIOClkDomain(self, clkdomain):
+self.gic.clk_domain = clkdomain
+self.l2x0_fake.clk_domain   = clkdomain
+self.a9scu.clkdomain= clkdomain
+self.local_cpu_timer.clk_domain = clkdomain
+
 # Attach I/O devices to specified bus object.  Can't do this
 # earlier, since the bus object itself is typically defined at the
 # System level.
@@ -282,12 +302,40 @@
self.rtc.pio   = bus.master
self.flash_fake.pio= bus.master
 
+# Set the clock domain for IO objects that are considered
+# to be far away from the cores.
+def offChipIOClkDomain(self, clkdomain):
+self.uart.clk_domain  = clkdomain
+self.realview_io.clk_domain   = clkdomain
+self.timer0.clk_domain= clkdomain
+self.timer1.clk_domain= clkdomain
+self.clcd.clk_domain  = clkdomain
+self.kmi0.clk_domain  = clkdomain
+self.kmi1.clk_domain  = clkdomain
+self.cf_ctrl.clk_domain   = clkdomain
+self.dmac_fake.clk_domain = clkdomain
+self.uart1_fake.clk_domain= clkdomain
+self.uart2_fake.clk_domain= clkdomain
+self.uart3_fake.clk_domain= clkdomain
+self.smc_fake.clk_domain  = clkdomain
+self.sp810_fake.clk_domain= clkdomain
+self.watchdog_fake.clk_domain = clkdomain
+self.gpio0_fake.clk_domain= clkdomain
+self.gpio1_fake.clk_domain= clkdomain
+self.gpio2_fake.clk_domain= clkdomain
+self.ssp_fake.clk_domain  = clkdomain
+self.sci_fake.clk_domain  = clkdomain
+self.aaci_fake.clk_domain = clkdomain
+self.mmc_fake.clk_domain  = clkdomain
+self.rtc.clk_domain   = clkdomain
+self.flash_fake.clk_domain= clkdomain
+
 # Reference for memory map and interrupt number
 # RealView Emulation Baseboard User Guide (ARM DUI 0143B)
 # Chapter 4: Programmer's Reference
 class RealViewEB(RealView):
 uart = Pl011(pio_addr=0x10009000, int_num=44)
-realview_io = RealViewCtrl(pio_addr=0x1000)
+realview_io = RealViewCtrl(pio_addr=0x1000, idreg=0x01400500)
 gic = Pl390(dist_addr=0x10041000, cpu_addr=0x1004)
 timer0 

Re: [gem5-dev] Review Request 2246: config: Add hooks to enable new config sys

2014-07-25 Thread Geoffrey Blake via gem5-dev


 On May 4, 2014, 11:56 p.m., Steve Reinhardt wrote:
  src/python/m5/simulate.py, line 117
  http://reviews.gem5.org/r/2246/diff/1/?file=39626#file39626line117
 
  for all these 'if hasattr()' changes (here and below, and in 
  simulate.py): seems like it would be much cleaner to make sure whatever 
  dummy objects are being inserted into the config tree support dummy 
  versions of these calls rather than add all these distributed checks all 
  over the place. Maybe create a special DummySimObject class or something?  
  I don't fully understand the use case here or perhaps I could be more 
  specific.
 
 Steve Reinhardt wrote:
 BTW, is this change independent of the command-line parameter stuff?  If 
 so, then it should be a separate changeset.
 
 Geoffrey Blake wrote:
 Internally we had this discussion and it was deemed a worse maintenance 
 headache having a DummySimobject C++ class over being handled purely in the 
 python code.  It can be a separate changeset as well
 
 Steve Reinhardt wrote:
 You didn't invite me to your discussion ;).  It seems to me like we've 
 established what the SimObject interface is, and we've already added dummy 
 versions of these methods to the base SimObject class so that we don't have 
 to put all these tests in, even though many derived classes don't implement 
 them.  So a DummySimObject class ought to be pretty trivial, and putting in 
 all these tests now seems like a step backwards to me.
 
 As I said initially, I don't really understand the use case for these 
 dummy objects anyway, or how you can call simulate() when you don't have 
 all the objects in place yet, so maybe I'm missing something, but so far I 
 remain unconvinced.
 
 Geoffrey Blake wrote:
 The dummy simobject is being used as a means to provide naming resolution 
 only.  In the rewrite of the config scripts, it was a desired feature to have 
 functions that created whole subsystems that we could call multiple times to 
 instantiate the desired system:
 
 cluster1 = makeCpuAndCaches()
 cluster2 = makeCpuAndCaches()
 
 SimObjects cluster1 and 2 would have identical sets of Simobjects as 
 children, but their final names would be unique (cluster1.cpu0.icache, 
 cluster2.cpu0.icache etc).  In the end Objects cluster1 and cluster2 aren't 
 instantiated (I have forgotten how now), but the simulated system still works 
 correctly because we made sure to connect up all the ports properly.  Making 
 an actual DummySimobject class is not hard, I did that in a previous 
 iteration.
 
 Ali Saidi wrote:
 My reasoning here was that we didn't really want to have a sim object 
 that was instantiated by did nothing, wasn't connected to anything and only 
 existed to provide containers for configuration.
 
 Steve Reinhardt wrote:
 That argument sounds reasonable on its own, but I feel like the 
 consequences of following that path ultimately make it the least 
 objectionable alternative.  (IMO, being less objectionable than sprinkling 
 'hasattr' conditionals all over is a pretty low bar.)
 
 It would be nice to avoid creating the useless C++ object if we could 
 have the python object support the full SimObject interface, but that won't 
 work because the python SimObject actually derives the interface dynamically 
 from the C++ version (see SimObject.__getattr__).  So the C++ object isn't 
 totally useless, in that it is serving as a proxy to export the C++ SimObject 
 API out through the python code.
 
 Also, if the purpose is to serve as a container, then a name like 
 Container or SimObjectContainer seems more appropriate.

Hi Steve,

I've updated this patch internally with the suggested edits and split it into 
two. They should be posted early monday to the reviewboard. I did go forward 
and re-create the container simobject as a C++ object and called it SubSystem.  


- Geoffrey


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2246/#review5082
---


On April 23, 2014, 12:22 p.m., Andreas Hansson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2246/
 ---
 
 (Updated April 23, 2014, 12:22 p.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10198:f749128e94c8
 ---
 config: Add hooks to enable new config sys
 
 This patch adds helper functions to SimObject.py, params.py and
 simulate.py to enable the new configuration system.  Functions like
 enumerateParams() in SimObject lets the config system auto-generate
 command line options for simobjects to be modified on the command
 line.
 
 Params in params.py have __call__() added
 to their definition to allow 

Re: [gem5-dev] Review Request 2246: config: Add hooks to enable new config sys

2014-07-10 Thread Geoffrey Blake via gem5-dev


 On May 4, 2014, 11:56 p.m., Steve Reinhardt wrote:
  src/python/m5/SimObject.py, line 654
  http://reviews.gem5.org/r/2246/diff/1/?file=39624#file39624line654
 
  This seems kind of complicated, but I'm going to wait for a 
  higher-level description of what it's doing before I dig into it.
 
 Geoffrey Blake wrote:
 The intent is that you can build a configuration system that exposes all 
 params to be settable on the command line (instead of the tiny fraction 
 available now) and a system that auto-updates its available settable command 
 line flags whenever a simobject's params are edited.  The other intention of 
 this function is to provide the argument parser with enough information about 
 each parameter to make unique command lines, and also use the ParamValue 
 itself as an input check to catch any errors in specifying a command as early 
 as possible.
 
 The enumerateParams() function takes a simobject instance and traces out 
 all the reachable parameters it contains and constructs information to be 
 presented on the command line.  For example, if you passed an instance of 
 O3_ARM_v7a_3 to enumerateParams() it will generate command lines to be 
 presented such as:
 --testsys-cpucluster-O3_ARM_v7a_3.branchPred.BTBTagSize
 --testsys-cpucluster-O3_ARM_v7a_3.fuPool.FUList0.count
 --testsys-cpucluster-O3_ARM_v7a_3.LQEntries
 
 

 
 Steve Reinhardt wrote:
 OK, I see.  So I take it there's some additional code needed in 
 se.py/fs.py/etc. to make this work?  Would it make sense to include that in 
 this patch, so people can see how to use it?

There is a large chunk code that is a full top to bottom re-write of the 
mainline configuration scripts, so it wouldn't make much sense to include in 
this patch.


 On May 4, 2014, 11:56 p.m., Steve Reinhardt wrote:
  src/python/m5/params.py, line 84
  http://reviews.gem5.org/r/2246/diff/1/?file=39625#file39625line84
 
  this seems clunky... would it be easier to list the classes that 
  *can't* be set on the command line?
 
 Geoffrey Blake wrote:
 I did it this way as a guard against adding new Params that may not be 
 able to be spec'ed on the command line and have the enumerateParams() 
 function skip it as the default behavior.  Flipping it to be a list of what 
 not to trace could cause things to go haywire if a new Param type was added 
 that was missing functionality and the developer forgot to put it in the do 
 not trace this blacklist.
 
 Steve Reinhardt wrote:
 I see. Looking at this again, I think a better solution would be to have 
 a class static variable like isCommandLineSettable, make a default value in 
 SimObject of False, and then override it to True in the subclasses that are 
 settable.

That would be a better option.


 On May 4, 2014, 11:56 p.m., Steve Reinhardt wrote:
  src/python/m5/simulate.py, line 117
  http://reviews.gem5.org/r/2246/diff/1/?file=39626#file39626line117
 
  for all these 'if hasattr()' changes (here and below, and in 
  simulate.py): seems like it would be much cleaner to make sure whatever 
  dummy objects are being inserted into the config tree support dummy 
  versions of these calls rather than add all these distributed checks all 
  over the place. Maybe create a special DummySimObject class or something?  
  I don't fully understand the use case here or perhaps I could be more 
  specific.
 
 Steve Reinhardt wrote:
 BTW, is this change independent of the command-line parameter stuff?  If 
 so, then it should be a separate changeset.
 
 Geoffrey Blake wrote:
 Internally we had this discussion and it was deemed a worse maintenance 
 headache having a DummySimobject C++ class over being handled purely in the 
 python code.  It can be a separate changeset as well
 
 Steve Reinhardt wrote:
 You didn't invite me to your discussion ;).  It seems to me like we've 
 established what the SimObject interface is, and we've already added dummy 
 versions of these methods to the base SimObject class so that we don't have 
 to put all these tests in, even though many derived classes don't implement 
 them.  So a DummySimObject class ought to be pretty trivial, and putting in 
 all these tests now seems like a step backwards to me.
 
 As I said initially, I don't really understand the use case for these 
 dummy objects anyway, or how you can call simulate() when you don't have 
 all the objects in place yet, so maybe I'm missing something, but so far I 
 remain unconvinced.

The dummy simobject is being used as a means to provide naming resolution only. 
 In the rewrite of the config scripts, it was a desired feature to have 
functions that created whole subsystems that we could call multiple times to 
instantiate the desired system:

cluster1 = makeCpuAndCaches()
cluster2 = makeCpuAndCaches()

SimObjects cluster1 and 2 would have identical sets of Simobjects as children, 
but their final names would be unique (cluster1.cpu0.icache, 

[gem5-dev] changeset in gem5: arm: Panics in miscreg read functions can be ...

2014-05-09 Thread Geoffrey Blake via gem5-dev
changeset c09802451018 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=c09802451018
description:
arm: Panics in miscreg read functions can be tripped by O3 model

Unimplemented miscregs for the generic timer were guarded by panics
in arm/isa.cc which can be tripped by the O3 model if it speculatively
executes a wrong path containing a mrs instruction with a bad miscreg
index. These registers were flagged as implemented and accessible.
This patch changes the miscreg info bit vector to flag them as
unimplemented and inaccessible. In this case, and UndefinedInst
fault will be generated if the register access is not trapped
by a hypervisor.

diffstat:

 src/arch/arm/isa.cc |  31 +++
 src/arch/arm/isa/insts/misc.isa |   4 ++--
 src/arch/arm/miscregs.cc|  34 +-
 3 files changed, 34 insertions(+), 35 deletions(-)

diffs (158 lines):

diff -r 7fef26827810 -r c09802451018 src/arch/arm/isa.cc
--- a/src/arch/arm/isa.cc   Fri May 09 18:58:46 2014 -0400
+++ b/src/arch/arm/isa.cc   Fri May 09 18:58:46 2014 -0400
@@ -729,30 +729,30 @@
 return getArchTimer(tc, tc-cpuId())-control();
   // PL1 phys. timer, secure
   //   AArch64
-  case MISCREG_CNTPS_CVAL_EL1:
-  case MISCREG_CNTPS_TVAL_EL1:
-  case MISCREG_CNTPS_CTL_EL1:
+  // case MISCREG_CNTPS_CVAL_EL1:
+  // case MISCREG_CNTPS_TVAL_EL1:
+  // case MISCREG_CNTPS_CTL_EL1:
   // PL2 phys. timer, non-secure
   //   AArch32
-  case MISCREG_CNTHCTL:
-  case MISCREG_CNTHP_CVAL:
-  case MISCREG_CNTHP_TVAL:
-  case MISCREG_CNTHP_CTL:
+  // case MISCREG_CNTHCTL:
+  // case MISCREG_CNTHP_CVAL:
+  // case MISCREG_CNTHP_TVAL:
+  // case MISCREG_CNTHP_CTL:
   //   AArch64
-  case MISCREG_CNTHCTL_EL2:
-  case MISCREG_CNTHP_CVAL_EL2:
-  case MISCREG_CNTHP_TVAL_EL2:
-  case MISCREG_CNTHP_CTL_EL2:
+  // case MISCREG_CNTHCTL_EL2:
+  // case MISCREG_CNTHP_CVAL_EL2:
+  // case MISCREG_CNTHP_TVAL_EL2:
+  // case MISCREG_CNTHP_CTL_EL2:
   // Virtual timer
   //   AArch32
-  case MISCREG_CNTV_CVAL:
-  case MISCREG_CNTV_TVAL:
-  case MISCREG_CNTV_CTL:
+  // case MISCREG_CNTV_CVAL:
+  // case MISCREG_CNTV_TVAL:
+  // case MISCREG_CNTV_CTL:
   //   AArch64
   // case MISCREG_CNTV_CVAL_EL2:
   // case MISCREG_CNTV_TVAL_EL2:
   // case MISCREG_CNTV_CTL_EL2:
-panic(Generic Timer register not implemented\n);
+  default:
 break;
 
 }
@@ -1902,7 +1902,6 @@
   // case MISCREG_CNTV_CVAL_EL2:
   // case MISCREG_CNTV_TVAL_EL2:
   // case MISCREG_CNTV_CTL_EL2:
-panic(Generic Timer register not implemented\n);
 break;
 }
 }
diff -r 7fef26827810 -r c09802451018 src/arch/arm/isa/insts/misc.isa
--- a/src/arch/arm/isa/insts/misc.isa   Fri May 09 18:58:46 2014 -0400
+++ b/src/arch/arm/isa/insts/misc.isa   Fri May 09 18:58:46 2014 -0400
@@ -851,7 +851,7 @@
 // if we're in non secure PL1 mode then we can trap regargless of whether
 // the register is accessable, in other modes we trap if only if the 
register
 // IS accessable.
-if (!canRead  !(hypTrap  !inUserMode(Cpsr)  !inSecureState(Scr, Cpsr))) 
{
+if (!canRead  !(hypTrap  !inUserMode(Cpsr)  !inSecureState(Scr, 
Cpsr))) {
 return new UndefinedInstruction(machInst, false, mnemonic);
 }
 if (hypTrap) {
@@ -906,7 +906,7 @@
 // if we're in non secure PL1 mode then we can trap regargless of whether
 // the register is accessable, in other modes we trap if only if the 
register
 // IS accessable.
-if (!canRead  !(hypTrap  !inUserMode(Cpsr)  !inSecureState(Scr, Cpsr))) 
{
+if (!canRead  !(hypTrap  !inUserMode(Cpsr)  !inSecureState(Scr, 
Cpsr))) {
 return new UndefinedInstruction(machInst, false, mnemonic);
 }
 if (hypTrap) {
diff -r 7fef26827810 -r c09802451018 src/arch/arm/miscregs.cc
--- a/src/arch/arm/miscregs.cc  Fri May 09 18:58:46 2014 -0400
+++ b/src/arch/arm/miscregs.cc  Fri May 09 18:58:46 2014 -0400
@@ -714,15 +714,15 @@
 // MISCREG_CNTP_CTL_S
 bitsetNUM_MISCREG_INFOS(string(00110011001)),
 // MISCREG_CNTV_TVAL
-bitsetNUM_MISCREG_INFOS(string(111)),
+bitsetNUM_MISCREG_INFOS(string(000)),
 // MISCREG_CNTV_CTL
-bitsetNUM_MISCREG_INFOS(string(111)),
+bitsetNUM_MISCREG_INFOS(string(000)),
 // MISCREG_CNTHCTL
-bitsetNUM_MISCREG_INFOS(string(1100111)),
+bitsetNUM_MISCREG_INFOS(string(0100100)),
 // MISCREG_CNTHP_TVAL
-bitsetNUM_MISCREG_INFOS(string(1100111)),
+bitsetNUM_MISCREG_INFOS(string(0100100)),
 // MISCREG_CNTHP_CTL
-bitsetNUM_MISCREG_INFOS(string(1100111)),
+

[gem5-dev] changeset in gem5: config: Avoid generating a reference to mysel...

2014-05-09 Thread Geoffrey Blake via gem5-dev
changeset 7d4d0cd3f7e5 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=7d4d0cd3f7e5
description:
config: Avoid generating a reference to myself for Parent.any

The unproxy code for Parent.any can generate a circular reference
in certain situations with classes hierarchies like those in 
ClockDomain.py.
This patch solves this by marking ouself as visited to make sure the
search does not resolve to a self-reference.

diffstat:

 src/python/m5/SimObject.py |   6 +-
 src/python/m5/proxy.py |  22 ++
 2 files changed, 23 insertions(+), 5 deletions(-)

diffs (61 lines):

diff -r e6d2e8083d9c -r 7d4d0cd3f7e5 src/python/m5/SimObject.py
--- a/src/python/m5/SimObject.pyFri May 09 18:58:47 2014 -0400
+++ b/src/python/m5/SimObject.pyFri May 09 18:58:47 2014 -0400
@@ -861,7 +861,11 @@
 
 found_obj = None
 for child in self._children.itervalues():
-if isinstance(child, ptype):
+visited = False
+if hasattr(child, '_visited'):
+  visited = getattr(child, '_visited')
+
+if isinstance(child, ptype) and not visited:
 if found_obj != None and child != found_obj:
 raise AttributeError, \
   'parent.any matched more than one: %s %s' % \
diff -r e6d2e8083d9c -r 7d4d0cd3f7e5 src/python/m5/proxy.py
--- a/src/python/m5/proxy.pyFri May 09 18:58:47 2014 -0400
+++ b/src/python/m5/proxy.pyFri May 09 18:58:47 2014 -0400
@@ -82,12 +82,19 @@
 result, done = self.find(obj)
 
 if self._search_up:
+# Search up the tree but mark ourself
+# as visited to avoid a self-reference
+self._visited = True
+obj._visited = True
 while not done:
 obj = obj._parent
 if not obj:
 break
 result, done = self.find(obj)
 
+self._visited = False
+base._visited = False
+
 if not done:
 raise AttributeError, \
   Can't resolve proxy '%s' of type '%s' from '%s' % \
@@ -151,10 +158,17 @@
 def find(self, obj):
 try:
 val = getattr(obj, self._attr)
-# for any additional unproxying to be done, pass the
-# current, rather than the original object so that proxy
-# has the right context
-obj = val
+visited = False
+if hasattr(val, '_visited'):
+visited = getattr(val, '_visited')
+
+if not visited:
+# for any additional unproxying to be done, pass the
+# current, rather than the original object so that proxy
+# has the right context
+obj = val
+else:
+return None, False
 except:
 return None, False
 while isproxy(val):
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev


[gem5-dev] changeset in gem5: arch, arm: Preserve TLB bootUncacheability wh...

2014-05-09 Thread Geoffrey Blake via gem5-dev
changeset e6d2e8083d9c in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=e6d2e8083d9c
description:
arch, arm: Preserve TLB bootUncacheability when switching CPUs

The ARM TLBs have a bootUncacheability flag used to make some loads
and stores become uncacheable when booting in FS mode. Later the
flag is cleared to let those loads and stores operate as normal.  When
doing a takeOverFrom(), this flag's state is not preserved and is
momentarily reset until the CPSR is touched. On single core runs this
is a non-issue. On multi-core runs this can lead to crashes on the O3
CPU model from the following series of events:
 1) takeOverFrom executed to switch from Atomic - O3
 2) All bootUncacheability flags are reset to true
 3) Core2 tries to execute a load covered by bootUncacheability, it
is flagged as uncacheable
 4) Core2's load needs to replay due to a pipeline flush
 3) Core1 core does an action on CPSR
 4) The handling code for CPSR then checks all other cores
to determine if bootUncacheability can be set to false
 5) Asynchronously set bootUncacheability on all cores to false
 6) Core2 replays load previously set as uncacheable and notices
it is now flagged as cacheable, leads to a panic.
This patch implements takeOverFrom() functionality for the ARM TLBs
to preserve flag values when switching from atomic - detailed.

diffstat:

 src/arch/alpha/tlb.hh |   2 ++
 src/arch/arm/tlb.cc   |  24 
 src/arch/arm/tlb.hh   |   2 ++
 src/arch/mips/tlb.hh  |   3 +++
 src/arch/power/tlb.hh |   2 ++
 src/arch/sparc/tlb.hh |   2 ++
 src/arch/x86/tlb.hh   |   2 ++
 src/cpu/base.cc   |   5 +
 src/sim/tlb.hh|   5 +
 9 files changed, 47 insertions(+), 0 deletions(-)

diffs (144 lines):

diff -r d717abc806aa -r e6d2e8083d9c src/arch/alpha/tlb.hh
--- a/src/arch/alpha/tlb.hh Fri May 09 18:58:47 2014 -0400
+++ b/src/arch/alpha/tlb.hh Fri May 09 18:58:47 2014 -0400
@@ -87,6 +87,8 @@
 TLB(const Params *p);
 virtual ~TLB();
 
+void takeOverFrom(BaseTLB *otlb) {}
+
 virtual void regStats();
 
 int getsize() const { return size; }
diff -r d717abc806aa -r e6d2e8083d9c src/arch/arm/tlb.cc
--- a/src/arch/arm/tlb.cc   Fri May 09 18:58:47 2014 -0400
+++ b/src/arch/arm/tlb.cc   Fri May 09 18:58:47 2014 -0400
@@ -354,6 +354,30 @@
 }
 
 void
+TLB::takeOverFrom(BaseTLB *_otlb)
+{
+TLB *otlb = dynamic_castTLB*(_otlb);
+/* Make sure we actually have a valid type */
+if (otlb) {
+_attr = otlb-_attr;
+haveLPAE = otlb-haveLPAE;
+directToStage2 = otlb-directToStage2;
+stage2Req = otlb-stage2Req;
+bootUncacheability = otlb-bootUncacheability;
+
+/* Sync the stage2 MMU if they exist in both
+ * the old CPU and the new
+ */
+if (!isStage2 
+stage2Tlb  otlb-stage2Tlb) {
+stage2Tlb-takeOverFrom(otlb-stage2Tlb);
+}
+} else {
+panic(Incompatible TLB type!);
+}
+}
+
+void
 TLB::serialize(ostream os)
 {
 DPRINTF(Checkpoint, Serializing Arm TLB\n);
diff -r d717abc806aa -r e6d2e8083d9c src/arch/arm/tlb.hh
--- a/src/arch/arm/tlb.hh   Fri May 09 18:58:47 2014 -0400
+++ b/src/arch/arm/tlb.hh   Fri May 09 18:58:47 2014 -0400
@@ -155,6 +155,8 @@
 
 virtual ~TLB();
 
+void takeOverFrom(BaseTLB *otlb);
+
 /// setup all the back pointers
 virtual void init();
 
diff -r d717abc806aa -r e6d2e8083d9c src/arch/mips/tlb.hh
--- a/src/arch/mips/tlb.hh  Fri May 09 18:58:47 2014 -0400
+++ b/src/arch/mips/tlb.hh  Fri May 09 18:58:47 2014 -0400
@@ -87,6 +87,9 @@
 int probeEntry(Addr vpn,uint8_t) const;
 MipsISA::PTE *getEntry(unsigned) const;
 virtual ~TLB();
+
+void takeOverFrom(BaseTLB *otlb) {}
+
 int smallPages;
 int getsize() const { return size; }
 
diff -r d717abc806aa -r e6d2e8083d9c src/arch/power/tlb.hh
--- a/src/arch/power/tlb.hh Fri May 09 18:58:47 2014 -0400
+++ b/src/arch/power/tlb.hh Fri May 09 18:58:47 2014 -0400
@@ -130,6 +130,8 @@
 TLB(const Params *p);
 virtual ~TLB();
 
+void takeOverFrom(BaseTLB *otlb) {}
+
 int probeEntry(Addr vpn,uint8_t) const;
 PowerISA::PTE *getEntry(unsigned) const;
 
diff -r d717abc806aa -r e6d2e8083d9c src/arch/sparc/tlb.hh
--- a/src/arch/sparc/tlb.hh Fri May 09 18:58:47 2014 -0400
+++ b/src/arch/sparc/tlb.hh Fri May 09 18:58:47 2014 -0400
@@ -154,6 +154,8 @@
 typedef SparcTLBParams Params;
 TLB(const Params *p);
 
+void takeOverFrom(BaseTLB *otlb) {}
+
 void
 demapPage(Addr vaddr, uint64_t asn)
 {
diff -r d717abc806aa -r e6d2e8083d9c src/arch/x86/tlb.hh
--- a/src/arch/x86/tlb.hh   Fri May 09 18:58:47 2014 -0400
+++ b/src/arch/x86/tlb.hh   Fri May 09 18:58:47 2014 -0400
@@ -75,6 +75,8 @@
 typedef