[m5-dev] changeset in m5: IDE, X86: Fix IDE controller BAR configuration f...

2010-11-21 Thread Gabe Black 
changeset f2ac8da7896e in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=f2ac8da7896e
description:
IDE,X86: Fix IDE controller BAR configuration for x86.

diffstat:

 src/dev/ide_ctrl.cc |  4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diffs (17 lines):

diff -r e10aff32c561 -r f2ac8da7896e src/dev/ide_ctrl.cc
--- a/src/dev/ide_ctrl.cc   Sat Nov 20 12:12:27 2010 -0800
+++ b/src/dev/ide_ctrl.cc   Mon Nov 22 02:33:47 2010 -0500
@@ -107,11 +107,11 @@
 primary.select(false);
 secondary.select(false);
 
-if ((BARAddrs[0] & ~BAR_IO_MASK) != 0){
+if ((BARAddrs[0] & ~BAR_IO_MASK) && !legacyIO[0]){
 primary.cmdAddr = BARAddrs[0];  primary.cmdSize = BARSize[0];
 primary.ctrlAddr = BARAddrs[1]; primary.ctrlSize = BARAddrs[1];
 }
-if ((BARAddrs[2] & ~BAR_IO_MASK) != 0){
+if ((BARAddrs[2] & ~BAR_IO_MASK) && !legacyIO[2]){
 secondary.cmdAddr = BARAddrs[2];  secondary.cmdSize = BARSize[2];
 secondary.ctrlAddr = BARAddrs[3]; secondary.ctrlSize = BARAddrs[3];
 }
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: ARM: Add support for a dumb IDE controller

2010-11-21 Thread Gabe Black 
Yeah, I figured it out, and a fix will be coming soon. If we're keeping
score I'm sure I've accidentally broken more things than you, so no
sweat. There will be a regression soon so this will be less likely to
happen again.

Gabe

Ali Saidi wrote:
> It's not an isa thing, it's hard-coded in the struct that is passed to the 
> ide driver. Either way, it-s not those values that is breaking it.  They do 
> nothing when they're 0.
>
> Ali
>
>
>
>
> On Nov 21, 2010, at 11:58 PM, Gabe Black wrote:
>
>   
>> As feared, this change does break x86 boot. I'll see if I can fix it.
>> Ironically I found this out while adding an X86 FS regression. We really
>> do need to figure out what's going on with those values and how that
>> maps to other ISAs.
>>
>> Gabe
>>
>> Gabe Black wrote:
>> 
 On 2010-11-08 17:56:14, Nathan Binkert wrote:

 
> src/dev/ide_ctrl.cc, line 454
> 
>
>Is this something that we should deal with on a per device basis, or 
> is this a more generic thing?  Also, is this something that should be 
> configured by the user, or is this something that's either fixed or 
> gleaned from the OS?
>
>   
 Ali Saidi wrote:
It seems rather arbitrary, but the world of function pointers that sets 
 this value in the OS is pretty deep. I think having the user configure it 
 is fine, there isn't a really good way we could grab it from the OS since 
 there isn't once place where there is a device struct that describes a ide 
 controller. These values only seem to apply to the IDE device.

 
>>> I'm also really curious what's going on with these values. We should 
>>> probably figure out what they're for before we go hacking them in. Also, 
>>> please make sure an X86 kernel still boots with this change. I remember 
>>> getting the IDE controller to work was a little finicky at least partially 
>>> because of its legacy fixed IO port locations, and it might break even 
>>> though other ISAs are ok.
>>>
>>>
>>> - Gabe
>>>
>>>
>>> ---
>>> This is an automatically generated e-mail. To reply, visit:
>>> http://reviews.m5sim.org/r/292/#review436
>>> ---
>>>
>>>
>>> On 2010-11-08 15:34:45, Ali Saidi wrote:
>>>
>>>   
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.m5sim.org/r/292/
 ---

 (Updated 2010-11-08 15:34:45)


 Review request for Default.


 Summary
 ---

 ARM: Add support for a dumb IDE controller


 Diffs
 -

  configs/common/FSConfig.py f61e079ad05e 
  src/dev/Ide.py f61e079ad05e 
  src/dev/arm/realview.cc f61e079ad05e 
  src/dev/ide_ctrl.hh f61e079ad05e 
  src/dev/ide_ctrl.cc f61e079ad05e 
  src/dev/pcidev.cc f61e079ad05e 

 Diff: http://reviews.m5sim.org/r/292/diff


 Testing
 ---


 Thanks,

 Ali



 
>>> ___
>>> m5-dev mailing list
>>> m5-dev@m5sim.org
>>> http://m5sim.org/mailman/listinfo/m5-dev
>>>
>>>   
>> 
>
> ___
> m5-dev mailing list
> m5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/m5-dev
>   

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: ARM: Add support for a dumb IDE controller

2010-11-21 Thread Ali Saidi 
It's not an isa thing, it's hard-coded in the struct that is passed to the ide 
driver. Either way, it-s not those values that is breaking it.  They do nothing 
when they're 0.

Ali




On Nov 21, 2010, at 11:58 PM, Gabe Black wrote:

> As feared, this change does break x86 boot. I'll see if I can fix it.
> Ironically I found this out while adding an X86 FS regression. We really
> do need to figure out what's going on with those values and how that
> maps to other ISAs.
> 
> Gabe
> 
> Gabe Black wrote:
>> 
>>> On 2010-11-08 17:56:14, Nathan Binkert wrote:
>>> 
 src/dev/ide_ctrl.cc, line 454
 
 
Is this something that we should deal with on a per device basis, or is 
 this a more generic thing?  Also, is this something that should be 
 configured by the user, or is this something that's either fixed or 
 gleaned from the OS?
 
>>> Ali Saidi wrote:
>>>It seems rather arbitrary, but the world of function pointers that sets 
>>> this value in the OS is pretty deep. I think having the user configure it 
>>> is fine, there isn't a really good way we could grab it from the OS since 
>>> there isn't once place where there is a device struct that describes a ide 
>>> controller. These values only seem to apply to the IDE device.
>>> 
>> 
>> I'm also really curious what's going on with these values. We should 
>> probably figure out what they're for before we go hacking them in. Also, 
>> please make sure an X86 kernel still boots with this change. I remember 
>> getting the IDE controller to work was a little finicky at least partially 
>> because of its legacy fixed IO port locations, and it might break even 
>> though other ISAs are ok.
>> 
>> 
>> - Gabe
>> 
>> 
>> ---
>> This is an automatically generated e-mail. To reply, visit:
>> http://reviews.m5sim.org/r/292/#review436
>> ---
>> 
>> 
>> On 2010-11-08 15:34:45, Ali Saidi wrote:
>> 
>>> ---
>>> This is an automatically generated e-mail. To reply, visit:
>>> http://reviews.m5sim.org/r/292/
>>> ---
>>> 
>>> (Updated 2010-11-08 15:34:45)
>>> 
>>> 
>>> Review request for Default.
>>> 
>>> 
>>> Summary
>>> ---
>>> 
>>> ARM: Add support for a dumb IDE controller
>>> 
>>> 
>>> Diffs
>>> -
>>> 
>>>  configs/common/FSConfig.py f61e079ad05e 
>>>  src/dev/Ide.py f61e079ad05e 
>>>  src/dev/arm/realview.cc f61e079ad05e 
>>>  src/dev/ide_ctrl.hh f61e079ad05e 
>>>  src/dev/ide_ctrl.cc f61e079ad05e 
>>>  src/dev/pcidev.cc f61e079ad05e 
>>> 
>>> Diff: http://reviews.m5sim.org/r/292/diff
>>> 
>>> 
>>> Testing
>>> ---
>>> 
>>> 
>>> Thanks,
>>> 
>>> Ali
>>> 
>>> 
>>> 
>> 
>> ___
>> m5-dev mailing list
>> m5-dev@m5sim.org
>> http://m5sim.org/mailman/listinfo/m5-dev
>> 
> 
> 

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: ARM: Add support for a dumb IDE controller

2010-11-21 Thread Gabe Black 
As feared, this change does break x86 boot. I'll see if I can fix it.
Ironically I found this out while adding an X86 FS regression. We really
do need to figure out what's going on with those values and how that
maps to other ISAs.

Gabe

Gabe Black wrote:
>   
>> On 2010-11-08 17:56:14, Nathan Binkert wrote:
>> 
>>> src/dev/ide_ctrl.cc, line 454
>>> 
>>>
>>> Is this something that we should deal with on a per device basis, or is 
>>> this a more generic thing?  Also, is this something that should be 
>>> configured by the user, or is this something that's either fixed or gleaned 
>>> from the OS?
>>>   
>> Ali Saidi wrote:
>> It seems rather arbitrary, but the world of function pointers that sets 
>> this value in the OS is pretty deep. I think having the user configure it is 
>> fine, there isn't a really good way we could grab it from the OS since there 
>> isn't once place where there is a device struct that describes a ide 
>> controller. These values only seem to apply to the IDE device.
>> 
>
> I'm also really curious what's going on with these values. We should probably 
> figure out what they're for before we go hacking them in. Also, please make 
> sure an X86 kernel still boots with this change. I remember getting the IDE 
> controller to work was a little finicky at least partially because of its 
> legacy fixed IO port locations, and it might break even though other ISAs are 
> ok.
>
>
> - Gabe
>
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/292/#review436
> ---
>
>
> On 2010-11-08 15:34:45, Ali Saidi wrote:
>   
>> ---
>> This is an automatically generated e-mail. To reply, visit:
>> http://reviews.m5sim.org/r/292/
>> ---
>>
>> (Updated 2010-11-08 15:34:45)
>>
>>
>> Review request for Default.
>>
>>
>> Summary
>> ---
>>
>> ARM: Add support for a dumb IDE controller
>>
>>
>> Diffs
>> -
>>
>>   configs/common/FSConfig.py f61e079ad05e 
>>   src/dev/Ide.py f61e079ad05e 
>>   src/dev/arm/realview.cc f61e079ad05e 
>>   src/dev/ide_ctrl.hh f61e079ad05e 
>>   src/dev/ide_ctrl.cc f61e079ad05e 
>>   src/dev/pcidev.cc f61e079ad05e 
>>
>> Diff: http://reviews.m5sim.org/r/292/diff
>>
>>
>> Testing
>> ---
>>
>>
>> Thanks,
>>
>> Ali
>>
>>
>> 
>
> ___
> m5-dev mailing list
> m5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/m5-dev
>   

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] Cron /z/m5/regression/do-regression --scratch all

2010-11-21 Thread Cron Daemon 
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/simple-timing passed.
* build/ALPHA_SE/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby 
passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/simple-atomic passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby 
passed.
* build/ALPHA_SE/tests/fast/long/50.vortex/alpha/tru64/simple-atomic passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/simple-atomic passed.
* build/ALPHA_SE/tests/fast/long/70.twolf/alpha/tru64/simple-atomic passed.
* build/ALPHA_SE/tests/fast/quick/30.eio-mp/alpha/eio/simple-timing-mp 
passed.
* build/ALPHA_SE/tests/fast/quick/01.hello-2T-smt/alpha/linux/o3-timing 
passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/o3-timing passed.
* build/ALPHA_SE/tests/fast/long/70.twolf/alpha/tru64/simple-timing passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/o3-timing passed.
* build/ALPHA_SE/tests/fast/quick/50.memtest/alpha/linux/memtest passed.
* build/ALPHA_SE/tests/fast/long/50.vortex/alpha/tru64/o3-timing passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/inorder-timing 
passed.
* build/ALPHA_SE/tests/fast/quick/30.eio-mp/alpha/eio/simple-atomic-mp 
passed.
* build/ALPHA_SE/tests/fast/long/70.twolf/alpha/tru64/o3-timing passed.
* build/ALPHA_SE/tests/fast/long/00.gzip/alpha/tru64/simple-timing passed.
* build/ALPHA_SE/tests/fast/quick/20.eio-short/alpha/eio/simple-atomic 
passed.
* build/ALPHA_SE/tests/fast/long/30.eon/alpha/tru64/simple-timing passed.
* build/ALPHA_SE/tests/fast/long/60.bzip2/alpha/tru64/simple-timing passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/simple-timing passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby 
passed.
* build/ALPHA_SE/tests/fast/long/50.vortex/alpha/tru64/simple-timing passed.
* build/ALPHA_SE/tests/fast/long/40.perlbmk/alpha/tru64/simple-atomic 
passed.
* build/ALPHA_SE/tests/fast/long/00.gzip/alpha/tru64/simple-atomic passed.
* build/ALPHA_SE/tests/fast/long/50.vortex/alpha/tru64/inorder-timing 
passed.
* build/ALPHA_SE/tests/fast/quick/20.eio-short/alpha/eio/simple-timing 
passed.
* build/ALPHA_SE/tests/fast/long/30.eon/alpha/tru64/o3-timing passed.
* build/ALPHA_SE/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby 
passed.
* build/ALPHA_SE/tests/fast/long/70.twolf/alpha/tru64/inorder-timing passed.
* build/ALPHA_SE/tests/fast/long/30.eon/alpha/tru64/simple-atomic passed.
* build/ALPHA_SE/tests/fast/long/60.bzip2/alpha/tru64/simple-atomic passed.
* build/ALPHA_SE/tests/fast/long/00.gzip/alpha/tru64/o3-timing passed.
* 
build/ALPHA_SE_MOESI_hammer/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby-MOESI_hammer
 passed.
* 
build/ALPHA_SE_MOESI_hammer/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby-MOESI_hammer
 passed.
* 
build/ALPHA_SE_MOESI_hammer/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby-MOESI_hammer
 passed.
* 
build/ALPHA_SE_MOESI_hammer/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby-MOESI_hammer
 passed.
* build/ALPHA_SE/tests/fast/long/40.perlbmk/alpha/tru64/simple-timing 
passed.
* 
build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby-MESI_CMP_directory
 passed.
* 
build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby-MESI_CMP_directory
 passed.
* 
build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby-MESI_CMP_directory
 passed.
* 
build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby-MESI_CMP_directory
 passed.
* 
build/ALPHA_SE_MOESI_CMP_directory/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby-MOESI_CMP_directory
 passed.
* 
build/ALPHA_SE_MOESI_CMP_directory/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby-MOESI_CMP_directory
 passed.
* 
build/ALPHA_SE_MOESI_CMP_directory/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby-MOESI_CMP_directory
 passed.
* 
build/ALPHA_SE_MOESI_CMP_directory/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby-MOESI_CMP_directory
 passed.
* 
build/ALPHA_SE_MOESI_CMP_token/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby-MOESI_CMP_token
 passed.
* 
build/ALPHA_SE_MOESI_CMP_token/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby-MOESI_CMP_token
 passed.
* 
build/ALPHA_SE_MOESI_CMP_token/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby-MOESI_CMP_token
 passed.
* 
build/ALPHA_SE_MOESI_CMP_token/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby-MOESI_CMP_token
 passed.
* 
build/ALPHA_FS/tests/fast/quick/10.linux-boot/alpha/linux/tsunami-simple-atomic-dual
 passed.
* 
build/ALPHA_FS/tests/fast/quick/10.linux-boot/alpha/linux/tsunami-simple-atomic 
passed.
* 
build/ALPHA_FS/tests/fast/quick/10.lin

Re: [m5-dev] Review Request: O3: Change when the memory ordering violation is checked to after dtlb lookup.

2010-11-21 Thread Gabe Black 

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/322/#review501
---


I think this was supposed to go before the dtlb timing one.

- Gabe


On 2010-11-19 16:13:55, Ali Saidi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/322/
> ---
> 
> (Updated 2010-11-19 16:13:55)
> 
> 
> Review request for Default.
> 
> 
> Summary
> ---
> 
> O3: Change when the memory ordering violation is checked to after dtlb lookup.
> 
> This patch is a follow-up to the dtlb_timing.patch. Separated intentionally to
> be understood easily.
> 
> Dtlb lookup now can finish (finishTranslation()) some time later from the tick
> it was initiated (initiateAccess()), checking and handling of memory ordering
> violation needs to happen after finishTranslation() is called for the
> instruction. The code that handling this violation used to be in 
> executeInsts()
> has been separated as handleMemOrderViolation().
> 
> Also, added cycleMemIssued variable to prevent memory instructions issued in 
> the
> same cycle are falsely blamed to have caused ordering violation.
> 
> 
> Diffs
> -
> 
>   src/cpu/base_dyn_inst.hh 6286bb50127e 
>   src/cpu/o3/iew.hh 6286bb50127e 
>   src/cpu/o3/iew_impl.hh 6286bb50127e 
>   src/cpu/o3/lsq_impl.hh 6286bb50127e 
>   src/cpu/o3/lsq_unit.hh 6286bb50127e 
>   src/cpu/o3/lsq_unit_impl.hh 6286bb50127e 
> 
> Diff: http://reviews.m5sim.org/r/322/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ali
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: O3: Make the DTLB translateTiming() a split transaction.

2010-11-21 Thread Gabe Black 

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/321/#review500
---



src/cpu/base_dyn_inst.hh


Please change to "Is the physical address valid (translation was 
successful)?"



