[Qemu-devel] [PATCH 0/7] kvm-i386: QEMU support for SMRAM

[Qemu-devel] [PATCH 0/7] kvm-i386: QEMU support for SMRAM

[Paolo Bonzini]

These are the new patches for SMRAM support, using multiple address spaces
and, thus, multiple KVMMemoryListeners.  No more layering violations, yay!

The meat is in patches 4 and 5, which parameterize KVM's memory listener
so that: 1) each memory listener handles its own array of memory slots;
2) each memory listener specifies an address space id for use in
KVM_SET_USER_MEMORY_REGION and KVM_GET_DIRTY_LOG.

Andrew Jones (1):
  kvm-all: put kvm_mem_flags to more work

Paolo Bonzini (6):
  kvm-all: remove useless typedef
  kvm-all: move internal types to kvm_int.h
  kvm-all: make KVM's memory listener more generic
  kvm-all: add support for multiple address spaces
  kvm-all: kvm_irqchip_create is not expected to fail
  target-i386: register a separate KVM address space including SMRAM regions

 include/sysemu/kvm_int.h |  40 ++++++++
 kvm-all.c                | 259 +++++++++++++++++++++++++----------------------
 target-i386/kvm.c        |  40 +++++++-
 3 files changed, 215 insertions(+), 124 deletions(-)
 create mode 100644 include/sysemu/kvm_int.h

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/7] kvm-all: put kvm_mem_flags to more work

[Paolo Bonzini]

From: Andrew Jones <drjones@redhat.com>

Currently kvm_mem_flags just translates bools to bits, let's
make it also determine the bools first. This avoids its parameter
list growing each time we add a flag.

Signed-off-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 kvm-all.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 17a3771..b459855 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -295,10 +295,14 @@ err:
  * dirty pages logging control
  */
 
-static int kvm_mem_flags(KVMState *s, bool log_dirty, bool readonly)
+static int kvm_mem_flags(MemoryRegion *mr)
 {
+    bool readonly = mr->readonly || memory_region_is_romd(mr);
     int flags = 0;
-    flags = log_dirty ? KVM_MEM_LOG_DIRTY_PAGES : 0;
+
+    if (memory_region_is_logging(mr)) {
+        flags |= KVM_MEM_LOG_DIRTY_PAGES;
+    }
     if (readonly && kvm_readonly_mem_allowed) {
         flags |= KVM_MEM_READONLY;
     }
@@ -313,7 +317,10 @@ static int kvm_slot_dirty_pages_log_change(KVMSlot *mem, bool log_dirty)
 
     old_flags = mem->flags;
 
-    flags = (mem->flags & ~mask) | kvm_mem_flags(s, log_dirty, false);
+    flags = mem->flags & ~mask;
+    if (log_dirty) {
+        flags |= KVM_MEM_LOG_DIRTY_PAGES;
+    }
     mem->flags = flags;
 
     /* If nothing changed effectively, no need to issue ioctl */
@@ -663,9 +670,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
     KVMSlot *mem, old;
     int err;
     MemoryRegion *mr = section->mr;
-    bool log_dirty = memory_region_is_logging(mr);
     bool writeable = !mr->readonly && !mr->rom_device;
-    bool readonly_flag = mr->readonly || memory_region_is_romd(mr);
     hwaddr start_addr = section->offset_within_address_space;
     ram_addr_t size = int128_get64(section->size);
     void *ram = NULL;
@@ -709,7 +714,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
             (ram - start_addr == mem->ram - mem->start_addr)) {
             /* The new slot fits into the existing one and comes with
              * identical parameters - update flags and done. */
-            kvm_slot_dirty_pages_log_change(mem, log_dirty);
+            kvm_slot_dirty_pages_log_change(mem, memory_region_is_logging(mr));
             return;
         }
 
@@ -742,7 +747,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
             mem->memory_size = old.memory_size;
             mem->start_addr = old.start_addr;
             mem->ram = old.ram;
