From: jonathan.austin@arm•com (Jonathan Austin)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v7 1/3] ARM: make cr_alignment read-only #ifndef CONFIG_CPU_CP15
Date: Tue, 15 Jan 2013 13:08:20 +0000 [thread overview]
Message-ID: <50F554C4.3020903@arm.com> (raw)
In-Reply-To: <1350462872-16805-2-git-send-email-u.kleine-koenig@pengutronix.de>
Hi Uwe,
On 17/10/12 09:34, Uwe Kleine-K?nig wrote:
> This makes cr_alignment a constant 0 to break code that tries to modify
> the value as it's likely that it's built on wrong assumption when
> CONFIG_CPU_CP15 isn't defined. For code that is only reading the value 0
> is more or less a fine value to report.
>
Without the context of some of the discussion that was had on the list
about how/why to do this, this description is a bit confusing...
I found myself asking "Why do we not #ifdef out uses of cr_alignment
based on CONFIG_CPU_CP15" - a question which it seems is answered by you
and Nicolas here:
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-March/089968.html
So, perhaps it would help to have a little bit more context in this
commit message?
I know that the cr_alignment stuff is currently tied up with
CONFIG_CPU_15, but I wonder if you looked at using the CCR.UNALIGN_TRP
bit and wiring that in to alignment trapping code? Is it something you
think would make sense for the work you're doing?
Also, see a couple of minor style/commenting points below...
> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix•de>
> ---
> unchanged since v6
> arch/arm/include/asm/cp15.h | 11 ++++++++++-
> arch/arm/kernel/head-common.S | 9 +++++++--
> arch/arm/mm/alignment.c | 2 ++
> arch/arm/mm/mmu.c | 17 +++++++++++++++++
> 4 files changed, 36 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/include/asm/cp15.h b/arch/arm/include/asm/cp15.h
> index 5ef4d80..d814435 100644
> --- a/arch/arm/include/asm/cp15.h
> +++ b/arch/arm/include/asm/cp15.h
> @@ -42,6 +42,8 @@
> #define vectors_high() (0)
> #endif
>
> +#ifdef CONFIG_CPU_CP15
> +
> extern unsigned long cr_no_alignment; /* defined in entry-armv.S */
> extern unsigned long cr_alignment; /* defined in entry-armv.S */
>
> @@ -82,6 +84,13 @@ static inline void set_copro_access(unsigned int val)
> isb();
> }
>
> -#endif
> +#else /* ifdef CONFIG_CPU_CP15 */
> +
> +#define cr_no_alignment UL(0)
> +#define cr_alignment UL(0)
> +
> +#endif /* ifdef CONFIG_CPU_CP15 / else */
> +
> +#endif /* ifndef __ASSEMBLY__ */
>
> #endif
> diff --git a/arch/arm/kernel/head-common.S b/arch/arm/kernel/head-common.S
> index 854bd22..2f560c5 100644
> --- a/arch/arm/kernel/head-common.S
> +++ b/arch/arm/kernel/head-common.S
> @@ -98,8 +98,9 @@ __mmap_switched:
> str r9, [r4] @ Save processor ID
> str r1, [r5] @ Save machine type
> str r2, [r6] @ Save atags pointer
> - bic r4, r0, #CR_A @ Clear 'A' bit
> - stmia r7, {r0, r4} @ Save control register values
> + cmp r7, #0
> + bicne r4, r0, #CR_A @ Clear 'A' bit
> + stmneia r7, {r0, r4} @ Save control register values
> b start_kernel
> ENDPROC(__mmap_switched)
>
> @@ -113,7 +114,11 @@ __mmap_switched_data:
> .long processor_id @ r4
> .long __machine_arch_type @ r5
> .long __atags_pointer @ r6
> +#ifdef CONFIG_CPU_CP15
> .long cr_alignment @ r7
> +#else
> + .long 0
This value still gets loaded in to r7, so it might be worth keeping the
comments going... They certainly help when reading the code.
> +#endif
> .long init_thread_union + THREAD_START_SP @ sp
> .size __mmap_switched_data, . - __mmap_switched_data
>
> diff --git a/arch/arm/mm/alignment.c b/arch/arm/mm/alignment.c
> index b9f60eb..5748094 100644
> --- a/arch/arm/mm/alignment.c
> +++ b/arch/arm/mm/alignment.c
> @@ -962,12 +962,14 @@ static int __init alignment_init(void)
> return -ENOMEM;
> #endif
>
> +#ifdef CONFIG_CPU_CP15
> if (cpu_is_v6_unaligned()) {
> cr_alignment &= ~CR_A;
> cr_no_alignment &= ~CR_A;
> set_cr(cr_alignment);
> ai_usermode = safe_usermode(ai_usermode, false);
> }
> +#endif
>
> hook_fault_code(FAULT_CODE_ALIGNMENT, do_alignment, SIGBUS, BUS_ADRALN,
> "alignment exception");
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index 941dfb9..b675918 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -97,6 +97,7 @@ static struct cachepolicy cache_policies[] __initdata = {
> }
> };
>
> +#ifdef CONFIG_CPU_CP15
> /*
> * These are useful for identifying cache coherency
> * problems by allowing the cache or the cache and
> @@ -195,6 +196,22 @@ void adjust_cr(unsigned long mask, unsigned long set)
> }
> #endif
>
> +#else
When you read this file in its complete form there's a lot of code
(including more preprocessor stuff) between the #ifdef and the #else.
Could you add
#else /* ifdef CONFIG_CPU_CP15 */
like you have in the other cases?
Jonny
next prev parent reply other threads:[~2013-01-15 13:08 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-17 8:34 [PATCH v7 0/3] Yet another update for the Cortex-M3 series Uwe Kleine-König
2012-10-17 8:34 ` [PATCH v7 1/3] ARM: make cr_alignment read-only #ifndef CONFIG_CPU_CP15 Uwe Kleine-König
2013-01-15 13:08 ` Jonathan Austin [this message]
2013-01-16 12:03 ` Uwe Kleine-König
2012-10-17 8:34 ` [PATCH v7 2/3] Cortex-M3: Add base support for Cortex-M3 Uwe Kleine-König
2012-10-17 8:34 ` [PATCH v7 3/3] Cortex-M3: Add support for exception handling Uwe Kleine-König
2013-01-11 11:05 ` Uwe Kleine-König
2013-01-11 11:09 ` Russell King - ARM Linux
2013-01-11 11:14 ` Uwe Kleine-König
2013-01-11 11:26 ` Uwe Kleine-König
2013-01-11 9:47 ` [PATCH v7 0/3] Yet another update for the Cortex-M3 series Uwe Kleine-König
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=50F554C4.3020903@arm.com \
--to=jonathan.austin@arm$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox