WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: [PATCH V2 1/3] Introduce log_start/log_stop in CPUPhysMe

To: anthony.perard@xxxxxxxxxx
Subject: [Xen-devel] Re: [PATCH V2 1/3] Introduce log_start/log_stop in CPUPhysMemoryClient
From: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
Date: Tue, 18 Jan 2011 19:20:35 +0100
Cc: Xen Devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, QEMU-devel <qemu-devel@xxxxxxxxxx>, Marcelo Tosatti <mtosatti@xxxxxxxxxx>
Delivery-date: Tue, 18 Jan 2011 10:21:29 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1295353522-31891-1-git-send-email-anthony.perard@xxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <alpine.DEB.1.10.1101171811180.784@xxxxxxxxxxxxxxxxxxxxxxx> <1295353522-31891-1-git-send-email-anthony.perard@xxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666
Send patch updates always with proper "[PATCH]" tag, they may get lost
otherwise.

On 2011-01-18 13:25, anthony.perard@xxxxxxxxxx wrote:
> From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> 
> In order to use log_start/log_stop with Xen as well in the vga code,
> this two operations have been put in CPUPhysMemoryClient.
> 
> The two new functions cpu_physical_log_start,cpu_physical_log_stop are
> used in hw/vga.c and replace the kvm_log_start/stop. With this, vga does
> no longer depends on kvm header.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> ---
>  cpu-all.h    |    6 ++++++
>  cpu-common.h |    4 ++++
>  exec.c       |   28 ++++++++++++++++++++++++++++
>  hw/vga.c     |   31 ++++++++++++++++---------------
>  hw/vhost.c   |    2 ++
>  kvm-all.c    |    8 ++++++--
>  kvm-stub.c   |   10 ----------
>  kvm.h        |    3 ---
>  8 files changed, 62 insertions(+), 30 deletions(-)
> 
> diff --git a/cpu-all.h b/cpu-all.h
> index 30ae17d..aaf5442 100644
> --- a/cpu-all.h
> +++ b/cpu-all.h
> @@ -957,6 +957,12 @@ int cpu_physical_memory_get_dirty_tracking(void);
>  int cpu_physical_sync_dirty_bitmap(target_phys_addr_t start_addr,
>                                     target_phys_addr_t end_addr);
>  
> +int cpu_physical_log_start(target_phys_addr_t start_addr,
> +                           ram_addr_t size);
> +
> +int cpu_physical_log_stop(target_phys_addr_t start_addr,
> +                           ram_addr_t size);
> +
>  void dump_exec_info(FILE *f, fprintf_function cpu_fprintf);
>  #endif /* !CONFIG_USER_ONLY */
>  
> diff --git a/cpu-common.h b/cpu-common.h
> index 8ec01f4..2344842 100644
> --- a/cpu-common.h
> +++ b/cpu-common.h
> @@ -88,6 +88,10 @@ struct CPUPhysMemoryClient {
>                               target_phys_addr_t end_addr);
>      int (*migration_log)(struct CPUPhysMemoryClient *client,
>                           int enable);
> +    int (*log_start)(struct CPUPhysMemoryClient *client,
> +                     target_phys_addr_t phys_addr, ram_addr_t size);
> +    int (*log_stop)(struct CPUPhysMemoryClient *client,
> +                    target_phys_addr_t phys_addr, ram_addr_t size);
>      QLIST_ENTRY(CPUPhysMemoryClient) list;
>  };
>  
> diff --git a/exec.c b/exec.c
> index c6ed96d..59a6426 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2073,6 +2073,34 @@ int cpu_physical_sync_dirty_bitmap(target_phys_addr_t 
> start_addr,
>      return ret;
>  }
>  
> +int cpu_physical_log_start(target_phys_addr_t start_addr,
> +                           ram_addr_t size)
> +{
> +    CPUPhysMemoryClient *client;
> +    QLIST_FOREACH(client, &memory_client_list, list) {
> +        if (client->log_start) {
> +            int r = client->log_start(client, start_addr, size);
> +            if (r < 0)
> +                return r;
> +        }
> +    }
> +    return 0;
> +}
> +
> +int cpu_physical_log_stop(target_phys_addr_t start_addr,
> +                          ram_addr_t size)
> +{
> +    CPUPhysMemoryClient *client;
> +    QLIST_FOREACH(client, &memory_client_list, list) {
> +        if (client->log_stop) {
> +            int r = client->log_stop(client, start_addr, size);
> +            if (r < 0)
> +                return r;
> +        }
> +    }
> +    return 0;
> +}
> +
>  static inline void tlb_update_dirty(CPUTLBEntry *tlb_entry)
>  {
>      ram_addr_t ram_addr;
> diff --git a/hw/vga.c b/hw/vga.c
> index c057f4f..a9bf172 100644
> --- a/hw/vga.c
> +++ b/hw/vga.c
> @@ -28,7 +28,6 @@
>  #include "vga_int.h"
>  #include "pixel_ops.h"
>  #include "qemu-timer.h"
> -#include "kvm.h"
>  
>  //#define DEBUG_VGA
>  //#define DEBUG_VGA_MEM
> @@ -1597,34 +1596,36 @@ static void vga_sync_dirty_bitmap(VGACommonState *s)
>  
>  void vga_dirty_log_start(VGACommonState *s)
>  {
> -    if (kvm_enabled() && s->map_addr)
> -        kvm_log_start(s->map_addr, s->map_end - s->map_addr);
> +    if (s->map_addr) {
> +        cpu_physical_log_start(s->map_addr, s->map_end - s->map_addr);
> +    }
>  
> -    if (kvm_enabled() && s->lfb_vram_mapped) {
> -        kvm_log_start(isa_mem_base + 0xa0000, 0x8000);
> -        kvm_log_start(isa_mem_base + 0xa8000, 0x8000);
> +    if (s->lfb_vram_mapped) {
> +        cpu_physical_log_start(isa_mem_base + 0xa0000, 0x8000);
> +        cpu_physical_log_start(isa_mem_base + 0xa8000, 0x8000);
>      }
>  
>  #ifdef CONFIG_BOCHS_VBE
> -    if (kvm_enabled() && s->vbe_mapped) {
> -        kvm_log_start(VBE_DISPI_LFB_PHYSICAL_ADDRESS, s->vram_size);
> +    if (s->vbe_mapped) {
> +        cpu_physical_log_start(VBE_DISPI_LFB_PHYSICAL_ADDRESS, s->vram_size);
>      }
>  #endif
>  }
>  
>  void vga_dirty_log_stop(VGACommonState *s)
>  {
> -    if (kvm_enabled() && s->map_addr)
> -     kvm_log_stop(s->map_addr, s->map_end - s->map_addr);
> +    if (s->map_addr) {
> +     cpu_physical_log_stop(s->map_addr, s->map_end - s->map_addr);
> +    }
>  
> -    if (kvm_enabled() && s->lfb_vram_mapped) {
> -     kvm_log_stop(isa_mem_base + 0xa0000, 0x8000);
> -     kvm_log_stop(isa_mem_base + 0xa8000, 0x8000);
> +    if (s->lfb_vram_mapped) {
> +     cpu_physical_log_stop(isa_mem_base + 0xa0000, 0x8000);
> +     cpu_physical_log_stop(isa_mem_base + 0xa8000, 0x8000);
>      }
>  
>  #ifdef CONFIG_BOCHS_VBE
> -    if (kvm_enabled() && s->vbe_mapped) {
> -     kvm_log_stop(VBE_DISPI_LFB_PHYSICAL_ADDRESS, s->vram_size);
> +    if (s->vbe_mapped) {
> +     cpu_physical_log_stop(VBE_DISPI_LFB_PHYSICAL_ADDRESS, s->vram_size);
>      }

Sorry in case they were present earlier and I missed them, but here are
tabs included that should be removed from new code. There is a
check-patch script floating around on the list, I think it's not yet
merged though.

>  #endif
>  }
> diff --git a/hw/vhost.c b/hw/vhost.c
> index 8586f66..932b372 100644
> --- a/hw/vhost.c
> +++ b/hw/vhost.c
> @@ -606,6 +606,8 @@ int vhost_dev_init(struct vhost_dev *hdev, int devfd)
>      hdev->client.set_memory = vhost_client_set_memory;
>      hdev->client.sync_dirty_bitmap = vhost_client_sync_dirty_bitmap;
>      hdev->client.migration_log = vhost_client_migration_log;
> +    hdev->client.log_start = NULL;
> +    hdev->client.log_stop = NULL;
>      hdev->mem = qemu_mallocz(offsetof(struct vhost_memory, regions));
>      hdev->log = NULL;
>      hdev->log_size = 0;
> diff --git a/kvm-all.c b/kvm-all.c
> index 37b99c7..aedfe33 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -267,14 +267,16 @@ static int 
> kvm_dirty_pages_log_change(target_phys_addr_t phys_addr,
>      return kvm_set_user_memory_region(s, mem);
>  }
>  
> -int kvm_log_start(target_phys_addr_t phys_addr, ram_addr_t size)
> +static int kvm_log_start(CPUPhysMemoryClient *client,
> +                         target_phys_addr_t phys_addr, ram_addr_t size)
>  {
>          return kvm_dirty_pages_log_change(phys_addr, size,
>                                            KVM_MEM_LOG_DIRTY_PAGES,
>                                            KVM_MEM_LOG_DIRTY_PAGES);
>  }
>  
> -int kvm_log_stop(target_phys_addr_t phys_addr, ram_addr_t size)
> +static int kvm_log_stop(CPUPhysMemoryClient *client,
> +                        target_phys_addr_t phys_addr, ram_addr_t size)
>  {
>          return kvm_dirty_pages_log_change(phys_addr, size,
>                                            0,
> @@ -596,6 +598,8 @@ static CPUPhysMemoryClient kvm_cpu_phys_memory_client = {
>       .set_memory = kvm_client_set_memory,
>       .sync_dirty_bitmap = kvm_client_sync_dirty_bitmap,
>       .migration_log = kvm_client_migration_log,
> +        .log_start = kvm_log_start,
> +        .log_stop = kvm_log_stop,
>  };
>  
>  int kvm_init(int smp_cpus)
> diff --git a/kvm-stub.c b/kvm-stub.c
> index 5384a4b..41324e4 100644
> --- a/kvm-stub.c
> +++ b/kvm-stub.c
> @@ -33,16 +33,6 @@ int kvm_init_vcpu(CPUState *env)
>      return -ENOSYS;
>  }
>  
> -int kvm_log_start(target_phys_addr_t phys_addr, ram_addr_t size)
> -{
> -    return -ENOSYS;
> -}
> -
> -int kvm_log_stop(target_phys_addr_t phys_addr, ram_addr_t size)
> -{
> -    return -ENOSYS;
> -}
> -
>  int kvm_coalesce_mmio_region(target_phys_addr_t start, ram_addr_t size)
>  {
>      return -ENOSYS;
> diff --git a/kvm.h b/kvm.h
> index 60a9b42..fc77b81 100644
> --- a/kvm.h
> +++ b/kvm.h
> @@ -49,9 +49,6 @@ int kvm_init_vcpu(CPUState *env);
>  int kvm_cpu_exec(CPUState *env);
>  
>  #if !defined(CONFIG_USER_ONLY)
> -int kvm_log_start(target_phys_addr_t phys_addr, ram_addr_t size);
> -int kvm_log_stop(target_phys_addr_t phys_addr, ram_addr_t size);
> -
>  void kvm_setup_guest_memory(void *start, size_t size);
>  
>  int kvm_coalesce_mmio_region(target_phys_addr_t start, ram_addr_t size);

Besides the tabs, I'm fine with it:
Reviewed-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>

Maybe this patch should be picked up by qemu-kvm's upstream branch (it
will definitely collide with my coding style patch if not more, but
that's easily resolvable there).

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel