[Qemu-devel] [PATCH 00/10] [PULL] qemu-kvm.git uq/master queue

[Qemu-devel] [PATCH 00/10] [PULL] qemu-kvm.git uq/master queue

[Marcelo Tosatti]

The following changes since commit a303f9e37b87ced34e966dc2c0b7f86bc5e74035:
  Blue Swirl (1):
        sh4: remove dead assignments, spotted by clang analyzer

are available in the git repository at:

  git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git uq/master

Jan Kiszka (1):
      KVM: x86: Add debug register saving and restoring

Marcelo Tosatti (8):
      target-i386: print EFER in cpu_dump_state
      kvm: handle internal error
      kvm_init_vcpu requires global lock held
      kvm: remove explicit kvm_arch_reset_vcpu from kvm_init_vcpu
      vga: fix typo in length passed to kvm_log_stop
      introduce leul_to_cpu
      kvm: port qemu-kvm's bitmap scanning
      introduce qemu_ram_map

Sheng Yang (1):
      kvm: allow qemu to set EPT identity mapping address

 bswap.h              |    2 +
 cpu-common.h         |    1 +
 cpus.c               |    2 +-
 exec.c               |   28 ++++++++++++++
 hw/vga.c             |    4 +-
 kvm-all.c            |   96 +++++++++++++++++++++++++++++++++++++++++---------
 kvm.h                |    1 +
 target-i386/helper.c |    1 +
 target-i386/kvm.c    |   81 +++++++++++++++++++++++++++++++++++++++++-
 9 files changed, 195 insertions(+), 21 deletions(-)

[Qemu-devel] [PATCH 01/10] KVM: x86: Add debug register saving and restoring

[Marcelo Tosatti]

From: Jan Kiszka <jan.kiszka@siemens.com>

Make use of the new KVM_GET/SET_DEBUGREGS to save/restore the x86 debug
registers.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
---
 kvm-all.c         |   11 ++++++++++
 kvm.h             |    1 +
 target-i386/kvm.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 0 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 2ede4b9..d050115 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -64,6 +64,7 @@ struct KVMState
     int migration_log;
     int vcpu_events;
     int robust_singlestep;
+    int debugregs;
 #ifdef KVM_CAP_SET_GUEST_DEBUG
     struct kvm_sw_breakpoint_head kvm_sw_breakpoints;
 #endif
@@ -664,6 +665,11 @@ int kvm_init(int smp_cpus)
         kvm_check_extension(s, KVM_CAP_X86_ROBUST_SINGLESTEP);
 #endif
 
+    s->debugregs = 0;
+#ifdef KVM_CAP_DEBUGREGS
+    s->debugregs = kvm_check_extension(s, KVM_CAP_DEBUGREGS);
+#endif
+
     ret = kvm_arch_init(s, smp_cpus);
     if (ret < 0)
         goto err;
@@ -939,6 +945,11 @@ int kvm_has_robust_singlestep(void)
     return kvm_state->robust_singlestep;
 }
 
+int kvm_has_debugregs(void)
+{
+    return kvm_state->debugregs;
+}
+
 void kvm_setup_guest_memory(void *start, size_t size)
 {
     if (!kvm_has_sync_mmu()) {
diff --git a/kvm.h b/kvm.h
index ae87d85..70bfbf8 100644
--- a/kvm.h
+++ b/kvm.h
@@ -40,6 +40,7 @@ int kvm_init(int smp_cpus);
 int kvm_has_sync_mmu(void);
 int kvm_has_vcpu_events(void);
 int kvm_has_robust_singlestep(void);
+int kvm_has_debugregs(void);
 
 #ifdef NEED_CPU_H
 int kvm_init_vcpu(CPUState *env);
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 5513472..bb6dafa 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -874,6 +874,53 @@ static int kvm_guest_debug_workarounds(CPUState *env)
     return ret;
 }
 
+static int kvm_put_debugregs(CPUState *env)
+{
+#ifdef KVM_CAP_DEBUGREGS
+    struct kvm_debugregs dbgregs;
+    int i;
+
+    if (!kvm_has_debugregs()) {
+        return 0;
+    }
+
+    for (i = 0; i < 4; i++) {
+        dbgregs.db[i] = env->dr[i];
+    }
+    dbgregs.dr6 = env->dr[6];
+    dbgregs.dr7 = env->dr[7];
+    dbgregs.flags = 0;
+
+    return kvm_vcpu_ioctl(env, KVM_SET_DEBUGREGS, &dbgregs);
+#else
+    return 0;
+#endif
+}
+
+static int kvm_get_debugregs(CPUState *env)
+{
+#ifdef KVM_CAP_DEBUGREGS
+    struct kvm_debugregs dbgregs;
+    int i, ret;
+
+    if (!kvm_has_debugregs()) {
+        return 0;
+    }
+
+    ret = kvm_vcpu_ioctl(env, KVM_GET_DEBUGREGS, &dbgregs);
+    if (ret < 0) {
+       return ret;
+    }
+    for (i = 0; i < 4; i++) {
+        env->dr[i] = dbgregs.db[i];
+    }
+    env->dr[4] = env->dr[6] = dbgregs.dr6;
+    env->dr[5] = env->dr[7] = dbgregs.dr7;
+#endif
+
+    return 0;
+}
+
 int kvm_arch_put_registers(CPUState *env, int level)
 {
     int ret;
@@ -909,6 +956,10 @@ int kvm_arch_put_registers(CPUState *env, int level)
     if (ret < 0)
         return ret;
 
+    ret = kvm_put_debugregs(env);
+    if (ret < 0)
+        return ret;
+
     return 0;
 }
 
@@ -940,6 +991,10 @@ int kvm_arch_get_registers(CPUState *env)
     if (ret < 0)
         return ret;
 
+    ret = kvm_get_debugregs(env);
+    if (ret < 0)
+        return ret;
+
     return 0;
 }
 
-- 
1.6.6.1

[Qemu-devel] [PATCH 02/10] target-i386: print EFER in cpu_dump_state

[Marcelo Tosatti]

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Avi Kivity <avi@redhat.com>
---
 target-i386/helper.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index 3835835..c9508a8 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -356,6 +356,7 @@ void cpu_dump_state(CPUState *env, FILE *f,
                         cc_op_name);
         }
     }