-            mem->flags = kvm_mem_flags(s, log_dirty, readonly_flag);
+            mem->flags = kvm_mem_flags(mr);
 
             err = kvm_set_user_memory_region(s, mem);
             if (err) {
@@ -763,7 +768,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
             mem->memory_size = start_addr - old.start_addr;
             mem->start_addr = old.start_addr;
             mem->ram = old.ram;
-            mem->flags =  kvm_mem_flags(s, log_dirty, readonly_flag);
+            mem->flags =  kvm_mem_flags(mr);
 
             err = kvm_set_user_memory_region(s, mem);
             if (err) {
@@ -787,7 +792,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
             size_delta = mem->start_addr - old.start_addr;
             mem->memory_size = old.memory_size - size_delta;
             mem->ram = old.ram + size_delta;
-            mem->flags = kvm_mem_flags(s, log_dirty, readonly_flag);
+            mem->flags = kvm_mem_flags(mr);
 
             err = kvm_set_user_memory_region(s, mem);
             if (err) {
@@ -809,7 +814,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
     mem->memory_size = size;
     mem->start_addr = start_addr;
     mem->ram = ram;
-    mem->flags = kvm_mem_flags(s, log_dirty, readonly_flag);
+    mem->flags = kvm_mem_flags(mr);
 
     err = kvm_set_user_memory_region(s, mem);
     if (err) {
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/7] kvm-all: remove useless typedef

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 kvm-all.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index b459855..bb9477c 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -69,8 +69,6 @@ typedef struct KVMSlot
     int flags;
 } KVMSlot;
 
-typedef struct kvm_dirty_log KVMDirtyLog;
-
 struct KVMState
 {
     AccelState parent_obj;
@@ -425,7 +423,7 @@ static int kvm_physical_sync_dirty_bitmap(MemoryRegionSection *section)
 {
     KVMState *s = kvm_state;
     unsigned long size, allocated_size = 0;
-    KVMDirtyLog d = {};
+    struct kvm_dirty_log d = {};
     KVMSlot *mem;
     int ret = 0;
     hwaddr start_addr = section->offset_within_address_space;
-- 
1.8.3.1

[Qemu-devel] [PATCH 3/7] kvm-all: move internal types to kvm_int.h

[Paolo Bonzini]

i386 code will have to define a different KVMMemoryListener.  Create
an internal header so that KVMSlot is not exposed outside.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/sysemu/kvm_int.h | 30 ++++++++++++++++++++++++++++++
 kvm-all.c                | 18 +-----------------
 2 files changed, 31 insertions(+), 17 deletions(-)
 create mode 100644 include/sysemu/kvm_int.h

diff --git a/include/sysemu/kvm_int.h b/include/sysemu/kvm_int.h
new file mode 100644
index 0000000..e8dcbd7
--- /dev/null
+++ b/include/sysemu/kvm_int.h
@@ -0,0 +1,30 @@
+/*
+ * Internal definitions for a target's KVM support
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef QEMU_KVM_INT_H
+#define QEMU_KVM_INT_H
+
+#include "sysemu/sysemu.h"
+#include "sysemu/accel.h"
+#include "sysemu/kvm.h"
+
+typedef struct KVMSlot
+{
+    hwaddr start_addr;
+    ram_addr_t memory_size;
+    void *ram;
+    int slot;
+    int flags;
+} KVMSlot;
+
+#define TYPE_KVM_ACCEL ACCEL_CLASS_NAME("kvm")
+
+#define KVM_STATE(obj) \
+    OBJECT_CHECK(KVMState, (obj), TYPE_KVM_ACCEL)
+
+#endif
diff --git a/kvm-all.c b/kvm-all.c
index bb9477c..4499ff8 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -24,13 +24,11 @@
 #include "qemu/atomic.h"
 #include "qemu/option.h"
 #include "qemu/config-file.h"
-#include "sysemu/sysemu.h"
-#include "sysemu/accel.h"
 #include "hw/hw.h"
 #include "hw/pci/msi.h"
 #include "hw/s390x/adapter.h"
 #include "exec/gdbstub.h"
-#include "sysemu/kvm.h"
+#include "sysemu/kvm_int.h"
 #include "qemu/bswap.h"
 #include "exec/memory.h"
 #include "exec/ram_addr.h"
@@ -60,15 +58,6 @@
 
 #define KVM_MSI_HASHTAB_SIZE    256
 
-typedef struct KVMSlot
-{
-    hwaddr start_addr;
-    ram_addr_t memory_size;
-    void *ram;
-    int slot;
-    int flags;
-} KVMSlot;
-
 struct KVMState
 {
     AccelState parent_obj;
@@ -107,11 +96,6 @@ struct KVMState
 #endif
 };
 
-#define TYPE_KVM_ACCEL ACCEL_CLASS_NAME("kvm")
-
-#define KVM_STATE(obj) \
-    OBJECT_CHECK(KVMState, (obj), TYPE_KVM_ACCEL)
-
 KVMState *kvm_state;
 bool kvm_kernel_irqchip;
 bool kvm_async_interrupts_allowed;
-- 
1.8.3.1

[Qemu-devel] [PATCH 4/7] kvm-all: make KVM's memory listener more generic

[Paolo Bonzini]

No semantic change, but s->slots and s->migration_log move into a new
struct KVMMemoryListener.  KVM's memory listener becomes a member of
struct KVMState, and becomes of type KVMMemoryListener.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/sysemu/kvm_int.h |   6 ++
 kvm-all.c                | 174 +++++++++++++++++++++++++++--------------------
 2 files changed, 106 insertions(+), 74 deletions(-)

diff --git a/include/sysemu/kvm_int.h b/include/sysemu/kvm_int.h
index e8dcbd7..f2698fa 100644
--- a/include/sysemu/kvm_int.h
+++ b/include/sysemu/kvm_int.h
@@ -22,6 +22,12 @@ typedef struct KVMSlot
     int flags;
 } KVMSlot;
 
+typedef struct KVMMemoryListener {
+    MemoryListener listener;
+    KVMSlot *slots;
+    int migration_log;
+} KVMMemoryListener;
+
 #define TYPE_KVM_ACCEL ACCEL_CLASS_NAME("kvm")
 
 #define KVM_STATE(obj) \
diff --git a/kvm-all.c b/kvm-all.c
index 4499ff8..2700182 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -62,7 +62,6 @@ struct KVMState
 {
     AccelState parent_obj;
 
-    KVMSlot *slots;
     int nr_slots;
     int fd;
     int vmfd;
@@ -70,7 +69,6 @@ struct KVMState
     struct kvm_coalesced_mmio_ring *coalesced_mmio_ring;
     bool coalesced_flush_in_progress;
     int broken_set_mem_region;
-    int migration_log;
     int vcpu_events;
     int robust_singlestep;
     int debugregs;
@@ -116,13 +114,14 @@ static const KVMCapabilityInfo kvm_required_capabilites[] = {
     KVM_CAP_LAST_INFO
 };
 
-static KVMSlot *kvm_get_free_slot(KVMState *s)
+static KVMSlot *kvm_get_free_slot(KVMMemoryListener *kml)
 {
+    KVMState *s = kvm_state;
     int i;
 
     for (i = 0; i < s->nr_slots; i++) {
-        if (s->slots[i].memory_size == 0) {
-            return &s->slots[i];
+        if (kml->slots[i].memory_size == 0) {
+            return &kml->slots[i];
         }
     }
 
@@ -131,12 +130,14 @@ static KVMSlot *kvm_get_free_slot(KVMState *s)
 
 bool kvm_has_free_slot(MachineState *ms)
 {
-    return kvm_get_free_slot(KVM_STATE(ms->accelerator));
+    KVMState *s = KVM_STATE(ms->accelerator);
+
+    return kvm_get_free_slot(&s->memory_listener);
 }
 
-static KVMSlot *kvm_alloc_slot(KVMState *s)
+static KVMSlot *kvm_alloc_slot(KVMMemoryListener *kml)
 {
-    KVMSlot *slot = kvm_get_free_slot(s);
+    KVMSlot *slot = kvm_get_free_slot(kml);
 
     if (slot) {
         return slot;
@@ -146,14 +147,15 @@ static KVMSlot *kvm_alloc_slot(KVMState *s)
     abort();
 }
 
-static KVMSlot *kvm_lookup_matching_slot(KVMState *s,
+static KVMSlot *kvm_lookup_matching_slot(KVMMemoryListener *kml,
                                          hwaddr start_addr,
                                          hwaddr end_addr)
 {
+    KVMState *s = kvm_state;
     int i;
 
     for (i = 0; i < s->nr_slots; i++) {
-        KVMSlot *mem = &s->slots[i];
+        KVMSlot *mem = &kml->slots[i];
 
         if (start_addr == mem->start_addr &&
             end_addr == mem->start_addr + mem->memory_size) {
@@ -167,15 +169,16 @@ static KVMSlot *kvm_lookup_matching_slot(KVMState *s,
 /*
  * Find overlapping slot with lowest start address
  */
-static KVMSlot *kvm_lookup_overlapping_slot(KVMState *s,
+static KVMSlot *kvm_lookup_overlapping_slot(KVMMemoryListener *kml,
                                             hwaddr start_addr,
                                             hwaddr end_addr)
 {
+    KVMState *s = kvm_state;
     KVMSlot *found = NULL;
     int i;
 
     for (i = 0; i < s->nr_slots; i++) {
-        KVMSlot *mem = &s->slots[i];
+        KVMSlot *mem = &kml->slots[i];
 
         if (mem->memory_size == 0 ||
             (found && found->start_addr < mem->start_addr)) {
@@ -194,10 +197,11 @@ static KVMSlot *kvm_lookup_overlapping_slot(KVMState *s,
 int kvm_physical_memory_addr_from_host(KVMState *s, void *ram,
                                        hwaddr *phys_addr)
 {
+    KVMMemoryListener *kml = &s->memory_listener;
     int i;
 
     for (i = 0; i < s->nr_slots; i++) {
-        KVMSlot *mem = &s->slots[i];
+        KVMSlot *mem = &kml->slots[i];
 
         if (ram >= mem->ram && ram < mem->ram + mem->memory_size) {
             *phys_addr = mem->start_addr + (ram - mem->ram);
@@ -208,15 +212,16 @@ int kvm_physical_memory_addr_from_host(KVMState *s, void *ram,
     return 0;
 }
 
-static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
+static int kvm_set_user_memory_region(KVMMemoryListener *kml, KVMSlot *slot)
 {
+    KVMState *s = kvm_state;
     struct kvm_userspace_memory_region mem;
 
     mem.slot = slot->slot;
     mem.guest_phys_addr = slot->start_addr;
     mem.userspace_addr = (unsigned long)slot->ram;
     mem.flags = slot->flags;
-    if (s->migration_log) {
+    if (kml->migration_log) {
         mem.flags |= KVM_MEM_LOG_DIRTY_PAGES;
     }
 
@@ -291,9 +296,9 @@ static int kvm_mem_flags(MemoryRegion *mr)
     return flags;
 }
 
-static int kvm_slot_dirty_pages_log_change(KVMSlot *mem, bool log_dirty)
+static int kvm_slot_dirty_pages_log_change(KVMMemoryListener *kml,
+                                           KVMSlot *mem, bool log_dirty)
 {
-    KVMState *s = kvm_state;
     int flags, mask = KVM_MEM_LOG_DIRTY_PAGES;
     int old_flags;
 
@@ -306,7 +311,7 @@ static int kvm_slot_dirty_pages_log_change(KVMSlot *mem, bool log_dirty)
     mem->flags = flags;
 
     /* If nothing changed effectively, no need to issue ioctl */
-    if (s->migration_log) {
+    if (kml->migration_log) {
         flags |= KVM_MEM_LOG_DIRTY_PAGES;
     }
 
@@ -314,14 +319,13 @@ static int kvm_slot_dirty_pages_log_change(KVMSlot *mem, bool log_dirty)
         return 0;
     }
 
-    return kvm_set_user_memory_region(s, mem);
+    return kvm_set_user_memory_region(kml, mem);
 }
 
-static int kvm_dirty_pages_log_change(hwaddr phys_addr,
+static int kvm_dirty_pages_log_change(KVMMemoryListener *kml, hwaddr phys_addr,
                                       ram_addr_t size, bool log_dirty)
 {
-    KVMState *s = kvm_state;
-    KVMSlot *mem = kvm_lookup_matching_slot(s, phys_addr, phys_addr + size);
+    KVMSlot *mem = kvm_lookup_matching_slot(kml, phys_addr, phys_addr + size);
 
     if (mem == NULL)  {
         fprintf(stderr, "BUG: %s: invalid parameters " TARGET_FMT_plx "-"
@@ -329,15 +333,16 @@ static int kvm_dirty_pages_log_change(hwaddr phys_addr,
                 (hwaddr)(phys_addr + size - 1));
         return -EINVAL;
     }
-    return kvm_slot_dirty_pages_log_change(mem, log_dirty);
+    return kvm_slot_dirty_pages_log_change(kml, mem, log_dirty);
 }
 
 static void kvm_log_start(MemoryListener *listener,
                           MemoryRegionSection *section)
 {
+    KVMMemoryListener *kml = container_of(listener, KVMMemoryListener, listener);
     int r;
 
-    r = kvm_dirty_pages_log_change(section->offset_within_address_space,
+    r = kvm_dirty_pages_log_change(kml, section->offset_within_address_space,
                                    int128_get64(section->size), true);
     if (r < 0) {
         abort();
@@ -347,25 +352,26 @@ static void kvm_log_start(MemoryListener *listener,
 static void kvm_log_stop(MemoryListener *listener,
                           MemoryRegionSection *section)
 {
+    KVMMemoryListener *kml = container_of(listener, KVMMemoryListener, listener);
     int r;
 
-    r = kvm_dirty_pages_log_change(section->offset_within_address_space,
+    r = kvm_dirty_pages_log_change(kml, section->offset_within_address_space,
                                    int128_get64(section->size), false);
     if (r < 0) {
         abort();
     }
 }
 
-static int kvm_set_migration_log(bool enable)
+static int kvm_set_migration_log(KVMMemoryListener *kml, bool enable)
 {
     KVMState *s = kvm_state;
     KVMSlot *mem;
     int i, err;
 
-    s->migration_log = enable;
+    kml->migration_log = enable;
 
     for (i = 0; i < s->nr_slots; i++) {
-        mem = &s->slots[i];
+        mem = &kml->slots[i];
 
         if (!mem->memory_size) {
             continue;
@@ -373,7 +379,7 @@ static int kvm_set_migration_log(bool enable)
         if (!!(mem->flags & KVM_MEM_LOG_DIRTY_PAGES) == enable) {
             continue;
         }
-        err = kvm_set_user_memory_region(s, mem);
+        err = kvm_set_user_memory_region(kml, mem);
         if (err) {
             return err;
         }
@@ -403,7 +409,8 @@ static int kvm_get_dirty_pages_log_range(MemoryRegionSection *section,
  * @start_add: start of logged region.
  * @end_addr: end of logged region.
  */
-static int kvm_physical_sync_dirty_bitmap(MemoryRegionSection *section)
+static int kvm_physical_sync_dirty_bitmap(KVMMemoryListener *kml,
+                                          MemoryRegionSection *section)
 {
     KVMState *s = kvm_state;
     unsigned long size, allocated_size = 0;
@@ -415,7 +422,7 @@ static int kvm_physical_sync_dirty_bitmap(MemoryRegionSection *section)
 
     d.dirty_bitmap = NULL;
     while (start_addr < end_addr) {
-        mem = kvm_lookup_overlapping_slot(s, start_addr, end_addr);
+        mem = kvm_lookup_overlapping_slot(kml, start_addr, end_addr);
         if (mem == NULL) {
             break;
         }
@@ -646,7 +653,8 @@ kvm_check_extension_list(KVMState *s, const KVMCapabilityInfo *list)
     return NULL;
 }
 
-static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
+static void kvm_set_phys_mem(KVMMemoryListener *kml,
+                             MemoryRegionSection *section, bool add)
 {
     KVMState *s = kvm_state;
     KVMSlot *mem, old;
@@ -686,7 +694,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
     ram = memory_region_get_ram_ptr(mr) + section->offset_within_region + delta;
 
     while (1) {
-        mem = kvm_lookup_overlapping_slot(s, start_addr, start_addr + size);
+        mem = kvm_lookup_overlapping_slot(kml, start_addr, start_addr + size);
         if (!mem) {
             break;
         }
@@ -696,19 +704,19 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
             (ram - start_addr == mem->ram - mem->start_addr)) {
             /* The new slot fits into the existing one and comes with
              * identical parameters - update flags and done. */
-            kvm_slot_dirty_pages_log_change(mem, memory_region_is_logging(mr));
+            kvm_slot_dirty_pages_log_change(kml, mem, memory_region_is_logging(mr));
             return;
         }
 
         old = *mem;
 
-        if ((mem->flags & KVM_MEM_LOG_DIRTY_PAGES) || s->migration_log) {
-            kvm_physical_sync_dirty_bitmap(section);
+        if ((mem->flags & KVM_MEM_LOG_DIRTY_PAGES) || kml->migration_log) {
+            kvm_physical_sync_dirty_bitmap(kml, section);
         }
 
         /* unregister the overlapping slot */
         mem->memory_size = 0;
-        err = kvm_set_user_memory_region(s, mem);
+        err = kvm_set_user_memory_region(kml, mem);
         if (err) {
             fprintf(stderr, "%s: error unregistering overlapping slot: %s\n",
                     __func__, strerror(-err));
@@ -725,13 +733,13 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
          * - and actually require a recent KVM version. */
         if (s->broken_set_mem_region &&
             old.start_addr == start_addr && old.memory_size < size && add) {
-            mem = kvm_alloc_slot(s);
+            mem = kvm_alloc_slot(kml);
             mem->memory_size = old.memory_size;
             mem->start_addr = old.start_addr;
             mem->ram = old.ram;
             mem->flags = kvm_mem_flags(mr);
 
-            err = kvm_set_user_memory_region(s, mem);
+            err = kvm_set_user_memory_region(kml, mem);
             if (err) {
                 fprintf(stderr, "%s: error updating slot: %s\n", __func__,
                         strerror(-err));
@@ -746,13 +754,13 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
 
         /* register prefix slot */
         if (old.start_addr < start_addr) {
-            mem = kvm_alloc_slot(s);
+            mem = kvm_alloc_slot(kml);
             mem->memory_size = start_addr - old.start_addr;
             mem->start_addr = old.start_addr;
             mem->ram = old.ram;
             mem->flags =  kvm_mem_flags(mr);
 
-            err = kvm_set_user_memory_region(s, mem);
+            err = kvm_set_user_memory_region(kml, mem);
             if (err) {
                 fprintf(stderr, "%s: error registering prefix slot: %s\n",
                         __func__, strerror(-err));
@@ -769,14 +777,14 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
         if (old.start_addr + old.memory_size > start_addr + size) {
             ram_addr_t size_delta;
 
-            mem = kvm_alloc_slot(s);
+            mem = kvm_alloc_slot(kml);
             mem->start_addr = start_addr + size;
             size_delta = mem->start_addr - old.start_addr;
             mem->memory_size = old.memory_size - size_delta;
             mem->ram = old.ram + size_delta;
             mem->flags = kvm_mem_flags(mr);
 
-            err = kvm_set_user_memory_region(s, mem);
+            err = kvm_set_user_memory_region(kml, mem);
             if (err) {
                 fprintf(stderr, "%s: error registering suffix slot: %s\n",
                         __func__, strerror(-err));
@@ -792,13 +800,13 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
     if (!add) {
         return;
     }
-    mem = kvm_alloc_slot(s);
+    mem = kvm_alloc_slot(kml);
     mem->memory_size = size;
     mem->start_addr = start_addr;
     mem->ram = ram;
     mem->flags = kvm_mem_flags(mr);
 
-    err = kvm_set_user_memory_region(s, mem);
+    err = kvm_set_user_memory_region(kml, mem);
     if (err) {
         fprintf(stderr, "%s: error registering slot: %s\n", __func__,
                 strerror(-err));
@@ -809,23 +817,28 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
 static void kvm_region_add(MemoryListener *listener,
                            MemoryRegionSection *section)
 {
+    KVMMemoryListener *kml = container_of(listener, KVMMemoryListener, listener);
+
     memory_region_ref(section->mr);
-    kvm_set_phys_mem(section, true);
+    kvm_set_phys_mem(kml, section, true);
 }
 
 static void kvm_region_del(MemoryListener *listener,
                            MemoryRegionSection *section)
 {
-    kvm_set_phys_mem(section, false);
+    KVMMemoryListener *kml = container_of(listener, KVMMemoryListener, listener);
+
+    kvm_set_phys_mem(kml, section, false);
     memory_region_unref(section->mr);
 }
 
 static void kvm_log_sync(MemoryListener *listener,
                          MemoryRegionSection *section)
 {
+    KVMMemoryListener *kml = container_of(listener, KVMMemoryListener, listener);
     int r;
 
-    r = kvm_physical_sync_dirty_bitmap(section);
+    r = kvm_physical_sync_dirty_bitmap(kml, section);
     if (r < 0) {
         abort();
     }
@@ -833,17 +846,19 @@ static void kvm_log_sync(MemoryListener *listener,
 
 static void kvm_log_global_start(struct MemoryListener *listener)
 {
+    KVMMemoryListener *kml = container_of(listener, KVMMemoryListener, listener);
     int r;
 
-    r = kvm_set_migration_log(1);
+    r = kvm_set_migration_log(kml, 1);
     assert(r >= 0);
 }
 
 static void kvm_log_global_stop(struct MemoryListener *listener)
 {
+    KVMMemoryListener *kml = container_of(listener, KVMMemoryListener, listener);
     int r;
 
-    r = kvm_set_migration_log(0);
+    r = kvm_set_migration_log(kml, 0);
     assert(r >= 0);
 }
 
@@ -916,20 +931,29 @@ static void kvm_io_ioeventfd_del(MemoryListener *listener,
     }
 }
 
-static MemoryListener kvm_memory_listener = {
-    .region_add = kvm_region_add,
-    .region_del = kvm_region_del,
-    .log_start = kvm_log_start,
-    .log_stop = kvm_log_stop,
-    .log_sync = kvm_log_sync,
-    .log_global_start = kvm_log_global_start,
-    .log_global_stop = kvm_log_global_stop,
-    .eventfd_add = kvm_mem_ioeventfd_add,
-    .eventfd_del = kvm_mem_ioeventfd_del,
-    .coalesced_mmio_add = kvm_coalesce_mmio_region,
-    .coalesced_mmio_del = kvm_uncoalesce_mmio_region,
-    .priority = 10,
-};
+static void kvm_memory_listener_register(KVMState *s,
+                                         KVMMemoryListener *kml,
+                                         AddressSpace *as)
+{
+    int i;
+
+    kml->slots = g_malloc0(s->nr_slots * sizeof(KVMSlot));
+
+    for (i = 0; i < s->nr_slots; i++) {
+        kml->slots[i].slot = i;
+    }
+
+    kml->listener.region_add = kvm_region_add;
+    kml->listener.region_del = kvm_region_del;
+    kml->listener.log_start = kvm_log_start;
+    kml->listener.log_stop = kvm_log_stop;
+    kml->listener.log_sync = kvm_log_sync;
+    kml->listener.log_global_start = kvm_log_global_start;
+    kml->listener.log_global_stop = kvm_log_global_stop;
+    kml->listener.priority = 10;
+
+    memory_listener_register(&kml->listener, as);
+}
 
 static MemoryListener kvm_io_listener = {
     .eventfd_add = kvm_io_ioeventfd_add,
@@ -1437,7 +1461,7 @@ static int kvm_init(MachineState *ms)
     KVMState *s;
     const KVMCapabilityInfo *missing_cap;
     int ret;
-    int i, type = 0;
+    int type = 0;
     const char *kvm_type;
 
     s = KVM_STATE(ms->accelerator);
@@ -1486,12 +1510,6 @@ static int kvm_init(MachineState *ms)
         s->nr_slots = 32;
     }
 
-    s->slots = g_malloc0(s->nr_slots * sizeof(KVMSlot));
-
-    for (i = 0; i < s->nr_slots; i++) {
-        s->slots[i].slot = i;
-    }
-
     /* check the vcpu limits */
     soft_vcpus_limit = kvm_recommended_vcpus(s);
     hard_vcpus_limit = kvm_max_vcpus(s);
@@ -1629,8 +1647,16 @@ static int kvm_init(MachineState *ms)
     }
 
     kvm_state = s;
-    memory_listener_register(&kvm_memory_listener, &address_space_memory);
-    memory_listener_register(&kvm_io_listener, &address_space_io);
+
+    s->memory_listener.listener.eventfd_add = kvm_mem_ioeventfd_add;
+    s->memory_listener.listener.eventfd_del = kvm_mem_ioeventfd_del;
+    s->memory_listener.listener.coalesced_mmio_add = kvm_coalesce_mmio_region;
+    s->memory_listener.listener.coalesced_mmio_del = kvm_uncoalesce_mmio_region;
+
+    kvm_memory_listener_register(s, &s->memory_listener,
+                                 &address_space_memory);
+    memory_listener_register(&kvm_io_listener,
+                             &address_space_io);
 
     s->many_ioeventfds = kvm_check_many_ioeventfds();
 
@@ -1646,7 +1672,7 @@ err:
     if (s->fd != -1) {
         close(s->fd);
     }
-    g_free(s->slots);
+    g_free(s->memory_listener.slots);
 
     return ret;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 5/7] kvm-all: add support for multiple address spaces

[Paolo Bonzini]

Make kvm_memory_listener_register public, and assign a kernel
address space id to each KVMMemoryListener.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/sysemu/kvm_int.h |  4 ++++
 kvm-all.c                | 13 ++++++-------
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/include/sysemu/kvm_int.h b/include/sysemu/kvm_int.h
index f2698fa..9d67a2a 100644
--- a/include/sysemu/kvm_int.h
+++ b/include/sysemu/kvm_int.h
@@ -26,6 +26,7 @@ typedef struct KVMMemoryListener {
     MemoryListener listener;
     KVMSlot *slots;
     int migration_log;
+    int as_id;
 } KVMMemoryListener;
 
 #define TYPE_KVM_ACCEL ACCEL_CLASS_NAME("kvm")
@@ -33,4 +34,7 @@ typedef struct KVMMemoryListener {
 #define KVM_STATE(obj) \
     OBJECT_CHECK(KVMState, (obj), TYPE_KVM_ACCEL)
 
+void kvm_memory_listener_register(KVMState *s, KVMMemoryListener *kml,
+                                  AddressSpace *as, int as_id);
+
 #endif
diff --git a/kvm-all.c b/kvm-all.c
index 2700182..f8552a9 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -217,7 +217,7 @@ static int kvm_set_user_memory_region(KVMMemoryListener *kml, KVMSlot *slot)
     KVMState *s = kvm_state;
     struct kvm_userspace_memory_region mem;
 
-    mem.slot = slot->slot;
+    mem.slot = slot->slot | (kml->as_id << 16);
     mem.guest_phys_addr = slot->start_addr;
     mem.userspace_addr = (unsigned long)slot->ram;
     mem.flags = slot->flags;
@@ -449,8 +449,7 @@ static int kvm_physical_sync_dirty_bitmap(KVMMemoryListener *kml,
         allocated_size = size;
         memset(d.dirty_bitmap, 0, allocated_size);
 
-        d.slot = mem->slot;
-
+        d.slot = mem->slot | (kml->as_id << 16);
         if (kvm_vm_ioctl(s, KVM_GET_DIRTY_LOG, &d) == -1) {
             DPRINTF("ioctl failed %d\n", errno);
             ret = -1;
@@ -931,13 +930,13 @@ static void kvm_io_ioeventfd_del(MemoryListener *listener,
     }
 }
 
-static void kvm_memory_listener_register(KVMState *s,
-                                         KVMMemoryListener *kml,
-                                         AddressSpace *as)
+void kvm_memory_listener_register(KVMState *s, KVMMemoryListener *kml,
+                                  AddressSpace *as, int as_id)
 {
     int i;
 
     kml->slots = g_malloc0(s->nr_slots * sizeof(KVMSlot));
+    kml->as_id = as_id;
 
     for (i = 0; i < s->nr_slots; i++) {
         kml->slots[i].slot = i;
@@ -1654,7 +1653,7 @@ static int kvm_init(MachineState *ms)
     s->memory_listener.listener.coalesced_mmio_del = kvm_uncoalesce_mmio_region;
 
     kvm_memory_listener_register(s, &s->memory_listener,
-                                 &address_space_memory);
+                                 &address_space_memory, 0);
     memory_listener_register(&kvm_io_listener,
                              &address_space_io);
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 6/7] kvm-all: kvm_irqchip_create is not expected to fail

[Paolo Bonzini]

KVM_CREATE_IRQCHIP should never fail, and so should its userspace
wrapper kvm_irqchip_create.  The function does not do anything
if the irqchip capability is not available, as is the case for PPC.

With this patch, kvm_arch_init can allocate memory and it will not
be leaked.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 kvm-all.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index f8552a9..ce18b40 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1391,27 +1391,31 @@ int kvm_irqchip_remove_irqfd_notifier(KVMState *s, EventNotifier *n, int virq)
            false);
 }
 
-static int kvm_irqchip_create(MachineState *machine, KVMState *s)
+static void kvm_irqchip_create(MachineState *machine, KVMState *s)
 {
     int ret;
 
-    if (!machine_kernel_irqchip_allowed(machine) ||
-        (!kvm_check_extension(s, KVM_CAP_IRQCHIP) &&
-         (kvm_vm_enable_cap(s, KVM_CAP_S390_IRQCHIP, 0) < 0))) {
-        return 0;
+    if (kvm_check_extension(s, KVM_CAP_IRQCHIP)) {
+        ;
+    } else if (kvm_check_extension(s, KVM_CAP_S390_IRQCHIP)) {
+        ret = kvm_vm_enable_cap(s, KVM_CAP_S390_IRQCHIP, 0);
+        if (ret < 0) {
+            fprintf(stderr, "Enable kernel irqchip failed: %s\n", strerror(-ret));
+            exit(1);
+        }
+    } else {
+        return;
     }
 
     /* First probe and see if there's a arch-specific hook to create the
      * in-kernel irqchip for us */
     ret = kvm_arch_irqchip_create(s);
-    if (ret < 0) {
-        return ret;
-    } else if (ret == 0) {
+    if (ret == 0) {
         ret = kvm_vm_ioctl(s, KVM_CREATE_IRQCHIP);
-        if (ret < 0) {
-            fprintf(stderr, "Create kernel irqchip failed\n");
-            return ret;
-        }
+    }
+    if (ret < 0) {
+        fprintf(stderr, "Create kernel irqchip failed: %s\n", strerror(-ret));
+        exit(1);
     }
 
     kvm_kernel_irqchip = true;
@@ -1422,8 +1426,6 @@ static int kvm_irqchip_create(MachineState *machine, KVMState *s)
     kvm_halt_in_kernel_allowed = true;
 
     kvm_init_irq_routing(s);
-
-    return 0;
 }
 
 /* Find number of supported CPUs using the recommended
@@ -1640,9 +1642,8 @@ static int kvm_init(MachineState *ms)
         goto err;
     }
 
-    ret = kvm_irqchip_create(ms, s);
-    if (ret < 0) {
-        goto err;
+    if (machine_kernel_irqchip_allowed(ms)) {
+        kvm_irqchip_create(ms, s);
     }
 
     kvm_state = s;
-- 
1.8.3.1

[Qemu-devel] [PATCH 7/7] target-i386: register a separate KVM address space including SMRAM regions

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target-i386/kvm.c | 40 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 09b4fc7..19ccc08 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -22,7 +22,7 @@
 
 #include "qemu-common.h"
 #include "sysemu/sysemu.h"
-#include "sysemu/kvm.h"
+#include "sysemu/kvm_int.h"
 #include "kvm_i386.h"
 #include "cpu.h"
 #include "exec/gdbstub.h"
@@ -846,6 +846,39 @@ static int kvm_get_supported_msrs(KVMState *s)
     return ret;
 }
 
+static Notifier smram_machine_done;
+static KVMMemoryListener smram_listener;
+static AddressSpace smram_address_space;
+static MemoryRegion smram_as_root;
+static MemoryRegion smram_as_mem;
+
+static void register_smram_listener(Notifier *n, void *unused)
+{
+    MemoryRegion *smram =
+        (MemoryRegion *) object_resolve_path("/machine/smram", NULL);
+
+    /* Outer container... */
+    memory_region_init(&smram_as_root, OBJECT(kvm_state), "mem-container-smram", ~0ull);
+    memory_region_set_enabled(&smram_as_root, true);
+
+    /* ... with two regions inside: normal system memory with low
+     * priority, and...
+     */
+    memory_region_init_alias(&smram_as_mem, OBJECT(kvm_state), "mem-smram",
+			     get_system_memory(), 0, ~0ull);
+    memory_region_add_subregion_overlap(&smram_as_root, 0, &smram_as_mem, 0);
+    memory_region_set_enabled(&smram_as_mem, true);
+
+    if (smram) {
+	    memory_region_add_subregion_overlap(&smram_as_root, 0, smram, 10);
+            memory_region_set_enabled(smram, true);
+    }
+
+    address_space_init(&smram_address_space, &smram_as_root, "KVM-SMRAM");
+    kvm_memory_listener_register(kvm_state, &smram_listener,
+				 &smram_address_space, 1);
+}
+
 int kvm_arch_init(MachineState *ms, KVMState *s)
 {
     uint64_t identity_base = 0xfffbc000;
@@ -904,6 +937,11 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
             return ret;
         }
     }
+
+    if (kvm_check_extension(s, KVM_CAP_X86_SMM)) {
+        smram_machine_done.notify = register_smram_listener;
+        qemu_add_machine_init_done_notifier(&smram_machine_done);
+    }
     return 0;
 }
 
-- 
1.8.3.1

[Qemu-devel] [RFC PATCH 0/7] x86: SMRAM implementation for KVM

[Paolo Bonzini]

This is the final piece of x86 SMM implementation, tested with PIIX
(low SMRAM) and Q35 (high SMRAM).

There is a problem---it has an awful layering violation in patch 5,
and an only slightly better one in patch 6.

If anyone has ideas, please speak up.  Note that it is not possible
to call KVM_SET_USER_MEMORY_REGION every time you enter or leave SMM,
because that would only work for single-processor virtual machines.

Paolo

Andrew Jones (1):
  kvm-all: put kvm_mem_flags to more work

Paolo Bonzini (6):
  kvm-all: remove useless typedef
  kvm-all: move KVMState definitions to kvm_int.h
  kvm-all: add KVM address space
  memory: add kvm_mem_flags to MemoryRegion
  i386: disable the region in /machine/smram when SMRAM is open
  kvm-i386: register SMRAM regions with KVM_MEM_X86_SMRAM

 hw/pci-host/piix.c       |   6 +++
 hw/pci-host/q35.c        |  20 ++++++---
 include/exec/memory.h    |   4 ++
 include/sysemu/kvm_int.h |  73 +++++++++++++++++++++++++++++++
 kvm-all.c                | 111 ++++++++++++++++-------------------------------
 memory.c                 |  17 ++++++--
 target-i386/kvm.c        |  27 ++++++++++++
 7 files changed, 173 insertions(+), 85 deletions(-)
 create mode 100644 include/sysemu/kvm_int.h

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/7] kvm-all: put kvm_mem_flags to more work

[Paolo Bonzini]

From: Andrew Jones <drjones@redhat.com>

Currently kvm_mem_flags just translates bools to bits, let's
make it also determine the bools first. This avoids its parameter
list growing each time we add a flag.

Signed-off-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 kvm-all.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 5ad4877..6e1a3f8 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -295,10 +295,14 @@ err:
  * dirty pages logging control
  */
 
-static int kvm_mem_flags(KVMState *s, bool log_dirty, bool readonly)
+static int kvm_mem_flags(MemoryRegion *mr)
 {
+    bool readonly = mr->readonly || memory_region_is_romd(mr);
     int flags = 0;
-    flags = log_dirty ? KVM_MEM_LOG_DIRTY_PAGES : 0;
+
+    if (memory_region_is_logging(mr)) {
+        flags |= KVM_MEM_LOG_DIRTY_PAGES;
+    }
     if (readonly && kvm_readonly_mem_allowed) {
         flags |= KVM_MEM_READONLY;
     }
@@ -313,7 +317,10 @@ static int kvm_slot_dirty_pages_log_change(KVMSlot *mem, bool log_dirty)
 
     old_flags = mem->flags;
 
-    flags = (mem->flags & ~mask) | kvm_mem_flags(s, log_dirty, false);
+    flags = mem->flags & ~mask;
+    if (log_dirty) {
+        flags |= KVM_MEM_LOG_DIRTY_PAGES;
+    }
     mem->flags = flags;
 
     /* If nothing changed effectively, no need to issue ioctl */
@@ -663,9 +670,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
     KVMSlot *mem, old;
     int err;
     MemoryRegion *mr = section->mr;
-    bool log_dirty = memory_region_is_logging(mr);
     bool writeable = !mr->readonly && !mr->rom_device;
-    bool readonly_flag = mr->readonly || memory_region_is_romd(mr);
     hwaddr start_addr = section->offset_within_address_space;
     ram_addr_t size = int128_get64(section->size);
     void *ram = NULL;
@@ -709,7 +714,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
             (ram - start_addr == mem->ram - mem->start_addr)) {
             /* The new slot fits into the existing one and comes with
              * identical parameters - update flags and done. */
-            kvm_slot_dirty_pages_log_change(mem, log_dirty);
+            kvm_slot_dirty_pages_log_change(mem, memory_region_is_logging(mr));
             return;
         }
 
@@ -742,7 +747,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
             mem->memory_size = old.memory_size;
             mem->start_addr = old.start_addr;
             mem->ram = old.ram;
-            mem->flags = kvm_mem_flags(s, log_dirty, readonly_flag);
+            mem->flags = kvm_mem_flags(mr);
 
             err = kvm_set_user_memory_region(s, mem);
             if (err) {
@@ -763,7 +768,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
             mem->memory_size = start_addr - old.start_addr;
             mem->start_addr = old.start_addr;
             mem->ram = old.ram;
-            mem->flags =  kvm_mem_flags(s, log_dirty, readonly_flag);
+            mem->flags =  kvm_mem_flags(mr);
 
             err = kvm_set_user_memory_region(s, mem);
             if (err) {
@@ -787,7 +792,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
             size_delta = mem->start_addr - old.start_addr;
             mem->memory_size = old.memory_size - size_delta;
             mem->ram = old.ram + size_delta;
-            mem->flags = kvm_mem_flags(s, log_dirty, readonly_flag);
+            mem->flags = kvm_mem_flags(mr);
 
             err = kvm_set_user_memory_region(s, mem);
             if (err) {
@@ -809,7 +814,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
     mem->memory_size = size;
     mem->start_addr = start_addr;
     mem->ram = ram;
-    mem->flags = kvm_mem_flags(s, log_dirty, readonly_flag);
+    mem->flags = kvm_mem_flags(mr);
 
     err = kvm_set_user_memory_region(s, mem);
     if (err) {
-- 
1.8.3.1