Skip to contentSkip to conversation

qemu-kvm.git uq/master queue

GIT PULL In review Marcelo Tosatti 5 participants 32 messages Apr 26, 2010 – May 3, 2010 KVM

lore
Share & tools
lore
#0 lore raw

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(-)
#1 lore raw

PATCH 1/10 KVM: x86: Add debug register saving and restoring

From: Jan Kiszka <[email protected]>

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

---
 kvm-all.c         |   11 ++++++++++
 kvm.h             |    1 +
 target-i386/kvm.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 0 deletions(-)
kvm-all.c +11 -0
index 2ede4b9..d050115 100644
@@ -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()) {
kvm.h +1 -0
index ae87d85..70bfbf8 100644
@@ -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);
target-i386/kvm.c +55 -0

73 lines not on this page · raw message

Signature · 1 line
1.6.6.1
#2 lore raw

PATCH 2/10 target-i386: print EFER in cpu_dump_state

In reply to Marcelo Tosatti
---
 target-i386/helper.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
target-i386/helper.c +1 -0
index 3835835..c9508a8 100644
@@ -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;
Signature · 1 line
1.6.6.1
#3 lore raw

PATCH 3/10 kvm: handle internal error

In reply to Marcelo Tosatti

Port qemu-kvm's KVM_EXIT_INTERNAL_ERROR handling to upstream.

---
 kvm-all.c |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)
kvm-all.c +31 -0
index d050115..9c8aa7d 100644
@@ -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
Signature · 1 line
1.6.6.1
#4 lore raw

PATCH 4/10 kvm: allow qemu to set EPT identity mapping address

In reply to Marcelo Tosatti

From: Sheng Yang <[email protected]>

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.

---
 target-i386/kvm.c |   26 +++++++++++++++++++++++++-
 1 files changed, 25 insertions(+), 1 deletions(-)
target-i386/kvm.c +25 -1
index bb6dafa..f73b47b 100644
@@ -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)
Signature · 1 line
1.6.6.1
#5 lore raw

PATCH 5/10 kvm_init_vcpu requires global lock held

In reply to Marcelo Tosatti

Since it accesses data protected by the lock.

---
 cpus.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
cpus.c +1 -1
index 8450ee4..2bf87d2 100644
@@ -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);
 
Signature · 1 line
1.6.6.1
#6 lore raw

PATCH 6/10 kvm: remove explicit kvm_arch_reset_vcpu from kvm_init_vcpu

In reply to Marcelo Tosatti

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

---
 kvm-all.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
kvm-all.c +0 -1
index 9c8aa7d..eabb097 100644
@@ -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;
Signature · 1 line
1.6.6.1
#7 lore raw

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

Quoted 13 lines · Marcelo Tosatti

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

---
  kvm-all.c |    1 -
  1 files changed, 0 insertions(+), 1 deletions(-)
kvm-all.c +0 -0
index 9c8aa7d..eabb097 100644

kvm-all.c int kvm_init_vcpu(CPUState *env) · line 213

Quoted 7 lines
kvm-all.c +0 -1
@@ -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

#8 lore raw

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

Quoted 8 lines · Anthony Liguori
Quoted 7 lines · Marcelo Tosatti

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

---
 kvm-all.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
Quoted 15 lines

kvm-all.c int kvm_init_vcpu(CPUState *env) · line 213

Quoted 12 lines
kvm-all.c +0 -1
index 9c8aa7d..eabb097 100644
@@ -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?

#9 lore raw

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

Quoted 27 lines · Marcelo Tosatti
Quoted 9 lines · Anthony Liguori
Quoted 7 lines · Marcelo Tosatti

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

---
  kvm-all.c |    1 -
  1 files changed, 0 insertions(+), 1 deletions(-)
Quoted 16 lines

kvm-all.c int kvm_init_vcpu(CPUState *env) · line 213

Quoted 13 lines
kvm-all.c +0 -1
index 9c8aa7d..eabb097 100644
@@ -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.

Quoted 6 lines

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

#10 lore raw

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

In reply to Marcelo Tosatti
Quoted 25 lines · Marcelo Tosatti
Quoted 8 lines · Anthony Liguori
Quoted 7 lines · Marcelo Tosatti

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

---
 kvm-all.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
Quoted 15 lines

kvm-all.c int kvm_init_vcpu(CPUState *env) · line 213

Quoted 12 lines
kvm-all.c +0 -1
index 9c8aa7d..eabb097 100644
@@ -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.

Quoted 5 lines

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.