+    cpu_fprintf(f, "EFER=%016" PRIx64 "\n", env->efer);
     if (flags & X86_DUMP_FPU) {
         int fptag;
         fptag = 0;
-- 
1.6.6.1

[Qemu-devel] [PATCH 03/10] kvm: handle internal error

[Marcelo Tosatti]

Port qemu-kvm's KVM_EXIT_INTERNAL_ERROR handling to upstream.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Avi Kivity <avi@redhat.com>
---
 kvm-all.c |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index d050115..9c8aa7d 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -730,6 +730,32 @@ static int kvm_handle_io(uint16_t port, void *data, int direction, int size,
     return 1;
 }
 
+#ifdef KVM_CAP_INTERNAL_ERROR_DATA
+static void kvm_handle_internal_error(CPUState *env, struct kvm_run *run)
+{
+
+    if (kvm_check_extension(kvm_state, KVM_CAP_INTERNAL_ERROR_DATA)) {
+        int i;
+
+        fprintf(stderr, "KVM internal error. Suberror: %d\n",
+                run->internal.suberror);
+
+        for (i = 0; i < run->internal.ndata; ++i) {
+            fprintf(stderr, "extra data[%d]: %"PRIx64"\n",
+                    i, (uint64_t)run->internal.data[i]);
+        }
+    }
+    cpu_dump_state(env, stderr, fprintf, 0);
+    if (run->internal.suberror == KVM_INTERNAL_ERROR_EMULATION) {
+        fprintf(stderr, "emulation failure\n");
+    }
+    /* FIXME: Should trigger a qmp message to let management know
+     * something went wrong.
+     */
+    vm_stop(0);
+}
+#endif
+
 void kvm_flush_coalesced_mmio_buffer(void)
 {
 #ifdef KVM_CAP_COALESCED_MMIO
@@ -845,6 +871,11 @@ int kvm_cpu_exec(CPUState *env)
         case KVM_EXIT_EXCEPTION:
             DPRINTF("kvm_exit_exception\n");
             break;
+#ifdef KVM_CAP_INTERNAL_ERROR_DATA
+        case KVM_EXIT_INTERNAL_ERROR:
+            kvm_handle_internal_error(env, run);
+            break;
+#endif
         case KVM_EXIT_DEBUG:
             DPRINTF("kvm_exit_debug\n");
 #ifdef KVM_CAP_SET_GUEST_DEBUG
-- 
1.6.6.1

[Qemu-devel] [PATCH 04/10] kvm: allow qemu to set EPT identity mapping address

[Marcelo Tosatti]

From: Sheng Yang <sheng@linux.intel.com>

If we use larger BIOS image than current 256KB, we would need move reserved
TSS and EPT identity mapping pages. Currently TSS support this, but not
EPT.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Avi Kivity <avi@redhat.com>
---
 target-i386/kvm.c |   26 +++++++++++++++++++++++++-
 1 files changed, 25 insertions(+), 1 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index bb6dafa..f73b47b 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -326,6 +326,25 @@ static int kvm_has_msr_star(CPUState *env)
     return 0;
 }
 
+static int kvm_init_identity_map_page(KVMState *s)
+{
+#ifdef KVM_CAP_SET_IDENTITY_MAP_ADDR
+    int ret;
+    uint64_t addr = 0xfffbc000;
+
+    if (!kvm_check_extension(s, KVM_CAP_SET_IDENTITY_MAP_ADDR)) {
+        return 0;
+    }
+
+    ret = kvm_vm_ioctl(s, KVM_SET_IDENTITY_MAP_ADDR, &addr);
+    if (ret < 0) {
+        fprintf(stderr, "kvm_set_identity_map_addr: %s\n", strerror(ret));
+        return ret;
+    }
+#endif
+    return 0;
+}
+
 int kvm_arch_init(KVMState *s, int smp_cpus)
 {
     int ret;
@@ -353,7 +372,12 @@ int kvm_arch_init(KVMState *s, int smp_cpus)
         perror("e820_add_entry() table is full");
         exit(1);
     }
-    return kvm_vm_ioctl(s, KVM_SET_TSS_ADDR, 0xfffbd000);
+    ret = kvm_vm_ioctl(s, KVM_SET_TSS_ADDR, 0xfffbd000);
+    if (ret < 0) {
+        return ret;
+    }
+
+    return kvm_init_identity_map_page(s);
 }
                     
 static void set_v8086_seg(struct kvm_segment *lhs, const SegmentCache *rhs)
-- 
1.6.6.1

[Qemu-devel] [PATCH 05/10] kvm_init_vcpu requires global lock held

[Marcelo Tosatti]

Since it accesses data protected by the lock.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 cpus.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/cpus.c b/cpus.c
index 8450ee4..2bf87d2 100644
--- a/cpus.c
+++ b/cpus.c
@@ -401,6 +401,7 @@ static void *kvm_cpu_thread_fn(void *arg)
 {
     CPUState *env = arg;
 
+    qemu_mutex_lock(&qemu_global_mutex);
     qemu_thread_self(env->thread);
     if (kvm_enabled())
         kvm_init_vcpu(env);
@@ -408,7 +409,6 @@ static void *kvm_cpu_thread_fn(void *arg)
     kvm_block_io_signals(env);
 
     /* signal CPU creation */
-    qemu_mutex_lock(&qemu_global_mutex);
     env->created = 1;
     qemu_cond_signal(&qemu_cpu_cond);
 
-- 
1.6.6.1

[Qemu-devel] [PATCH 06/10] kvm: remove explicit kvm_arch_reset_vcpu from kvm_init_vcpu

[Marcelo Tosatti]

This is now done via the initialization's qemu_system_reset call.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 kvm-all.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 9c8aa7d..eabb097 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -208,7 +208,6 @@ int kvm_init_vcpu(CPUState *env)
     ret = kvm_arch_init_vcpu(env);
     if (ret == 0) {
         qemu_register_reset(kvm_reset_vcpu, env);
-        kvm_arch_reset_vcpu(env);
     }
 err:
     return ret;
-- 
1.6.6.1

[Qemu-devel] Re: [PATCH 06/10] kvm: remove explicit kvm_arch_reset_vcpu from kvm_init_vcpu

[Anthony Liguori]

On 04/26/2010 12:59 PM, Marcelo Tosatti wrote:
> This is now done via the initialization's qemu_system_reset call.
>
> Signed-off-by: Avi Kivity<avi@redhat.com>
> ---
>   kvm-all.c |    1 -
>   1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/kvm-all.c b/kvm-all.c
> index 9c8aa7d..eabb097 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -208,7 +208,6 @@ int kvm_init_vcpu(CPUState *env)
>       ret = kvm_arch_init_vcpu(env);
>       if (ret == 0) {
>           qemu_register_reset(kvm_reset_vcpu, env);
> -        kvm_arch_reset_vcpu(env);
>       }
>   err:
>       return ret;
>    

This breaks -enable-kvm in upstream qemu.  No progress is ever made in 
the guest.

Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [PATCH 06/10] kvm: remove explicit kvm_arch_reset_vcpu from kvm_init_vcpu

[Marcelo Tosatti]

On Wed, Apr 28, 2010 at 10:39:06AM -0500, Anthony Liguori wrote:
> On 04/26/2010 12:59 PM, Marcelo Tosatti wrote:
> >This is now done via the initialization's qemu_system_reset call.
> >
> >Signed-off-by: Avi Kivity<avi@redhat.com>
> >---
> >  kvm-all.c |    1 -
> >  1 files changed, 0 insertions(+), 1 deletions(-)
> >
> >diff --git a/kvm-all.c b/kvm-all.c
> >index 9c8aa7d..eabb097 100644
> >--- a/kvm-all.c
> >+++ b/kvm-all.c
> >@@ -208,7 +208,6 @@ int kvm_init_vcpu(CPUState *env)
> >      ret = kvm_arch_init_vcpu(env);
> >      if (ret == 0) {
> >          qemu_register_reset(kvm_reset_vcpu, env);
> >-        kvm_arch_reset_vcpu(env);
> >      }
> >  err:
> >      return ret;
> 
> This breaks -enable-kvm in upstream qemu.  No progress is ever made
> in the guest.

These patches have been autotested, and just confirmed manually that    
this patch does not break -enable-kvm (as the reset handler is now 
called through the notifier).

What is your qemu command line?

Re: [Qemu-devel] Re: [PATCH 06/10] kvm: remove explicit kvm_arch_reset_vcpu from kvm_init_vcpu

[Marcelo Tosatti]

On Wed, Apr 28, 2010 at 01:22:14PM -0300, Marcelo Tosatti wrote:
> On Wed, Apr 28, 2010 at 10:39:06AM -0500, Anthony Liguori wrote:
> > On 04/26/2010 12:59 PM, Marcelo Tosatti wrote:
> > >This is now done via the initialization's qemu_system_reset call.
> > >
> > >Signed-off-by: Avi Kivity<avi@redhat.com>
> > >---
> > >  kvm-all.c |    1 -
> > >  1 files changed, 0 insertions(+), 1 deletions(-)
> > >
> > >diff --git a/kvm-all.c b/kvm-all.c
> > >index 9c8aa7d..eabb097 100644
> > >--- a/kvm-all.c
> > >+++ b/kvm-all.c
> > >@@ -208,7 +208,6 @@ int kvm_init_vcpu(CPUState *env)
> > >      ret = kvm_arch_init_vcpu(env);
> > >      if (ret == 0) {
> > >          qemu_register_reset(kvm_reset_vcpu, env);
> > >-        kvm_arch_reset_vcpu(env);
> > >      }
> > >  err:
> > >      return ret;
> > 
> > This breaks -enable-kvm in upstream qemu.  No progress is ever made
> > in the guest.
> 
> These patches have been autotested, and just confirmed manually that    
> this patch does not break -enable-kvm (as the reset handler is now 
> called through the notifier).
> 
> What is your qemu command line?

Just drop it please.

Re: [Qemu-devel] Re: [PATCH 06/10] kvm: remove explicit kvm_arch_reset_vcpu from kvm_init_vcpu

[Anthony Liguori]

On 04/28/2010 11:22 AM, Marcelo Tosatti wrote:
> On Wed, Apr 28, 2010 at 10:39:06AM -0500, Anthony Liguori wrote:
>    
>> On 04/26/2010 12:59 PM, Marcelo Tosatti wrote:
>>      
>>> This is now done via the initialization's qemu_system_reset call.
>>>
>>> Signed-off-by: Avi Kivity<avi@redhat.com>
>>> ---
>>>   kvm-all.c |    1 -
>>>   1 files changed, 0 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/kvm-all.c b/kvm-all.c
>>> index 9c8aa7d..eabb097 100644
>>> --- a/kvm-all.c
>>> +++ b/kvm-all.c
>>> @@ -208,7 +208,6 @@ int kvm_init_vcpu(CPUState *env)
>>>       ret = kvm_arch_init_vcpu(env);
>>>       if (ret == 0) {
>>>           qemu_register_reset(kvm_reset_vcpu, env);
>>> -        kvm_arch_reset_vcpu(env);
>>>       }
>>>   err:
>>>       return ret;
>>>        
>> This breaks -enable-kvm in upstream qemu.  No progress is ever made
>> in the guest.
>>      
> These patches have been autotested, and just confirmed manually that
> this patch does not break -enable-kvm (as the reset handler is now
> called through the notifier).
>
> What is your qemu command line?
>    

x86_64-softmmu/qemu-system-x86_64 -hda ~/images/linux.img -snapshot 
-enable-kvm -L ~/git/qemu/pc-bios

This is with the very latest tip.  I can publish a branch if that's 
helpful.  If you have an old tree, I wouldn't be surprised if it didn't 
show up.

Regards,

Anthony Liguori

[Qemu-devel] [PATCH 07/10] vga: fix typo in length passed to kvm_log_stop

[Marcelo Tosatti]

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
---
 hw/vga.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/vga.c b/hw/vga.c
index 845dbcc..db72115 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -1618,8 +1618,8 @@ void vga_dirty_log_stop(VGACommonState *s)
 	kvm_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, 0x80000);
-	kvm_log_stop(isa_mem_base + 0xa8000, 0x80000);
+	kvm_log_stop(isa_mem_base + 0xa0000, 0x8000);
+	kvm_log_stop(isa_mem_base + 0xa8000, 0x8000);
     }
 
 #ifdef CONFIG_BOCHS_VBE
-- 
1.6.6.1

[Qemu-devel] [PATCH 08/10] introduce leul_to_cpu

[Marcelo Tosatti]

To be used by next patch.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
---
 bswap.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/bswap.h b/bswap.h
index aace9b7..956f3fa 100644
--- a/bswap.h
+++ b/bswap.h
@@ -205,8 +205,10 @@ static inline void cpu_to_be32wu(uint32_t *p, uint32_t v)
 
 #ifdef HOST_WORDS_BIGENDIAN
 #define cpu_to_32wu cpu_to_be32wu
+#define leul_to_cpu(v) le ## HOST_LONG_BITS ## _to_cpu(v)
 #else
 #define cpu_to_32wu cpu_to_le32wu
+#define leul_to_cpu(v) (v)
 #endif
 
 #undef le_bswap
-- 
1.6.6.1

[Qemu-devel] [PATCH 09/10] kvm: port qemu-kvm's bitmap scanning

[Marcelo Tosatti]

Which is significantly faster.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
---
 kvm-all.c |   53 +++++++++++++++++++++++++++++++++++++----------------
 1 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index eabb097..ef45418 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -26,6 +26,7 @@
 #include "hw/hw.h"
 #include "gdbstub.h"
 #include "kvm.h"
+#include "bswap.h"
 
 /* KVM uses PAGE_SIZE in it's definition of COALESCED_MMIO_MAX */
 #define PAGE_SIZE TARGET_PAGE_SIZE
@@ -282,11 +283,41 @@ static int kvm_set_migration_log(int enable)
     return 0;
 }
 
-static int test_le_bit(unsigned long nr, unsigned char *addr)
+/* get kvm's dirty pages bitmap and update qemu's */
+static int kvm_get_dirty_pages_log_range(unsigned long start_addr,
+                                         unsigned long *bitmap,
+                                         unsigned long offset,
+                                         unsigned long mem_size)
 {
-    return (addr[nr >> 3] >> (nr & 7)) & 1;
+    unsigned int i, j;
+    unsigned long page_number, addr, addr1, c;
+    ram_addr_t ram_addr;
+    unsigned int len = ((mem_size / TARGET_PAGE_SIZE) + HOST_LONG_BITS - 1) /
+        HOST_LONG_BITS;
+
+    /*
+     * bitmap-traveling is faster than memory-traveling (for addr...)
+     * especially when most of the memory is not dirty.
+     */
+    for (i = 0; i < len; i++) {
+        if (bitmap[i] != 0) {
+            c = leul_to_cpu(bitmap[i]);
+            do {
+                j = ffsl(c) - 1;
+                c &= ~(1ul << j);
+                page_number = i * HOST_LONG_BITS + j;
+                addr1 = page_number * TARGET_PAGE_SIZE;
+                addr = offset + addr1;
+                ram_addr = cpu_get_physical_page_desc(addr);
+                cpu_physical_memory_set_dirty(ram_addr);
+            } while (c != 0);
+        }
+    }
+    return 0;
 }
 
