Re: [PATCH] kvm: Flush coalesced MMIO buffer periodly

[Avi Kivity <avi@redhat.com>]

On 01/22/2010 04:22 AM, Sheng Yang wrote:
> The default action of coalesced MMIO is, cache the writing in buffer, until:
> 1. The buffer is full.
> 2. Or the exit to QEmu due to other reasons.
>
> But this would result in a very late writing in some condition.
> 1. The each time write to MMIO content is small.
> 2. The writing interval is big.
> 3. No need for input or accessing other devices frequently.
>
> This issue was observed in a experimental embbed system. The test image
> simply print "test" every 1 seconds. The output in QEmu meets expectation,
> but the output in KVM is delayed for seconds.
>
> Per Avi's suggestion, I hooked a flushing for coalesced MMIO buffer in VGA
> update handler. By this way, We don't need vcpu explicit exit to QEmu to
> handle this issue.
>
> Signed-off-by: Sheng Yang<sheng@linux.intel.com>
> ---
>
> Like this?
>
>   qemu-kvm.c |   26 ++++++++++++++++++++++++--
>   qemu-kvm.h |    6 ++++++
>   vl.c       |    2 ++
>   3 files changed, 32 insertions(+), 2 deletions(-)
>
>
>
> +#ifdef KVM_CAP_COALESCED_MMIO
> +void kvm_flush_coalesced_mmio_buffer(void)
> +{
> +    if (kvm_state->coalesced_mmio_ring) {
> +        struct kvm_coalesced_mmio_ring *ring =
> +            kvm_state->coalesced_mmio_ring;
> +        while (ring->first != ring->last) {
> +            cpu_physical_memory_rw(ring->coalesced_mmio[ring->first].phys_addr,
> +&ring->coalesced_mmio[ring->first].data[0],
> +                           ring->coalesced_mmio[ring->first].len, 1);
> +            smp_wmb();
> +            ring->first = (ring->first + 1) % KVM_COALESCED_MMIO_MAX;
> +        }
> +    }
> +}
> +#endif
> +
>    

qemu has a coalesced mmio api, (qemu_register_colaesced_mmio), so please 
follow it (stubs in exec.c, and implementation in kvm-specific files).  
Please send the patch against the uq/master branch (which follow upstream).

> diff --git a/vl.c b/vl.c
> index 9edea10..64902f2 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3235,6 +3235,7 @@ static void gui_update(void *opaque)
>               interval = dcl->gui_timer_interval;
>           dcl = dcl->next;
>       }
> +    kvm_flush_coalesced_mmio_buffer();
>       qemu_mod_timer(ds->gui_timer, interval + qemu_get_clock(rt_clock));
>   }
>    

Better to do that before the call to dpy_refresh().

>
> @@ -3242,6 +3243,7 @@ static void nographic_update(void *opaque)
>   {
>       uint64_t interval = GUI_REFRESH_INTERVAL;
>
> +    kvm_flush_coalesced_mmio_buffer();
>       qemu_mod_timer(nographic_timer, interval + qemu_get_clock(rt_clock));
>   }
>    

Any need to do it here?

(why does nographic_update use a timer?)

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.