> On June 19, 2014, 7:36 p.m., Steve Reinhardt wrote:
> > Why is Addr being parsed using toMemorySize() in the first place? That
> > seems wrong. At least some of the places Addr is used with a size (like
> > RealView.max_mem_size), I think the problem is that the param should really
> > be a Param.MemorySize to begin with.
>
> Mitch Hayenga wrote:
> Hi Steve, I'm the one that made this edit.
>
> I agree that the call to "toMemorySize()" on an address seems strange,
> but it's an idiom that seems pretty well spread throughout gem5. Saying that
> something lives at "512MB" for an address is used in multiple places.
>
>
> 1. It's pretty well baked into the AddrRange() param as most places that
> call it give it a starting address or size in MB, GB, etc which is then
> directly passed to Addr() within the param class.
>
> common/FSConfig.py
> 143: self.mem_ranges = [AddrRange(Addr('1MB'), size = '64MB'),
> 144: AddrRange(Addr('2GB'), size ='256MB')]
> 405: self.mem_ranges = [AddrRange('3GB'),
> 406: AddrRange(Addr('4GB'), size = excess_mem_size)]
>
>
> 2. It's also used on other systems for arithmetic.
>
> src/arch/sparc/SparcSystem.py
> 59: hypervisor_addr = Param.Addr(Addr('64kB') + _rom_base,
> 61: openboot_addr = Param.Addr(Addr('512kB') + _rom_base,
>
>
>
>
> It could be changed over, but this would require changing multiple other
> places in the code. Changing these places, which are really trying to
> directly set an "Addr" to "MemorySize", also seems wrong though. In the
> FSConfig.py example, we're trying to specify the starting address, not a
> MemorySize.
>
> Either way is going to have some bad semantics (unless we get rid of the
> ability to nicely specify starting addresses via "sizes").
>
> Andreas Hansson wrote:
> Any additional comments?
Thanks for the response, Mitch.
The patch itself makes sense; we certainly do want people to be able to use hex
constants for addresses. It's really the status quo that I am annoyed with :).
I agree that it's a reasonable idiom to use a size to indicate an address. It
would still be nice if we actually used MemorySize when we wanted a size though.
My one request would be, now that we've worked through this, to add a brief
comment above the existing toMemorySize() call (or maybe above the whole
try/except clause) to add some of this perspective to the code itself.
- Steve
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2292/#review5144
-----------------------------------------------------------
On June 12, 2014, 3:47 p.m., Ali Saidi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/2292/
> -----------------------------------------------------------
>
> (Updated June 12, 2014, 3:47 p.m.)
>
>
> Review request for Default.
>
>
> Repository: gem5
>
>
> Description
> -------
>
> Changeset 10240:d4f21d820604
> ---------------------------
> python: Change parsing of Addr so hex values work from scripts
>
> When passed from a configuration script with a hexadecimal value (like
> "0x80000000"), gem5 would error out. This is because it would call
> "toMemorySize" which requires the argument to end with a size specifier (like
> 1MB, etc).
>
> This modification makes it so raw hex values can be passed through Addr
> parameters from the configuration scripts.
>
>
> Diffs
> -----
>
> src/arch/arm/ArmSystem.py a2bb75a474fd
> src/python/m5/params.py a2bb75a474fd
>
> Diff: http://reviews.gem5.org/r/2292/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ali Saidi
>
>
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
