-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3619/#review8645
-----------------------------------------------------------
Thanks for implementing this! Overall, I think this is the right approach.
However, I think you could refactor it a bit to make it cleaner. I think you
could move most of the new packet handling logic to the KVMCpuPort, this would
largely hide the fact that it support both timing and atomic from the rest of
the CPU. I'd suggest something along these lines:
* Tick submitIO(pkt): Send the packet (and cleanup storage in atomic).
Return the delay (0 for timing).
* Statis nextIOState(): Return RunningMMIOPending if there are pending
timing accesses, RunningServiceCompletion otherwise.
* recvReqRetry(): Resubmit the deferred packet.
* recvTimingResp(pkt): Clean up pkt and try to submit the next deferred
packet. When the last packet has received a response, call cpu->finishTiming()
which updates the state of the CPU.
Using these helper functions, the MMIO handlers would be almost identical to
before. Instead of calling port->submitAtomic(pkt) you'd call
port->submitIO(pkt). Before exiting the IO handler, you set the state to
port->nextIOState().
src/cpu/kvm/base.cc (line 192)
<http://reviews.gem5.org/r/3619/#comment7498>
I think you need to submit the next deferred packet here.
src/cpu/kvm/base.cc (lines 1097 - 1111)
<http://reviews.gem5.org/r/3619/#comment7496>
Refactor this into a helper function that takes a packet and returns a
delay. You're using the exact same code when handling an IO instruction on x86.
src/cpu/kvm/x86_cpu.cc (lines 1347 - 1349)
<http://reviews.gem5.org/r/3619/#comment7497>
Since you potentially submit multiple timing packets, you can't reuse the
req without ending up in a double free situation.
- Andreas Sandberg
On Aug. 16, 2016, 11:28 p.m., Michael LeBeane wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3619/
> -----------------------------------------------------------
>
> (Updated Aug. 16, 2016, 11:28 p.m.)
>
>
> Review request for Default.
>
>
> Repository: gem5
>
>
> Description
> -------
>
> Changeset 11561:4595cc3848fc
> ---------------------------
> kvm: Support timing accesses for KVM cpu
> This patch enables timing accesses for KVM cpu. A new state,
> RunningMMIOPending, is added to indicate that there are outstanding timing
> requests generated by KVM in the system. KVM's tick() is disabled and the
> simulation does not enter into KVM until all outstanding timing requests have
> completed. The main motivation for this is to allow KVM CPU to perform MMIO
> in Ruby, since Ruby does not support atomic accesses.
>
>
> Diffs
> -----
>
> src/cpu/kvm/base.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c
> src/cpu/kvm/base.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c
> src/cpu/kvm/x86_cpu.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c
>
> Diff: http://reviews.gem5.org/r/3619/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michael LeBeane
>
>
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
