[Qemu-devel] Re: [PATCH 7/7] Fake dirty loggin when it's not there

[Jan Kiszka <jan.kiszka@siemens.com>]

Alexander Graf wrote:
> Some KVM platforms don't support dirty logging yet, like IA64 and PPC,
> so in order to still have screen updates on those, we need to fake it.
> 
> This patch just tells the getter function for dirty bitmaps, that all
> pages within a slot are dirty when the slot has dirty logging enabled.
> 
> That way we can implement dirty logging on those platforms sometime when
> it drags down performance, but share the rest of the code with dirty
> logging capable platforms.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  kvm-all.c |   13 ++++++++++++-
>  1 files changed, 12 insertions(+), 1 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 60b76cf..72b7935 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -300,6 +300,7 @@ int kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr,
>      KVMDirtyLog d;
>      KVMSlot *mem;
>      int ret = 0;
> +    int r;
>  
>      d.dirty_bitmap = NULL;
>      while (start_addr < end_addr) {
> @@ -308,6 +309,11 @@ int kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr,
>              break;
>          }
>  
> +        /* We didn't activate dirty logging? Don't care then. */
> +        if(!(mem->flags & KVM_MEM_LOG_DIRTY_PAGES)) {
> +            continue;
> +        }
> +

Isn't this worth a separate patch? Do we already run into this patch on
dirty-logging archs (ie. x86)? I'm still wondering why it's needed.

>          size = ((mem->memory_size >> TARGET_PAGE_BITS) + 7) / 8;
>          if (!d.dirty_bitmap) {
>              d.dirty_bitmap = qemu_malloc(size);
> @@ -319,7 +325,8 @@ int kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr,
>  
>          d.slot = mem->slot;
>  
> -        if (kvm_vm_ioctl(s, KVM_GET_DIRTY_LOG, &d) == -1) {
> +        r = kvm_vm_ioctl(s, KVM_GET_DIRTY_LOG, &d);
> +        if (r == -EINVAL) {

I would say, better exclude 0 and the error you for missing support here
than to assume only EINVAL is a "real" error.

>              dprintf("ioctl failed %d\n", errno);
>              ret = -1;
>              break;
> @@ -335,6 +342,10 @@ int kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr,
>  
>              if ((bitmap[word] >> bit) & 1) {
>                  cpu_physical_memory_set_dirty(addr);
> +            } else if (r < 0) {
> +                /* When our KVM implementation doesn't know about dirty logging
> +                 * we can just assume it's always dirty and be fine. */
> +                cpu_physical_memory_set_dirty(addr);
>              }
>          }
>          start_addr = phys_addr;

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux