[Qemu-devel] [RFC PATCH v1 0/4] ARM/AARCH64: Runtime page size computation

[Qemu-devel] [RFC PATCH v1 0/4] ARM/AARCH64: Runtime page size computation

[vijayak]

From: Vijaya Kumar K <vijayak@cavium.com>

For AARCH64, minimum page size is 4K, for ARM32 bit platforms
minimum page size is 1K.
With this patch series, the page size is calculated at run-time
based on cpu model emulated.

Having higher/optimal page size, improves emulation and migration
performance.

Vijaya Kumar K (4):
  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
  target-arm: Compute page size based on ARM target cpu type

 exec.c                |    5 +++--
 hw/arm/virt.c         |   48 +++++++++++++++++++++++++++++++++++++++++
 include/hw/boards.h   |    1 +
 include/qemu-common.h |    1 +
 migration/ram.c       |    4 +++-
 target-arm/cpu.h      |   12 ++++++-----
 translate-all.c       |   57 ++++++++++++++++++++++++++++++-------------------
 vl.c                  |   10 +++++++++
 8 files changed, 108 insertions(+), 30 deletions(-)

-- 
1.7.9.5

[Qemu-devel] [RFC PATCH v1 1/4] migration: Remove static allocation of xzblre cache buffer

[vijayak]

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>
---
 migration/ram.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/migration/ram.c b/migration/ram.c
index 844ea46..ba3b352 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)
 {
@@ -1437,6 +1437,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;
@@ -1893,6 +1894,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.7.9.5

[Qemu-devel] [RFC PATCH v1 2/4] exec.c: Remove static allocation of sub_section of sub_page

[vijayak]

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>
---
 exec.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/exec.c b/exec.c
index a9d465b..e803a41 100644
--- a/exec.c
+++ b/exec.c
@@ -154,7 +154,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
@@ -1151,6 +1151,7 @@ static void phys_section_destroy(MemoryRegion *mr)
     if (have_sub_page) {
         subpage_t *subpage = container_of(mr, subpage_t, iomem);
         object_unref(OBJECT(&subpage->iomem));
+        g_free(subpage->sub_section);
         g_free(subpage);
     }
 }
@@ -2272,7 +2273,7 @@ static subpage_t *subpage_init(AddressSpace *as, hwaddr base)
     subpage_t *mmio;
 
     mmio = g_malloc0(sizeof(subpage_t));
-
+    mmio->sub_section = g_malloc0(TARGET_PAGE_SIZE * sizeof(uint16_t));
     mmio->as = as;
     mmio->base = base;
     memory_region_init_io(&mmio->iomem, NULL, &subpage_ops, mmio,
-- 
1.7.9.5

Re: [Qemu-devel] [RFC PATCH v1 2/4] exec.c: Remove static allocation of sub_section of sub_page

[Peter Maydell]

On 13 June 2016 at 10:08,  <vijayak@caviumnetworks.com> 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>
> ---
>  exec.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index a9d465b..e803a41 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -154,7 +154,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
> @@ -1151,6 +1151,7 @@ static void phys_section_destroy(MemoryRegion *mr)
>      if (have_sub_page) {
>          subpage_t *subpage = container_of(mr, subpage_t, iomem);
>          object_unref(OBJECT(&subpage->iomem));
> +        g_free(subpage->sub_section);
>          g_free(subpage);
>      }
>  }
> @@ -2272,7 +2273,7 @@ static subpage_t *subpage_init(AddressSpace *as, hwaddr base)
>      subpage_t *mmio;
>
>      mmio = g_malloc0(sizeof(subpage_t));
> -
> +    mmio->sub_section = g_malloc0(TARGET_PAGE_SIZE * sizeof(uint16_t));

You can write this as
   = g_new0(uint16_t, TARGET_PAGE_SIZE);

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH v1 2/4] exec.c: Remove static allocation of sub_section of sub_page

[Paolo Bonzini]

