[PATCH v2 2/4] ARM: nommu: dynamic exception base address setting

[PATCH v2 2/4] ARM: nommu: dynamic exception base address setting

[Vladimir Murzin]

On 08/03/17 03:21, Greg Ungerer wrote:
> 
> Hi Afzal,
> 
> On 21/01/17 19:20, afzal mohammed write:
>> No-MMU dynamic exception base address configuration on CP15
>> processors. In the case of low vectors, decision based on whether
>> security extensions are enabled & whether remap vectors to RAM
>> CONFIG option is selected.
>>
>> For no-MMU without CP15, current default value of 0x0 is retained.
>>
>> Signed-off-by: afzal mohammed <afzal.mohd.ma@gmail.com>
> 
> This patch (which is in mainline now as commit f8300a0b5d) breaks
> my patch series to support running the Versatile QEMU target with
> a nommu configured linux kernel. This series can be found here:
> 
>  http://lists.infradead.org/pipermail/linux-arm-kernel/2017-February/490653.html
> 
> The problem is that QEMU is failing out with:
> 
>  qemu: fatal: Trying to execute code outside RAM or ROM at 0x41069264
> 
> when this your patch is applied.
> 
> 
>> diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
>> index 2740967727e2..20ac52579952 100644
>> --- a/arch/arm/mm/nommu.c
>> +++ b/arch/arm/mm/nommu.c
>> @@ -11,6 +11,7 @@
>>  #include <linux/kernel.h>
>>  
>>  #include <asm/cacheflush.h>
>> +#include <asm/cp15.h>
>>  #include <asm/sections.h>
>>  #include <asm/page.h>
>>  #include <asm/setup.h>
>> @@ -22,6 +23,8 @@
>>  
>>  #include "mm.h"
>>  
>> +unsigned long vectors_base;
>> +
>>  #ifdef CONFIG_ARM_MPU
>>  struct mpu_rgn_info mpu_rgn_info;
>>  
>> @@ -278,15 +281,60 @@ static void sanity_check_meminfo_mpu(void) {}
>>  static void __init mpu_setup(void) {}
>>  #endif /* CONFIG_ARM_MPU */
>>  
>> +#ifdef CONFIG_CPU_CP15
>> +#ifdef CONFIG_CPU_HIGH_VECTOR
>> +static unsigned long __init setup_vectors_base(void)
>> +{
>> +	unsigned long reg = get_cr();
>> +
>> +	set_cr(reg | CR_V);
>> +	return 0xffff0000;
>> +}
>> +#else /* CONFIG_CPU_HIGH_VECTOR */
>> +/* Write exception base address to VBAR */
>> +static inline void set_vbar(unsigned long val)
>> +{
>> +	asm("mcr p15, 0, %0, c12, c0, 0" : : "r" (val) : "cc");
>> +}
>> +
>> +/*
>> + * Security extensions, bits[7:4], permitted values,
>> + * 0b0000 - not implemented, 0b0001/0b0010 - implemented
>> + */
>> +static inline bool security_extensions_enabled(void)
>> +{
>> +	return !!cpuid_feature_extract(CPUID_EXT_PFR1, 4);
> 
> The problem is here. This ends up generating the asm code:
> 
>   2ebacc:       ee103f31        mrc     15, 0, r3, cr0, cr1, {1}
> 
> QEMU loses it on running this (confirmed by single stepping here).
> 
> Is this valid for an ARM926EJ?
> Or is it QEMU that is at fault here...
> 
> I can see that this would be valid for an ARM11 for example
> (http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0360f/CHDGIJFB.html)
> 
> But I could not see that it is valid on an ARM926
> (http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0198e/I1003211.html)
> 
> Maybe I am looking in the wrong place though.
> 
> Thoughts?

I'm wondering if something like bellow would help?

diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
index 3b5c7aa..25542ec 100644
--- a/arch/arm/mm/nommu.c
+++ b/arch/arm/mm/nommu.c
@@ -303,7 +303,11 @@ static inline void set_vbar(unsigned long val)
  */
 static inline bool security_extensions_enabled(void)
 {
+#if __LINUX_ARM_ARCH__ < 6
+	return 0;
+#else
 	return !!cpuid_feature_extract(CPUID_EXT_PFR1, 4);
+#endif
 }
 
 static unsigned long __init setup_vectors_base(void)

Cheers
Vladimir

> 
> Regards
> Greg
> 
> 

[PATCH v2 2/4] ARM: nommu: dynamic exception base address setting

[Greg Ungerer]

Hi Vladimir,

On 08/03/17 19:56, Vladimir Murzin wrote:
> On 08/03/17 03:21, Greg Ungerer wrote:
>> On 21/01/17 19:20, afzal mohammed write:
>>> No-MMU dynamic exception base address configuration on CP15
>>> processors. In the case of low vectors, decision based on whether
>>> security extensions are enabled & whether remap vectors to RAM
>>> CONFIG option is selected.
>>>
>>> For no-MMU without CP15, current default value of 0x0 is retained.
>>>
>>> Signed-off-by: afzal mohammed <afzal.mohd.ma@gmail.com>
>>
>> This patch (which is in mainline now as commit f8300a0b5d) breaks
>> my patch series to support running the Versatile QEMU target with
>> a nommu configured linux kernel. This series can be found here:
>>
>>  http://lists.infradead.org/pipermail/linux-arm-kernel/2017-February/490653.html
>>
>> The problem is that QEMU is failing out with:
>>
>>  qemu: fatal: Trying to execute code outside RAM or ROM at 0x41069264
>>
>> when this your patch is applied.
>>
>>
>>> diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
>>> index 2740967727e2..20ac52579952 100644
>>> --- a/arch/arm/mm/nommu.c
>>> +++ b/arch/arm/mm/nommu.c
>>> @@ -11,6 +11,7 @@
>>>  #include <linux/kernel.h>
>>>
>>>  #include <asm/cacheflush.h>
>>> +#include <asm/cp15.h>
>>>  #include <asm/sections.h>
>>>  #include <asm/page.h>
>>>  #include <asm/setup.h>
>>> @@ -22,6 +23,8 @@
>>>
>>>  #include "mm.h"
>>>
>>> +unsigned long vectors_base;
>>> +
>>>  #ifdef CONFIG_ARM_MPU
>>>  struct mpu_rgn_info mpu_rgn_info;
>>>
>>> @@ -278,15 +281,60 @@ static void sanity_check_meminfo_mpu(void) {}
>>>  static void __init mpu_setup(void) {}
>>>  #endif /* CONFIG_ARM_MPU */
>>>
>>> +#ifdef CONFIG_CPU_CP15
>>> +#ifdef CONFIG_CPU_HIGH_VECTOR
>>> +static unsigned long __init setup_vectors_base(void)
>>> +{
>>> +	unsigned long reg = get_cr();
>>> +
>>> +	set_cr(reg | CR_V);
>>> +	return 0xffff0000;
>>> +}
>>> +#else /* CONFIG_CPU_HIGH_VECTOR */
>>> +/* Write exception base address to VBAR */
>>> +static inline void set_vbar(unsigned long val)
>>> +{
>>> +	asm("mcr p15, 0, %0, c12, c0, 0" : : "r" (val) : "cc");
>>> +}
>>> +
>>> +/*
>>> + * Security extensions, bits[7:4], permitted values,
>>> + * 0b0000 - not implemented, 0b0001/0b0010 - implemented
>>> + */
>>> +static inline bool security_extensions_enabled(void)
>>> +{
>>> +	return !!cpuid_feature_extract(CPUID_EXT_PFR1, 4);
>>
>> The problem is here. This ends up generating the asm code:
>>
>>   2ebacc:       ee103f31        mrc     15, 0, r3, cr0, cr1, {1}
>>
>> QEMU loses it on running this (confirmed by single stepping here).
>>
>> Is this valid for an ARM926EJ?
>> Or is it QEMU that is at fault here...
>>
>> I can see that this would be valid for an ARM11 for example
>> (http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0360f/CHDGIJFB.html)
>>
>> But I could not see that it is valid on an ARM926
>> (http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0198e/I1003211.html)
>>
>> Maybe I am looking in the wrong place though.
>>
>> Thoughts?
>
> I'm wondering if something like bellow would help?
>
> diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
> index 3b5c7aa..25542ec 100644
> --- a/arch/arm/mm/nommu.c
> +++ b/arch/arm/mm/nommu.c
> @@ -303,7 +303,11 @@ static inline void set_vbar(unsigned long val)
>   */
>  static inline bool security_extensions_enabled(void)
>  {
> +#if __LINUX_ARM_ARCH__ < 6
> +	return 0;
> +#else
>  	return !!cpuid_feature_extract(CPUID_EXT_PFR1, 4);
> +#endif
>  }
>
>  static unsigned long __init setup_vectors_base(void)

Yes, that fixes the problem. Boots up fine with that applied.

Regards
Greg

[PATCH v2 2/4] ARM: nommu: dynamic exception base address setting

[afzal mohammed]

Hi Greg,

Was standing on one leg & hoping that nothing breaks, knocked down,

On Wed, Mar 08, 2017 at 01:21:36PM +1000, Greg Ungerer wrote:

> This patch (which is in mainline now as commit f8300a0b5d) breaks
> my patch series to support running the Versatile QEMU target with
> a nommu configured linux kernel.

Sorry

> > +	return !!cpuid_feature_extract(CPUID_EXT_PFR1, 4);
> 
> The problem is here. This ends up generating the asm code:
> 
>   2ebacc:       ee103f31        mrc     15, 0, r3, cr0, cr1, {1}

Thanks for finding the root cause


i have in mind the diff at the end (need to recheck it's correctness),
saw your response that Vladimir's fix works for you, either (if this
works) way, let's fix ASAP.

Regards
afzal

---8<---
diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
index 3b5c7aaf9c76..081562f5436e 100644
--- a/arch/arm/mm/nommu.c
+++ b/arch/arm/mm/nommu.c
@@ -303,7 +303,9 @@ static inline void set_vbar(unsigned long val)
  */
 static inline bool security_extensions_enabled(void)
 {
-	return !!cpuid_feature_extract(CPUID_EXT_PFR1, 4);
+	if ((read_cpuid_id() & 0x000F0000) == 0x000F0000)
+		return !!cpuid_feature_extract(CPUID_EXT_PFR1, 4);
+	return 0;
 }
 
 static unsigned long __init setup_vectors_base(void)

[PATCH v2 2/4] ARM: nommu: dynamic exception base address setting

[Greg Ungerer]

Hi Afzal,

On 09/03/17 02:22, afzal mohammed wrote:
> Was standing on one leg & hoping that nothing breaks, knocked down,

I think you are still standing... Breaking an out-of-tree patch
is a bit like the tree that falls in the forest that no-one hears :-)


> On Wed, Mar 08, 2017 at 01:21:36PM +1000, Greg Ungerer wrote:
> 
>> This patch (which is in mainline now as commit f8300a0b5d) breaks
>> my patch series to support running the Versatile QEMU target with
>> a nommu configured linux kernel.
> 
> Sorry
>
>>> +	return !!cpuid_feature_extract(CPUID_EXT_PFR1, 4);
>>
>> The problem is here. This ends up generating the asm code:
>>
>>   2ebacc:       ee103f31        mrc     15, 0, r3, cr0, cr1, {1}
> 
> Thanks for finding the root cause
> 
> 
> i have in mind the diff at the end (need to recheck it's correctness),
> saw your response that Vladimir's fix works for you, either (if this
> works) way, let's fix ASAP.

The patch below works too. Thanks for the quick response.

Regards
Greg


> ---8<---
> diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
> index 3b5c7aaf9c76..081562f5436e 100644
> --- a/arch/arm/mm/nommu.c
> +++ b/arch/arm/mm/nommu.c
> @@ -303,7 +303,9 @@ static inline void set_vbar(unsigned long val)
>   */
>  static inline bool security_extensions_enabled(void)
>  {
> -	return !!cpuid_feature_extract(CPUID_EXT_PFR1, 4);
> +	if ((read_cpuid_id() & 0x000F0000) == 0x000F0000)
> +		return !!cpuid_feature_extract(CPUID_EXT_PFR1, 4);
> +	return 0;
>  }
>  
>  static unsigned long __init setup_vectors_base(void)
> 

[PATCH v2 2/4] ARM: nommu: dynamic exception base address setting

[afzal mohammed]

Hi,

On Thu, Mar 09, 2017 at 09:51:58AM +1000, Greg Ungerer wrote:
> The patch below works too. Thanks for the quick response.

Thanks Greg for verifying

Vladimir, would the diff like below okay ?

Regards
afzal

> > ---8<---
> > diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
> > index 3b5c7aaf9c76..081562f5436e 100644
> > --- a/arch/arm/mm/nommu.c
> > +++ b/arch/arm/mm/nommu.c
> > @@ -303,7 +303,9 @@ static inline void set_vbar(unsigned long val)
> >   */
> >  static inline bool security_extensions_enabled(void)
> >  {
> > -	return !!cpuid_feature_extract(CPUID_EXT_PFR1, 4);
> > +	if ((read_cpuid_id() & 0x000F0000) == 0x000F0000)
> > +		return !!cpuid_feature_extract(CPUID_EXT_PFR1, 4);
> > +	return 0;
> >  }
> >  
> >  static unsigned long __init setup_vectors_base(void)
> > 

[PATCH v2 2/4] ARM: nommu: dynamic exception base address setting

[Vladimir Murzin]

On 09/03/17 12:46, afzal mohammed wrote:
> Hi,
> 
> On Thu, Mar 09, 2017 at 09:51:58AM +1000, Greg Ungerer wrote:
>> The patch below works too. Thanks for the quick response.
> 
> Thanks Greg for verifying
> 
> Vladimir, would the diff like below okay ?

Well, we should ask Russell which version he (dis)likes :)

Cheers
Vladimir

> 
> Regards
> afzal
> 
>>> ---8<---
>>> diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
>>> index 3b5c7aaf9c76..081562f5436e 100644
>>> --- a/arch/arm/mm/nommu.c
>>> +++ b/arch/arm/mm/nommu.c
>>> @@ -303,7 +303,9 @@ static inline void set_vbar(unsigned long val)
>>>   */
>>>  static inline bool security_extensions_enabled(void)
>>>  {
>>> -	return !!cpuid_feature_extract(CPUID_EXT_PFR1, 4);
>>> +	if ((read_cpuid_id() & 0x000F0000) == 0x000F0000)
>>> +		return !!cpuid_feature_extract(CPUID_EXT_PFR1, 4);
>>> +	return 0;
>>>  }
>>>  
>>>  static unsigned long __init setup_vectors_base(void)
>>>
> 

[PATCH v2 2/4] ARM: nommu: dynamic exception base address setting

[afzal mohammed]

Hi,

i was thinking of sending a patch to fix the Greg's issue today as
will be losing access to the boards shortly for a few days, and then
came to know that -next won't be there next week, so once back after a
few days plan to post the fix unless some one overtakes me. And
perhaps more suggestion/feedback can be obtained by that time, rather
an excuse for laziness ;)