+#define ALIGN(x, y)  (((x)+(y)-1) & ~((y)-1))
+
 /**
  * kvm_physical_sync_dirty_bitmap - Grab dirty bitmap from kernel space
  * This function updates qemu's dirty bitmap using cpu_physical_memory_set_dirty().
@@ -300,8 +331,6 @@ static int kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr,
 {
     KVMState *s = kvm_state;
     unsigned long size, allocated_size = 0;
-    target_phys_addr_t phys_addr;
-    ram_addr_t addr;
     KVMDirtyLog d;
     KVMSlot *mem;
     int ret = 0;
@@ -313,7 +342,7 @@ static int kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr,
             break;
         }
 
-        size = ((mem->memory_size >> TARGET_PAGE_BITS) + 7) / 8;
+        size = ALIGN(((mem->memory_size) >> TARGET_PAGE_BITS), HOST_LONG_BITS) / 8;
         if (!d.dirty_bitmap) {
             d.dirty_bitmap = qemu_malloc(size);
         } else if (size > allocated_size) {
@@ -330,17 +359,9 @@ static int kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr,
             break;
         }
 
-        for (phys_addr = mem->start_addr, addr = mem->phys_offset;
-             phys_addr < mem->start_addr + mem->memory_size;
-             phys_addr += TARGET_PAGE_SIZE, addr += TARGET_PAGE_SIZE) {
-            unsigned char *bitmap = (unsigned char *)d.dirty_bitmap;
-            unsigned nr = (phys_addr - mem->start_addr) >> TARGET_PAGE_BITS;
-
-            if (test_le_bit(nr, bitmap)) {
-                cpu_physical_memory_set_dirty(addr);
-            }
-        }
-        start_addr = phys_addr;
+        kvm_get_dirty_pages_log_range(mem->start_addr, d.dirty_bitmap,
+                                      mem->start_addr, mem->memory_size);
+        start_addr = mem->start_addr + mem->memory_size;
     }
     qemu_free(d.dirty_bitmap);
 
-- 
1.6.6.1

[Qemu-devel] Re: [PATCH 09/10] kvm: port qemu-kvm's bitmap scanning

[Yoshiaki Tamura]

Hi,

This patch may conflict with the patch I posted on April 19.

http://www.mail-archive.com/qemu-devel@nongnu.org/msg29941.html

If Marcelo's is going to be merged, I need to rebase the above to it.
It would be helpful if you could tell me the plan.

Thanks,

Yoshi

2010/4/27 Marcelo Tosatti <mtosatti@redhat.com>:
> Which is significantly faster.
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> ---
>  kvm-all.c |   53 +++++++++++++++++++++++++++++++++++++----------------
>  1 files changed, 37 insertions(+), 16 deletions(-)
>
> diff --git a/kvm-all.c b/kvm-all.c
> index eabb097..ef45418 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -26,6 +26,7 @@
>  #include "hw/hw.h"
>  #include "gdbstub.h"
>  #include "kvm.h"
> +#include "bswap.h"
>
>  /* KVM uses PAGE_SIZE in it's definition of COALESCED_MMIO_MAX */
>  #define PAGE_SIZE TARGET_PAGE_SIZE
> @@ -282,11 +283,41 @@ static int kvm_set_migration_log(int enable)
>     return 0;
>  }
>
> -static int test_le_bit(unsigned long nr, unsigned char *addr)
> +/* get kvm's dirty pages bitmap and update qemu's */
> +static int kvm_get_dirty_pages_log_range(unsigned long start_addr,
> +                                         unsigned long *bitmap,
> +                                         unsigned long offset,
> +                                         unsigned long mem_size)
>  {
> -    return (addr[nr >> 3] >> (nr & 7)) & 1;
> +    unsigned int i, j;
> +    unsigned long page_number, addr, addr1, c;
> +    ram_addr_t ram_addr;
> +    unsigned int len = ((mem_size / TARGET_PAGE_SIZE) + HOST_LONG_BITS - 1) /
> +        HOST_LONG_BITS;
> +
> +    /*
> +     * bitmap-traveling is faster than memory-traveling (for addr...)
> +     * especially when most of the memory is not dirty.
> +     */
> +    for (i = 0; i < len; i++) {
> +        if (bitmap[i] != 0) {
> +            c = leul_to_cpu(bitmap[i]);
> +            do {
> +                j = ffsl(c) - 1;
> +                c &= ~(1ul << j);
> +                page_number = i * HOST_LONG_BITS + j;
> +                addr1 = page_number * TARGET_PAGE_SIZE;
> +                addr = offset + addr1;
> +                ram_addr = cpu_get_physical_page_desc(addr);
> +                cpu_physical_memory_set_dirty(ram_addr);
> +            } while (c != 0);
> +        }
> +    }
> +    return 0;
>  }
>
> +#define ALIGN(x, y)  (((x)+(y)-1) & ~((y)-1))
> +
>  /**
>  * kvm_physical_sync_dirty_bitmap - Grab dirty bitmap from kernel space
>  * This function updates qemu's dirty bitmap using cpu_physical_memory_set_dirty().
> @@ -300,8 +331,6 @@ static int kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr,
>  {
>     KVMState *s = kvm_state;
>     unsigned long size, allocated_size = 0;
> -    target_phys_addr_t phys_addr;
> -    ram_addr_t addr;
>     KVMDirtyLog d;
>     KVMSlot *mem;
>     int ret = 0;
> @@ -313,7 +342,7 @@ static int kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr,
>             break;
>         }
>
> -        size = ((mem->memory_size >> TARGET_PAGE_BITS) + 7) / 8;
> +        size = ALIGN(((mem->memory_size) >> TARGET_PAGE_BITS), HOST_LONG_BITS) / 8;
>         if (!d.dirty_bitmap) {
>             d.dirty_bitmap = qemu_malloc(size);
>         } else if (size > allocated_size) {
> @@ -330,17 +359,9 @@ static int kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr,
>             break;
>         }
>
> -        for (phys_addr = mem->start_addr, addr = mem->phys_offset;
> -             phys_addr < mem->start_addr + mem->memory_size;
> -             phys_addr += TARGET_PAGE_SIZE, addr += TARGET_PAGE_SIZE) {
> -            unsigned char *bitmap = (unsigned char *)d.dirty_bitmap;
> -            unsigned nr = (phys_addr - mem->start_addr) >> TARGET_PAGE_BITS;
> -
> -            if (test_le_bit(nr, bitmap)) {
> -                cpu_physical_memory_set_dirty(addr);
> -            }
> -        }
> -        start_addr = phys_addr;
> +        kvm_get_dirty_pages_log_range(mem->start_addr, d.dirty_bitmap,
> +                                      mem->start_addr, mem->memory_size);
> +        start_addr = mem->start_addr + mem->memory_size;
>     }
>     qemu_free(d.dirty_bitmap);
>
> --
> 1.6.6.1

[Qemu-devel] Re: [PATCH 09/10] kvm: port qemu-kvm's bitmap scanning

[Marcelo Tosatti]

On Tue, Apr 27, 2010 at 01:49:15PM +0900, Yoshiaki Tamura wrote:
> Hi,
> 
> This patch may conflict with the patch I posted on April 19.
> 
> http://www.mail-archive.com/qemu-devel@nongnu.org/msg29941.html
> 
> If Marcelo's is going to be merged, I need to rebase the above to it.
> It would be helpful if you could tell me the plan.
> 
> Thanks,
> 
> Yoshi

Yoshi,

Whatever gets in first, the other patch will have to rebased. I'm
fine either way.

[Qemu-devel] Re: [PATCH 09/10] kvm: port qemu-kvm's bitmap scanning

[Yoshiaki Tamura]

Marcelo Tosatti wrote:
> On Tue, Apr 27, 2010 at 01:49:15PM +0900, Yoshiaki Tamura wrote:
>> Hi,
>>
>> This patch may conflict with the patch I posted on April 19.
>>
>> http://www.mail-archive.com/qemu-devel@nongnu.org/msg29941.html
>>
>> If Marcelo's is going to be merged, I need to rebase the above to it.
>> It would be helpful if you could tell me the plan.
>>
>> Thanks,
>>
>> Yoshi
>
> Yoshi,
>
> Whatever gets in first, the other patch will have to rebased. I'm
> fine either way.

I'm happy to rebase too.  Wanted to know if there is a plan, so that I can work 
beforehand, but never mind.

[Qemu-devel] [PATCH 10/10] introduce qemu_ram_map

[Marcelo Tosatti]

Which allows drivers to register an mmaped region into ram block mappings.
To be used by device assignment driver.

CC: Cam Macdonell <cam@cs.ualberta.ca>
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
---
 cpu-common.h |    1 +
 exec.c       |   28 ++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/cpu-common.h b/cpu-common.h
index b24cecc..2dfde6f 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -40,6 +40,7 @@ static inline void cpu_register_physical_memory(target_phys_addr_t start_addr,
 }
 
 ram_addr_t cpu_get_physical_page_desc(target_phys_addr_t addr);
+ram_addr_t qemu_ram_map(ram_addr_t size, void *host);
 ram_addr_t qemu_ram_alloc(ram_addr_t);
 void qemu_ram_free(ram_addr_t addr);
 /* This should only be used for ram local to a device.  */
diff --git a/exec.c b/exec.c
index 14d1fd7..648a9c9 100644
--- a/exec.c
+++ b/exec.c
@@ -2789,6 +2789,34 @@ static void *file_ram_alloc(ram_addr_t memory, const char *path)
 }
 #endif
 
