qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 0/7] Runtime pagesize computation
@ 2016-10-11 17:08 Peter Maydell
  2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 1/7] migration: Remove static allocation of xzblre cache buffer Peter Maydell
                   ` (10 more replies)
  0 siblings, 11 replies; 17+ messages in thread
From: Peter Maydell @ 2016-10-11 17:08 UTC (permalink / raw)
  To: qemu-arm, qemu-devel
  Cc: patches, paolo, Vijaya Kumar K, Dr . David Alan Gilbert,
	Laurent Desnogues, Richard Henderson

(I posted version 2 way back in June...)

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). In v3 I have included a
patch which adds the target-page-bits to the migration stream
so that you get a reasonably nice error message if you try
to migrate between source and destination which disagree about
the page size.

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.


Patches 1-3 have been reviewed, so 4-7 are the interesting
ones.

REQUEST FOR TESTING/PERFORMANCE BENCHMARKING:

I have only very lightly tested these and haven't attempted
to measure performance at all. Further testing and
benchmarking reports are therefore welcome.

In particular I would like to know how much of an
effect on TCG performance the assert() in the definition
of TARGET_PAGE_BITS has, so comparisons of all three configs:
 * before this patchset (ie TARGET_PAGE_BITS compile time const)
 * with this patchset (with assert in TARGET_PAGE_BITS)
 * this patchset, but with the assert commented out


Changes v2->v3:
 * rebased
 * added new patch to migrate the page size if it's not the default
 * virt-2.7 and earlier machines stick with the old
   default page size to avoid a compat break
 * in patch 3, v_l1_size is pulled into a local when we're
   using it as a loop upper bound

thanks
-- PMM


Peter Maydell (4):
  cpu: Support a target CPU having a variable page size
  migration/savevm.c: migrate non-default 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          |  4 +++
 include/exec/cpu-all.h |  9 +++++++
 include/hw/boards.h    |  7 +++++
 include/qemu-common.h  | 12 +++++++++
 migration/ram.c        |  4 ++-
 migration/savevm.c     | 49 ++++++++++++++++++++++++++++++++++
 target-arm/cpu.c       | 24 +++++++++++++++++
 target-arm/cpu.h       |  9 ++++---
 translate-all.c        | 71 ++++++++++++++++++++++++++++++++------------------
 vl.c                   | 10 +++++++
 11 files changed, 213 insertions(+), 33 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH v3 1/7] migration: Remove static allocation of xzblre cache buffer
  2016-10-11 17:08 [Qemu-devel] [PATCH v3 0/7] Runtime pagesize computation Peter Maydell
@ 2016-10-11 17:08 ` Peter Maydell
  2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 2/7] exec.c: Remove static allocation of sub_section of sub_page Peter Maydell
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2016-10-11 17:08 UTC (permalink / raw)
  To: qemu-arm, qemu-devel
  Cc: patches, paolo, Vijaya Kumar K, Dr . David Alan Gilbert,
	Laurent Desnogues, Richard Henderson

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>
Reviewed-by: Richard Henderson <rth@twiddle.net>
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 c8ec9f2..b5bde00 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);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH v3 2/7] exec.c: Remove static allocation of sub_section of sub_page
  2016-10-11 17:08 [Qemu-devel] [PATCH v3 0/7] Runtime pagesize computation Peter Maydell
  2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 1/7] migration: Remove static allocation of xzblre cache buffer Peter Maydell
@ 2016-10-11 17:08 ` Peter Maydell
  2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 3/7] translate-all.c: Compute L1 page table properties at runtime Peter Maydell
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2016-10-11 17:08 UTC (permalink / raw)
  To: qemu-arm, qemu-devel
  Cc: patches, paolo, Vijaya Kumar K, Dr . David Alan Gilbert,
	Laurent Desnogues, Richard Henderson

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]
Reviewed-by: Richard Henderson <rth@twiddle.net>
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 374c364..86b09ac 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
@@ -2205,8 +2205,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,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH v3 3/7] translate-all.c: Compute L1 page table properties at runtime
  2016-10-11 17:08 [Qemu-devel] [PATCH v3 0/7] Runtime pagesize computation Peter Maydell
  2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 1/7] migration: Remove static allocation of xzblre cache buffer Peter Maydell
  2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 2/7] exec.c: Remove static allocation of sub_section of sub_page Peter Maydell
@ 2016-10-11 17:08 ` Peter Maydell
  2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 4/7] cpu: Support a target CPU having a variable page size Peter Maydell
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2016-10-11 17:08 UTC (permalink / raw)
  To: qemu-arm, qemu-devel
  Cc: patches, paolo, Vijaya Kumar K, Dr . David Alan Gilbert,
	Laurent Desnogues, Richard Henderson

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
 put v_l1_size into a local where used as a loop limit]
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 translate-all.c | 71 +++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 46 insertions(+), 25 deletions(-)

diff --git a/translate-all.c b/translate-all.c
index 8ca393c9d..86b45a1 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
@@ -332,6 +351,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
@@ -408,10 +429,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) {
@@ -826,10 +847,10 @@ static void page_flush_tb_1(int level, void **lp)
 
 static void page_flush_tb(void)
 {
-    int i;
+    int i, l1_sz = v_l1_size;
 
-    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 < l1_sz; i++) {
+        page_flush_tb_1(v_l2_levels, l1_map + i);
     }
 }
 
@@ -1883,16 +1904,16 @@ static int walk_memory_regions_1(struct walk_memory_regions_data *data,
 int walk_memory_regions(void *priv, walk_memory_regions_fn fn)
 {
     struct walk_memory_regions_data data;
-    uintptr_t i;
+    uintptr_t i, l1_sz = v_l1_size;
 
     data.fn = fn;
     data.priv = priv;
     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 < l1_sz; 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;
         }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH v3 4/7] cpu: Support a target CPU having a variable page size
  2016-10-11 17:08 [Qemu-devel] [PATCH v3 0/7] Runtime pagesize computation Peter Maydell
                   ` (2 preceding siblings ...)
  2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 3/7] translate-all.c: Compute L1 page table properties at runtime Peter Maydell
@ 2016-10-11 17:08 ` Peter Maydell
  2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 5/7] migration/savevm.c: migrate non-default " Peter Maydell
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2016-10-11 17:08 UTC (permalink / raw)
  To: qemu-arm, qemu-devel
  Cc: patches, paolo, Vijaya Kumar K, Dr . David Alan Gilbert,
	Laurent Desnogues, Richard Henderson

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 |  9 +++++++++
 include/hw/boards.h    |  7 +++++++
 include/qemu-common.h  | 12 ++++++++++++
 vl.c                   | 10 ++++++++++
 5 files changed, 80 insertions(+)

diff --git a/exec.c b/exec.c
index 86b09ac..25d972f 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 {
@@ -2797,6 +2831,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 b6a7059..861260d 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -189,6 +189,15 @@ 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; })
+#else
+#define TARGET_PAGE_BITS_MIN 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 e46a744..a51da9c 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -86,6 +86,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 >*/
@@ -124,6 +130,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 9e8b0bd..7e6e4fe 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -82,6 +82,18 @@ bool tcg_enabled(void);
 void cpu_exec_init_all(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
  * Receives data into a (part of) iovec from a socket,
  * yielding when there is no data in the socket.
diff --git a/vl.c b/vl.c
index eb3c5ee..5feaae9 100644
--- a/vl.c
+++ b/vl.c
@@ -4088,6 +4088,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) {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH v3 5/7] migration/savevm.c: migrate non-default page size
  2016-10-11 17:08 [Qemu-devel] [PATCH v3 0/7] Runtime pagesize computation Peter Maydell
                   ` (3 preceding siblings ...)
  2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 4/7] cpu: Support a target CPU having a variable page size Peter Maydell
@ 2016-10-11 17:08 ` Peter Maydell
  2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 6/7] target-arm: Make page size a runtime setting Peter Maydell
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2016-10-11 17:08 UTC (permalink / raw)
  To: qemu-arm, qemu-devel
  Cc: patches, paolo, Vijaya Kumar K, Dr . David Alan Gilbert,
	Laurent Desnogues, Richard Henderson

