[Qemu-devel] [PATCH v2 0/6] Runtime pagesize computation

[Qemu-devel] [PATCH v2 0/6] Runtime pagesize computation

[Peter Maydell]

This set of patches is a development based on the ones from Vijaya:
the general idea is similar but I have tried to improve the interface
for defining the page size a bit.  I've also tweaked patches 2 and 3
to address code review comments.

The basic idea here is that:
 * the target CPU implementation has to opt into variable page size
   by defining TARGET_PAGE_BITS_VARY, and then calling
   set_preferred_target_page_bits() in its realize function
   with whatever the CPU as instantiated actually requires
 * the machine also has to opt in, by specifying a new MachineClass
   field which states the value they guarantee will be no greater
   than the preferred page size for any CPU they create
 * we finalize the decision about page size in cpu_exec_init_all()
   (and then later attempts to create CPUs which can't cope with
   that decision are failed)

I would ideally have liked to finalize things much later, but
this is in practice hugely difficult because so many things
(in particular all the address space/memory system code)
assume the target page size is known.

Note that setting minimum_page-bits for a machine is a migration
compatibility break (the RAM migration format assumes both sides
have the same idea of a page size). Mismatches will probably
result in an unhelpfully obscure migration failure. (Possibly we
could have the migration send the page size if it was different
from TARGET_PAGE_BITS_MIN so we at least got a nice error?)

This could in theory be extended to the user-mode binaries,
but for the moment I have just required them to define a
fixed TARGET_PAGE_BITS.

NB: I have only very lightly tested these and haven't attempted
to measure performance at all. There is an assert() in the
definition of TARGET_PAGE_BITS which is good for making sure
it isn't used before it's valid but not so good for speed.

thanks
-- PMM


Peter Maydell (3):
  cpu: Support a target CPU having a variable page size
  target-arm: Make page size a runtime setting
  hw/arm/virt: Set minimum_page_bits to 12

Vijaya Kumar K (3):
  migration: Remove static allocation of xzblre cache buffer
  exec.c: Remove static allocation of sub_section of sub_page
  translate-all.c: Compute L1 page table properties at runtime

 exec.c                 | 47 ++++++++++++++++++++++++++++++++---
 hw/arm/virt.c          |  2 ++
 include/exec/cpu-all.h |  8 ++++++
 include/hw/boards.h    |  7 ++++++
 include/qemu-common.h  | 13 ++++++++++
 migration/ram.c        |  4 ++-
 target-arm/cpu.c       | 24 ++++++++++++++++++
 target-arm/cpu.h       |  9 ++++---
 translate-all.c        | 67 +++++++++++++++++++++++++++++++++-----------------
 vl.c                   | 10 ++++++++
 10 files changed, 160 insertions(+), 31 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH v2 1/6] migration: Remove static allocation of xzblre cache buffer

[Peter Maydell]

From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>

Allocate xzblre zero page cache buffer dynamically.
Remove dependency on TARGET_PAGE_SIZE to make run-time
page size detection for arm platforms.

Signed-off-by: Vijaya Kumar K <vijayak@cavium.com>
Message-id: 1465808915-4887-2-git-send-email-vijayak@caviumnetworks.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 migration/ram.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/migration/ram.c b/migration/ram.c
index 815bc0e..b665741 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -69,7 +69,7 @@ static uint64_t bitmap_sync_count;
 /* 0x80 is reserved in migration.h start with 0x100 next */
 #define RAM_SAVE_FLAG_COMPRESS_PAGE    0x100
 
-static const uint8_t ZERO_TARGET_PAGE[TARGET_PAGE_SIZE];
+static uint8_t *ZERO_TARGET_PAGE;
 
 static inline bool is_zero_range(uint8_t *p, uint64_t size)
 {
@@ -1429,6 +1429,7 @@ static void ram_migration_cleanup(void *opaque)
         cache_fini(XBZRLE.cache);
         g_free(XBZRLE.encoded_buf);
         g_free(XBZRLE.current_buf);
+        g_free(ZERO_TARGET_PAGE);
         XBZRLE.cache = NULL;
         XBZRLE.encoded_buf = NULL;
         XBZRLE.current_buf = NULL;
@@ -1887,6 +1888,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
 
     if (migrate_use_xbzrle()) {
         XBZRLE_cache_lock();
+        ZERO_TARGET_PAGE = g_malloc0(TARGET_PAGE_SIZE);
         XBZRLE.cache = cache_init(migrate_xbzrle_cache_size() /
                                   TARGET_PAGE_SIZE,
                                   TARGET_PAGE_SIZE);
-- 
1.9.1

[Qemu-devel] [PATCH v2 2/6] exec.c: Remove static allocation of sub_section of sub_page

[Peter Maydell]

From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>

Allocate sub_section dynamically. Remove dependency
on TARGET_PAGE_SIZE to make run-time page size detection
for arm platforms.

Signed-off-by: Vijaya Kumar K <vijayak@cavium.com>
Message-id: 1465808915-4887-3-git-send-email-vijayak@caviumnetworks.com
[PMM: use flexible array member rather than separate malloc
 so we don't need an extra pointer deref when using it]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 exec.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/exec.c b/exec.c
index 0122ef7..8eaeb0c 100644
--- a/exec.c
+++ b/exec.c
@@ -153,7 +153,7 @@ typedef struct subpage_t {
     MemoryRegion iomem;
     AddressSpace *as;
     hwaddr base;
-    uint16_t sub_section[TARGET_PAGE_SIZE];
+    uint16_t sub_section[];
 } subpage_t;
 
 #define PHYS_SECTION_UNASSIGNED 0
@@ -2270,8 +2270,7 @@ static subpage_t *subpage_init(AddressSpace *as, hwaddr base)
 {
     subpage_t *mmio;
 
-    mmio = g_malloc0(sizeof(subpage_t));
-
+    mmio = g_malloc0(sizeof(subpage_t) + TARGET_PAGE_SIZE * sizeof(uint16_t));
     mmio->as = as;
     mmio->base = base;
     memory_region_init_io(&mmio->iomem, NULL, &subpage_ops, mmio,
-- 
1.9.1

[Qemu-devel] [PATCH v2 3/6] translate-all.c: Compute L1 page table properties at runtime

[Peter Maydell]

From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>

Remove L1 page mapping table properties computing
statically using macros which is dependent on
TARGET_PAGE_BITS. Drop macros V_L1_SIZE, V_L1_SHIFT,
V_L1_BITS macros and replace with variables which are
computed at early stage of VM boot.

Removing dependency can help to make TARGET_PAGE_BITS
dynamic.

Signed-off-by: Vijaya Kumar K <vijayak@cavium.com>
Message-id: 1465808915-4887-4-git-send-email-vijayak@caviumnetworks.com
[PMM:
 assert(v_l1_shift % V_L2_BITS == 0)
 cache v_l2_levels
 initialize from page_init() rather than vl.c
 minor code style fixes]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 translate-all.c | 67 +++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 44 insertions(+), 23 deletions(-)

diff --git a/translate-all.c b/translate-all.c
index 3f402df..e59efdb 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -97,25 +97,24 @@ typedef struct PageDesc {
 #define V_L2_BITS 10
 #define V_L2_SIZE (1 << V_L2_BITS)
 
-/* The bits remaining after N lower levels of page tables.  */
-#define V_L1_BITS_REM \
-    ((L1_MAP_ADDR_SPACE_BITS - TARGET_PAGE_BITS) % V_L2_BITS)
-
-#if V_L1_BITS_REM < 4
-#define V_L1_BITS  (V_L1_BITS_REM + V_L2_BITS)
-#else
-#define V_L1_BITS  V_L1_BITS_REM
-#endif
-
-#define V_L1_SIZE  ((target_ulong)1 << V_L1_BITS)
-
-#define V_L1_SHIFT (L1_MAP_ADDR_SPACE_BITS - TARGET_PAGE_BITS - V_L1_BITS)
-
 uintptr_t qemu_host_page_size;
 intptr_t qemu_host_page_mask;
 
-/* The bottom level has pointers to PageDesc */
-static void *l1_map[V_L1_SIZE];
+/*
+ * L1 Mapping properties
+ */
+static int v_l1_size;
+static int v_l1_shift;
+static int v_l2_levels;
+
+/* The bottom level has pointers to PageDesc, and is indexed by
+ * anything from 4 to (V_L2_BITS + 3) bits, depending on target page size.
+ */
+#define V_L1_MIN_BITS 4
+#define V_L1_MAX_BITS (V_L2_BITS + 3)
+#define V_L1_MAX_SIZE (1 << V_L1_MAX_BITS)
+
+static void *l1_map[V_L1_MAX_SIZE];
 
 /* code generation context */
 TCGContext tcg_ctx;
@@ -125,6 +124,26 @@ TCGContext tcg_ctx;
 __thread int have_tb_lock;
 #endif
 
+static void page_table_config_init(void)
+{
+    uint32_t v_l1_bits;
+
+    assert(TARGET_PAGE_BITS);
+    /* The bits remaining after N lower levels of page tables.  */
+    v_l1_bits = (L1_MAP_ADDR_SPACE_BITS - TARGET_PAGE_BITS) % V_L2_BITS;
+    if (v_l1_bits < V_L1_MIN_BITS) {
+        v_l1_bits += V_L2_BITS;
+    }
+
+    v_l1_size = 1 << v_l1_bits;
+    v_l1_shift = L1_MAP_ADDR_SPACE_BITS - TARGET_PAGE_BITS - v_l1_bits;
+    v_l2_levels = v_l1_shift / V_L2_BITS - 1;
+
+    assert(v_l1_bits <= V_L1_MAX_BITS);
+    assert(v_l1_shift % V_L2_BITS == 0);
+    assert(v_l2_levels >= 0);
+}
+
 void tb_lock(void)
 {
 #ifdef CONFIG_USER_ONLY
@@ -330,6 +349,8 @@ void page_size_init(void)
 static void page_init(void)
 {
     page_size_init();
+    page_table_config_init();
+
 #if defined(CONFIG_BSD) && defined(CONFIG_USER_ONLY)
     {
 #ifdef HAVE_KINFO_GETVMMAP
@@ -406,10 +427,10 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, int alloc)
     int i;
 
     /* Level 1.  Always allocated.  */
-    lp = l1_map + ((index >> V_L1_SHIFT) & (V_L1_SIZE - 1));
+    lp = l1_map + ((index >> v_l1_shift) & (v_l1_size - 1));
 
     /* Level 2..N-1.  */
-    for (i = V_L1_SHIFT / V_L2_BITS - 1; i > 0; i--) {
+    for (i = v_l2_levels; i > 0; i--) {
         void **p = atomic_rcu_read(lp);
 
         if (p == NULL) {
@@ -825,8 +846,8 @@ static void page_flush_tb(void)
 {
     int i;
 
-    for (i = 0; i < V_L1_SIZE; i++) {
-        page_flush_tb_1(V_L1_SHIFT / V_L2_BITS - 1, l1_map + i);
+    for (i = 0; i < v_l1_size; i++) {
+        page_flush_tb_1(v_l2_levels, l1_map + i);
     }
 }
 
@@ -1850,9 +1871,9 @@ int walk_memory_regions(void *priv, walk_memory_regions_fn fn)
     data.start = -1u;
     data.prot = 0;
 
-    for (i = 0; i < V_L1_SIZE; i++) {
-        int rc = walk_memory_regions_1(&data, (target_ulong)i << (V_L1_SHIFT + TARGET_PAGE_BITS),
-                                       V_L1_SHIFT / V_L2_BITS - 1, l1_map + i);
+    for (i = 0; i < v_l1_size; i++) {
+        target_ulong base = i << (v_l1_shift + TARGET_PAGE_BITS);
+        int rc = walk_memory_regions_1(&data, base, v_l2_levels, l1_map + i);
         if (rc != 0) {
             return rc;
         }
-- 
1.9.1

[Qemu-devel] [PATCH v2 4/6] cpu: Support a target CPU having a variable page size

[Peter Maydell]

Support target CPUs having a page size which isn't knownn
at compile time. To use this, the CPU implementation should:
 * define TARGET_PAGE_BITS_VARY
 * not define TARGET_PAGE_BITS
 * define TARGET_PAGE_BITS_MIN to the smallest value it
   might possibly want for TARGET_PAGE_BITS
 * call set_preferred_target_page_bits() in its realize
   function to indicate the actual preferred target page
   size for the CPU (and report any error from it)

In CONFIG_USER_ONLY, the CPU implementation should continue
to define TARGET_PAGE_BITS appropriately for the guest
OS page size.

Machines which want to take advantage of having the page
size something larger than TARGET_PAGE_BITS_MIN must
set the MachineClass minimum_page_bits field to a value
which they guarantee will be no greater than the preferred
page size for any CPU they create.

Note that changing the target page size by setting
minimum_page_bits is a migration compatibility break
for that machine.

For debugging purposes, attempts to use TARGET_PAGE_SIZE
before it has been finally confirmed will assert.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 exec.c                 | 42 ++++++++++++++++++++++++++++++++++++++++++
 include/exec/cpu-all.h |  8 ++++++++
 include/hw/boards.h    |  7 +++++++
 include/qemu-common.h  | 13 +++++++++++++
 vl.c                   | 10 ++++++++++
 5 files changed, 80 insertions(+)

diff --git a/exec.c b/exec.c
index 8eaeb0c..027ce53 100644
--- a/exec.c
+++ b/exec.c
@@ -93,6 +93,11 @@ static MemoryRegion io_mem_unassigned;
 
 #endif
 
+#ifdef TARGET_PAGE_BITS_VARY
+int target_page_bits;
+bool target_page_bits_decided;
+#endif
+
 struct CPUTailQ cpus = QTAILQ_HEAD_INITIALIZER(cpus);
 /* current CPU in the current thread. It is only valid inside
    cpu_exec() */
@@ -102,8 +107,37 @@ __thread CPUState *current_cpu;
    2 = Adaptive rate instruction counting.  */
 int use_icount;
 
+bool set_preferred_target_page_bits(int bits)
+{
+    /* The target page size is the lowest common denominator for all
+     * the CPUs in the system, so we can only make it smaller, never
+     * larger. And we can't make it smaller once we've committed to
+     * a particular size.
+     */
+#ifdef TARGET_PAGE_BITS_VARY
+    assert(bits >= TARGET_PAGE_BITS_MIN);
+    if (target_page_bits == 0 || target_page_bits > bits) {
+        if (target_page_bits_decided) {
+            return false;
+        }
+        target_page_bits = bits;
+    }
+#endif
+    return true;
+}
+
 #if !defined(CONFIG_USER_ONLY)
 
+static void finalize_target_page_bits(void)
+{
+#ifdef TARGET_PAGE_BITS_VARY
+    if (target_page_bits == 0) {
+        target_page_bits = TARGET_PAGE_BITS_MIN;
+    }
+    target_page_bits_decided = true;
+#endif
+}
+
 typedef struct PhysPageEntry PhysPageEntry;
 
 struct PhysPageEntry {
@@ -2862,6 +2896,14 @@ void cpu_register_map_client(QEMUBH *bh)
 void cpu_exec_init_all(void)
 {
     qemu_mutex_init(&ram_list.mutex);
+    /* The data structures we set up here depend on knowing the page size,
+     * so no more changes can be made after this point.
+     * In an ideal world, nothing we did before we had finished the
+     * machine setup would care about the target page size, and we could
+     * do this much later, rather than requiring board models to state
+     * up front what their requirements are.
+     */
+    finalize_target_page_bits();
     io_mem_init();
     memory_map_init();
     qemu_mutex_init(&map_client_list_lock);
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 9f38edf..dde3cd4 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -189,6 +189,14 @@ void address_space_stq(AddressSpace *as, hwaddr addr, uint64_t val,
 
 /* page related stuff */
 
+#ifdef TARGET_PAGE_BITS_VARY
+extern bool target_page_bits_decided;
+extern int target_page_bits;
+#define TARGET_PAGE_BITS ({ assert(target_page_bits_decided); \
+                            target_page_bits; })
+
+#endif
+
 #define TARGET_PAGE_SIZE (1 << TARGET_PAGE_BITS)
 #define TARGET_PAGE_MASK ~(TARGET_PAGE_SIZE - 1)
 #define TARGET_PAGE_ALIGN(addr) (((addr) + TARGET_PAGE_SIZE - 1) & TARGET_PAGE_MASK)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 3ed6155..11808bc 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -85,6 +85,12 @@ typedef struct {
  *    Returns a @HotpluggableCPUList, which describes CPUs objects which
  *    could be added with -device/device_add.
  *    Caller is responsible for freeing returned list.
+ * @minimum_page_bits:
+ *    If non-zero, the board promises never to create a CPU with a page size
+ *    smaller than this, so QEMU can use a more efficient larger page
+ *    size than the target architecture's minimum. (Attempting to create
+ *    such a CPU will fail.) Note that changing this is a migration
+ *    compatibility break for the machine.
  */
 struct MachineClass {
     /*< private >*/
@@ -123,6 +129,7 @@ struct MachineClass {
     ram_addr_t default_ram_size;
     bool option_rom_has_mr;
     bool rom_file_has_mr;
+    int minimum_page_bits;
 
     HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
                                            DeviceState *dev);
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 1f2cb94..fd9d4eb 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -76,6 +76,19 @@ void tcg_exec_init(unsigned long tb_size);
 bool tcg_enabled(void);
 
 void cpu_exec_init_all(void);
+void cpu_exec_machine_creation_done(void);
+
+/**
+ * set_preferred_target_page_bits:
+ * @bits: number of bits needed to represent an address within the page
+ *
+ * Set the preferred target page size (the actual target page
+ * size may be smaller than any given CPU's preference).
+ * Returns true on success, false on failure (which can only happen
+ * if this is called after the system has already finalized its
+ * choice of page size and the requested page size is smaller than that).
+ */
+bool set_preferred_target_page_bits(int bits);
 
 /**
  * Sends a (part of) iovec down a socket, yielding when the socket is full, or
diff --git a/vl.c b/vl.c
index 2f63eb4..61ae073 100644
--- a/vl.c
+++ b/vl.c
@@ -4038,6 +4038,16 @@ int main(int argc, char **argv, char **envp)
     }
     object_property_add_child(object_get_root(), "machine",
                               OBJECT(current_machine), &error_abort);
+
+    if (machine_class->minimum_page_bits) {
+        if (!set_preferred_target_page_bits(machine_class->minimum_page_bits)) {
+            /* This would be a board error: specifying a minimum smaller than
+             * a target's compile-time fixed setting.
+             */
+            g_assert_not_reached();
+        }
+    }
+
     cpu_exec_init_all();
 
     if (machine_class->hw_version) {
-- 
1.9.1

[Qemu-devel] [PATCH v2 5/6] target-arm: Make page size a runtime setting

[Peter Maydell]

Rather than defining TARGET_PAGE_BITS to always be 10,
switch to using a value picked at runtime. This allows us
to use 4K pages for modern ARM CPUs (and in particular all
64-bit CPUs) without having to drop support for the old
ARMv5 CPUs which had 1K pages.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu.c | 24 ++++++++++++++++++++++++
 target-arm/cpu.h |  9 +++++----
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index ce8b8f4..d807b3f 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -576,6 +576,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
     ARMCPU *cpu = ARM_CPU(dev);
     ARMCPUClass *acc = ARM_CPU_GET_CLASS(dev);
     CPUARMState *env = &cpu->env;
+    int pagebits;
 
     /* Some features automatically imply others: */
     if (arm_feature(env, ARM_FEATURE_V8)) {
@@ -631,6 +632,29 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
         set_feature(env, ARM_FEATURE_THUMB_DSP);
     }
 
+    if (arm_feature(env, ARM_FEATURE_V7) &&
+        !arm_feature(env, ARM_FEATURE_M) &&
+        !arm_feature(env, ARM_FEATURE_MPU)) {
+        /* v7VMSA drops support for the old ARMv5 tiny pages, so we
+         * can use 4K pages.
+         */
+        pagebits = 12;
+    } else {
+        /* For CPUs which might have tiny 1K pages, or which have an
+         * MPU and might have small region sizes, stick with 1K pages.
+         */
+        pagebits = 10;
+    }
+    if (!set_preferred_target_page_bits(pagebits)) {
+        /* This can only ever happen for hotplugging a CPU, or if
+         * the board code incorrectly creates a CPU which it has
+         * promised via minimum_page_size that it will not.
+         */
+        error_setg(errp, "This CPU requires a smaller page size than the "
+                   "system is using");
+        return;
+    }
+
     if (cpu->reset_hivecs) {
             cpu->reset_sctlr |= (1 << 13);
     }
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 7938ddc..073f91f 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1767,10 +1767,11 @@ bool write_cpustate_to_list(ARMCPU *cpu);
 #if defined(CONFIG_USER_ONLY)
 #define TARGET_PAGE_BITS 12
 #else
-/* The ARM MMU allows 1k pages.  */
-/* ??? Linux doesn't actually use these, and they're deprecated in recent
-   architecture revisions.  Maybe a configure option to disable them.  */
-#define TARGET_PAGE_BITS 10
+/* ARMv7 and later CPUs have 4K pages minimum, but ARMv5 and v6
+ * have to support 1K tiny pages.
+ */
+#define TARGET_PAGE_BITS_VARY
+#define TARGET_PAGE_BITS_MIN 10
 #endif
 
 #if defined(TARGET_AARCH64)
-- 
1.9.1

[Qemu-devel] [PATCH v2 6/6] hw/arm/virt: Set minimum_page_bits to 12

[Peter Maydell]

Since the virt board model will never create a CPU which is
pre-ARMv7, we know that our minimum page size is 4K and can
set minimum_page_bits accordingly, for improved performance.

Note that this is a migration compatibility break.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/virt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index c5c125e..f9b51aa 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1440,6 +1440,8 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
     mc->block_default_type = IF_VIRTIO;
     mc->no_cdrom = 1;
     mc->pci_allow_0_address = true;
+    /* We know we will never create a pre-ARMv7 CPU which needs 1K pages */
+    mc->minimum_page_bits = 12;
 }
 
 static const TypeInfo virt_machine_info = {
-- 
1.9.1

Re: [Qemu-devel] [PATCH v2 4/6] cpu: Support a target CPU having a variable page size

[Andrew Jones]

On Tue, Jun 21, 2016 at 06:09:32PM +0100, Peter Maydell wrote:
> Support target CPUs having a page size which isn't knownn
> at compile time. To use this, the CPU implementation should:
>  * define TARGET_PAGE_BITS_VARY
>  * not define TARGET_PAGE_BITS
>  * define TARGET_PAGE_BITS_MIN to the smallest value it
>    might possibly want for TARGET_PAGE_BITS
>  * call set_preferred_target_page_bits() in its realize
>    function to indicate the actual preferred target page
>    size for the CPU (and report any error from it)
> 
> In CONFIG_USER_ONLY, the CPU implementation should continue
> to define TARGET_PAGE_BITS appropriately for the guest
> OS page size.
> 
> Machines which want to take advantage of having the page
> size something larger than TARGET_PAGE_BITS_MIN must
> set the MachineClass minimum_page_bits field to a value
> which they guarantee will be no greater than the preferred
> page size for any CPU they create.
> 
> Note that changing the target page size by setting
> minimum_page_bits is a migration compatibility break
> for that machine.
> 
> For debugging purposes, attempts to use TARGET_PAGE_SIZE
> before it has been finally confirmed will assert.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  exec.c                 | 42 ++++++++++++++++++++++++++++++++++++++++++
>  include/exec/cpu-all.h |  8 ++++++++
>  include/hw/boards.h    |  7 +++++++
>  include/qemu-common.h  | 13 +++++++++++++
>  vl.c                   | 10 ++++++++++
>  5 files changed, 80 insertions(+)
> 
> diff --git a/exec.c b/exec.c
> index 8eaeb0c..027ce53 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -93,6 +93,11 @@ static MemoryRegion io_mem_unassigned;
>  
>  #endif
>  
> +#ifdef TARGET_PAGE_BITS_VARY
> +int target_page_bits;
> +bool target_page_bits_decided;
> +#endif
> +
>  struct CPUTailQ cpus = QTAILQ_HEAD_INITIALIZER(cpus);
>  /* current CPU in the current thread. It is only valid inside
>     cpu_exec() */
> @@ -102,8 +107,37 @@ __thread CPUState *current_cpu;
>     2 = Adaptive rate instruction counting.  */
>  int use_icount;
>  
> +bool set_preferred_target_page_bits(int bits)
> +{
> +    /* The target page size is the lowest common denominator for all
> +     * the CPUs in the system, so we can only make it smaller, never
> +     * larger. And we can't make it smaller once we've committed to
> +     * a particular size.
> +     */
> +#ifdef TARGET_PAGE_BITS_VARY
> +    assert(bits >= TARGET_PAGE_BITS_MIN);
> +    if (target_page_bits == 0 || target_page_bits > bits) {
> +        if (target_page_bits_decided) {
> +            return false;
> +        }
> +        target_page_bits = bits;
> +    }
> +#endif
> +    return true;
> +}
> +
>  #if !defined(CONFIG_USER_ONLY)
>  
> +static void finalize_target_page_bits(void)
> +{
> +#ifdef TARGET_PAGE_BITS_VARY
> +    if (target_page_bits == 0) {
> +        target_page_bits = TARGET_PAGE_BITS_MIN;
> +    }
> +    target_page_bits_decided = true;
> +#endif
> +}
> +
>  typedef struct PhysPageEntry PhysPageEntry;
>  
>  struct PhysPageEntry {
> @@ -2862,6 +2896,14 @@ void cpu_register_map_client(QEMUBH *bh)
>  void cpu_exec_init_all(void)
>  {
>      qemu_mutex_init(&ram_list.mutex);
> +    /* The data structures we set up here depend on knowing the page size,
> +     * so no more changes can be made after this point.
> +     * In an ideal world, nothing we did before we had finished the
> +     * machine setup would care about the target page size, and we could
> +     * do this much later, rather than requiring board models to state
> +     * up front what their requirements are.
> +     */
> +    finalize_target_page_bits();
>      io_mem_init();
>      memory_map_init();
>      qemu_mutex_init(&map_client_list_lock);
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index 9f38edf..dde3cd4 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -189,6 +189,14 @@ void address_space_stq(AddressSpace *as, hwaddr addr, uint64_t val,
>  
>  /* page related stuff */
>  
> +#ifdef TARGET_PAGE_BITS_VARY
> +extern bool target_page_bits_decided;
> +extern int target_page_bits;
> +#define TARGET_PAGE_BITS ({ assert(target_page_bits_decided); \
> +                            target_page_bits; })
> +
> +#endif
> +
>  #define TARGET_PAGE_SIZE (1 << TARGET_PAGE_BITS)
>  #define TARGET_PAGE_MASK ~(TARGET_PAGE_SIZE - 1)
>  #define TARGET_PAGE_ALIGN(addr) (((addr) + TARGET_PAGE_SIZE - 1) & TARGET_PAGE_MASK)
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 3ed6155..11808bc 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -85,6 +85,12 @@ typedef struct {
>   *    Returns a @HotpluggableCPUList, which describes CPUs objects which
>   *    could be added with -device/device_add.
>   *    Caller is responsible for freeing returned list.
> + * @minimum_page_bits:
> + *    If non-zero, the board promises never to create a CPU with a page size
> + *    smaller than this, so QEMU can use a more efficient larger page
> + *    size than the target architecture's minimum. (Attempting to create
> + *    such a CPU will fail.) Note that changing this is a migration
> + *    compatibility break for the machine.
>   */
>  struct MachineClass {
>      /*< private >*/
> @@ -123,6 +129,7 @@ struct MachineClass {
>      ram_addr_t default_ram_size;
>      bool option_rom_has_mr;
>      bool rom_file_has_mr;
> +    int minimum_page_bits;
>  
>      HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
>                                             DeviceState *dev);
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index 1f2cb94..fd9d4eb 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -76,6 +76,19 @@ void tcg_exec_init(unsigned long tb_size);
>  bool tcg_enabled(void);
>  
>  void cpu_exec_init_all(void);
> +void cpu_exec_machine_creation_done(void);

Was the above added here by mistake?

> +
> +/**
> + * set_preferred_target_page_bits:
> + * @bits: number of bits needed to represent an address within the page
> + *
> + * Set the preferred target page size (the actual target page
> + * size may be smaller than any given CPU's preference).
> + * Returns true on success, false on failure (which can only happen
> + * if this is called after the system has already finalized its
> + * choice of page size and the requested page size is smaller than that).
> + */
> +bool set_preferred_target_page_bits(int bits);
>  
>  /**
>   * Sends a (part of) iovec down a socket, yielding when the socket is full, or
> diff --git a/vl.c b/vl.c
> index 2f63eb4..61ae073 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4038,6 +4038,16 @@ int main(int argc, char **argv, char **envp)
>      }
>      object_property_add_child(object_get_root(), "machine",
>                                OBJECT(current_machine), &error_abort);
> +
> +    if (machine_class->minimum_page_bits) {
> +        if (!set_preferred_target_page_bits(machine_class->minimum_page_bits)) {
> +            /* This would be a board error: specifying a minimum smaller than
> +             * a target's compile-time fixed setting.
> +             */
> +            g_assert_not_reached();
> +        }
> +    }
> +
>      cpu_exec_init_all();
>  
>      if (machine_class->hw_version) {
> -- 
> 1.9.1
> 
>

Re: [Qemu-devel] [PATCH v2 6/6] hw/arm/virt: Set minimum_page_bits to 12

[Andrew Jones]

On Tue, Jun 21, 2016 at 06:09:34PM +0100, Peter Maydell wrote:
> Since the virt board model will never create a CPU which is
> pre-ARMv7, we know that our minimum page size is 4K and can
> set minimum_page_bits accordingly, for improved performance.
> 
> Note that this is a migration compatibility break.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/virt.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index c5c125e..f9b51aa 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1440,6 +1440,8 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>      mc->block_default_type = IF_VIRTIO;
>      mc->no_cdrom = 1;
>      mc->pci_allow_0_address = true;
> +    /* We know we will never create a pre-ARMv7 CPU which needs 1K pages */
> +    mc->minimum_page_bits = 12;
>  }

As this breaks migration, then I guess we also need

@@ -1507,5 +1510,6 @@ static void virt_machine_2_6_options(MachineClass *mc)
 {
     virt_machine_2_7_options(mc);
     SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_6);
+    mc->minimum_page_bits = 10;
 }
 DEFINE_VIRT_MACHINE(2, 6)

>  
>  static const TypeInfo virt_machine_info = {
> -- 
> 1.9.1
> 
> 

Re: [Qemu-devel] [PATCH v2 4/6] cpu: Support a target CPU having a variable page size

[Peter Maydell]

On 21 June 2016 at 19:26, Andrew Jones <drjones@redhat.com> wrote:
> On Tue, Jun 21, 2016 at 06:09:32PM +0100, Peter Maydell wrote:
>> --- a/include/qemu-common.h
>> +++ b/include/qemu-common.h
>> @@ -76,6 +76,19 @@ void tcg_exec_init(unsigned long tb_size);
>>  bool tcg_enabled(void);
>>
>>  void cpu_exec_init_all(void);
>> +void cpu_exec_machine_creation_done(void);
>
> Was the above added here by mistake?

Yes; leftover from an attempt at finalizing the page
size much later in init; I deleted the function but forgot
the prototype (and gcc doesn't warn about that).

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 6/6] hw/arm/virt: Set minimum_page_bits to 12

[Peter Maydell]

On 21 June 2016 at 19:45, Andrew Jones <drjones@redhat.com> wrote:
> On Tue, Jun 21, 2016 at 06:09:34PM +0100, Peter Maydell wrote:
>> Since the virt board model will never create a CPU which is
>> pre-ARMv7, we know that our minimum page size is 4K and can
>> set minimum_page_bits accordingly, for improved performance.
>>
>> Note that this is a migration compatibility break.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  hw/arm/virt.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index c5c125e..f9b51aa 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -1440,6 +1440,8 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>>      mc->block_default_type = IF_VIRTIO;
>>      mc->no_cdrom = 1;
>>      mc->pci_allow_0_address = true;
>> +    /* We know we will never create a pre-ARMv7 CPU which needs 1K pages */
>> +    mc->minimum_page_bits = 12;
>>  }
>
> As this breaks migration, then I guess we also need
>
> @@ -1507,5 +1510,6 @@ static void virt_machine_2_6_options(MachineClass *mc)
>  {
>      virt_machine_2_7_options(mc);
>      SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_6);
> +    mc->minimum_page_bits = 10;
>  }
>  DEFINE_VIRT_MACHINE(2, 6)

Doesn't hurt, but are we trying to claim migration
compat between different QEMU versions with the
versioned-machine names ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 1/6] migration: Remove static allocation of xzblre cache buffer

[Richard Henderson]

On 06/21/2016 10:09 AM, Peter Maydell wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
>
> Allocate xzblre zero page cache buffer dynamically.
> Remove dependency on TARGET_PAGE_SIZE to make run-time
> page size detection for arm platforms.
>
> Signed-off-by: Vijaya Kumar K <vijayak@cavium.com>
> Message-id: 1465808915-4887-2-git-send-email-vijayak@caviumnetworks.com
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  migration/ram.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [PATCH v2 2/6] exec.c: Remove static allocation of sub_section of sub_page

[Richard Henderson]

On 06/21/2016 10:09 AM, Peter Maydell wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
>
> Allocate sub_section dynamically. Remove dependency
> on TARGET_PAGE_SIZE to make run-time page size detection
> for arm platforms.
>
> Signed-off-by: Vijaya Kumar K <vijayak@cavium.com>
> Message-id: 1465808915-4887-3-git-send-email-vijayak@caviumnetworks.com
> [PMM: use flexible array member rather than separate malloc
>  so we don't need an extra pointer deref when using it]
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  exec.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] Fw: [Qemu-arm] [PATCH v2 2/6] exec.c: Remove static allocation of sub_section of sub_page

[Vijay Kilari]

> ________________________________
> From: Qemu-arm <qemu-arm-bounces+vijayak=caviumnetworks.com@nongnu.org> on
> behalf of Peter Maydell <peter.maydell@linaro.org>
> Sent: Tuesday, June 21, 2016 10:39 PM
> To: qemu-arm@nongnu.org; qemu-devel@nongnu.org
> Cc: Paolo Bonzini; Kumar, Vijaya; patches@linaro.org
> Subject: [Qemu-arm] [PATCH v2 2/6] exec.c: Remove static allocation of
> sub_section of sub_page
>
> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
>
> Allocate sub_section dynamically. Remove dependency
> on TARGET_PAGE_SIZE to make run-time page size detection
> for arm platforms.
>
> Signed-off-by: Vijaya Kumar K <vijayak@cavium.com>
> Message-id: 1465808915-4887-3-git-send-email-vijayak@caviumnetworks.com
> [PMM: use flexible array member rather than separate malloc
>  so we don't need an extra pointer deref when using it]
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  exec.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 0122ef7..8eaeb0c 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -153,7 +153,7 @@ typedef struct subpage_t {
>      MemoryRegion iomem;
>      AddressSpace *as;
>      hwaddr base;
> -    uint16_t sub_section[TARGET_PAGE_SIZE];
> +    uint16_t sub_section[];
   NIT: Comment that this variable should be last member of this
struct will be helpful.

>  } subpage_t;
>
>  #define PHYS_SECTION_UNASSIGNED 0
> @@ -2270,8 +2270,7 @@ static subpage_t *subpage_init(AddressSpace *as,
> hwaddr base)
>  {
>      subpage_t *mmio;
>
> -    mmio = g_malloc0(sizeof(subpage_t));
> -
> +    mmio = g_malloc0(sizeof(subpage_t) + TARGET_PAGE_SIZE *
> sizeof(uint16_t));

NIT: sizeof(mmio->sub_section[0]) looks better than sizeof(uint16_t)

>      mmio->as = as;
>      mmio->base = base;
>      memory_region_init_io(&mmio->iomem, NULL, &subpage_ops, mmio,
> --
> 1.9.1
>
>

Re: [Qemu-devel] [PATCH v2 3/6] translate-all.c: Compute L1 page table properties at runtime

[Richard Henderson]

On 06/21/2016 10:09 AM, Peter Maydell wrote:
> @@ -825,8 +846,8 @@ static void page_flush_tb(void)
>  {
>      int i;
>
> -    for (i = 0; i < V_L1_SIZE; i++) {
> -        page_flush_tb_1(V_L1_SHIFT / V_L2_BITS - 1, l1_map + i);
> +    for (i = 0; i < v_l1_size; i++) {
> +        page_flush_tb_1(v_l2_levels, l1_map + i);
>      }
>  }
>
> @@ -1850,9 +1871,9 @@ int walk_memory_regions(void *priv, walk_memory_regions_fn fn)
>      data.start = -1u;
>      data.prot = 0;
>
> -    for (i = 0; i < V_L1_SIZE; i++) {
> -        int rc = walk_memory_regions_1(&data, (target_ulong)i << (V_L1_SHIFT + TARGET_PAGE_BITS),
> -                                       V_L1_SHIFT / V_L2_BITS - 1, l1_map + i);
> +    for (i = 0; i < v_l1_size; i++) {
> +        target_ulong base = i << (v_l1_shift + TARGET_PAGE_BITS);
> +        int rc = walk_memory_regions_1(&data, base, v_l2_levels, l1_map + i);

It would probably be good to cache v_l1_size in a local, so that we don't keep 
re-reading it around the loop when we know it doesn't change.

Otherwise,

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] Fw: [Qemu-arm] [PATCH v2 2/6] exec.c: Remove static allocation of sub_section of sub_page

[Richard Henderson]

On 06/21/2016 11:55 PM, Vijay Kilari wrote:
>> > +    uint16_t sub_section[];
>    NIT: Comment that this variable should be last member of this
> struct will be helpful.
>

No comment required.  The compiler will error otherwise.


r~

Re: [Qemu-devel] [PATCH v2 6/6] hw/arm/virt: Set minimum_page_bits to 12

[Andrew Jones]

On Tue, Jun 21, 2016 at 08:47:39PM +0100, Peter Maydell wrote:
> On 21 June 2016 at 19:45, Andrew Jones <drjones@redhat.com> wrote:
> > On Tue, Jun 21, 2016 at 06:09:34PM +0100, Peter Maydell wrote:
> >> Since the virt board model will never create a CPU which is
> >> pre-ARMv7, we know that our minimum page size is 4K and can
> >> set minimum_page_bits accordingly, for improved performance.
> >>
> >> Note that this is a migration compatibility break.
> >>
> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> >> ---
> >>  hw/arm/virt.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> >> index c5c125e..f9b51aa 100644
> >> --- a/hw/arm/virt.c
> >> +++ b/hw/arm/virt.c
> >> @@ -1440,6 +1440,8 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
> >>      mc->block_default_type = IF_VIRTIO;
> >>      mc->no_cdrom = 1;
> >>      mc->pci_allow_0_address = true;
> >> +    /* We know we will never create a pre-ARMv7 CPU which needs 1K pages */
> >> +    mc->minimum_page_bits = 12;
> >>  }
> >
> > As this breaks migration, then I guess we also need
> >
> > @@ -1507,5 +1510,6 @@ static void virt_machine_2_6_options(MachineClass *mc)
> >  {
> >      virt_machine_2_7_options(mc);
> >      SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_6);
> > +    mc->minimum_page_bits = 10;
> >  }
> >  DEFINE_VIRT_MACHINE(2, 6)
> 
> Doesn't hurt, but are we trying to claim migration
> compat between different QEMU versions with the
> versioned-machine names ?

I'm still learning how best to approach compat/migration concerns.
I've CC'ed David to help.

Up until now my main concern has been keeping the RHEL-x.y mach-virt
(and now upstream versioned mach-virt) type from changing as new
features are added. In which case, I think the above makes sense
regardless. With respect to migration I'm unsure of the claims we
can/should make. IMO, we'd ideally be able to always migrate a
versioned-machine (obviously running with a QEMU that supports that
version) to a host with a later QEMU (which, being later, means it
also supports that version)

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 6/6] hw/arm/virt: Set minimum_page_bits to 12

[Paolo Bonzini]

On 21/06/2016 21:47, Peter Maydell wrote:
>> >
>> > As this breaks migration, then I guess we also need
>> >
>> > @@ -1507,5 +1510,6 @@ static void virt_machine_2_6_options(MachineClass *mc)
>> >  {
>> >      virt_machine_2_7_options(mc);
>> >      SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_6);
>> > +    mc->minimum_page_bits = 10;
>> >  }
>> >  DEFINE_VIRT_MACHINE(2, 6)
> Doesn't hurt, but are we trying to claim migration
> compat between different QEMU versions with the
> versioned-machine names ?

For PC machines we are (but only towards newer QEMU versions).

Paolo

Re: [Qemu-devel] [PATCH v2 6/6] hw/arm/virt: Set minimum_page_bits to 12

[Dr. David Alan Gilbert]

* Andrew Jones (drjones@redhat.com) wrote:
> On Tue, Jun 21, 2016 at 08:47:39PM +0100, Peter Maydell wrote:
> > On 21 June 2016 at 19:45, Andrew Jones <drjones@redhat.com> wrote:
> > > On Tue, Jun 21, 2016 at 06:09:34PM +0100, Peter Maydell wrote:
> > >> Since the virt board model will never create a CPU which is
> > >> pre-ARMv7, we know that our minimum page size is 4K and can
> > >> set minimum_page_bits accordingly, for improved performance.
> > >>
> > >> Note that this is a migration compatibility break.
> > >>
> > >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > >> ---
> > >>  hw/arm/virt.c | 2 ++
> > >>  1 file changed, 2 insertions(+)
> > >>
> > >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > >> index c5c125e..f9b51aa 100644
> > >> --- a/hw/arm/virt.c
> > >> +++ b/hw/arm/virt.c
> > >> @@ -1440,6 +1440,8 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
> > >>      mc->block_default_type = IF_VIRTIO;
> > >>      mc->no_cdrom = 1;
> > >>      mc->pci_allow_0_address = true;
> > >> +    /* We know we will never create a pre-ARMv7 CPU which needs 1K pages */
> > >> +    mc->minimum_page_bits = 12;
> > >>  }
> > >
> > > As this breaks migration, then I guess we also need
> > >
> > > @@ -1507,5 +1510,6 @@ static void virt_machine_2_6_options(MachineClass *mc)
> > >  {
> > >      virt_machine_2_7_options(mc);
> > >      SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_6);
> > > +    mc->minimum_page_bits = 10;
> > >  }
> > >  DEFINE_VIRT_MACHINE(2, 6)
> > 
> > Doesn't hurt, but are we trying to claim migration
> > compat between different QEMU versions with the
> > versioned-machine names ?
> 
> I'm still learning how best to approach compat/migration concerns.
> I've CC'ed David to help.
> 
> Up until now my main concern has been keeping the RHEL-x.y mach-virt
> (and now upstream versioned mach-virt) type from changing as new
> features are added. In which case, I think the above makes sense
> regardless. With respect to migration I'm unsure of the claims we
> can/should make. IMO, we'd ideally be able to always migrate a
> versioned-machine (obviously running with a QEMU that supports that
> version) to a host with a later QEMU (which, being later, means it
> also supports that version)

Yes, my hope is that any versioned machine type should migrate to
a newer qemu with the same machine type set.

There are really two separate things that we state with the machine
versioning:
  a) that the guest view is the same
  b) that the migration format is the same

Dave

> Thanks,
> drew
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 6/6] hw/arm/virt: Set minimum_page_bits to 12

[Peter Maydell]

On 22 June 2016 at 12:35, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> Yes, my hope is that any versioned machine type should migrate to
> a newer qemu with the same machine type set.
>
> There are really two separate things that we state with the machine
> versioning:
>   a) that the guest view is the same
>   b) that the migration format is the same

Well, this is true for PC. But my impression when we started
applying versioned machine types to ARM virt was that it was
signing up to (a) but not (yet) (b)...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 0/6] Runtime pagesize computation

[Dr. David Alan Gilbert]

* Peter Maydell (peter.maydell@linaro.org) wrote:
> This set of patches is a development based on the ones from Vijaya:
> the general idea is similar but I have tried to improve the interface
> for defining the page size a bit.  I've also tweaked patches 2 and 3
> to address code review comments.
> 
> The basic idea here is that:
>  * the target CPU implementation has to opt into variable page size
>    by defining TARGET_PAGE_BITS_VARY, and then calling
>    set_preferred_target_page_bits() in its realize function
>    with whatever the CPU as instantiated actually requires
>  * the machine also has to opt in, by specifying a new MachineClass
>    field which states the value they guarantee will be no greater
>    than the preferred page size for any CPU they create
>  * we finalize the decision about page size in cpu_exec_init_all()
>    (and then later attempts to create CPUs which can't cope with
>    that decision are failed)
> 
> I would ideally have liked to finalize things much later, but
> this is in practice hugely difficult because so many things
> (in particular all the address space/memory system code)
> assume the target page size is known.
> 
> Note that setting minimum_page-bits for a machine is a migration
> compatibility break (the RAM migration format assumes both sides
> have the same idea of a page size). Mismatches will probably
> result in an unhelpfully obscure migration failure. (Possibly we
> could have the migration send the page size if it was different
> from TARGET_PAGE_BITS_MIN so we at least got a nice error?)

Yes, that would be a good idea;  how about adding it as a subsection
in the vmstate_configuration (migration/savevm.c); if you only
send it in the case where it's not the existing size then it shouldn't
break any old formats.  That configuration block gets loaded
very early.

Dave

> 
> This could in theory be extended to the user-mode binaries,
> but for the moment I have just required them to define a
> fixed TARGET_PAGE_BITS.
> 
> NB: I have only very lightly tested these and haven't attempted
> to measure performance at all. There is an assert() in the
> definition of TARGET_PAGE_BITS which is good for making sure
> it isn't used before it's valid but not so good for speed.
> 
> thanks
> -- PMM
> 
> 
> Peter Maydell (3):
>   cpu: Support a target CPU having a variable page size
>   target-arm: Make page size a runtime setting
>   hw/arm/virt: Set minimum_page_bits to 12
> 
> Vijaya Kumar K (3):
>   migration: Remove static allocation of xzblre cache buffer
>   exec.c: Remove static allocation of sub_section of sub_page
>   translate-all.c: Compute L1 page table properties at runtime
> 
>  exec.c                 | 47 ++++++++++++++++++++++++++++++++---
>  hw/arm/virt.c          |  2 ++
>  include/exec/cpu-all.h |  8 ++++++
>  include/hw/boards.h    |  7 ++++++
>  include/qemu-common.h  | 13 ++++++++++
>  migration/ram.c        |  4 ++-
>  target-arm/cpu.c       | 24 ++++++++++++++++++
>  target-arm/cpu.h       |  9 ++++---
>  translate-all.c        | 67 +++++++++++++++++++++++++++++++++-----------------
>  vl.c                   | 10 ++++++++
>  10 files changed, 160 insertions(+), 31 deletions(-)
> 
> -- 
> 1.9.1
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 6/6] hw/arm/virt: Set minimum_page_bits to 12

[Dr. David Alan Gilbert]

* Peter Maydell (peter.maydell@linaro.org) wrote:
> On 22 June 2016 at 12:35, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> > Yes, my hope is that any versioned machine type should migrate to
> > a newer qemu with the same machine type set.
> >
> > There are really two separate things that we state with the machine
> > versioning:
> >   a) that the guest view is the same
> >   b) that the migration format is the same
> 
> Well, this is true for PC.

PC, ppc/spapr, and I think s390.

> But my impression when we started
> applying versioned machine types to ARM virt was that it was
> signing up to (a) but not (yet) (b)...

So that would make ARM special; the problem is we have no way
to communicate to the user that it's special.

Dave

> 
> thanks
> -- PMM
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 6/6] hw/arm/virt: Set minimum_page_bits to 12

[Peter Maydell]

On 22 June 2016 at 12:54, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> * Peter Maydell (peter.maydell@linaro.org) wrote:
>> On 22 June 2016 at 12:35, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
>> > Yes, my hope is that any versioned machine type should migrate to
>> > a newer qemu with the same machine type set.
>> >
>> > There are really two separate things that we state with the machine
>> > versioning:
>> >   a) that the guest view is the same
>> >   b) that the migration format is the same
>>
>> Well, this is true for PC.
>
> PC, ppc/spapr, and I think s390.
>
>> But my impression when we started
>> applying versioned machine types to ARM virt was that it was
>> signing up to (a) but not (yet) (b)...
>
> So that would make ARM special; the problem is we have no way
> to communicate to the user that it's special.

Is anybody testing that migration between versions for
virt works?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 6/6] hw/arm/virt: Set minimum_page_bits to 12

[Dr. David Alan Gilbert]

* Peter Maydell (peter.maydell@linaro.org) wrote:
> On 22 June 2016 at 12:54, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> > * Peter Maydell (peter.maydell@linaro.org) wrote:
> >> On 22 June 2016 at 12:35, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> >> > Yes, my hope is that any versioned machine type should migrate to
> >> > a newer qemu with the same machine type set.
> >> >
> >> > There are really two separate things that we state with the machine
> >> > versioning:
> >> >   a) that the guest view is the same
> >> >   b) that the migration format is the same
> >>
> >> Well, this is true for PC.
> >
> > PC, ppc/spapr, and I think s390.
> >
> >> But my impression when we started
> >> applying versioned machine types to ARM virt was that it was
> >> signing up to (a) but not (yet) (b)...
> >
> > So that would make ARM special; the problem is we have no way
> > to communicate to the user that it's special.
> 
> Is anybody testing that migration between versions for
> virt works?

