> On July 1, 2015, 3:19 p.m., Andreas Hansson wrote: > > src/mem/SerialLink.py, line 60 > > <http://reviews.gem5.org/r/2935/diff/1/?file=47354#file47354line60> > > > > Do we also need lane width? Lane signalling rate? > > > > No need to add right now, merely wondering if we should not try and get > > these things in place.
May be the default value of 1 for this parameter is confusing. num_lanes means lane_width, and in the HMC.py script it is assigned to num_lanes_per_link which is 16 in HMC. I can change its name or its default value if you say so. > On July 1, 2015, 3:19 p.m., Andreas Hansson wrote: > > src/mem/serial_link.cc, line 146 > > <http://reviews.gem5.org/r/2935/diff/1/?file=47356#file47356line146> > > > > I think we should really be paying for both, but do so on the request > > side, and here simply check that they are 0. I agree that we should account for this delay, but since it will be a global change and it may break things, I suggest to address it in a separate patch. > On July 1, 2015, 3:19 p.m., Andreas Hansson wrote: > > src/mem/serial_link.cc, line 150 > > <http://reviews.gem5.org/r/2935/diff/1/?file=47356#file47356line150> > > > > Could you add a comment here. What is it you are trying to model? > > De-serialisation? Yes exactly, I assume that the serializer component at the transmitter side does not need to receive the whole packet to start the serialization (this assumption is consistent with the standard). But the deserializer waits for the complete packet to check its integrity first. So everytime a packet crosses a serial link, we should account for its deserialization latency only. I will added a comment for this line. > On July 1, 2015, 3:19 p.m., Andreas Hansson wrote: > > src/mem/serial_link.cc, line 155 > > <http://reviews.gem5.org/r/2935/diff/1/?file=47356#file47356line155> > > > > We have added schedTimingResp with a boolean to force in-order > > insertion for coherency reasons. I can post that patch and we can use the > > same thing here. That would be great. > On July 1, 2015, 3:19 p.m., Andreas Hansson wrote: > > src/mem/serial_link.cc, line 167 > > <http://reviews.gem5.org/r/2935/diff/1/?file=47356#file47356line167> > > > > I would suggest we sink any packets with memInhibitAsserted here, and > > also any CleanEvicts (see other patches). Could you please explain more what is required? I took a look at other components and in the recvTimingResp() I did not see any sinking for the packets with memInhibitAsserted. > On July 1, 2015, 3:19 p.m., Andreas Hansson wrote: > > src/mem/serial_link.cc, line 201 > > <http://reviews.gem5.org/r/2935/diff/1/?file=47356#file47356line201> > > > > We should really pay for the serialisation delay from the crossbar > > here, and any headerDelay as well. I agree, and would like to try to address this in a separate patch. > On July 1, 2015, 3:19 p.m., Andreas Hansson wrote: > > src/mem/serial_link.cc, line 205 > > <http://reviews.gem5.org/r/2935/diff/1/?file=47356#file47356line205> > > > > Comments please :-) This is similar to what happens for the request packets: The serializer will start serialization as soon as it receives the first flit, but the deserializer (at the host side in this case), will have to wait to receive the whole packet. So we only account for the deserialization latency. I added a comment for this line. - Erfan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2935/#review6658 ----------------------------------------------------------- On July 8, 2015, 7:42 a.m., Erfan Azarkhish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2935/ > ----------------------------------------------------------- > > (Updated July 8, 2015, 7:42 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > SerialLink is simply a copy of gem5's Bridge class with the ability to > account for serialization > latency of the packets. I did not inherit SerialLink from Bridge, because it > required > some modifications in the Bridge. > This patch and the subsequent chain of patches aim to model a simple Hybrid > Memory Cube device in gem5 > Please apply the complete chain before running the simulation. > > > Diffs > ----- > > src/mem/SerialLink.py PRE-CREATION > src/mem/serial_link.hh PRE-CREATION > src/mem/serial_link.cc PRE-CREATION > > Diff: http://reviews.gem5.org/r/2935/diff/ > > > Testing > ------- > > > Thanks, > > Erfan Azarkhish > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