#11 lore raw

PATCH 7/10 vga: fix typo in length passed to kvm_log_stop

In reply to Marcelo Tosatti
---
 hw/vga.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
hw/vga.c +2 -2
index 845dbcc..db72115 100644
@@ -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
Signature · 1 line
1.6.6.1
#12 lore raw

PATCH 8/10 introduce leul_to_cpu

In reply to Marcelo Tosatti

To be used by next patch.

---
 bswap.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
bswap.h +2 -0
index aace9b7..956f3fa 100644
@@ -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
Signature · 1 line
1.6.6.1
#13 lore raw

PATCH 9/10 kvm: port qemu-kvm's bitmap scanning

In reply to Marcelo Tosatti

Which is significantly faster.

---
 kvm-all.c |   53 +++++++++++++++++++++++++++++++++++++----------------
 1 files changed, 37 insertions(+), 16 deletions(-)
kvm-all.c +37 -16
index eabb097..ef45418 100644
@@ -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);
 
Signature · 1 line
1.6.6.1
#14 lore raw

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

Hi,

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

http://www.mail-archive.com/[email protected]/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 <[email protected]>:

Quoted 104 lines

Which is significantly faster.

---
 kvm-all.c |   53 +++++++++++++++++++++++++++++++++++++----------------

 1 files changed, 37 insertions(+), 16 deletions(-)
kvm-all.c +37 -17
index eabb097..ef45418 100644
@@ -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

#15 lore raw

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

Quoted 4 lines · Yoshiaki Tamura

Hi,

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

Quoted 8 lines

http://www.mail-archive.com/[email protected]/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.

#16 lore raw

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

Quoted 10 lines · Marcelo Tosatti
Quoted 9 lines · Yoshiaki Tamura

Hi,

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

http://www.mail-archive.com/[email protected]/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.

Quoted 8 lines
Quoted 3 lines

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.

#17 lore raw

PATCH 10/10 introduce qemu_ram_map

In reply to Marcelo Tosatti

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

---
 cpu-common.h |    1 +
 exec.c       |   28 ++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 0 deletions(-)
cpu-common.h +1 -0
index b24cecc..2dfde6f 100644
@@ -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.  */
exec.c +28 -0
index 14d1fd7..648a9c9 100644
@@ -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;
Signature · 1 line
1.6.6.1
#18 lore raw

Re: [PATCH 10/10] introduce qemu_ram_map

Quoted 3 lines · Marcelo Tosatti

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

Quoted 59 lines
---
  cpu-common.h |    1 +
  exec.c       |   28 ++++++++++++++++++++++++++++
  2 files changed, 29 insertions(+), 0 deletions(-)
cpu-common.h +1 -0
index b24cecc..2dfde6f 100644
@@ -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.  */
exec.c +28 -0
index 14d1fd7..648a9c9 100644
@@ -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;
#19 lore raw

Re: [PATCH 10/10] introduce qemu_ram_map

Quoted 10 lines · Anthony Liguori
Quoted 2 lines · Marcelo Tosatti

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.

Quoted 8 lines

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

#20 lore raw

Re: [PATCH 10/10] introduce qemu_ram_map

Quoted 9 lines · Marcelo Tosatti
Quoted 7 lines · Anthony Liguori
Quoted 3 lines · Marcelo Tosatti

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.

Quoted 9 lines
Quoted 5 lines

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

Quoted 9 lines
Quoted 9 lines

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

#21 lore raw

Re: [PATCH 10/10] introduce qemu_ram_map

Quoted 26 lines · Anthony Liguori
Quoted 16 lines · Marcelo Tosatti
Quoted 13 lines · Anthony Liguori
Quoted 5 lines · Marcelo Tosatti

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:

Quoted 8 lines
Quoted 3 lines

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.

Quoted 4 lines

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?

Quoted 16 lines

Regards,

Anthony Liguori

Quoted 9 lines
Quoted 9 lines

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

#22 lore raw

Re: [PATCH 10/10] introduce qemu_ram_map

Quoted 32 lines · Cam Macdonell
Quoted 26 lines · Anthony Liguori
Quoted 16 lines · Marcelo Tosatti
Quoted 13 lines · Anthony Liguori
Quoted 5 lines · Marcelo Tosatti

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:

Quoted 8 lines
Quoted 3 lines

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.

Quoted 4 lines

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.

Quoted 4 lines

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.

#23 lore raw

Re: [PATCH 10/10] introduce qemu_ram_map

In reply to Marcelo Tosatti
Quoted 4 lines · Marcelo Tosatti

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

Quoted 59 lines
---
  cpu-common.h |    1 +
  exec.c       |   28 ++++++++++++++++++++++++++++
  2 files changed, 29 insertions(+), 0 deletions(-)
cpu-common.h +1 -0
index b24cecc..2dfde6f 100644
@@ -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.  */
exec.c +28 -0
index 14d1fd7..648a9c9 100644
@@ -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;
#24 lore raw

Re: [PATCH 10/10] introduce qemu_ram_map

Quoted 8 lines · Anthony Liguori
Quoted 3 lines · Marcelo Tosatti

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.

hw/device-assignment.c +17 -23
Index: qemu-kvm/hw/device-assignment.c
===================================================================
@@ -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);
hw/device-assignment.h +1 -1
Index: qemu-kvm/hw/device-assignment.h
===================================================================
@@ -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 */
#25 lore raw

Re: [PATCH 10/10] introduce qemu_ram_map

Quoted 8 lines · Marcelo Tosatti
Quoted 6 lines · Anthony Liguori
Quoted 4 lines · Marcelo Tosatti

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

Quoted 7 lines
Quoted 4 lines

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.

Quoted 72 lines

Index: qemu-kvm/hw/device-assignment.c
===================================================================

hw/device-assignment.c +17 -19
@@ -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

hw/device-assignment.c line 729

Quoted 10 lines
hw/device-assignment.c +0 -4
@@ -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

Quoted 14 lines

Index: qemu-kvm/hw/device-assignment.h
===================================================================

hw/device-assignment.h +1 -1
@@ -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 */
#26 lore raw

Re: [PATCH 10/10] introduce qemu_ram_map

Quoted 6 lines · Anthony Liguori
Quoted 5 lines · Marcelo Tosatti
Quoted 4 lines · Anthony Liguori
Quoted 3 lines · Marcelo Tosatti

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

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

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

Quoted 5 lines
Quoted 5 lines
hw/device-assignment.c +0 -1
@@ -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)
Quoted 8 lines

hw/device-assignment.c

Quoted 6 lines
hw/device-assignment.c +0 -3
-                    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.

#27 lore raw

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

Quoted 38 lines · Marcelo Tosatti
Quoted 10 lines · Anthony Liguori
Quoted 8 lines · Marcelo Tosatti
Quoted 6 lines · Anthony Liguori
Quoted 4 lines · Marcelo Tosatti

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

Quoted 9 lines
Quoted 7 lines
Quoted 4 lines

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

Quoted 5 lines
Quoted 5 lines
hw/device-assignment.c +0 -1
@@ -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)
Quoted 9 lines

hw/device-assignment.c

Quoted 7 lines
hw/device-assignment.c +0 -3
-                    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?

Quoted 4 lines

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

Quoted 3 lines

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

Yes, I am.

Quoted 2 lines
#28 lore raw

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

Quoted 28 lines · Anthony Liguori
Quoted 27 lines · Marcelo Tosatti
Quoted 6 lines · Anthony Liguori
Quoted 5 lines · Marcelo Tosatti
Quoted 4 lines · Anthony Liguori
Quoted 3 lines · Marcelo Tosatti

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

Quoted 6 lines
Quoted 5 lines
Quoted 3 lines

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

Quoted 12 lines

hw/device-assignment.c line 729

Quoted 11 lines
hw/device-assignment.c +0 -4
@@ -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?

Quoted 7 lines
Quoted 3 lines

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.

#29 lore raw

Re: [PATCH 10/10] introduce qemu_ram_map

In reply to Marcelo Tosatti
Quoted 62 lines · Marcelo Tosatti

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

---
 cpu-common.h |    1 +
 exec.c       |   28 ++++++++++++++++++++++++++++

 2 files changed, 29 insertions(+), 0 deletions(-)
cpu-common.h +1 -0
index b24cecc..2dfde6f 100644
@@ -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.  */
exec.c +28 -1
index 14d1fd7..648a9c9 100644
@@ -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;
--
Quoted 2 lines

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.

#30 lore raw

Re: [PATCH 10/10] introduce qemu_ram_map

Quoted 32 lines · Cam Macdonell
Quoted 32 lines
exec.c +28 -0
+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;

Quoted 8 lines
Quoted 3 lines

--
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.

#31 lore raw
In reply to Marcelo Tosatti
Quoted 8 lines · 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

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

Regards,

Anthony Liguori

Quoted 27 lines

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(-)