> On Aug. 23, 2015, 7:12 p.m., Joel Hestness wrote:
> > Hi Andreas. I'm wondering why this patch was checked in 
> > (http://repo.gem5.org/gem5/rev/842f56345a42). It doesn't have a 'Ship It', 
> > and the review request is only 4 days old.
> > 
> > This change also breaks some packet use cases we have in gem5-gpu. 
> > Specifically, this change now requires that Requests be translated or 
> > otherwise find a valid physical address before a packet can be constructed. 
> > I'm not convinced that this is a reasonable requirement, especially since 
> > packets can be used between components that operate completely on virtual 
> > addresses.
> > 
> > Can you please revert this change?

Hi Joel,

I called out for feedback, and Steve was the only one to respond. Since the 
patch was fairly trivial and the functionality was never used I opted to not 
gate other changes on this minor modification.

Could you explain your use-case a bit better? Packets _need_ a physical 
address. Previously there was an option to create a va-only request, then 
create the packet, then do the va->pa. Note that the packet always had to hava 
physical address in both cases. The late setting was _never_ used in the main 
gem5 codebase and opened up for nasty bugs (trust me on this one).

How is the gem5-gpu use-case different from what is happening in the main 
codebase? Why not first create the request (do any translation), and then 
create the packet?


- Andreas


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


On Aug. 19, 2015, 9:06 a.m., Andreas Hansson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3044/
> -----------------------------------------------------------
> 
> (Updated Aug. 19, 2015, 9:06 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> Changeset 11055:dc0a15e6e113
> ---------------------------
> mem: Reflect that packet address and size are always valid
> 
> This patch simplifies the packet, and removes the possibility of
> creating a packet without a valid address and/or size. Under no
> circumstances are these fields set at a later point, and thus they
> really have to be provided at construction time.
> 
> The patch also fixes a case there the MinorCPU creates a packet
> without a valid address and size, only to later delete it.
> 
> 
> Diffs
> -----
> 
>   src/cpu/minor/lsq.cc 110cce93d398 
>   src/mem/packet.hh 110cce93d398 
> 
> Diff: http://reviews.gem5.org/r/3044/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andreas Hansson
> 
>

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

Reply via email to