On 13/06/2016 11:08, vijayak@caviumnetworks.com 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>
> ---
>  exec.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index a9d465b..e803a41 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -154,7 +154,7 @@ typedef struct subpage_t {
>      MemoryRegion iomem;
>      AddressSpace *as;
>      hwaddr base;
> -    uint16_t sub_section[TARGET_PAGE_SIZE];
> +    uint16_t *sub_section;

Please make this a flexible array member instead, so that you can avoid
the extra pointer dereference.

Thanks,

Paolo

>  } subpage_t;
>  
>  #define PHYS_SECTION_UNASSIGNED 0
> @@ -1151,6 +1151,7 @@ static void phys_section_destroy(MemoryRegion *mr)
>      if (have_sub_page) {
>          subpage_t *subpage = container_of(mr, subpage_t, iomem);
>          object_unref(OBJECT(&subpage->iomem));
> +        g_free(subpage->sub_section);
>          g_free(subpage);
>      }
>  }
> @@ -2272,7 +2273,7 @@ static subpage_t *subpage_init(AddressSpace *as, hwaddr base)
>      subpage_t *mmio;
>  
>      mmio = g_malloc0(sizeof(subpage_t));
> -
> +    mmio->sub_section = g_malloc0(TARGET_PAGE_SIZE * sizeof(uint16_t));
>      mmio->as = as;
>      mmio->base = base;
>      memory_region_init_io(&mmio->iomem, NULL, &subpage_ops, mmio,
> 

Re: [Qemu-devel] [RFC PATCH v1 2/4] exec.c: Remove static allocation of sub_section of sub_page

[Vijay Kilari]

Hi Paolo,

On Mon, Jun 13, 2016 at 3:22 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 13/06/2016 11:08, vijayak@caviumnetworks.com 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>
>> ---
>>  exec.c |    5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/exec.c b/exec.c
>> index a9d465b..e803a41 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -154,7 +154,7 @@ typedef struct subpage_t {
>>      MemoryRegion iomem;
>>      AddressSpace *as;
>>      hwaddr base;
>> -    uint16_t sub_section[TARGET_PAGE_SIZE];
>> +    uint16_t *sub_section;
>
> Please make this a flexible array member instead, so that you can avoid
> the extra pointer dereference.

What do you mean by flexible array member?. please give more info.

Re: [Qemu-devel] [RFC PATCH v1 2/4] exec.c: Remove static allocation of sub_section of sub_page

[Peter Maydell]

On 17 June 2016 at 11:14, Vijay Kilari <vijay.kilari@gmail.com> wrote:
> Hi Paolo,
>
> On Mon, Jun 13, 2016 at 3:22 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> diff --git a/exec.c b/exec.c
>>> index a9d465b..e803a41 100644
>>> --- a/exec.c
>>> +++ b/exec.c
>>> @@ -154,7 +154,7 @@ typedef struct subpage_t {
>>>      MemoryRegion iomem;
>>>      AddressSpace *as;
>>>      hwaddr base;
>>> -    uint16_t sub_section[TARGET_PAGE_SIZE];
>>> +    uint16_t *sub_section;
>>
>> Please make this a flexible array member instead, so that you can avoid
>> the extra pointer dereference.
>
> What do you mean by flexible array member?. please give more info.

https://en.wikipedia.org/wiki/Flexible_array_member

thanks
-- PMM

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

[vijayak]

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>
---
 include/qemu-common.h |    1 +
 translate-all.c       |   57 ++++++++++++++++++++++++++++++-------------------
 vl.c                  |    3 +++
 3 files changed, 39 insertions(+), 22 deletions(-)

diff --git a/include/qemu-common.h b/include/qemu-common.h
index 1f2cb94..d5f0450 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -129,6 +129,7 @@ int parse_debug_env(const char *name, int max, int initial);
 
 const char *qemu_ether_ntoa(const MACAddr *mac);
 void page_size_init(void);
+void init_l1_page_table_param(void);
 
 /* returns non-zero if dump is in progress, otherwise zero is
  * returned. */
diff --git a/translate-all.c b/translate-all.c
index 118e7d3..a580ca9 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -57,6 +57,7 @@
 #include "qemu/bitmap.h"
 #include "qemu/timer.h"
 #include "exec/log.h"
+#include "qemu/error-report.h"
 
 //#define DEBUG_TB_INVALIDATE
 //#define DEBUG_FLUSH
@@ -99,25 +100,18 @@ 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;
 
+/*
+ * L1 Mapping properties
+ */
+static unsigned long v_l1_bits;
+static unsigned long v_l1_size;
+static unsigned long v_l1_shift;
+
 /* The bottom level has pointers to PageDesc */
-static void *l1_map[V_L1_SIZE];
+static void *l1_map;
 
 /* code generation context */
 TCGContext tcg_ctx;
@@ -127,6 +121,25 @@ TCGContext tcg_ctx;
 __thread int have_tb_lock;
 #endif
 
+void init_l1_page_table_param(void)
+{
+    uint32_t v_l1_bits_rem;
+
+    assert(TARGET_PAGE_BITS);
+    /* The bits remaining after N lower levels of page tables.  */
+    v_l1_bits_rem = ((L1_MAP_ADDR_SPACE_BITS - TARGET_PAGE_BITS) % V_L2_BITS);
+    if (v_l1_bits_rem < 4)
+        v_l1_bits = (v_l1_bits_rem + V_L2_BITS);
+    else
+        v_l1_bits = v_l1_bits_rem;
+
+    v_l1_size = ((target_ulong)1 << v_l1_bits);
+    v_l1_shift = (L1_MAP_ADDR_SPACE_BITS - TARGET_PAGE_BITS - v_l1_bits);
+    l1_map = g_malloc0(v_l1_size * sizeof(void *));
+    if (!l1_map)
+        error_report("Allocation faile for L1 MAP table\n");
+}
+
 void tb_lock(void)
 {
 #ifdef CONFIG_USER_ONLY
@@ -408,10 +421,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_l1_shift / V_L2_BITS - 1; i > 0; i--) {
         void **p = atomic_rcu_read(lp);
 
         if (p == NULL) {
@@ -819,8 +832,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_l1_shift / V_L2_BITS - 1, l1_map + i);
     }
 }
 
@@ -1825,9 +1838,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++) {
+        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);
         if (rc != 0) {
             return rc;
         }
diff --git a/vl.c b/vl.c
index b0bcc25..b6da265 100644
--- a/vl.c
+++ b/vl.c
@@ -4044,6 +4044,9 @@ int main(int argc, char **argv, char **envp)
     }
     object_property_add_child(object_get_root(), "machine",
                               OBJECT(current_machine), &error_abort);
+
+    init_l1_page_table_param();
+
     cpu_exec_init_all();
 
     if (machine_class->hw_version) {
-- 
1.7.9.5

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

[Paolo Bonzini]

On 13/06/2016 11:08, vijayak@caviumnetworks.com wrote:
> 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>
> ---
>  include/qemu-common.h |    1 +
>  translate-all.c       |   57 ++++++++++++++++++++++++++++++-------------------
>  vl.c                  |    3 +++
>  3 files changed, 39 insertions(+), 22 deletions(-)
> 
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index 1f2cb94..d5f0450 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -129,6 +129,7 @@ int parse_debug_env(const char *name, int max, int initial);
>  
>  const char *qemu_ether_ntoa(const MACAddr *mac);
>  void page_size_init(void);
> +void init_l1_page_table_param(void);
>  
>  /* returns non-zero if dump is in progress, otherwise zero is
>   * returned. */
> diff --git a/translate-all.c b/translate-all.c
> index 118e7d3..a580ca9 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -57,6 +57,7 @@
>  #include "qemu/bitmap.h"
>  #include "qemu/timer.h"
>  #include "exec/log.h"
> +#include "qemu/error-report.h"
>  
>  //#define DEBUG_TB_INVALIDATE
>  //#define DEBUG_FLUSH
> @@ -99,25 +100,18 @@ 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;
>  
> +/*
> + * L1 Mapping properties
> + */
> +static unsigned long v_l1_bits;
> +static unsigned long v_l1_size;
> +static unsigned long v_l1_shift;

Please make these uint8_t.

>  /* The bottom level has pointers to PageDesc */
> -static void *l1_map[V_L1_SIZE];
> +static void *l1_map;

You can make this array have a static V_L2_SIZE * 16 size too.  Peter,
what do you think?

>  /* code generation context */
>  TCGContext tcg_ctx;
> @@ -127,6 +121,25 @@ TCGContext tcg_ctx;
>  __thread int have_tb_lock;
>  #endif
>  
> +void init_l1_page_table_param(void)
> +{
> +    uint32_t v_l1_bits_rem;
> +
> +    assert(TARGET_PAGE_BITS);
> +    /* The bits remaining after N lower levels of page tables.  */
> +    v_l1_bits_rem = ((L1_MAP_ADDR_SPACE_BITS - TARGET_PAGE_BITS) % V_L2_BITS);
> +    if (v_l1_bits_rem < 4)
> +        v_l1_bits = (v_l1_bits_rem + V_L2_BITS);
> +    else
> +        v_l1_bits = v_l1_bits_rem;
> +
> +    v_l1_size = ((target_ulong)1 << v_l1_bits);
> +    v_l1_shift = (L1_MAP_ADDR_SPACE_BITS - TARGET_PAGE_BITS - v_l1_bits);

Please assert that v_l1_shift % V_L2_BITS == 0.

> +    l1_map = g_malloc0(v_l1_size * sizeof(void *));
> +    if (!l1_map)
> +        error_report("Allocation faile for L1 MAP table\n");
> +}
> +
>  void tb_lock(void)
>  {
>  #ifdef CONFIG_USER_ONLY
> @@ -408,10 +421,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_l1_shift / V_L2_BITS - 1; i > 0; i--) {

Please cache v_l1_shift / V_L2_BITS - 1 into a new variable v_l2_levels.

>          void **p = atomic_rcu_read(lp);
>  
>          if (p == NULL) {
> @@ -819,8 +832,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_l1_shift / V_L2_BITS - 1, l1_map + i);
>      }
>  }
>  
> @@ -1825,9 +1838,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++) {
> +        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);
>          if (rc != 0) {
>              return rc;
>          }
> diff --git a/vl.c b/vl.c
> index b0bcc25..b6da265 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4044,6 +4044,9 @@ int main(int argc, char **argv, char **envp)
>      }
>      object_property_add_child(object_get_root(), "machine",
>                                OBJECT(current_machine), &error_abort);
> +
> +    init_l1_page_table_param();

Please call this from cpu_exec_init_all instead.

Thanks,

Paolo

>      cpu_exec_init_all();
>  
>      if (machine_class->hw_version) {
> 

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

[Peter Maydell]

On 13 June 2016 at 10:25, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 13/06/2016 11:08, vijayak@caviumnetworks.com wrote:
>> 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>
>> ---
>>  include/qemu-common.h |    1 +
>>  translate-all.c       |   57 ++++++++++++++++++++++++++++++-------------------
>>  vl.c                  |    3 +++
>>  3 files changed, 39 insertions(+), 22 deletions(-)
>>
>> diff --git a/include/qemu-common.h b/include/qemu-common.h
>> index 1f2cb94..d5f0450 100644
>> --- a/include/qemu-common.h
>> +++ b/include/qemu-common.h
>> @@ -129,6 +129,7 @@ int parse_debug_env(const char *name, int max, int initial);
>>
>>  const char *qemu_ether_ntoa(const MACAddr *mac);
>>  void page_size_init(void);
>> +void init_l1_page_table_param(void);
>>
>>  /* returns non-zero if dump is in progress, otherwise zero is
>>   * returned. */
>> diff --git a/translate-all.c b/translate-all.c
>> index 118e7d3..a580ca9 100644
>> --- a/translate-all.c
>> +++ b/translate-all.c
>> @@ -57,6 +57,7 @@
>>  #include "qemu/bitmap.h"
>>  #include "qemu/timer.h"
>>  #include "exec/log.h"
>> +#include "qemu/error-report.h"
>>
>>  //#define DEBUG_TB_INVALIDATE
>>  //#define DEBUG_FLUSH
>> @@ -99,25 +100,18 @@ 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;
>>
>> +/*
>> + * L1 Mapping properties
>> + */
>> +static unsigned long v_l1_bits;
>> +static unsigned long v_l1_size;
>> +static unsigned long v_l1_shift;
>
> Please make these uint8_t.
>
>>  /* The bottom level has pointers to PageDesc */
>> -static void *l1_map[V_L1_SIZE];
>> +static void *l1_map;
>
> You can make this array have a static V_L2_SIZE * 16 size too.  Peter,
> what do you think?

I don't know this code well enough to have an informed view,
but we only allocate this once at startup, right? I'm not sure
why making it a static array would be better?

thanks
-- PMM

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

[Paolo Bonzini]

On 13/06/2016 11:36, Peter Maydell wrote:
>>>  /* The bottom level has pointers to PageDesc */
>>> >> -static void *l1_map[V_L1_SIZE];
>>> >> +static void *l1_map;
>> >
>> > You can make this array have a static V_L2_SIZE * 16 size too.  Peter,
>> > what do you think?
> I don't know this code well enough to have an informed view,
> but we only allocate this once at startup, right? I'm not sure
> why making it a static array would be better?

It makes accesses faster by avoiding a pointer load.  On one hand it
might be just microoptimization, on the other hand... death by one
thousand papercuts...

Paolo

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

[Peter Maydell]

On 13 June 2016 at 10:25, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 13/06/2016 11:08, vijayak@caviumnetworks.com wrote:
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -4044,6 +4044,9 @@ int main(int argc, char **argv, char **envp)
>>      }
>>      object_property_add_child(object_get_root(), "machine",
>>                                OBJECT(current_machine), &error_abort);
>> +
>> +    init_l1_page_table_param();
>
> Please call this from cpu_exec_init_all instead.

If you're going to call it that early (ie before machine init)
than I think we might as well just call it from the existing
page_init() function in translate-all.c instead, right?
(gets called when the tcg accelerator is inited; we don't need
to set up the l1 map if we're not using TCG).

thanks
-- PMM

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

[Paolo Bonzini]

On 21/06/2016 13:53, Peter Maydell wrote:
>>> >>      object_property_add_child(object_get_root(), "machine",
>>> >>                                OBJECT(current_machine), &error_abort);
>>> >> +
>>> >> +    init_l1_page_table_param();
>> >
>> > Please call this from cpu_exec_init_all instead.
> If you're going to call it that early (ie before machine init)
> than I think we might as well just call it from the existing
> page_init() function in translate-all.c instead, right?
> (gets called when the tcg accelerator is inited; we don't need
> to set up the l1 map if we're not using TCG).

Yes, good idea.

Paolo

[Qemu-devel] [RFC PATCH v1 4/4] target-arm: Compute page size based on ARM target cpu type

[vijayak]

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

Replace TARGET_PAGE_BITS with arm_target_page_size function
in order to fetch page size at run-time.

Introduced MachineClass callback to compute target page
size at the early boot before memory initialization.
This callback is currently implemented for ARM platforms.
Based on cpu_model, the page size is updated in
target_page_bits which is defined as TARGET_PAGE_BITS.

Signed-off-by: Vijaya Kumar K <vijayak@cavium.com>
---
 hw/arm/virt.c       |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/boards.h |    1 +
 target-arm/cpu.h    |   12 +++++++-----
 vl.c                |    7 +++++++
 4 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 73113cf..37aab33 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -90,6 +90,12 @@ typedef struct {
     int32_t gic_version;
 } VirtMachineState;
 
+/*
+ * Holds TARGET_AARCH_64_PAGE_BITS or TARGET_ARM_PAGE_BITS
+ * based on the the cpu type emulated at runtime.
+ */
+static uint32_t target_page_bits;
+
 #define TYPE_VIRT_MACHINE   MACHINE_TYPE_NAME("virt")
 #define VIRT_MACHINE(obj) \
     OBJECT_CHECK(VirtMachineState, (obj), TYPE_VIRT_MACHINE)
@@ -1099,6 +1105,47 @@ void virt_guest_info_machine_done(Notifier *notifier, void *data)
     virt_build_smbios(&guest_info_state->info);
 }
 
+static void machvirt_update_target_page_size(const char *cpu_model)
+{
+    char **cpustr;
+    ObjectClass *oc, *parent;
+    const char *parent_type;
+    /* Set to default page size */
+    uint32_t page_bits = TARGET_ARM_PAGE_BITS;
+
+    cpustr = g_strsplit(cpu_model, ",", 2);
+    if (!strcmp(cpustr[0], "host")) {
+#ifdef __aarch64__
+        page_bits = TARGET_AARCH64_PAGE_BITS;
+#else
+        page_bits = TARGET_ARM_PAGE_BITS;
+#endif
+        goto out;
+    }
+
+    oc = cpu_class_by_name(TYPE_ARM_CPU, cpustr[0]);
+    if (!oc) {
+        oc = cpu_class_by_name(TYPE_AARCH64_CPU, cpustr[0]);
+        if (!oc)
+           goto out;
+    }
+
+    parent = object_class_get_parent(oc);
+    if (!parent)
+       goto out;
+
+    parent_type = object_class_get_name(parent);
+    if (!strcmp(parent_type, TYPE_AARCH64_CPU))
+       page_bits = TARGET_AARCH64_PAGE_BITS;
+out:
+    target_page_bits = page_bits;
+}
+
+uint32_t arm_get_target_page_bits(void)
+{
+    return target_page_bits;
+}
+
 static void machvirt_init(MachineState *machine)
 {
     VirtMachineState *vms = VIRT_MACHINE(machine);
@@ -1376,6 +1423,7 @@ 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;
+    mc->update_target_page_size = machvirt_update_target_page_size;
 }
 
 static const TypeInfo virt_machine_info = {
diff --git a/include/hw/boards.h b/include/hw/boards.h
index d268bd0..1e8abb6 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -124,6 +124,7 @@ struct MachineClass {
                                            DeviceState *dev);
     unsigned (*cpu_index_to_socket_id)(unsigned cpu_index);
     CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
+    void (*update_target_page_size)(const char *cpu_model);
 };
 
 /**
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 17d8051..f593620 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1743,14 +1743,16 @@ bool write_cpustate_to_list(ARMCPU *cpu);
 #define ARM_CPUID_TI915T      0x54029152
 #define ARM_CPUID_TI925T      0x54029252
 
-#if defined(CONFIG_USER_ONLY)
-#define TARGET_PAGE_BITS 12
-#else
+uint32_t arm_get_target_page_bits(void);
+/*
+ * Minimum page size for aarch64 is 4K
+ */
+#define TARGET_AARCH64_PAGE_BITS 12
 /* 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
-#endif
+#define TARGET_ARM_PAGE_BITS 10
+#define TARGET_PAGE_BITS arm_get_target_page_bits()
 
 #if defined(TARGET_AARCH64)
 #  define TARGET_PHYS_ADDR_SPACE_BITS 48
diff --git a/vl.c b/vl.c
index b6da265..a317cf1 100644
--- a/vl.c
+++ b/vl.c
@@ -4045,6 +4045,13 @@ int main(int argc, char **argv, char **envp)
     object_property_add_child(object_get_root(), "machine",
                               OBJECT(current_machine), &error_abort);
 
+    /*
+     * Compute target page size dynamically if arch supports
+     * multiple page sizes. Ex: ARM
+     */
+    if (machine_class->update_target_page_size)
+        machine_class->update_target_page_size(cpu_model);
+
     init_l1_page_table_param();
 
     cpu_exec_init_all();
-- 
1.7.9.5

Re: [Qemu-devel] [RFC PATCH v1 4/4] target-arm: Compute page size based on ARM target cpu type

[Paolo Bonzini]

On 13/06/2016 11:08, vijayak@caviumnetworks.com wrote:
> + */
> +#define TARGET_AARCH64_PAGE_BITS 12
>  /* 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
> -#endif
> +#define TARGET_ARM_PAGE_BITS 10
> +#define TARGET_PAGE_BITS arm_get_target_page_bits()

Please avoid the function call and just make it a variable target_page_bits.

Paolo

Re: [Qemu-devel] [RFC PATCH v1 4/4] target-arm: Compute page size based on ARM target cpu type

[Peter Maydell]

On 13 June 2016 at 10:08,  <vijayak@caviumnetworks.com> wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
>
> Replace TARGET_PAGE_BITS with arm_target_page_size function
> in order to fetch page size at run-time.
>
> Introduced MachineClass callback to compute target page
> size at the early boot before memory initialization.
> This callback is currently implemented for ARM platforms.
> Based on cpu_model, the page size is updated in
> target_page_bits which is defined as TARGET_PAGE_BITS.
>
> Signed-off-by: Vijaya Kumar K <vijayak@cavium.com>
> ---
>  hw/arm/virt.c       |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/boards.h |    1 +
>  target-arm/cpu.h    |   12 +++++++-----
>  vl.c                |    7 +++++++
>  4 files changed, 63 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 73113cf..37aab33 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -90,6 +90,12 @@ typedef struct {
>      int32_t gic_version;
>  } VirtMachineState;
>
> +/*
> + * Holds TARGET_AARCH_64_PAGE_BITS or TARGET_ARM_PAGE_BITS
> + * based on the the cpu type emulated at runtime.
> + */
> +static uint32_t target_page_bits;

The CPU page size is not specific to the 'virt' board, so this
is the wrong place to do this. You should identify the
page size in arm_cpu_realizefn() based on the set of feature
bits the CPU has: anything with ARM_FEATURE_V7 has a 4K page
table (this includes a lot of 32-bit CPUs).

CPU realize is pretty late in startup so you may need
to rearrange some other stuff to be sure that it will
work OK. If that absolutely can't work then we could do
this in CPU init, but that would be a bit messier.

> --- a/vl.c
> +++ b/vl.c
> @@ -4045,6 +4045,13 @@ int main(int argc, char **argv, char **envp)
>      object_property_add_child(object_get_root(), "machine",
>                                OBJECT(current_machine), &error_abort);
>
> +    /*
> +     * Compute target page size dynamically if arch supports
> +     * multiple page sizes. Ex: ARM
> +     */
> +    if (machine_class->update_target_page_size)
> +        machine_class->update_target_page_size(cpu_model);
> +
>      init_l1_page_table_param();

Page size isn't board specific so you don't need any of these hook
functions in the machine class.

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH v1 4/4] target-arm: Compute page size based on ARM target cpu type

[Peter Maydell]

On 13 June 2016 at 10:43, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 13 June 2016 at 10:08,  <vijayak@caviumnetworks.com> wrote:
>> +/*
>> + * Holds TARGET_AARCH_64_PAGE_BITS or TARGET_ARM_PAGE_BITS
>> + * based on the the cpu type emulated at runtime.
>> + */
>> +static uint32_t target_page_bits;
>
> The CPU page size is not specific to the 'virt' board, so this
> is the wrong place to do this. You should identify the
> page size in arm_cpu_realizefn() based on the set of feature
> bits the CPU has: anything with ARM_FEATURE_V7 has a 4K page
> table (this includes a lot of 32-bit CPUs).

Actually that should be "with ARM_FEATURE_V7 and not
ARM_FEATURE_MPU", or we'll break the PMSA code.

Note that you'll also need to handle systems where the
different CPUs in it disagree about the preferred target
page size -- the xlnx-ep108 board can have both
Cortex-A53 (prefers 4K) and Cortex-R5 (prefers 1K) CPUs in it.
"Use the smallest value required by any CPU on the board"
is probably the best approach.

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH v1 4/4] target-arm: Compute page size based on ARM target cpu type

[Vijay Kilari]

On Mon, Jun 13, 2016 at 3:40 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 13 June 2016 at 10:43, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 13 June 2016 at 10:08,  <vijayak@caviumnetworks.com> wrote:
>>> +/*
>>> + * Holds TARGET_AARCH_64_PAGE_BITS or TARGET_ARM_PAGE_BITS
>>> + * based on the the cpu type emulated at runtime.
>>> + */
>>> +static uint32_t target_page_bits;
>>
>> The CPU page size is not specific to the 'virt' board, so this
>> is the wrong place to do this. You should identify the
>> page size in arm_cpu_realizefn() based on the set of feature
>> bits the CPU has: anything with ARM_FEATURE_V7 has a 4K page
>> table (this includes a lot of 32-bit CPUs).

  cpu_init and cpu_realizefn() of required cpu model is called in
machvirt_init(),
which is quite late in the initialization sequence.
The cpu_exec_init_all() which calls memory_map_init() is called very
early stage,
is where TARGET_PAGE_BITS information is required.

In order to get feature information of CPU early, one option is to
create cpu object
early, initialize it. This means moving some cpu object creation and
initalization
code from machvirt_init().

>
> Actually that should be "with ARM_FEATURE_V7 and not
> ARM_FEATURE_MPU", or we'll break the PMSA code.
>
> Note that you'll also need to handle systems where the
> different CPUs in it disagree about the preferred target
> page size -- the xlnx-ep108 board can have both
> Cortex-A53 (prefers 4K) and Cortex-R5 (prefers 1K) CPUs in it.
> "Use the smallest value required by any CPU on the board"
> is probably the best approach.

How -cpu options are passed for xlnx-ep108 board in qemu command?.

>
> thanks
> -- PMM

Re: [Qemu-devel] [RFC PATCH v1 4/4] target-arm: Compute page size based on ARM target cpu type

[Peter Maydell]

On 14 June 2016 at 12:14, Vijay Kilari <vijay.kilari@gmail.com> wrote:
> On Mon, Jun 13, 2016 at 3:40 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 13 June 2016 at 10:43, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 13 June 2016 at 10:08,  <vijayak@caviumnetworks.com> wrote:
>>>> +/*
>>>> + * Holds TARGET_AARCH_64_PAGE_BITS or TARGET_ARM_PAGE_BITS
>>>> + * based on the the cpu type emulated at runtime.
>>>> + */
>>>> +static uint32_t target_page_bits;
>>>
>>> The CPU page size is not specific to the 'virt' board, so this
>>> is the wrong place to do this. You should identify the
>>> page size in arm_cpu_realizefn() based on the set of feature
>>> bits the CPU has: anything with ARM_FEATURE_V7 has a 4K page
>>> table (this includes a lot of 32-bit CPUs).
>
>   cpu_init and cpu_realizefn() of required cpu model is called in
> machvirt_init(),
> which is quite late in the initialization sequence.
> The cpu_exec_init_all() which calls memory_map_init() is called very
> early stage,
> is where TARGET_PAGE_BITS information is required.
>
> In order to get feature information of CPU early, one option is to
> create cpu object
> early, initialize it. This means moving some cpu object creation and
> initalization
> code from machvirt_init().

It would be better to delay the point at which we allocate
the data structures which care about page size, rather than
moving init of the CPU earlier.

Also we should consider what happens if we have decided
the page size is X, and then a CPU is hotplugged which
requires a page size Y where Y < X.

>> Actually that should be "with ARM_FEATURE_V7 and not
>> ARM_FEATURE_MPU", or we'll break the PMSA code.
>>
>> Note that you'll also need to handle systems where the
>> different CPUs in it disagree about the preferred target
>> page size -- the xlnx-ep108 board can have both
>> Cortex-A53 (prefers 4K) and Cortex-R5 (prefers 1K) CPUs in it.
>> "Use the smallest value required by any CPU on the board"
>> is probably the best approach.
>
> How -cpu options are passed for xlnx-ep108 board in qemu command?

They aren't -- you always get the cpus the board has in real h/w.

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH v1 4/4] target-arm: Compute page size based on ARM target cpu type

[Richard Henderson]

On 06/14/2016 04:36 AM, Peter Maydell wrote:
> It would be better to delay the point at which we allocate
> the data structures which care about page size, rather than
> moving init of the CPU earlier.

It would be *best* if we could re-initialize and re-allocate these data
structures so that we can follow the current page size as it changes.

Yes, this might require flushing just about everything, but it's the kind of
thing that's likely to happen only at system startup.  After that, everything
benefits from having the correct (larger) page size.

> Also we should consider what happens if we have decided
> the page size is X, and then a CPU is hotplugged which
> requires a page size Y where Y < X.

Is that a real possibility?  Or trivially true because the new cpu has yet to
be initialized by the OS to use the regular OS page size.


r~

Re: [Qemu-devel] [RFC PATCH v1 4/4] target-arm: Compute page size based on ARM target cpu type

[Vijay Kilari]

On Fri, Jun 17, 2016 at 1:12 AM, Richard Henderson <rth@twiddle.net> wrote:
> On 06/14/2016 04:36 AM, Peter Maydell wrote:
>> It would be better to delay the point at which we allocate
>> the data structures which care about page size, rather than
>> moving init of the CPU earlier.
>
> It would be *best* if we could re-initialize and re-allocate these data
> structures so that we can follow the current page size as it changes.
>
> Yes, this might require flushing just about everything, but it's the kind of
> thing that's likely to happen only at system startup.  After that, everything
> benefits from having the correct (larger) page size.

I tried shuffling the memory initialization code after cpu initialization.
but it was full mess. So could not proceed further.

However, I tried early creation cpu objects instead of doing it from
machvirt_init.
With this, initfn of the cpu model is called earlier and feature set is updated.
With that I could fetch arm architecture info. Based on this we can
choose page size.
I can send out RFC patch.

>
>> Also we should consider what happens if we have decided
>> the page size is X, and then a CPU is hotplugged which
>> requires a page size Y where Y < X.
>
> Is that a real possibility?  Or trivially true because the new cpu has yet to
> be initialized by the OS to use the regular OS page size.
>
>
> r~

Re: [Qemu-devel] [RFC PATCH v1 4/4] target-arm: Compute page size based on ARM target cpu type

[Peter Maydell]

On 17 June 2016 at 11:20, Vijay Kilari <vijay.kilari@gmail.com> wrote:
> On Fri, Jun 17, 2016 at 1:12 AM, Richard Henderson <rth@twiddle.net> wrote:
>> On 06/14/2016 04:36 AM, Peter Maydell wrote:
>>> It would be better to delay the point at which we allocate
>>> the data structures which care about page size, rather than
>>> moving init of the CPU earlier.
>>
>> It would be *best* if we could re-initialize and re-allocate these data
>> structures so that we can follow the current page size as it changes.
>>
>> Yes, this might require flushing just about everything, but it's the kind of
>> thing that's likely to happen only at system startup.  After that, everything
>> benefits from having the correct (larger) page size.
>
> I tried shuffling the memory initialization code after cpu initialization.
> but it was full mess. So could not proceed further.
>
> However, I tried early creation cpu objects instead of doing it from
> machvirt_init.
> With this, initfn of the cpu model is called earlier and feature set is updated.
> With that I could fetch arm architecture info. Based on this we can
> choose page size.

This won't work, because machvirt_init needs to be able to specify
properties of the CPU. I think we need to solve the issues with
dynamically reinitializing and reallocating the data structures,
as rth suggests.

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH v1 4/4] target-arm: Compute page size based on ARM target cpu type

[Vijay Kilari]

On Fri, Jun 17, 2016 at 4:00 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 17 June 2016 at 11:20, Vijay Kilari <vijay.kilari@gmail.com> wrote:
>> On Fri, Jun 17, 2016 at 1:12 AM, Richard Henderson <rth@twiddle.net> wrote:
>>> On 06/14/2016 04:36 AM, Peter Maydell wrote:
>>>> It would be better to delay the point at which we allocate
>>>> the data structures which care about page size, rather than
>>>> moving init of the CPU earlier.
>>>
>>> It would be *best* if we could re-initialize and re-allocate these data
>>> structures so that we can follow the current page size as it changes.
>>>
>>> Yes, this might require flushing just about everything, but it's the kind of
>>> thing that's likely to happen only at system startup.  After that, everything
>>> benefits from having the correct (larger) page size.
>>
>> I tried shuffling the memory initialization code after cpu initialization.
>> but it was full mess. So could not proceed further.
>>
>> However, I tried early creation cpu objects instead of doing it from
>> machvirt_init.
>> With this, initfn of the cpu model is called earlier and feature set is updated.
>> With that I could fetch arm architecture info. Based on this we can
>> choose page size.
>
> This won't work, because machvirt_init needs to be able to specify
> properties of the CPU. I think we need to solve the issues with
> dynamically reinitializing and reallocating the data structures,
> as rth suggests.

In early init, only cpu object is created and does not specify
properties of the CPU.
machvirt_init will reuse already created cpu object and specify
properties of the CPU.

Regards
Vijay

Re: [Qemu-devel] [RFC PATCH v1 4/4] target-arm: Compute page size based on ARM target cpu type

[Peter Maydell]

On 17 June 2016 at 11:39, Vijay Kilari <vijay.kilari@gmail.com> wrote:
> On Fri, Jun 17, 2016 at 4:00 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> This won't work, because machvirt_init needs to be able to specify
>> properties of the CPU. I think we need to solve the issues with
>> dynamically reinitializing and reallocating the data structures,
>> as rth suggests.
>
> In early init, only cpu object is created and does not specify
> properties of the CPU.
> machvirt_init will reuse already created cpu object and specify
> properties of the CPU.

You don't know how many CPUs to create, though, and some of the
properties defined by the machine init function include those
which tell you whether the page size should be 1K or 4K.

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH v1 4/4] target-arm: Compute page size based on ARM target cpu type

[Peter Maydell]

On 17 June 2016 at 11:20, Vijay Kilari <vijay.kilari@gmail.com> wrote:
> On Fri, Jun 17, 2016 at 1:12 AM, Richard Henderson <rth@twiddle.net> wrote:
>> On 06/14/2016 04:36 AM, Peter Maydell wrote:
>>> It would be better to delay the point at which we allocate
>>> the data structures which care about page size, rather than
>>> moving init of the CPU earlier.
>>
>> It would be *best* if we could re-initialize and re-allocate these data
>> structures so that we can follow the current page size as it changes.
>>
>> Yes, this might require flushing just about everything, but it's the kind of
>> thing that's likely to happen only at system startup.  After that, everything
>> benefits from having the correct (larger) page size.
>
> I tried shuffling the memory initialization code after cpu initialization.
> but it was full mess. So could not proceed further.
>
> However, I tried early creation cpu objects instead of doing it from
> machvirt_init.

I'm going to have a go at this and see if I can come up with a
nice looking arrangement. Will try to send some patches by the
end of the week...

thanks
-- PMM