+ram_addr_t qemu_ram_map(ram_addr_t size, void *host)
+{
+    RAMBlock *new_block;
+
+    size = TARGET_PAGE_ALIGN(size);
+    new_block = qemu_malloc(sizeof(*new_block));
+
+    new_block->host = host;
+
+    new_block->offset = last_ram_offset;
+    new_block->length = size;
+
+    new_block->next = ram_blocks;
+    ram_blocks = new_block;
+
+    phys_ram_dirty = qemu_realloc(phys_ram_dirty,
+        (last_ram_offset + size) >> TARGET_PAGE_BITS);
+    memset(phys_ram_dirty + (last_ram_offset >> TARGET_PAGE_BITS),
+           0xff, size >> TARGET_PAGE_BITS);
+
+    last_ram_offset += size;
+
+    if (kvm_enabled())
+        kvm_setup_guest_memory(new_block->host, size);
+
+    return new_block->offset;
+}
+
 ram_addr_t qemu_ram_alloc(ram_addr_t size)
 {
     RAMBlock *new_block;
-- 
1.6.6.1

[Qemu-devel] Re: [PATCH 10/10] introduce qemu_ram_map

[Anthony Liguori]

On 04/26/2010 12:59 PM, Marcelo Tosatti wrote:
> Which allows drivers to register an mmaped region into ram block mappings.
> To be used by device assignment driver.
>    

This doesn't make much sense to me.

Do you use this like:

qemu_ram_map(64k, ptr);
assert(qemu_ram_alloc(64k) == ptr);

If so, I think this is not the best API.  I'd rather see qemu_ram_map() 
register a symbolic name for the region and for there to be a 
qemu_ram_alloc() variant that allocated by name.

Regards,

Anthony Liguori

> CC: Cam Macdonell<cam@cs.ualberta.ca>
> Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com>
> ---
>   cpu-common.h |    1 +
>   exec.c       |   28 ++++++++++++++++++++++++++++
>   2 files changed, 29 insertions(+), 0 deletions(-)
>
> diff --git a/cpu-common.h b/cpu-common.h
> index b24cecc..2dfde6f 100644
> --- a/cpu-common.h
> +++ b/cpu-common.h
> @@ -40,6 +40,7 @@ static inline void cpu_register_physical_memory(target_phys_addr_t start_addr,
>   }
>
>   ram_addr_t cpu_get_physical_page_desc(target_phys_addr_t addr);
> +ram_addr_t qemu_ram_map(ram_addr_t size, void *host);
>   ram_addr_t qemu_ram_alloc(ram_addr_t);
>   void qemu_ram_free(ram_addr_t addr);
>   /* This should only be used for ram local to a device.  */
> diff --git a/exec.c b/exec.c
> index 14d1fd7..648a9c9 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2789,6 +2789,34 @@ static void *file_ram_alloc(ram_addr_t memory, const char *path)
>   }
>   #endif
>
> +ram_addr_t qemu_ram_map(ram_addr_t size, void *host)
> +{
> +    RAMBlock *new_block;
> +
> +    size = TARGET_PAGE_ALIGN(size);
> +    new_block = qemu_malloc(sizeof(*new_block));
> +
> +    new_block->host = host;
> +
> +    new_block->offset = last_ram_offset;
> +    new_block->length = size;
> +
> +    new_block->next = ram_blocks;
> +    ram_blocks = new_block;
> +
> +    phys_ram_dirty = qemu_realloc(phys_ram_dirty,
> +        (last_ram_offset + size)>>  TARGET_PAGE_BITS);
> +    memset(phys_ram_dirty + (last_ram_offset>>  TARGET_PAGE_BITS),
> +           0xff, size>>  TARGET_PAGE_BITS);
> +
> +    last_ram_offset += size;
> +
> +    if (kvm_enabled())
> +        kvm_setup_guest_memory(new_block->host, size);
> +
> +    return new_block->offset;
> +}
> +
>   ram_addr_t qemu_ram_alloc(ram_addr_t size)
>   {
>       RAMBlock *new_block;
>    

[Qemu-devel] Re: [PATCH 10/10] introduce qemu_ram_map

[Marcelo Tosatti]

On Mon, Apr 26, 2010 at 01:27:30PM -0500, Anthony Liguori wrote:
> On 04/26/2010 12:59 PM, Marcelo Tosatti wrote:
> >Which allows drivers to register an mmaped region into ram block mappings.
> >To be used by device assignment driver.
> 
> This doesn't make much sense to me.
> 
> Do you use this like:
> 
> qemu_ram_map(64k, ptr);
> assert(qemu_ram_alloc(64k) == ptr);

No. hw/device-assignment.c in qemu-kvm mmaps
/sys/bus/pci/devices/x:y:z/resourcen (the PCI devices memory regions) to
the guest.

> If so, I think this is not the best API.  I'd rather see
> qemu_ram_map() register a symbolic name for the region and for there
> to be a qemu_ram_alloc() variant that allocated by name.
> 
> Regards,
> 
> Anthony Liguori
> 

[Qemu-devel] Re: [PATCH 10/10] introduce qemu_ram_map

[Anthony Liguori]

On 04/26/2010 01:49 PM, Marcelo Tosatti wrote:
> On Mon, Apr 26, 2010 at 01:27:30PM -0500, Anthony Liguori wrote:
>    
>> On 04/26/2010 12:59 PM, Marcelo Tosatti wrote:
>>      
>>> Which allows drivers to register an mmaped region into ram block mappings.
>>> To be used by device assignment driver.
>>>        
>> This doesn't make much sense to me.
>>
>> Do you use this like:
>>
>> qemu_ram_map(64k, ptr);
>> assert(qemu_ram_alloc(64k) == ptr);
>>      
> No. hw/device-assignment.c in qemu-kvm mmaps
> /sys/bus/pci/devices/x:y:z/resourcen (the PCI devices memory regions) to
> the guest.
>    

I understand, but how do you use qemu_ram_map() to actually map that 
memory to a given PCI device resource?  I assume you rely on it getting 
put on the front of the list so that the next qemu_ram_alloc() will be 
at that location.

Regards,

Anthony Liguori

>> If so, I think this is not the best API.  I'd rather see
>> qemu_ram_map() register a symbolic name for the region and for there
>> to be a qemu_ram_alloc() variant that allocated by name.
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>      

[Qemu-devel] Re: [PATCH 10/10] introduce qemu_ram_map

[Cam Macdonell]

On Mon, Apr 26, 2010 at 12:54 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 04/26/2010 01:49 PM, Marcelo Tosatti wrote:
>>
>> On Mon, Apr 26, 2010 at 01:27:30PM -0500, Anthony Liguori wrote:
>>
>>>
>>> On 04/26/2010 12:59 PM, Marcelo Tosatti wrote:
>>>
>>>>
>>>> Which allows drivers to register an mmaped region into ram block
>>>> mappings.
>>>> To be used by device assignment driver.
>>>>
>>>
>>> This doesn't make much sense to me.
>>>
>>> Do you use this like:
>>>
>>> qemu_ram_map(64k, ptr);
>>> assert(qemu_ram_alloc(64k) == ptr);
>>>
>>
>> No. hw/device-assignment.c in qemu-kvm mmaps
>> /sys/bus/pci/devices/x:y:z/resourcen (the PCI devices memory regions) to
>> the guest.
>>
>
> I understand, but how do you use qemu_ram_map() to actually map that memory
> to a given PCI device resource?  I assume you rely on it getting put on the
> front of the list so that the next qemu_ram_alloc() will be at that
> location.

In my shared memory patch, I passed the offset returned from
qemu_ram_mmap to cpu_register_physical_memory from within the map
function passed to pci_register_bar.  Could the same not be done?  Is
there something incorrect with this approach?

>
> Regards,
>
> Anthony Liguori
>
>>> If so, I think this is not the best API.  I'd rather see
>>> qemu_ram_map() register a symbolic name for the region and for there
>>> to be a qemu_ram_alloc() variant that allocated by name.
>>>
>>> Regards,
>>>
>>> Anthony Liguori
>>>
>>>
>
>

[Qemu-devel] Re: [PATCH 10/10] introduce qemu_ram_map

[Marcelo Tosatti]

On Tue, Apr 27, 2010 at 08:28:15AM -0600, Cam Macdonell wrote:
> On Mon, Apr 26, 2010 at 12:54 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> > On 04/26/2010 01:49 PM, Marcelo Tosatti wrote:
> >>
> >> On Mon, Apr 26, 2010 at 01:27:30PM -0500, Anthony Liguori wrote:
> >>
> >>>
> >>> On 04/26/2010 12:59 PM, Marcelo Tosatti wrote:
> >>>
> >>>>
> >>>> Which allows drivers to register an mmaped region into ram block
> >>>> mappings.
> >>>> To be used by device assignment driver.
> >>>>
> >>>
> >>> This doesn't make much sense to me.
> >>>
> >>> Do you use this like:
> >>>
> >>> qemu_ram_map(64k, ptr);
> >>> assert(qemu_ram_alloc(64k) == ptr);
> >>>
> >>
> >> No. hw/device-assignment.c in qemu-kvm mmaps
> >> /sys/bus/pci/devices/x:y:z/resourcen (the PCI devices memory regions) to
> >> the guest.
> >>
> >
> > I understand, but how do you use qemu_ram_map() to actually map that memory
> > to a given PCI device resource?  I assume you rely on it getting put on the
> > front of the list so that the next qemu_ram_alloc() will be at that
> > location.
> 
> In my shared memory patch, I passed the offset returned from
> qemu_ram_mmap to cpu_register_physical_memory from within the map
> function passed to pci_register_bar.  Could the same not be done?  Is
> there something incorrect with this approach?

No, its correct. Its similar to what hw/device-assignment.c in qemu-kvm 
will do.

[Qemu-devel] Re: [PATCH 10/10] introduce qemu_ram_map

[Anthony Liguori]

On 04/26/2010 12:59 PM, Marcelo Tosatti wrote:
> Which allows drivers to register an mmaped region into ram block mappings.
> To be used by device assignment driver.
>
>    

This is not kvm specific and not required by this pull request so it 
shouldn't really be part of the pull.  Something like this should only 
be added when there's an actual consumer.

Regards,

Anthony Liguori

> CC: Cam Macdonell<cam@cs.ualberta.ca>
> Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com>
> ---
>   cpu-common.h |    1 +
>   exec.c       |   28 ++++++++++++++++++++++++++++
>   2 files changed, 29 insertions(+), 0 deletions(-)
>
> diff --git a/cpu-common.h b/cpu-common.h
> index b24cecc..2dfde6f 100644
> --- a/cpu-common.h
> +++ b/cpu-common.h
> @@ -40,6 +40,7 @@ static inline void cpu_register_physical_memory(target_phys_addr_t start_addr,
>   }
>
>   ram_addr_t cpu_get_physical_page_desc(target_phys_addr_t addr);
> +ram_addr_t qemu_ram_map(ram_addr_t size, void *host);
>   ram_addr_t qemu_ram_alloc(ram_addr_t);
>   void qemu_ram_free(ram_addr_t addr);
>   /* This should only be used for ram local to a device.  */
> diff --git a/exec.c b/exec.c
> index 14d1fd7..648a9c9 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2789,6 +2789,34 @@ static void *file_ram_alloc(ram_addr_t memory, const char *path)
>   }
>   #endif
>
> +ram_addr_t qemu_ram_map(ram_addr_t size, void *host)
> +{
> +    RAMBlock *new_block;
> +
> +    size = TARGET_PAGE_ALIGN(size);
> +    new_block = qemu_malloc(sizeof(*new_block));
> +
> +    new_block->host = host;
> +
> +    new_block->offset = last_ram_offset;
> +    new_block->length = size;
> +
> +    new_block->next = ram_blocks;
> +    ram_blocks = new_block;
> +
> +    phys_ram_dirty = qemu_realloc(phys_ram_dirty,
> +        (last_ram_offset + size)>>  TARGET_PAGE_BITS);
> +    memset(phys_ram_dirty + (last_ram_offset>>  TARGET_PAGE_BITS),
> +           0xff, size>>  TARGET_PAGE_BITS);
> +
> +    last_ram_offset += size;
> +
> +    if (kvm_enabled())
> +        kvm_setup_guest_memory(new_block->host, size);
> +
> +    return new_block->offset;
> +}
> +
>   ram_addr_t qemu_ram_alloc(ram_addr_t size)
>   {
>       RAMBlock *new_block;
>    

[Qemu-devel] Re: [PATCH 10/10] introduce qemu_ram_map

[Marcelo Tosatti]

On Mon, Apr 26, 2010 at 01:29:06PM -0500, Anthony Liguori wrote:
> On 04/26/2010 12:59 PM, Marcelo Tosatti wrote:
> >Which allows drivers to register an mmaped region into ram block mappings.
> >To be used by device assignment driver.
> >
> 
> This is not kvm specific and not required by this pull request so it
> shouldn't really be part of the pull.  Something like this should
> only be added when there's an actual consumer.

The user will be hw/device-assignment.c in qemu-kvm. And also Cam has
the need for a similar interface for shared memory drivers.

Index: qemu-kvm/hw/device-assignment.c
===================================================================
--- qemu-kvm.orig/hw/device-assignment.c	2010-04-22 16:21:30.000000000 -0400
+++ qemu-kvm/hw/device-assignment.c	2010-04-22 17:36:57.000000000 -0400
@@ -256,10 +256,7 @@
     AssignedDevice *r_dev = container_of(pci_dev, AssignedDevice, dev);
     AssignedDevRegion *region = &r_dev->v_addrs[region_num];
     PCIRegion *real_region = &r_dev->real_device.regions[region_num];
-    pcibus_t old_ephys = region->e_physbase;
-    pcibus_t old_esize = region->e_size;
-    int first_map = (region->e_size == 0);
-    int ret = 0;
+    int ret = 0, flags = 0;
 
     DEBUG("e_phys=%08" FMT_PCIBUS " r_virt=%p type=%d len=%08" FMT_PCIBUS " region_num=%d \n",
           e_phys, region->u.r_virtbase, type, e_size, region_num);
@@ -267,30 +264,22 @@
     region->e_physbase = e_phys;
     region->e_size = e_size;
 
-    if (!first_map)
-	kvm_destroy_phys_mem(kvm_context, old_ephys,
-                             TARGET_PAGE_ALIGN(old_esize));
-
     if (e_size > 0) {
+
+        if (region_num == PCI_ROM_SLOT)
+            flags |= IO_MEM_ROM;
+
+        cpu_register_physical_memory(e_phys, e_size, region->memory_index | flags);
+
         /* deal with MSI-X MMIO page */
         if (real_region->base_addr <= r_dev->msix_table_addr &&
                 real_region->base_addr + real_region->size >=
                 r_dev->msix_table_addr) {
             int offset = r_dev->msix_table_addr - real_region->base_addr;
-            ret = munmap(region->u.r_virtbase + offset, TARGET_PAGE_SIZE);
-            if (ret == 0)
-                DEBUG("munmap done, virt_base 0x%p\n",
-                        region->u.r_virtbase + offset);
-            else {
-                fprintf(stderr, "%s: fail munmap msix table!\n", __func__);
-                exit(1);
-            }
+
             cpu_register_physical_memory(e_phys + offset,
                     TARGET_PAGE_SIZE, r_dev->mmio_index);
         }
-	ret = kvm_register_phys_mem(kvm_context, e_phys,
-                                    region->u.r_virtbase,
-                                    TARGET_PAGE_ALIGN(e_size), 0);
     }
 
     if (ret != 0) {
@@ -539,6 +528,15 @@
             pci_dev->v_addrs[i].u.r_virtbase +=
                 (cur_region->base_addr & 0xFFF);
 
+
+            if (!slow_map) {
+                void *virtbase = pci_dev->v_addrs[i].u.r_virtbase;
+
+                pci_dev->v_addrs[i].memory_index = qemu_ram_map(cur_region->size,
+                                                                virtbase);
+            } else
+                pci_dev->v_addrs[i].memory_index = 0;
+
             pci_register_bar((PCIDevice *) pci_dev, i,
                              cur_region->size, t,
                              slow_map ? assigned_dev_iomem_map_slow
@@ -726,10 +724,6 @@
                 kvm_remove_ioperm_data(region->u.r_baseport, region->r_size);
                 continue;
             } else if (pci_region->type & IORESOURCE_MEM) {
-                if (region->e_size > 0)
-                    kvm_destroy_phys_mem(kvm_context, region->e_physbase,
-                                         TARGET_PAGE_ALIGN(region->e_size));
-
                 if (region->u.r_virtbase) {
                     int ret = munmap(region->u.r_virtbase,
                                      (pci_region->size + 0xFFF) & 0xFFFFF000);
Index: qemu-kvm/hw/device-assignment.h
===================================================================
--- qemu-kvm.orig/hw/device-assignment.h	2010-04-22 16:21:30.000000000 -0400
+++ qemu-kvm/hw/device-assignment.h	2010-04-22 16:24:32.000000000 -0400
@@ -63,7 +63,7 @@
 
 typedef struct {
     pcibus_t e_physbase;
-    uint32_t memory_index;
+    ram_addr_t memory_index;
     union {
         void *r_virtbase;    /* mmapped access address for memory regions */
         uint32_t r_baseport; /* the base guest port for I/O regions */

[Qemu-devel] Re: [PATCH 10/10] introduce qemu_ram_map

[Anthony Liguori]

On 04/26/2010 01:50 PM, Marcelo Tosatti wrote:
> On Mon, Apr 26, 2010 at 01:29:06PM -0500, Anthony Liguori wrote:
>    
>> On 04/26/2010 12:59 PM, Marcelo Tosatti wrote:
>>      
>>> Which allows drivers to register an mmaped region into ram block mappings.
>>> To be used by device assignment driver.
>>>
>>>        
>> This is not kvm specific and not required by this pull request so it
>> shouldn't really be part of the pull.  Something like this should
>> only be added when there's an actual consumer.
>>      
> The user will be hw/device-assignment.c in qemu-kvm. And also Cam has
> the need for a similar interface for shared memory drivers.
>    

It should be part of one of those submissions.

> Index: qemu-kvm/hw/device-assignment.c
> ===================================================================
> --- qemu-kvm.orig/hw/device-assignment.c	2010-04-22 16:21:30.000000000 -0400
> +++ qemu-kvm/hw/device-assignment.c	2010-04-22 17:36:57.000000000 -0400
> @@ -256,10 +256,7 @@
>       AssignedDevice *r_dev = container_of(pci_dev, AssignedDevice, dev);
>       AssignedDevRegion *region =&r_dev->v_addrs[region_num];
>       PCIRegion *real_region =&r_dev->real_device.regions[region_num];
> -    pcibus_t old_ephys = region->e_physbase;
> -    pcibus_t old_esize = region->e_size;
> -    int first_map = (region->e_size == 0);
> -    int ret = 0;
> +    int ret = 0, flags = 0;
>
>       DEBUG("e_phys=%08" FMT_PCIBUS " r_virt=%p type=%d len=%08" FMT_PCIBUS " region_num=%d \n",
>             e_phys, region->u.r_virtbase, type, e_size, region_num);
> @@ -267,30 +264,22 @@
>       region->e_physbase = e_phys;
>       region->e_size = e_size;
>
> -    if (!first_map)
> -	kvm_destroy_phys_mem(kvm_context, old_ephys,
> -                             TARGET_PAGE_ALIGN(old_esize));
> -
>       if (e_size>  0) {
> +
> +        if (region_num == PCI_ROM_SLOT)
> +            flags |= IO_MEM_ROM;
> +
> +        cpu_register_physical_memory(e_phys, e_size, region->memory_index | flags);
> +
>           /* deal with MSI-X MMIO page */
>           if (real_region->base_addr<= r_dev->msix_table_addr&&
>                   real_region->base_addr + real_region->size>=
>                   r_dev->msix_table_addr) {
>               int offset = r_dev->msix_table_addr - real_region->base_addr;
> -            ret = munmap(region->u.r_virtbase + offset, TARGET_PAGE_SIZE);
> -            if (ret == 0)
> -                DEBUG("munmap done, virt_base 0x%p\n",
> -                        region->u.r_virtbase + offset);
> -            else {
> -                fprintf(stderr, "%s: fail munmap msix table!\n", __func__);
> -                exit(1);
> -            }
> +
>               cpu_register_physical_memory(e_phys + offset,
>                       TARGET_PAGE_SIZE, r_dev->mmio_index);
>           }
> -	ret = kvm_register_phys_mem(kvm_context, e_phys,
> -                                    region->u.r_virtbase,
> -                                    TARGET_PAGE_ALIGN(e_size), 0);
>       }
>
>       if (ret != 0) {
> @@ -539,6 +528,15 @@
>               pci_dev->v_addrs[i].u.r_virtbase +=
>                   (cur_region->base_addr&  0xFFF);
>
> +
> +            if (!slow_map) {
> +                void *virtbase = pci_dev->v_addrs[i].u.r_virtbase;
> +
> +                pci_dev->v_addrs[i].memory_index = qemu_ram_map(cur_region->size,
> +                                                                virtbase);
> +            } else
> +                pci_dev->v_addrs[i].memory_index = 0;
> +
>               pci_register_bar((PCIDevice *) pci_dev, i,
>                                cur_region->size, t,
>                                slow_map ? assigned_dev_iomem_map_slow
> @@ -726,10 +724,6 @@
>                   kvm_remove_ioperm_data(region->u.r_baseport, region->r_size);
>                   continue;
>               } else if (pci_region->type&  IORESOURCE_MEM) {
> -                if (region->e_size>  0)
> -                    kvm_destroy_phys_mem(kvm_context, region->e_physbase,
> -                                         TARGET_PAGE_ALIGN(region->e_size));
> -
>                   if (region->u.r_virtbase) {
>                       int ret = munmap(region->u.r_virtbase,
>                                        (pci_region->size + 0xFFF)&  0xFFFFF000);
>    

How does hot unplug get dealt with?

Regards,

Anthony Liguori

> Index: qemu-kvm/hw/device-assignment.h
> ===================================================================
> --- qemu-kvm.orig/hw/device-assignment.h	2010-04-22 16:21:30.000000000 -0400
> +++ qemu-kvm/hw/device-assignment.h	2010-04-22 16:24:32.000000000 -0400
> @@ -63,7 +63,7 @@
>
>   typedef struct {
>       pcibus_t e_physbase;
> -    uint32_t memory_index;
> +    ram_addr_t memory_index;
>       union {
>           void *r_virtbase;    /* mmapped access address for memory regions */
>           uint32_t r_baseport; /* the base guest port for I/O regions */
>    

[Qemu-devel] Re: [PATCH 10/10] introduce qemu_ram_map

[Marcelo Tosatti]

On Mon, Apr 26, 2010 at 01:57:37PM -0500, Anthony Liguori wrote:
> On 04/26/2010 01:50 PM, Marcelo Tosatti wrote:
> >On Mon, Apr 26, 2010 at 01:29:06PM -0500, Anthony Liguori wrote:
> >>On 04/26/2010 12:59 PM, Marcelo Tosatti wrote:
> >>>Which allows drivers to register an mmaped region into ram block mappings.
> >>>To be used by device assignment driver.
> >>>
> >>This is not kvm specific and not required by this pull request so it
> >>shouldn't really be part of the pull.  Something like this should
> >>only be added when there's an actual consumer.
> >The user will be hw/device-assignment.c in qemu-kvm. And also Cam has
> >the need for a similar interface for shared memory drivers.
> 
> It should be part of one of those submissions.

OK

> >@@ -726,10 +724,6 @@
> >                  kvm_remove_ioperm_data(region->u.r_baseport, region->r_size);
> >                  continue;
> >              } else if (pci_region->type&  IORESOURCE_MEM) {
> >-                if (region->e_size>  0)
> >-                    kvm_destroy_phys_mem(kvm_context, region->e_physbase,
> >-                                         TARGET_PAGE_ALIGN(region->e_size));
> >-
> >                  if (region->u.r_virtbase) {
> >                      int ret = munmap(region->u.r_virtbase,
> >                                       (pci_region->size + 0xFFF)&  0xFFFFF000);
> 
> How does hot unplug get dealt with?

The regions will have such mappings unmapped from QEMU (and KVM) via
cpu_register_physical_memory(IO_MEM_UNASSIGNED) via
pci_unregister_io_regions.

Just pushed a new tree without the patch, please pull if you
are OK with the other changes.

Re: [Qemu-devel] Re: [PATCH 10/10] introduce qemu_ram_map

[Anthony Liguori]

On 04/26/2010 02:14 PM, Marcelo Tosatti wrote:
> On Mon, Apr 26, 2010 at 01:57:37PM -0500, Anthony Liguori wrote:
>    
>> On 04/26/2010 01:50 PM, Marcelo Tosatti wrote:
>>      
>>> On Mon, Apr 26, 2010 at 01:29:06PM -0500, Anthony Liguori wrote:
>>>        
>>>> On 04/26/2010 12:59 PM, Marcelo Tosatti wrote:
>>>>          
>>>>> Which allows drivers to register an mmaped region into ram block mappings.
>>>>> To be used by device assignment driver.
>>>>>
>>>>>            
>>>> This is not kvm specific and not required by this pull request so it
>>>> shouldn't really be part of the pull.  Something like this should
>>>> only be added when there's an actual consumer.
>>>>          
>>> The user will be hw/device-assignment.c in qemu-kvm. And also Cam has
>>> the need for a similar interface for shared memory drivers.
>>>        
>> It should be part of one of those submissions.
>>      
> OK
>
>    
>>> @@ -726,10 +724,6 @@
>>>                   kvm_remove_ioperm_data(region->u.r_baseport, region->r_size);
>>>                   continue;
>>>               } else if (pci_region->type&   IORESOURCE_MEM) {
>>> -                if (region->e_size>   0)
>>> -                    kvm_destroy_phys_mem(kvm_context, region->e_physbase,
>>> -                                         TARGET_PAGE_ALIGN(region->e_size));
>>> -
>>>                   if (region->u.r_virtbase) {
>>>                       int ret = munmap(region->u.r_virtbase,
>>>                                        (pci_region->size + 0xFFF)&   0xFFFFF000);
>>>        
>> How does hot unplug get dealt with?
>>      
> The regions will have such mappings unmapped from QEMU (and KVM) via
> cpu_register_physical_memory(IO_MEM_UNASSIGNED) via
> pci_unregister_io_regions.
>    

But how do you qemu_ram_unmap()?  I see you munmap() that address but it 
looks like the qemu ram region gets leaked pointing to an invalid pointer.

Regards,

Anthony Liguori

> Just pushed a new tree without the patch, please pull if you
> are OK with the other changes.
>    

Yes, I am.
>
>    

Re: [Qemu-devel] Re: [PATCH 10/10] introduce qemu_ram_map

[Marcelo Tosatti]

On Mon, Apr 26, 2010 at 02:20:42PM -0500, Anthony Liguori wrote:
> On 04/26/2010 02:14 PM, Marcelo Tosatti wrote:
> >On Mon, Apr 26, 2010 at 01:57:37PM -0500, Anthony Liguori wrote:
> >>On 04/26/2010 01:50 PM, Marcelo Tosatti wrote:
> >>>On Mon, Apr 26, 2010 at 01:29:06PM -0500, Anthony Liguori wrote:
> >>>>On 04/26/2010 12:59 PM, Marcelo Tosatti wrote:
> >>>>>Which allows drivers to register an mmaped region into ram block mappings.
> >>>>>To be used by device assignment driver.
> >>>>>
> >>>>This is not kvm specific and not required by this pull request so it
> >>>>shouldn't really be part of the pull.  Something like this should
> >>>>only be added when there's an actual consumer.
> >>>The user will be hw/device-assignment.c in qemu-kvm. And also Cam has
> >>>the need for a similar interface for shared memory drivers.
> >>It should be part of one of those submissions.
> >OK
> >
> >>>@@ -726,10 +724,6 @@
> >>>                  kvm_remove_ioperm_data(region->u.r_baseport, region->r_size);
> >>>                  continue;
> >>>              } else if (pci_region->type&   IORESOURCE_MEM) {
> >>>-                if (region->e_size>   0)
> >>>-                    kvm_destroy_phys_mem(kvm_context, region->e_physbase,
> >>>-                                         TARGET_PAGE_ALIGN(region->e_size));
> >>>-
> >>>                  if (region->u.r_virtbase) {
> >>>                      int ret = munmap(region->u.r_virtbase,
> >>>                                       (pci_region->size + 0xFFF)&   0xFFFFF000);
> >>How does hot unplug get dealt with?
> >The regions will have such mappings unmapped from QEMU (and KVM) via
> >cpu_register_physical_memory(IO_MEM_UNASSIGNED) via
> >pci_unregister_io_regions.
> 
> But how do you qemu_ram_unmap()?  I see you munmap() that address
> but it looks like the qemu ram region gets leaked pointing to an
> invalid pointer.

Yes, qemu_ram_free() is not implemented. last_ram_offset always moves
forward. But there should be no references to the memory mapping
anymore, after the device is hot-unplugged.

[Qemu-devel] Re: [PATCH 10/10] introduce qemu_ram_map

[Cam Macdonell]

On Mon, Apr 26, 2010 at 11:59 AM, Marcelo Tosatti <mtosatti@redhat.com> wrote:
> Which allows drivers to register an mmaped region into ram block mappings.
> To be used by device assignment driver.
>
> CC: Cam Macdonell <cam@cs.ualberta.ca>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> ---
>  cpu-common.h |    1 +
>  exec.c       |   28 ++++++++++++++++++++++++++++
>  2 files changed, 29 insertions(+), 0 deletions(-)
>
> diff --git a/cpu-common.h b/cpu-common.h
> index b24cecc..2dfde6f 100644
> --- a/cpu-common.h
> +++ b/cpu-common.h
> @@ -40,6 +40,7 @@ static inline void cpu_register_physical_memory(target_phys_addr_t start_addr,
>  }
>
>  ram_addr_t cpu_get_physical_page_desc(target_phys_addr_t addr);
> +ram_addr_t qemu_ram_map(ram_addr_t size, void *host);
>  ram_addr_t qemu_ram_alloc(ram_addr_t);
>  void qemu_ram_free(ram_addr_t addr);
>  /* This should only be used for ram local to a device.  */
> diff --git a/exec.c b/exec.c
> index 14d1fd7..648a9c9 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2789,6 +2789,34 @@ static void *file_ram_alloc(ram_addr_t memory, const char *path)
>  }
>  #endif
>
> +ram_addr_t qemu_ram_map(ram_addr_t size, void *host)
> +{
> +    RAMBlock *new_block;
> +
> +    size = TARGET_PAGE_ALIGN(size);
> +    new_block = qemu_malloc(sizeof(*new_block));
> +
> +    new_block->host = host;
> +
> +    new_block->offset = last_ram_offset;
> +    new_block->length = size;
> +
> +    new_block->next = ram_blocks;
> +    ram_blocks = new_block;
> +
> +    phys_ram_dirty = qemu_realloc(phys_ram_dirty,
> +        (last_ram_offset + size) >> TARGET_PAGE_BITS);
> +    memset(phys_ram_dirty + (last_ram_offset >> TARGET_PAGE_BITS),
> +           0xff, size >> TARGET_PAGE_BITS);
> +
> +    last_ram_offset += size;
> +
> +    if (kvm_enabled())
> +        kvm_setup_guest_memory(new_block->host, size);
> +
> +    return new_block->offset;
> +}
> +
>  ram_addr_t qemu_ram_alloc(ram_addr_t size)
>  {
>     RAMBlock *new_block;
> --
> 1.6.6.1
>

Sorry for being late to reply, is there a strong reason not to have
the function handle the mmap itself?  As As Anthony points out, that
way we don't have worry about realloc changing the pointer in the
function.

[Qemu-devel] Re: [PATCH 10/10] introduce qemu_ram_map

[Marcelo Tosatti]

On Tue, Apr 27, 2010 at 08:32:27AM -0600, Cam Macdonell wrote:
> >
> > +ram_addr_t qemu_ram_map(ram_addr_t size, void *host)
> > +{
> > +    RAMBlock *new_block;
> > +
> > +    size = TARGET_PAGE_ALIGN(size);
> > +    new_block = qemu_malloc(sizeof(*new_block));
> > +
> > +    new_block->host = host;
> > +
> > +    new_block->offset = last_ram_offset;
> > +    new_block->length = size;
> > +
> > +    new_block->next = ram_blocks;
> > +    ram_blocks = new_block;
> > +
> > +    phys_ram_dirty = qemu_realloc(phys_ram_dirty,
> > +        (last_ram_offset + size) >> TARGET_PAGE_BITS);
> > +    memset(phys_ram_dirty + (last_ram_offset >> TARGET_PAGE_BITS),
> > +           0xff, size >> TARGET_PAGE_BITS);
> > +
> > +    last_ram_offset += size;
> > +
> > +    if (kvm_enabled())
> > +        kvm_setup_guest_memory(new_block->host, size);
> > +
> > +    return new_block->offset;
> > +}
> > +
> >  ram_addr_t qemu_ram_alloc(ram_addr_t size)
> >  {
> >     RAMBlock *new_block;
> > --
> > 1.6.6.1
> >
> 
> Sorry for being late to reply, is there a strong reason not to have
> the function handle the mmap itself?  As As Anthony points out, that
> way we don't have worry about realloc changing the pointer in the
> function.

The caller might want a different protection for the memory map.

[Qemu-devel] Re: [PATCH 00/10] [PULL] qemu-kvm.git uq/master queue

[Anthony Liguori]

On 04/26/2010 12:58 PM, Marcelo Tosatti wrote:
> The following changes since commit a303f9e37b87ced34e966dc2c0b7f86bc5e74035:
>    Blue Swirl (1):
>          sh4: remove dead assignments, spotted by clang analyzer
>
> are available in the git repository at:
>
>    git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git uq/master
>    

Pulled (after you dropped 10/10 and 6/10).  Thanks!

Regards,

Anthony Liguori

> Jan Kiszka (1):
>        KVM: x86: Add debug register saving and restoring
>
> Marcelo Tosatti (8):
>        target-i386: print EFER in cpu_dump_state
>        kvm: handle internal error
>        kvm_init_vcpu requires global lock held
>        kvm: remove explicit kvm_arch_reset_vcpu from kvm_init_vcpu
>        vga: fix typo in length passed to kvm_log_stop
>        introduce leul_to_cpu
>        kvm: port qemu-kvm's bitmap scanning
>        introduce qemu_ram_map
>
> Sheng Yang (1):
>        kvm: allow qemu to set EPT identity mapping address
>
>   bswap.h              |    2 +
>   cpu-common.h         |    1 +
>   cpus.c               |    2 +-
>   exec.c               |   28 ++++++++++++++
>   hw/vga.c             |    4 +-
>   kvm-all.c            |   96 +++++++++++++++++++++++++++++++++++++++++---------
>   kvm.h                |    1 +
>   target-i386/helper.c |    1 +
>   target-i386/kvm.c    |   81 +++++++++++++++++++++++++++++++++++++++++-
>   9 files changed, 195 insertions(+), 21 deletions(-)
>