Add a subsection to vmstate_configuration which is present
only if the guest is using a target page size which is
different from the default. This allows us to helpfully
diagnose attempts to migrate between machines which
are using different target page sizes.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 migration/savevm.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/migration/savevm.c b/migration/savevm.c
index 33a2911..48a47fb 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -265,6 +265,7 @@ typedef struct SaveState {
     bool skip_configuration;
     uint32_t len;
     const char *name;
+    uint32_t target_page_bits;
 } SaveState;
 
 static SaveState savevm_state = {
@@ -286,6 +287,19 @@ static void configuration_pre_save(void *opaque)
 
     state->len = strlen(current_name);
     state->name = current_name;
+    state->target_page_bits = TARGET_PAGE_BITS;
+}
+
+static int configuration_pre_load(void *opaque)
+{
+    SaveState *state = opaque;
+
+    /* If there is no target-page-bits subsection it means the source
+     * predates the variable-target-page-bits support and is using the
+     * minimum possible value for this CPU.
+     */
+    state->target_page_bits = TARGET_PAGE_BITS_MIN;
+    return 0;
 }
 
 static int configuration_post_load(void *opaque, int version_id)
@@ -298,12 +312,43 @@ static int configuration_post_load(void *opaque, int version_id)
                      (int) state->len, state->name, current_name);
         return -EINVAL;
     }
+
+    if (state->target_page_bits != TARGET_PAGE_BITS) {
+        error_report("Received TARGET_PAGE_BITS is %d but local is %d",
+                     state->target_page_bits, TARGET_PAGE_BITS);
+        return -EINVAL;
+    }
+
     return 0;
 }
 
+/* The target-page-bits subsection is present only if the
+ * target page size is not the same as the default (ie the
+ * minimum page size for a variable-page-size guest CPU).
+ * If it is present then it contains the actual target page
+ * bits for the machine, and migration will fail if the
+ * two ends don't agree about it.
+ */
+static bool vmstate_target_page_bits_needed(void *opaque)
+{
+    return TARGET_PAGE_BITS > TARGET_PAGE_BITS_MIN;
+}
+
+static const VMStateDescription vmstate_target_page_bits = {
+    .name = "configuration/target-page-bits",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = vmstate_target_page_bits_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(target_page_bits, SaveState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_configuration = {
     .name = "configuration",
     .version_id = 1,
+    .pre_load = configuration_pre_load,
     .post_load = configuration_post_load,
     .pre_save = configuration_pre_save,
     .fields = (VMStateField[]) {
@@ -311,6 +356,10 @@ static const VMStateDescription vmstate_configuration = {
         VMSTATE_VBUFFER_ALLOC_UINT32(name, SaveState, 0, NULL, 0, len),
         VMSTATE_END_OF_LIST()
     },
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_target_page_bits,
+        NULL
+    }
 };
 
 static void dump_vmstate_vmsd(FILE *out_file,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH v3 6/7] target-arm: Make page size a runtime setting
  2016-10-11 17:08 [Qemu-devel] [PATCH v3 0/7] Runtime pagesize computation Peter Maydell
                   ` (4 preceding siblings ...)
  2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 5/7] migration/savevm.c: migrate non-default " Peter Maydell
@ 2016-10-11 17:08 ` Peter Maydell
  2016-10-12 13:33   ` Andrew Jones
  2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 7/7] hw/arm/virt: Set minimum_page_bits to 12 Peter Maydell
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 17+ messages in thread
From: Peter Maydell @ 2016-10-11 17:08 UTC (permalink / raw)
  To: qemu-arm, qemu-devel
  Cc: patches, paolo, Vijaya Kumar K, Dr . David Alan Gilbert,
	Laurent Desnogues, Richard Henderson

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 1b9540e..c94a324 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 76d824d..37d6eb3 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1766,10 +1766,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)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH v3 7/7] hw/arm/virt: Set minimum_page_bits to 12
  2016-10-11 17:08 [Qemu-devel] [PATCH v3 0/7] Runtime pagesize computation Peter Maydell
                   ` (5 preceding siblings ...)
  2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 6/7] target-arm: Make page size a runtime setting Peter Maydell
@ 2016-10-11 17:08 ` Peter Maydell
  2016-10-11 17:38 ` [Qemu-devel] [Qemu-arm] [PATCH v3 0/7] Runtime pagesize computation Peter Maydell
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2016-10-11 17:08 UTC (permalink / raw)
  To: qemu-arm, qemu-devel
  Cc: patches, paolo, Vijaya Kumar K, Dr . David Alan Gilbert,
	Laurent Desnogues, Richard Henderson

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, so
we introduce it only for the virt-2.8 machine and onward;
virt-2.7 continues using the old 1K pages.

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

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 795740d..abef00c 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1496,6 +1496,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 = {
@@ -1563,6 +1565,8 @@ static void virt_machine_2_7_options(MachineClass *mc)
 {
     virt_machine_2_8_options(mc);
     SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_7);
+    /* Stick with 1K pages for migration compatibility */
+    mc->minimum_page_bits = 0;
 }
 DEFINE_VIRT_MACHINE(2, 7)
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH v3 0/7] Runtime pagesize computation
  2016-10-11 17:08 [Qemu-devel] [PATCH v3 0/7] Runtime pagesize computation Peter Maydell
                   ` (6 preceding siblings ...)
  2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 7/7] hw/arm/virt: Set minimum_page_bits to 12 Peter Maydell
@ 2016-10-11 17:38 ` Peter Maydell
  2016-10-11 19:20 ` [Qemu-devel] " Richard Henderson
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2016-10-11 17:38 UTC (permalink / raw)
  To: qemu-arm, QEMU Developers
  Cc: Patch Tracking, Vijaya Kumar K, Dr . David Alan Gilbert,
	Laurent Desnogues, Richard Henderson, Paolo Bonzini

Oops, I messed up Paolo's email address sending this.
Sorry about that...

thanks
-- PMM

On 11 October 2016 at 18:08, Peter Maydell <peter.maydell@linaro.org> wrote:
> (I posted version 2 way back in June...)
>
> 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). In v3 I have included a
> patch which adds the target-page-bits to the migration stream
> so that you get a reasonably nice error message if you try
> to migrate between source and destination which disagree about
> the page size.
>
> 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.
>
>
> Patches 1-3 have been reviewed, so 4-7 are the interesting
> ones.
>
> REQUEST FOR TESTING/PERFORMANCE BENCHMARKING:
>
> I have only very lightly tested these and haven't attempted
> to measure performance at all. Further testing and
> benchmarking reports are therefore welcome.
>
> In particular I would like to know how much of an
> effect on TCG performance the assert() in the definition
> of TARGET_PAGE_BITS has, so comparisons of all three configs:
>  * before this patchset (ie TARGET_PAGE_BITS compile time const)
>  * with this patchset (with assert in TARGET_PAGE_BITS)
>  * this patchset, but with the assert commented out
>
>
> Changes v2->v3:
>  * rebased
>  * added new patch to migrate the page size if it's not the default
>  * virt-2.7 and earlier machines stick with the old
>    default page size to avoid a compat break
>  * in patch 3, v_l1_size is pulled into a local when we're
>    using it as a loop upper bound
>
> thanks
> -- PMM
>
>
> Peter Maydell (4):
>   cpu: Support a target CPU having a variable page size
>   migration/savevm.c: migrate non-default 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          |  4 +++
>  include/exec/cpu-all.h |  9 +++++++
>  include/hw/boards.h    |  7 +++++
>  include/qemu-common.h  | 12 +++++++++
>  migration/ram.c        |  4 ++-
>  migration/savevm.c     | 49 ++++++++++++++++++++++++++++++++++
>  target-arm/cpu.c       | 24 +++++++++++++++++
>  target-arm/cpu.h       |  9 ++++---
>  translate-all.c        | 71 ++++++++++++++++++++++++++++++++------------------
>  vl.c                   | 10 +++++++
>  11 files changed, 213 insertions(+), 33 deletions(-)

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH v3 0/7] Runtime pagesize computation
  2016-10-11 17:08 [Qemu-devel] [PATCH v3 0/7] Runtime pagesize computation Peter Maydell
                   ` (7 preceding siblings ...)
  2016-10-11 17:38 ` [Qemu-devel] [Qemu-arm] [PATCH v3 0/7] Runtime pagesize computation Peter Maydell
@ 2016-10-11 19:20 ` Richard Henderson
  2016-10-11 20:18   ` Peter Maydell
  2016-10-12  0:57 ` no-reply
  2016-10-21 15:00 ` [Qemu-devel] [Qemu-arm] " Peter Maydell
  10 siblings, 1 reply; 17+ messages in thread
From: Richard Henderson @ 2016-10-11 19:20 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel
  Cc: patches, paolo, Vijaya Kumar K, Dr . David Alan Gilbert,
	Laurent Desnogues

On 10/11/2016 12:08 PM, Peter Maydell wrote:
> 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.

Unfortunate.  I suppose that 4k is still better than 1k, but I was hoping to 
get 16k or 64k (or higher) when the OS is configured to use such.  I.e. totally 
dynamically configurable upon write to the appropriate cpu register.

Given how the memory subsystem already dynamically reconfigures itself for 
changes in address_space topology, I assumed page size changes would be trivial 
and fall out naturally.

Anyway, patches 4-7 get my

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


r~

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH v3 0/7] Runtime pagesize computation
  2016-10-11 19:20 ` [Qemu-devel] " Richard Henderson
@ 2016-10-11 20:18   ` Peter Maydell
  2016-10-12  1:34     ` Richard Henderson
  0 siblings, 1 reply; 17+ messages in thread
From: Peter Maydell @ 2016-10-11 20:18 UTC (permalink / raw)
  To: Richard Henderson
  Cc: qemu-arm, QEMU Developers, Patch Tracking, paolo, Vijaya Kumar K,
	Dr . David Alan Gilbert, Laurent Desnogues

On 11 October 2016 at 12:20, Richard Henderson <rth@twiddle.net> wrote:
> On 10/11/2016 12:08 PM, Peter Maydell wrote:
>> 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.

> Unfortunate.  I suppose that 4k is still better than 1k, but
> I was hoping to get 16k or 64k (or higher) when the OS is
> configured to use such.  I.e. totally dynamically configurable
> upon write to the appropriate cpu register.

I think that would run into problems with migration:
the migration stream all works in guest-pages of ram and
a mismatch means migration doesn't work.

> Given how the memory subsystem already dynamically reconfigures
> itself for changes in address_space topology, I assumed page size
> changes would be trivial and fall out naturally.

The trouble is that all the data structures work in terms
of page sizes (even though we support sub-page allocations
those are still done by carving up a page-size chunk).
It could probably be done but it looked like a gargantuan
task so I decided this was a better compromise.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH v3 0/7] Runtime pagesize computation
  2016-10-11 17:08 [Qemu-devel] [PATCH v3 0/7] Runtime pagesize computation Peter Maydell
                   ` (8 preceding siblings ...)
  2016-10-11 19:20 ` [Qemu-devel] " Richard Henderson
@ 2016-10-12  0:57 ` no-reply
  2016-10-21 15:00 ` [Qemu-devel] [Qemu-arm] " Peter Maydell
  10 siblings, 0 replies; 17+ messages in thread
From: no-reply @ 2016-10-12  0:57 UTC (permalink / raw)
  To: peter.maydell
  Cc: famz, qemu-arm, qemu-devel, patches, Vijaya.Kumar, dgilbert,
	paolo, laurent.desnogues, rth

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Message-id: 1476205699-28857-1-git-send-email-peter.maydell@linaro.org
Subject: [Qemu-devel] [PATCH v3 0/7] Runtime pagesize computation
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git show --no-patch --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
5a1d89b hw/arm/virt: Set minimum_page_bits to 12
c3c85d5 target-arm: Make page size a runtime setting
b43a8b0 migration/savevm.c: migrate non-default page size
994a37f cpu: Support a target CPU having a variable page size
570aff0 translate-all.c: Compute L1 page table properties at runtime
9891732 exec.c: Remove static allocation of sub_section of sub_page
60c7e4c migration: Remove static allocation of xzblre cache buffer

=== OUTPUT BEGIN ===
Checking PATCH 1/7: migration: Remove static allocation of xzblre cache buffer...
Checking PATCH 2/7: exec.c: Remove static allocation of sub_section of sub_page...
Checking PATCH 3/7: translate-all.c: Compute L1 page table properties at runtime...
Checking PATCH 4/7: cpu: Support a target CPU having a variable page size...
Checking PATCH 5/7: migration/savevm.c: migrate non-default page size...
ERROR: spaces required around that '*' (ctx:VxV)
#96: FILE: migration/savevm.c:359:
+    .subsections = (const VMStateDescription*[]) {
                                             ^

total: 1 errors, 0 warnings, 79 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 6/7: target-arm: Make page size a runtime setting...
Checking PATCH 7/7: hw/arm/virt: Set minimum_page_bits to 12...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH v3 0/7] Runtime pagesize computation
  2016-10-11 20:18   ` Peter Maydell
@ 2016-10-12  1:34     ` Richard Henderson
  0 siblings, 0 replies; 17+ messages in thread
From: Richard Henderson @ 2016-10-12  1:34 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-arm, QEMU Developers, Patch Tracking, paolo, Vijaya Kumar K,
	Dr . David Alan Gilbert, Laurent Desnogues

On 10/11/2016 03:18 PM, Peter Maydell wrote:
> On 11 October 2016 at 12:20, Richard Henderson <rth@twiddle.net> wrote:
>> On 10/11/2016 12:08 PM, Peter Maydell wrote:
>>> 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.
>
>> Unfortunate.  I suppose that 4k is still better than 1k, but
>> I was hoping to get 16k or 64k (or higher) when the OS is
>> configured to use such.  I.e. totally dynamically configurable
>> upon write to the appropriate cpu register.
>
> I think that would run into problems with migration:
> the migration stream all works in guest-pages of ram and
> a mismatch means migration doesn't work.

Perhaps migration should use definitions based off of TARGET_PAGE_BITS_MIN? 
Dunno how big of a job that would be...

> The trouble is that all the data structures work in terms
> of page sizes (even though we support sub-page allocations
> those are still done by carving up a page-size chunk).
> It could probably be done but it looked like a gargantuan
> task so I decided this was a better compromise.

Fair enough.  This is still an improvement for an interesting guest.


r~

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH v3 6/7] target-arm: Make page size a runtime setting
  2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 6/7] target-arm: Make page size a runtime setting Peter Maydell
@ 2016-10-12 13:33   ` Andrew Jones
  2016-10-12 13:40     ` Peter Maydell
  0 siblings, 1 reply; 17+ messages in thread
From: Andrew Jones @ 2016-10-12 13:33 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-arm, qemu-devel, patches, Vijaya Kumar K,
	Dr . David Alan Gilbert, paolo, Laurent Desnogues,
	Richard Henderson

On Tue, Oct 11, 2016 at 06:08:18PM +0100, Peter Maydell wrote:
> 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 1b9540e..c94a324 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");

I'm not sure about this. IIUC, then with this it won't be possible to
create a board that sets up its cpus with the preferred target page bits
greater than the cpu's default set above, even when the cpu supports it.
For example, I may want a board that will only use AArch64 cpus with 64K
pages. In that case I'd like to set the minimum to 16 bits, but then that
would result in this error. I think we should only set the default when a
preference hasn't already been given. And, maybe we should also provide
a maximum to sanity check against? (side note: if we provide a maximum,
then we could use it in arch_dump.c for the dump info's block size,
which must be the maximum page size the cpu supports.)

Thanks,
drew

> +        return;
> +    }
> +
>      if (cpu->reset_hivecs) {
>              cpu->reset_sctlr |= (1 << 13);
>      }
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 76d824d..37d6eb3 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1766,10 +1766,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)
> -- 
> 2.7.4
> 
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH v3 6/7] target-arm: Make page size a runtime setting
  2016-10-12 13:33   ` Andrew Jones
@ 2016-10-12 13:40     ` Peter Maydell
  2016-10-13  7:39       ` Andrew Jones
  0 siblings, 1 reply; 17+ messages in thread
From: Peter Maydell @ 2016-10-12 13:40 UTC (permalink / raw)
  To: Andrew Jones
  Cc: qemu-arm, QEMU Developers, Patch Tracking, Vijaya Kumar K,
	Dr . David Alan Gilbert, paolo, Laurent Desnogues,
	Richard Henderson

On 12 October 2016 at 14:33, Andrew Jones <drjones@redhat.com> wrote:
> On Tue, Oct 11, 2016 at 06:08:18PM +0100, Peter Maydell wrote:
>> 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>

>> +    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");
>
> I'm not sure about this. IIUC, then with this it won't be possible to
> create a board that sets up its cpus with the preferred target page bits
> greater than the cpu's default set above, even when the cpu supports it.
> For example, I may want a board that will only use AArch64 cpus with 64K
> pages. In that case I'd like to set the minimum to 16 bits, but then that
> would result in this error. I think we should only set the default when a
> preference hasn't already been given. And, maybe we should also provide
> a maximum to sanity check against? (side note: if we provide a maximum,
> then we could use it in arch_dump.c for the dump info's block size,
> which must be the maximum page size the cpu supports.)

If we had a CPU which supported only 64K pages then we would
make this if-ladder set pagebits appropriately. But we don't
have any of those, so it's a bit moot. If the CPU can be
configured by the guest to use 4K pages then the board
must not have set the preferred-page-size to something
larger, or the guest will fall over when it tries it.
That's what this check is protecting against.

The CPU's maximum page size isn't very interesting for this
patchset, because we only care about the minimum (which is
what the TLB needs to use). If the board code was somehow
buggy and requested a page size greater than the CPU's
maximum, then it will also be greater than the CPU's
minimum, and be caught by this error message.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH v3 6/7] target-arm: Make page size a runtime setting
  2016-10-12 13:40     ` Peter Maydell
@ 2016-10-13  7:39       ` Andrew Jones
  0 siblings, 0 replies; 17+ messages in thread
From: Andrew Jones @ 2016-10-13  7:39 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Patch Tracking, Vijaya Kumar K, QEMU Developers,
	Dr . David Alan Gilbert, qemu-arm, paolo, Laurent Desnogues,
	Richard Henderson

On Wed, Oct 12, 2016 at 02:40:30PM +0100, Peter Maydell wrote:
> On 12 October 2016 at 14:33, Andrew Jones <drjones@redhat.com> wrote:
> > On Tue, Oct 11, 2016 at 06:08:18PM +0100, Peter Maydell wrote:
> >> 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>
> 
> >> +    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");
> >
> > I'm not sure about this. IIUC, then with this it won't be possible to
> > create a board that sets up its cpus with the preferred target page bits
> > greater than the cpu's default set above, even when the cpu supports it.
> > For example, I may want a board that will only use AArch64 cpus with 64K
> > pages. In that case I'd like to set the minimum to 16 bits, but then that
> > would result in this error. I think we should only set the default when a
> > preference hasn't already been given. And, maybe we should also provide
> > a maximum to sanity check against? (side note: if we provide a maximum,
> > then we could use it in arch_dump.c for the dump info's block size,
> > which must be the maximum page size the cpu supports.)
> 
> If we had a CPU which supported only 64K pages then we would
> make this if-ladder set pagebits appropriately. But we don't
> have any of those, so it's a bit moot. If the CPU can be
> configured by the guest to use 4K pages then the board
> must not have set the preferred-page-size to something
> larger, or the guest will fall over when it tries it.
> That's what this check is protecting against.

Fair enough and understood, but there is a case where we can know
what page size to use. When booting with -kernel we can probe the
kernel image's header (see Documentation/arm64/booting.txt) If the
magic number matches, then we can check the header flags to see
what the kernel page size is.

An API like the one in this series would be useful for setting
the page size to match the guest kernel's, but only if the API
doesn't override the selection.

> 
> The CPU's maximum page size isn't very interesting for this
> patchset, because we only care about the minimum (which is
> what the TLB needs to use). If the board code was somehow
> buggy and requested a page size greater than the CPU's
> maximum, then it will also be greater than the CPU's
> minimum, and be caught by this error message.

Agreed we don't need a maximum as it is now. I think we may
if we allow preferences to be greater than the minimum though.

Thanks,
drew

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH v3 0/7] Runtime pagesize computation
  2016-10-11 17:08 [Qemu-devel] [PATCH v3 0/7] Runtime pagesize computation Peter Maydell
                   ` (9 preceding siblings ...)
  2016-10-12  0:57 ` no-reply
@ 2016-10-21 15:00 ` Peter Maydell
  10 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2016-10-21 15:00 UTC (permalink / raw)
  To: qemu-arm, QEMU Developers
  Cc: patches@linaro.org, Vijaya Kumar K, Dr . David Alan Gilbert,
	Laurent Desnogues, Richard Henderson, Paolo Bonzini

On 11 October 2016 at 18:08, Peter Maydell <peter.maydell@linaro.org> wrote:
> (I posted version 2 way back in June...)
>
> 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)

> REQUEST FOR TESTING/PERFORMANCE BENCHMARKING:
>
> I have only very lightly tested these and haven't attempted
> to measure performance at all. Further testing and
> benchmarking reports are therefore welcome.
>
> In particular I would like to know how much of an
> effect on TCG performance the assert() in the definition
> of TARGET_PAGE_BITS has, so comparisons of all three configs:
>  * before this patchset (ie TARGET_PAGE_BITS compile time const)
>  * with this patchset (with assert in TARGET_PAGE_BITS)
>  * this patchset, but with the assert commented out

So I found the time to do some benchmarking. This is of
an ARMv7 guest running a workload that is "boot up
kernel and userspace (via systemd), run a compilation,
then shutdown". This took about 5m9s to run with current
master, and 4m14s with this patchset, cutting nearly 20%
off the runtime. Removing the assert from TARGET_PAGE_BITS
was barely noticeable (down to 4m13s) so I think we should
leave that in.

On that basis and given the code review from rth I'm going to
put this into target-arm.next.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2016-10-21 15:00 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-11 17:08 [Qemu-devel] [PATCH v3 0/7] Runtime pagesize computation Peter Maydell
2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 1/7] migration: Remove static allocation of xzblre cache buffer Peter Maydell
2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 2/7] exec.c: Remove static allocation of sub_section of sub_page Peter Maydell
2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 3/7] translate-all.c: Compute L1 page table properties at runtime Peter Maydell
2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 4/7] cpu: Support a target CPU having a variable page size Peter Maydell
2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 5/7] migration/savevm.c: migrate non-default " Peter Maydell
2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 6/7] target-arm: Make page size a runtime setting Peter Maydell
2016-10-12 13:33   ` Andrew Jones
2016-10-12 13:40     ` Peter Maydell
2016-10-13  7:39       ` Andrew Jones
2016-10-11 17:08 ` [Qemu-devel] [PATCH v3 7/7] hw/arm/virt: Set minimum_page_bits to 12 Peter Maydell
2016-10-11 17:38 ` [Qemu-devel] [Qemu-arm] [PATCH v3 0/7] Runtime pagesize computation Peter Maydell
2016-10-11 19:20 ` [Qemu-devel] " Richard Henderson
2016-10-11 20:18   ` Peter Maydell
2016-10-12  1:34     ` Richard Henderson
2016-10-12  0:57 ` no-reply
2016-10-21 15:00 ` [Qemu-devel] [Qemu-arm] " Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).