I don't know (Perhaps Drew knows), but I'd love to add
a make check test for the x86 equivalent, but there's two
things I'm stuck on;
   1) What to run in the guest to check it survives
   2) How to know we have an old qemu binary to check.

  I was thinking for (2) I could use an environment variable
and if it's not set then skip the test.
  For (1) perhaps a trivial little image (like I use for postcopy)
that wouldn't check much except that the CPU/serial is still happy,
but at least migration completed OK.

  You could use virt-test/avocado to run a test with a full guest;
it has options to specify source and destination qemu's separately.

Dave

> 
> thanks
> -- PMM
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 6/6] hw/arm/virt: Set minimum_page_bits to 12

[Andrew Jones]

On Wed, Jun 22, 2016 at 01:02:04PM +0100, Peter Maydell wrote:
> On 22 June 2016 at 12:54, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> > * Peter Maydell (peter.maydell@linaro.org) wrote:
> >> On 22 June 2016 at 12:35, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> >> > Yes, my hope is that any versioned machine type should migrate to
> >> > a newer qemu with the same machine type set.
> >> >
> >> > There are really two separate things that we state with the machine
> >> > versioning:
> >> >   a) that the guest view is the same
> >> >   b) that the migration format is the same
> >>
> >> Well, this is true for PC.
> >
> > PC, ppc/spapr, and I think s390.
> >
> >> But my impression when we started
> >> applying versioned machine types to ARM virt was that it was
> >> signing up to (a) but not (yet) (b)...
> >
> > So that would make ARM special; the problem is we have no way
> > to communicate to the user that it's special.
> 
> Is anybody testing that migration between versions for
> virt works?

