On 03.12.20 00:25, Frank Yang wrote:


On Wed, Dec 2, 2020 at 2:57 PM Alexander Graf <ag...@csgraf.de <mailto:ag...@csgraf.de>> wrote:


    On 02.12.20 23:46, Frank Yang wrote:


    On Wed, Dec 2, 2020 at 2:28 PM Alexander Graf <ag...@csgraf.de
    <mailto:ag...@csgraf.de>> wrote:


        On 02.12.20 23:19, Frank Yang wrote:

        From downstream:
        
https://android-review.googlesource.com/c/platform/external/qemu/+/1515002
        
<https://android-review.googlesource.com/c/platform/external/qemu/+/1515002>

        Based on v3 of Alexander Graf's patches

        https://patchew.org/QEMU/20201202190408.2041-1-ag...@csgraf.de
        <https://patchew.org/QEMU/20201202190408.2041-1-ag...@csgraf.de>

        We need to adjust CNTVOFF_EL2 so that time doesnt warp. 
        Even though we
        can set separate CNTVOFF_EL2 values per vCPU, it just is not
        worth the
        require effort to do that accurately---with individual
        values, even if
        they are a tiny bit off it can result in a lockup due to
        inconsistent
        time differences between vCPUs. So just use a global
        approximate value
        for now.

        Not tested in upstream yet, but Android emulator snapshots
        work without
        time warp now.

        Signed-off-by: Lingfeng Yang <l...@google.com
        <mailto:l...@google.com>>


        If we just always make CNTV start at the same 0 as
        QEMU_CLOCK_VIRTUAL, we should be able to just recover the
        offset after migration by looking at QEMU_CLOCK_VIRTUAL to
        set CNTVOFF, right?

        That would end up much easier than this patch I hope.



    The virtual clock interfaces/implementations in QEMU seem complex
    to me relative to the fix needed here and they don't seem to
    compute ticks with mach_absolute_time() (which in this case we
    want since we want to compute in timer ticks instead of having to
    mess with ns / cycle conversions). I do agree this patch does
    seem more complicated on the surface though versus "just" setting
    cntvoff directly to some value. Maybe we should simplify the
    QEMU_CLOCK_VIRTUAL implementation first to maintain
    CNTVOFF_EL2/CNTV using mach_absolute_time() first?


    So QEMU_CLOCK_VIRTUAL calls cpu_get_clock() which just adds an
    offset to gettimeofday(). This offset is already part of the live
    migration stream[1]. So if you just configure CNTVOFF_EL2 based on
    QEMU_CLOCK_VIRTUAL adjusted by the clock frequency on vcpu init,
    you should have everything you need. You can do that on every CPU
    init even, as the virtual clock will just be 0 on start.

    The only thing we need to change then is to move the WFI from a
    direct call to mach_absolute_time() to also check the virtual
    clock instead. I would hope that gettimeofday() calls
    mach_absolute_time() in the background too to speed it up.

Sounds plausible, but I noticed that we also have cpu_ticks_offset as part of the migration stream, and I prefer mach_absolute_time() (ticks) instead of seconds in WFI as well as it makes the calculation more accurate (ticks against ticks instead of conversion between ns and ticks).

Should we look at integrating this with cpu_ticks_offset instead?


The big benefit of reusing the virtual clock is that you create compatible migration streams between TCG and HVF, no? You'd lose out on that with a hack like this.


Alex


Reply via email to