> On Sept. 1, 2012, 4:41 p.m., Andreas Hansson wrote:
> > src/arch/arm/isa.cc, line 235
> > <http://reviews.gem5.org/r/1344/diff/5/?file=29386#file29386line235>
> >
> >     I really think we have to find a better way of doing this. String 
> > matching should, in my opinion, never be used to determine the type of a 
> > module as it is setting ourselves up for hidden assumptions.
> >     
> >     Do you agree?
> 
> Anthony Gutierrez wrote:
>     I agree. However, there is no current way to determine if we have caches 
> as far as I can tell. Returning a value other than 0 for the cache-related 
> fields here, when there are no caches, is wrong. And it affects the behavior 
> of the kernel at the very least. My previous patch added a cacheline size 
> param to the CPU model. This solved the problem of getting the line size as 
> well as determining if we have caches. I can revert this back to that.
> 
> Andreas Hansson wrote:
>     I think what you suggest might be the best option at the moment.
>     
>     Ultimately, I am a strong believer of the notion of creating a "CPU" 
> wrapper that contains cores (what is called CPU today) and caches, as already 
> mentioned a few times.
>     
>     Any other ideas?
> 
> Ali Saidi wrote:
>     I think that is it, but it's a question of biting the bullet and doing 
> the coding. The other issue here is that if you start a system without caches 
> and then add them later, what happens? I'm not sure if the kernel caches the 
> response of the register or if it reads it every time. If the latter that is 
> fine, but if it's the prior then we have a problem because we need to be able 
> to specify these values even when the cache doesn't exist. Probably we're 
> going to want a default CTR value in the ARM system that is checked for being 
> reasonable against the real cache when it starts up.
>     
>     Tony,
>     
>     could you look how this value is read/cached in the kernel?
>     
>     Thanks,
>     Ali
>     
>     
>

Well, from my previous work with the invalidation instructions I know that this 
register can be consulted in some versions of the kernel when doing cache 
invalidations. E.g., it will read this register to get the line size, then 
iterate over the range to be invalidated in line-sized steps. Some versions 
used the CSSIDR. In the use-cases I saw it does not cache the value, it reads 
the register each time. I don't know where else it could be used, and since the 
invalidation instructions are not implemented, I don't think it will affect 
correctness. But, it definitely can skew things like instruction counts. E.g, 
in the original implementation (where the CTR reg held a constant default 
value, that also happened to be off because it used values representing bytes 
instead of words) the cache inv loop was being executed 2-3x less than it 
should because the line size was 2-3x too large.


- Anthony


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


On Sept. 1, 2012, 10:12 a.m., Anthony Gutierrez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/1344/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2012, 10:12 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Description
> -------
> 
> Changeset 9182:3377bf0ff708
> ---------------------------
> ARM: don't set cache fields of CTR reg if there are no caches
> 
> currently the CTR reg sets its fields based on the block size of
> the peer connected to its inst port. in the cache where there are
> no caches it doesn't make sense to do so. this patch sets only
> the relevant fields of the CTR reg if there are no caches and
> leaves the cache-related fields as 0.
> 
> 
> Diffs
> -----
> 
>   src/arch/arm/isa.cc 42807286d6cbaa33b43942d7d15ec34cd5dd5d1d 
> 
> Diff: http://reviews.gem5.org/r/1344/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Anthony Gutierrez
> 
>

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to