We've only tested migration between identically configured hosts
so far. This is partly due to the fact that we haven't released
a RHEL machine type yet, i.e. we're working towards a RHEL-7.3
machine type on a RHEL-7.3 QEMU, which will be the first and only
we have. When we have a development 7.4 branch to work with then
we'd test a 7.3 machine migration from a 7.3 host to a 7.4 host
as well.

I'll add trying an upstream 2.6 machine type migration from a
2.6 QEMU to a latest QEMU to our TODO.

Thanks,
drew

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 0/6] Runtime pagesize computation

[Peter Maydell]

On 21 June 2016 at 18:09, Peter Maydell <peter.maydell@linaro.org> wrote:
> This set of patches is a development based on the ones from Vijaya:
> the general idea is similar but I have tried to improve the interface
> for defining the page size a bit.  I've also tweaked patches 2 and 3
> to address code review comments.

> NB: I have only very lightly tested these and haven't attempted
> to measure performance at all. There is an assert() in the
> definition of TARGET_PAGE_BITS which is good for making sure
> it isn't used before it's valid but not so good for speed.

Vijaya, are you in a position to test this patchset for
performance? Presumably you have a test case benchmark you're
looking to improve here?

thanks
-- PMM

Re: [Qemu-devel] Fw: [Qemu-arm] [PATCH v2 0/6] Runtime pagesize computation

[Vijay Kilari]

On Wed, Jun 29, 2016 at 12:24 PM, Kumar, Vijaya <Vijaya.Kumar@cavium.com> wrote:
>
>
>
> ________________________________
> From: Peter Maydell <peter.maydell@linaro.org>
> Sent: Tuesday, June 28, 2016 1:46 PM
> To: qemu-arm; QEMU Developers
> Cc: Paolo Bonzini; Kumar, Vijaya; Patch Tracking
> Subject: Re: [Qemu-arm] [PATCH v2 0/6] Runtime pagesize computation
>
> On 21 June 2016 at 18:09, Peter Maydell <peter.maydell@linaro.org> wrote:
>> This set of patches is a development based on the ones from Vijaya:
>> the general idea is similar but I have tried to improve the interface
>> for defining the page size a bit.  I've also tweaked patches 2 and 3
>> to address code review comments.
>
>> NB: I have only very lightly tested these and haven't attempted
>> to measure performance at all. There is an assert() in the
>> definition of TARGET_PAGE_BITS which is good for making sure
>> it isn't used before it's valid but not so good for speed.
>
> Vijaya, are you in a position to test this patchset for
> performance? Presumably you have a test case benchmark you're
> looking to improve here?

I have tested the patches and the test case that I was trying was
Live migration of Idle VM on arm64 platform.
VM migrated is with 4 VCPUS and 8GB RAM running CentOS.

With page bits 10 (1K), the live migration time is 5.8 sec

capabilities: xbzrle: off rdma-pin-all: off auto-converge: off
zero-blocks: off compress: off events: off x-postcopy-ram: off
Migration status: completed
total time: 5857 milliseconds
downtime: 102 milliseconds
setup: 14 milliseconds
transferred ram: 336081 kbytes
throughput: 470.21 mbps
remaining ram: 0 kbytes
total ram: 8519872 kbytes
duplicate: 8271539 pages
skipped: 0 pages
normal: 261340 pages
normal bytes: 261340 kbytes
dirty sync count: 3

With page bits 12 (4K), live migration time is 2.9 sec

capabilities: xbzrle: off rdma-pin-all: off auto-converge: off
zero-blocks: off compress: off events: off x-postcopy-ram: off
Migration status: completed
total time: 2974 milliseconds
downtime: 76 milliseconds
setup: 5 milliseconds
transferred ram: 301327 kbytes
throughput: 830.30 mbps
remaining ram: 0 kbytes
total ram: 8519872 kbytes
duplicate: 2062398 pages
skipped: 0 pages
normal: 70662 pages
normal bytes: 282648 kbytes
dirty sync count: 3

Regards
Vijay
>
> thanks
> -- PMM

Re: [Qemu-devel] Fw: [Qemu-arm] [PATCH v2 0/6] Runtime pagesize computation

[Vijay Kilari]

Hi Peter,

  Any update on this patch set. Is it merged?

On Wed, Jun 29, 2016 at 12:30 PM, Vijay Kilari <vijay.kilari@gmail.com> wrote:
> On Wed, Jun 29, 2016 at 12:24 PM, Kumar, Vijaya <Vijaya.Kumar@cavium.com> wrote:
>>
>>
>>
>> ________________________________
>> From: Peter Maydell <peter.maydell@linaro.org>
>> Sent: Tuesday, June 28, 2016 1:46 PM
>> To: qemu-arm; QEMU Developers
>> Cc: Paolo Bonzini; Kumar, Vijaya; Patch Tracking
>> Subject: Re: [Qemu-arm] [PATCH v2 0/6] Runtime pagesize computation
>>
>> On 21 June 2016 at 18:09, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> This set of patches is a development based on the ones from Vijaya:
>>> the general idea is similar but I have tried to improve the interface
>>> for defining the page size a bit.  I've also tweaked patches 2 and 3
>>> to address code review comments.
>>
>>> NB: I have only very lightly tested these and haven't attempted
>>> to measure performance at all. There is an assert() in the
>>> definition of TARGET_PAGE_BITS which is good for making sure
>>> it isn't used before it's valid but not so good for speed.
>>
>> Vijaya, are you in a position to test this patchset for
>> performance? Presumably you have a test case benchmark you're
>> looking to improve here?
>
> I have tested the patches and the test case that I was trying was
> Live migration of Idle VM on arm64 platform.
> VM migrated is with 4 VCPUS and 8GB RAM running CentOS.
>
> With page bits 10 (1K), the live migration time is 5.8 sec
>
> capabilities: xbzrle: off rdma-pin-all: off auto-converge: off
> zero-blocks: off compress: off events: off x-postcopy-ram: off
> Migration status: completed
> total time: 5857 milliseconds
> downtime: 102 milliseconds
> setup: 14 milliseconds
> transferred ram: 336081 kbytes
> throughput: 470.21 mbps
> remaining ram: 0 kbytes
> total ram: 8519872 kbytes
> duplicate: 8271539 pages
> skipped: 0 pages
> normal: 261340 pages
> normal bytes: 261340 kbytes
> dirty sync count: 3
>
> With page bits 12 (4K), live migration time is 2.9 sec
>
> capabilities: xbzrle: off rdma-pin-all: off auto-converge: off
> zero-blocks: off compress: off events: off x-postcopy-ram: off
> Migration status: completed
> total time: 2974 milliseconds
> downtime: 76 milliseconds
> setup: 5 milliseconds
> transferred ram: 301327 kbytes
> throughput: 830.30 mbps
> remaining ram: 0 kbytes
> total ram: 8519872 kbytes
> duplicate: 2062398 pages
> skipped: 0 pages
> normal: 70662 pages
> normal bytes: 282648 kbytes
> dirty sync count: 3
>
> Regards
> Vijay
>>
>> thanks
>> -- PMM

Re: [Qemu-devel] Fw: [Qemu-arm] [PATCH v2 0/6] Runtime pagesize computation

[Peter Maydell]

On 19 July 2016 at 12:01, Vijay Kilari <vijay.kilari@gmail.com> wrote:
> Hi Peter,
>
>   Any update on this patch set. Is it merged?

No. It's an RFC patchset really, and it was posted during softfreeze,
so I never expected or intended it to be merged before the 2.7
release. It also needs a respin to address some of the review comments
I got on it.

What would be useful in getting it further forward would be
some good performance benchmarking of more than just migration.
In particular whether it improves the speed in TCG emulation
mode of aarch64 guests and whether that assert in the definition
of TARGET_PAGE_BITS is particularly performance-draining.

thanks
-- PMM

Re: [Qemu-devel] Fw: [Qemu-arm] [PATCH v2 0/6] Runtime pagesize computation

[Peter Maydell]

On 19 July 2016 at 12:04, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 19 July 2016 at 12:01, Vijay Kilari <vijay.kilari@gmail.com> wrote:
>> Hi Peter,
>>
>>   Any update on this patch set. Is it merged?
>
> No. It's an RFC patchset really, and it was posted during softfreeze,
> so I never expected or intended it to be merged before the 2.7
> release. It also needs a respin to address some of the review comments
> I got on it.
>
> What would be useful in getting it further forward would be
> some good performance benchmarking of more than just migration.
> In particular whether it improves the speed in TCG emulation
> mode of aarch64 guests and whether that assert in the definition
> of TARGET_PAGE_BITS is particularly performance-draining.

If I do a respin of this patchset is anybody willing to do
the actual perf benchmarking of the TCG emulation?
Otherwise this is unlikely to go into 2.8.

thanks
-- PMM

Re: [Qemu-devel] Fw: [Qemu-arm] [PATCH v2 0/6] Runtime pagesize computation

[Vijay Kilari]

Hi Peter,

On Fri, Oct 7, 2016 at 7:50 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 19 July 2016 at 12:04, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 19 July 2016 at 12:01, Vijay Kilari <vijay.kilari@gmail.com> wrote:
>>> Hi Peter,
>>>
>>>   Any update on this patch set. Is it merged?
>>
>> No. It's an RFC patchset really, and it was posted during softfreeze,
>> so I never expected or intended it to be merged before the 2.7
>> release. It also needs a respin to address some of the review comments
>> I got on it.
>>
>> What would be useful in getting it further forward would be
>> some good performance benchmarking of more than just migration.
>> In particular whether it improves the speed in TCG emulation
>> mode of aarch64 guests and whether that assert in the definition
>> of TARGET_PAGE_BITS is particularly performance-draining.
>
> If I do a respin of this patchset is anybody willing to do
> the actual perf benchmarking of the TCG emulation?
> Otherwise this is unlikely to go into 2.8.

 I can help you. Let me know details on benchmarkings to be done.
Also, a reference/details about running arm64 guests in TCG mode
would be helpful.

Regards
Vijay