[gem5-dev] changeset in gem5: config: Fix typo in Float param
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...
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
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...
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 ...
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
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
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 ...
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...
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...
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