src/cpu/base_dyn_inst.hh


Please change to "Is address translation finished, faulting or not?"



src/cpu/base_dyn_inst.hh


data may not need to be on the next line. It might need to be if the lines 
above are jst on the very edge.



src/cpu/base_dyn_inst.hh


MachineCheckFaults aren't for uncached loads, they're for accesses that 
fall off the end of the world and cause a machine check. To the best of my 
knowledge these won't be returned immediately since there would be a delay 
getting to them through the memory system, but something in the middle might 
use the fault to signal a strange error.



src/cpu/base_dyn_inst_impl.hh


A new copyright might not be justified here.



src/cpu/o3/lsq_unit.hh


Copyright probably not justified again.



src/cpu/o3/lsq_unit_impl.hh


I don't think this is true. A fault could have happened by this point, 
especially when there isn't a table walker. initiateAcc itself could fault 
without even performing an access if, for instance, it was a privileged 
instruction executed in a non-privileged mode.



src/cpu/o3/lsq_unit_impl.hh


Why is this code being removed?



src/cpu/o3/lsq_unit_impl.hh


I don't totally follow what's going on here. Why are you putting these in a 
new structure instead of checking them in place? This might be fine, but I 
don't understand what's going on so I'm not sure if there's a flaw in the 
logic. One thing I'd be especially careful of here are translations finishing 
out of order and breaking the ordering checks.



