Re: [Qemu-devel] [RFC PATCH v1 1/2] target-arm: Update page size for aarch64

[Peter Maydell]

On 31 May 2016 at 10:04, Vijay Kilari  wrote:
> On Wed, Apr 6, 2016 at 8:31 PM, Vijay Kilari  wrote:
>> On Mon, Apr 4, 2016 at 10:14 PM, Peter Maydell  
>> wrote:
>>> If you want to avoid defining TARGET_PAGE_SIZE to the
>>> lowest-common-denominator 1K, then yes, you'd need to
>>> choose it at runtime. That could be painful to implement.

> The L1 page table size, L1 shift are dependent on TARGET_PAGE_BITS(page size).
> as shown in snippet code below from translate-all.c

Yes, that's why I said "painful to implement" :-)

> How about adding CONFIG_PAGE_SIZE option to configure?.

I don't want this to be a configure option, because QEMU
should just work for everybody. Otherwise we have some QEMU
binaries which silently don't implement the architecture/CPU
that they ought to.

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH v1 1/2] target-arm: Update page size for aarch64

[Vijay Kilari]

Hi Peter

On Wed, Apr 6, 2016 at 8:31 PM, Vijay Kilari  wrote:
> On Mon, Apr 4, 2016 at 10:14 PM, Peter Maydell  
> wrote:
>> On 4 April 2016 at 17:40, Vijay Kilari  wrote:
>>> On Mon, Apr 4, 2016 at 7:14 PM, Peter Maydell  
>>> wrote:
 I agree that this would definitely improve performance (both for
 migration and for emulated guests), but I'm afraid this breaks
 running 32-bit ARMv5 and ARMv7M guests with this QEMU binary,
 so we can't do this. If we want to allow the minimum page size to
 be bigger than 1K for AArch64 CPUs then we need to make it a
 runtime settable thing rather than compile-time (which is not
 an entirely trivial thing).
>>>
>>> Do you mean to say that based on -cpu type qemu option
>>> choose the page size at runtime?
>>
>> If you want to avoid defining TARGET_PAGE_SIZE to the
>> lowest-common-denominator 1K, then yes, you'd need to
>> choose it at runtime. That could be painful to implement.
>
> Had a look at it. Needs some changes in common code as well.
> I will send this as a separate patch series and drop this patch
> from this series.

The L1 page table size, L1 shift are dependent on TARGET_PAGE_BITS(page size).
as shown in snippet code below from translate-all.c

/* 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)

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

How about adding CONFIG_PAGE_SIZE option to configure?.

Re: [Qemu-devel] [RFC PATCH v1 1/2] target-arm: Update page size for aarch64

[Vijay Kilari]

On Mon, Apr 4, 2016 at 10:14 PM, Peter Maydell  wrote:
> On 4 April 2016 at 17:40, Vijay Kilari  wrote:
>> On Mon, Apr 4, 2016 at 7:14 PM, Peter Maydell  
>> wrote:
>>> I agree that this would definitely improve performance (both for
>>> migration and for emulated guests), but I'm afraid this breaks
>>> running 32-bit ARMv5 and ARMv7M guests with this QEMU binary,
>>> so we can't do this. If we want to allow the minimum page size to
>>> be bigger than 1K for AArch64 CPUs then we need to make it a
>>> runtime settable thing rather than compile-time (which is not
>>> an entirely trivial thing).
>>
>> Do you mean to say that based on -cpu type qemu option
>> choose the page size at runtime?
>
> If you want to avoid defining TARGET_PAGE_SIZE to the
> lowest-common-denominator 1K, then yes, you'd need to
> choose it at runtime. That could be painful to implement.

Had a look at it. Needs some changes in common code as well.
I will send this as a separate patch series and drop this patch
from this series.

>
> thanks
> -- PMM

Re: [Qemu-devel] [RFC PATCH v1 1/2] target-arm: Update page size for aarch64

[Vijay Kilari]

On Mon, Apr 4, 2016 at 7:14 PM, Peter Maydell  wrote:
> On 4 April 2016 at 14:39,   wrote:
>> From: Vijay 
>>
>> Set target page size to minimum 4K for aarch64.
>> This helps to reduce live migration downtime significantly.
>>
>> Signed-off-by: Vijaya Kumar K 
>> ---
>>  target-arm/cpu.h |7 +++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
>> index 066ff67..2e4b48f 100644
>> --- a/target-arm/cpu.h
>> +++ b/target-arm/cpu.h
>> @@ -1562,11 +1562,18 @@ bool write_cpustate_to_list(ARMCPU *cpu);
>>  #if defined(CONFIG_USER_ONLY)
>>  #define TARGET_PAGE_BITS 12
>>  #else
>> +/*
>> + * Aarch64 support minimum 4K page size
>> + */
>> +#if defined(TARGET_AARCH64)
>> +#define TARGET_PAGE_BITS 12
>
> I agree that this would definitely improve performance (both for
> migration and for emulated guests), but I'm afraid this breaks
> running 32-bit ARMv5 and ARMv7M guests with this QEMU binary,
> so we can't do this. If we want to allow the minimum page size to
> be bigger than 1K for AArch64 CPUs then we need to make it a
> runtime settable thing rather than compile-time (which is not
> an entirely trivial thing).

Do you mean to say that based on -cpu type qemu option
choose the page size at runtime?

>
>> +#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
>>  #endif
>> +#endif
>>
>>  #if defined(TARGET_AARCH64)
>>  #  define TARGET_PHYS_ADDR_SPACE_BITS 48
>
> thanks
> -- PMM

Re: [Qemu-devel] [RFC PATCH v1 1/2] target-arm: Update page size for aarch64

[Peter Maydell]

On 4 April 2016 at 17:40, Vijay Kilari  wrote:
> On Mon, Apr 4, 2016 at 7:14 PM, Peter Maydell  
> wrote:
>> I agree that this would definitely improve performance (both for
>> migration and for emulated guests), but I'm afraid this breaks
>> running 32-bit ARMv5 and ARMv7M guests with this QEMU binary,
>> so we can't do this. If we want to allow the minimum page size to
>> be bigger than 1K for AArch64 CPUs then we need to make it a
>> runtime settable thing rather than compile-time (which is not
>> an entirely trivial thing).
>
> Do you mean to say that based on -cpu type qemu option
> choose the page size at runtime?

If you want to avoid defining TARGET_PAGE_SIZE to the
lowest-common-denominator 1K, then yes, you'd need to
choose it at runtime. That could be painful to implement.

thanks
-- PMM

[Qemu-devel] [RFC PATCH v1 1/2] target-arm: Update page size for aarch64

[vijayak]

From: Vijay 

Set target page size to minimum 4K for aarch64.
This helps to reduce live migration downtime significantly.

Signed-off-by: Vijaya Kumar K 
---
 target-arm/cpu.h |7 +++
 1 file changed, 7 insertions(+)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 066ff67..2e4b48f 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1562,11 +1562,18 @@ bool write_cpustate_to_list(ARMCPU *cpu);
 #if defined(CONFIG_USER_ONLY)
 #define TARGET_PAGE_BITS 12
 #else
+/*
+ * Aarch64 support minimum 4K page size
+ */
+#if defined(TARGET_AARCH64)
+#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
 #endif
+#endif
 
 #if defined(TARGET_AARCH64)
 #  define TARGET_PHYS_ADDR_SPACE_BITS 48
-- 
1.7.9.5

Re: [Qemu-devel] [RFC PATCH v1 1/2] target-arm: Update page size for aarch64

[Peter Maydell]

On 4 April 2016 at 14:39,   wrote:
> From: Vijay 
>
> Set target page size to minimum 4K for aarch64.
> This helps to reduce live migration downtime significantly.
>
> Signed-off-by: Vijaya Kumar K 
> ---
>  target-arm/cpu.h |7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 066ff67..2e4b48f 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1562,11 +1562,18 @@ bool write_cpustate_to_list(ARMCPU *cpu);
>  #if defined(CONFIG_USER_ONLY)
>  #define TARGET_PAGE_BITS 12
>  #else
> +/*
> + * Aarch64 support minimum 4K page size
> + */
> +#if defined(TARGET_AARCH64)
> +#define TARGET_PAGE_BITS 12

I agree that this would definitely improve performance (both for
migration and for emulated guests), but I'm afraid this breaks
running 32-bit ARMv5 and ARMv7M guests with this QEMU binary,
so we can't do this. If we want to allow the minimum page size to
be bigger than 1K for AArch64 CPUs then we need to make it a
runtime settable thing rather than compile-time (which is not
an entirely trivial thing).

> +#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
>  #endif
> +#endif
>
>  #if defined(TARGET_AARCH64)
>  #  define TARGET_PHYS_ADDR_SPACE_BITS 48

thanks
-- PMM