Regards
afzal

[PATCH v2 2/4] ARM: nommu: dynamic exception base address setting

[afzal mohammed]

No-MMU dynamic exception base address configuration on CP15
processors. In the case of low vectors, decision based on whether
security extensions are enabled & whether remap vectors to RAM
CONFIG option is selected.

For no-MMU without CP15, current default value of 0x0 is retained.

Signed-off-by: afzal mohammed <afzal.mohd.ma@gmail.com>
---

v2:
 Use existing helpers to detect security extensions
 Rewrite a CPP step to C for readability

 arch/arm/mm/nommu.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 50 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
index 2740967727e2..20ac52579952 100644
--- a/arch/arm/mm/nommu.c
+++ b/arch/arm/mm/nommu.c
@@ -11,6 +11,7 @@
 #include <linux/kernel.h>
 
 #include <asm/cacheflush.h>
+#include <asm/cp15.h>
 #include <asm/sections.h>
 #include <asm/page.h>
 #include <asm/setup.h>
@@ -22,6 +23,8 @@
 
 #include "mm.h"
 
+unsigned long vectors_base;
+
 #ifdef CONFIG_ARM_MPU
 struct mpu_rgn_info mpu_rgn_info;
 
@@ -278,15 +281,60 @@ static void sanity_check_meminfo_mpu(void) {}
 static void __init mpu_setup(void) {}
 #endif /* CONFIG_ARM_MPU */
 
+#ifdef CONFIG_CPU_CP15
+#ifdef CONFIG_CPU_HIGH_VECTOR
+static unsigned long __init setup_vectors_base(void)
+{
+	unsigned long reg = get_cr();
+
+	set_cr(reg | CR_V);
+	return 0xffff0000;
+}
+#else /* CONFIG_CPU_HIGH_VECTOR */
+/* Write exception base address to VBAR */
+static inline void set_vbar(unsigned long val)
+{
+	asm("mcr p15, 0, %0, c12, c0, 0" : : "r" (val) : "cc");
+}
+
+/*
+ * Security extensions, bits[7:4], permitted values,
+ * 0b0000 - not implemented, 0b0001/0b0010 - implemented
+ */
+static inline bool security_extensions_enabled(void)
+{
+	return !!cpuid_feature_extract(CPUID_EXT_PFR1, 4);
+}
+
+static unsigned long __init setup_vectors_base(void)
+{
+	unsigned long base = 0, reg = get_cr();
+
+	set_cr(reg & ~CR_V);
+	if (security_extensions_enabled()) {
+		if (IS_ENABLED(CONFIG_REMAP_VECTORS_TO_RAM))
+			base = CONFIG_DRAM_BASE;
+		set_vbar(base);
+	} else if (IS_ENABLED(CONFIG_REMAP_VECTORS_TO_RAM)) {
+		if (CONFIG_DRAM_BASE != 0)
+			pr_err("Security extensions not enabled, vectors cannot be remapped to RAM, vectors base will be 0x00000000\n");
+	}
+
+	return base;
+}
+#endif /* CONFIG_CPU_HIGH_VECTOR */
+#endif /* CONFIG_CPU_CP15 */
+
 void __init arm_mm_memblock_reserve(void)
 {
 #ifndef CONFIG_CPU_V7M
+	vectors_base = IS_ENABLED(CONFIG_CPU_CP15) ? setup_vectors_base() : 0;
 	/*
 	 * Register the exception vector page.
 	 * some architectures which the DRAM is the exception vector to trap,
 	 * alloc_page breaks with error, although it is not NULL, but "0."
 	 */
-	memblock_reserve(CONFIG_VECTORS_BASE, 2 * PAGE_SIZE);
+	memblock_reserve(vectors_base, 2 * PAGE_SIZE);
 #else /* ifndef CONFIG_CPU_V7M */
 	/*
 	 * There is no dedicated vector page on V7-M. So nothing needs to be
@@ -310,7 +358,7 @@ void __init sanity_check_meminfo(void)
  */
 void __init paging_init(const struct machine_desc *mdesc)
 {
-	early_trap_init((void *)CONFIG_VECTORS_BASE);
+	early_trap_init((void *)vectors_base);
 	mpu_setup();
 	bootmem_init();
 }
-- 
2.11.0

[PATCH v2 2/4] ARM: nommu: dynamic exception base address setting

[afzal mohammed]

No-MMU dynamic exception base address configuration on CP15
processors. In the case of low vectors, decision based on whether
security extensions are enabled & whether remap vectors to RAM
CONFIG option is selected.

For no-MMU without CP15, current default value of 0x0 is retained.

Signed-off-by: afzal mohammed <afzal.mohd.ma@gmail.com>
---

v2:
 Use existing helpers to detect security extensions
 Rewrite a CPP step to C for readability

 arch/arm/mm/nommu.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 50 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
index 2740967727e2..20ac52579952 100644
--- a/arch/arm/mm/nommu.c
+++ b/arch/arm/mm/nommu.c
@@ -11,6 +11,7 @@
 #include <linux/kernel.h>
 
 #include <asm/cacheflush.h>
+#include <asm/cp15.h>
 #include <asm/sections.h>
 #include <asm/page.h>
 #include <asm/setup.h>
@@ -22,6 +23,8 @@
 
 #include "mm.h"
 
+unsigned long vectors_base;
+
 #ifdef CONFIG_ARM_MPU
 struct mpu_rgn_info mpu_rgn_info;
 
@@ -278,15 +281,60 @@ static void sanity_check_meminfo_mpu(void) {}
 static void __init mpu_setup(void) {}
 #endif /* CONFIG_ARM_MPU */
 
+#ifdef CONFIG_CPU_CP15
+#ifdef CONFIG_CPU_HIGH_VECTOR
+static unsigned long __init setup_vectors_base(void)
+{
+	unsigned long reg = get_cr();
+
+	set_cr(reg | CR_V);
+	return 0xffff0000;
+}
+#else /* CONFIG_CPU_HIGH_VECTOR */
+/* Write exception base address to VBAR */
+static inline void set_vbar(unsigned long val)
+{
+	asm("mcr p15, 0, %0, c12, c0, 0" : : "r" (val) : "cc");
+}
+
+/*
+ * Security extensions, bits[7:4], permitted values,
+ * 0b0000 - not implemented, 0b0001/0b0010 - implemented
+ */
+static inline bool security_extensions_enabled(void)
+{
+	return !!cpuid_feature_extract(CPUID_EXT_PFR1, 4);
+}
+
+static unsigned long __init setup_vectors_base(void)
+{
+	unsigned long base = 0, reg = get_cr();
+
+	set_cr(reg & ~CR_V);
+	if (security_extensions_enabled()) {
+		if (IS_ENABLED(CONFIG_REMAP_VECTORS_TO_RAM))
+			base = CONFIG_DRAM_BASE;
+		set_vbar(base);
+	} else if (IS_ENABLED(CONFIG_REMAP_VECTORS_TO_RAM)) {
+		if (CONFIG_DRAM_BASE != 0)
+			pr_err("Security extensions not enabled, vectors cannot be remapped to RAM, vectors base will be 0x00000000\n");
+	}
+
+	return base;
+}
+#endif /* CONFIG_CPU_HIGH_VECTOR */
+#endif /* CONFIG_CPU_CP15 */
+
 void __init arm_mm_memblock_reserve(void)
 {
 #ifndef CONFIG_CPU_V7M
+	vectors_base = IS_ENABLED(CONFIG_CPU_CP15) ? setup_vectors_base() : 0;
 	/*
 	 * Register the exception vector page.
 	 * some architectures which the DRAM is the exception vector to trap,
 	 * alloc_page breaks with error, although it is not NULL, but "0."
 	 */
-	memblock_reserve(CONFIG_VECTORS_BASE, 2 * PAGE_SIZE);
+	memblock_reserve(vectors_base, 2 * PAGE_SIZE);
 #else /* ifndef CONFIG_CPU_V7M */
 	/*
 	 * There is no dedicated vector page on V7-M. So nothing needs to be
@@ -310,7 +358,7 @@ void __init sanity_check_meminfo(void)
  */
 void __init paging_init(const struct machine_desc *mdesc)
 {
-	early_trap_init((void *)CONFIG_VECTORS_BASE);
+	early_trap_init((void *)vectors_base);
 	mpu_setup();
 	bootmem_init();
 }
-- 
2.11.0