src/cpu/o3/lsq_unit_impl.hh


The else and the if shouldn't be on two different lines.



src/cpu/o3/lsq_unit_impl.hh


Shouldn't be on a new line.


- Gabe


On 2010-11-19 16:13:39, Ali Saidi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/321/
> ---
> 
> (Updated 2010-11-19 16:13:39)
> 
> 
> Review request for Default.
> 
> 
> Summary
> ---
> 
> O3: Make the DTLB translateTiming() a split transaction.
> ISAs that have hardware page table walkers need this. For those ISAs,
> initiateTranslation can return with NoFault, but with the translation
> unfinished.
> 
> In the BaseDynInst::read() function, actual memory(cache) access calling
> cpu->read() used to occur right after initiateTranslation() function call. 
> This
> access has been moved to the finishTranslation() so that it occurs when the
> translation finishes, either by TLB hit or TLB Miss followed by hardware
> page-walk. Faults occured during translation is detected in 
> finishTranslation()
> and handled during the IEW stage tick()->handleTranslationFault().
> 
> This is a work in progress, but I want to make sure there aren't any issues...
> 
> 
> Diffs
> -
> 
>   src/arch/arm/tlb.cc 6286bb50127e 
>   src/cpu/base_dyn_inst.hh 6286bb50127e 
>   src/cpu/base_dyn_inst_impl.hh 6286bb50127e 
>   src/cpu/o3/commit_impl.hh 6286bb50127e 
>   src/cpu/o3/iew_impl.hh 6286bb50127e 
>   src/cpu/o3/lsq.hh 6286bb50127e 
>   src/cpu/o3/lsq_impl.hh 6286bb50127e 
>   src/cpu/o3/lsq_unit.hh 6286bb50127e 
>   src/cpu/o3/lsq_unit_impl.hh 6286bb50127e 
> 
> Diff: http://reviews.m5sim.org/r/321/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ali
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: O3: Support squashing all state after special instruction

2010-11-21 Thread Gabe Black 

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/320/#review499
---


Generally I like it. I'll trust that the actual mechanism in the commit stage 
is correct. There are some mostly superficial tweaks that need to happen, but 
then I'm ok with it.


src/arch/sparc/isa/decoder.isa


This line is probably too long now.



src/cpu/o3/commit_impl.hh


Since this is used in only one spot this intermediate variable probably 
isn't necessary and obfuscates what's going on a bit.



src/cpu/o3/commit_impl.hh


I'm guessing here, but does commit already have code that does this? Do we 
want to pull that out into a function? Whether or not this sort of thing is 
already being done (and I can't imagine it's not) this is a nice mostly 
independent blob of code doing something fairly specific, so it'd be a great 
candidate to put in a function and simplify this one.


- Gabe


On 2010-11-19 16:12:52, Ali Saidi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/320/
> ---
> 
> (Updated 2010-11-19 16:12:52)
> 
> 
> Review request for Default.
> 
> 
> Summary
> ---
> 
> O3: Support squashing all state after special instruction
> 
> For SPARC ASIs are added to the ExtMachInst. If the ASI is changed simply
> marking the instruction as Serializing isn't enough beacuse that only
> stops rename. This provides a mechanism to squash all the instructions
> and refetch them
> 
> 
> Diffs
> -
> 
>   src/arch/sparc/isa/decoder.isa 6286bb50127e 
>   src/cpu/base_dyn_inst.hh 6286bb50127e 
>   src/cpu/o3/commit_impl.hh 6286bb50127e 
>   src/cpu/static_inst.hh 6286bb50127e 
> 
> Diff: http://reviews.m5sim.org/r/320/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ali
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: O3: Support SWAP and predicated loads/store in ARM.

2010-11-21 Thread Gabe Black 

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/319/#review498
---



src/arch/arm/isa/insts/swap.isa


Is this line too long now? Also, I'm not sure what's going on with the new 
'IsStoreConditional' flag. Doesn't there need to be a load locked somewhere? 
Also, can't those fail? Would the x86 version of locking be more appropriate 
since it can't? Why is any flag necessary since the swap is (I think) done 
atomically through the memory request itself? Or are we moving away from that?



src/cpu/o3/iew_impl.hh


The only difference between this else if and the if above it is that 
activityThisCycle is called for the if but not the else if. I'm not sure that's 
correct, though admittedly the use of the activity stuff in O3 is a little 
mysterious to me, but in any case I don't think it would hurt anything to call 
it in both cases and merge those two blocks.


- Gabe


On 2010-11-19 16:12:35, Ali Saidi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/319/
> ---
> 
> (Updated 2010-11-19 16:12:35)
> 
> 
> Review request for Default.
> 
> 
> Summary
> ---
> 
> O3: Support SWAP and predicated loads/store in ARM.
> 
> 
> Diffs
> -
> 
>   src/arch/arm/isa/insts/swap.isa 6286bb50127e 
>   src/cpu/o3/iew.hh 6286bb50127e 
>   src/cpu/o3/iew_impl.hh 6286bb50127e 
>   src/cpu/o3/lsq_unit_impl.hh 6286bb50127e 
> 
> Diff: http://reviews.m5sim.org/r/319/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ali
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: ARM: Support switchover with hardware table walkers

2010-11-21 Thread Gabe Black 

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/318/#review497
---



src/cpu/base.cc


I like how this seems pretty general purpose and things shouldn't get lost 
in the shuffle this way (I think) but it would be nice if it didn't assume 
there was only one port that needed to be moved. Off hand I don't have any 
suggestion to fix it that wouldn't be overly cumbersome.


- Gabe


On 2010-11-19 16:11:58, Ali Saidi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/318/
> ---
> 
> (Updated 2010-11-19 16:11:58)
> 
> 
> Review request for Default.
> 
> 
> Summary
> ---
> 
> ARM: Support switchover with hardware table walkers
> 
> 
> Diffs
> -
> 
>   src/arch/arm/table_walker.cc 6286bb50127e 
>   src/arch/arm/tlb.hh 6286bb50127e 
>   src/arch/arm/tlb.cc 6286bb50127e 
>   src/cpu/base.cc 6286bb50127e 
>   src/sim/tlb.hh 6286bb50127e 
> 
> Diff: http://reviews.m5sim.org/r/318/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ali
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev