On Tue, 2008-07-01 at 00:32 -0500, Kumar Gala wrote: > To allow for a single kernel image on e500 v1/v2/mc we need to fixup lwsync > at runtime. On e500v1/v2 lwsync causes an illop so we need to patch up > the code. We default to 'sync' since that is always safe and if the cpu > is capable we will replace 'sync' with 'lwsync'. > > We introduce CPU_FTR_LWSYNC as a way to determine at runtime if this is > needed. This flag could be moved elsewhere since we dont really use it > for the normal CPU_FTR purpose. > > Finally we only store the relative offset in the fixup section to keep it > as small as possible rather than using a full fixup_entry.
How many entries are we talking? I guess it's not much bother to have a
separate section.
> diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
> index 9e83add..0109e7f 100644
> --- a/arch/powerpc/kernel/setup_32.c
> +++ b/arch/powerpc/kernel/setup_32.c
> @@ -101,6 +101,10 @@ unsigned long __init early_init(unsigned long dt_ptr)
> PTRRELOC(&__start___ftr_fixup),
> PTRRELOC(&__stop___ftr_fixup));
>
> + do_lwsync_fixups(spec->cpu_features,
> + PTRRELOC(&__start___lwsync_fixup),
> + PTRRELOC(&__stop___lwsync_fixup));
> +
This could be changed to use cur_cpu_spec->cpu_features, and then all
the call sites would be passing that, which would mean
do_lwsync_fixups() could just check cur_cpu_spec->cpu_features directly.
> diff --git a/arch/powerpc/lib/feature-fixups.c
> b/arch/powerpc/lib/feature-fixups.c
> index 48e1ed8..ac5f5a1 100644
> --- a/arch/powerpc/lib/feature-fixups.c
> +++ b/arch/powerpc/lib/feature-fixups.c
> @@ -110,6 +110,22 @@ void do_feature_fixups(unsigned long value, void
> *fixup_start, void *fixup_end)
> }
> }
>
> +void do_lwsync_fixups(unsigned long value, void *fixup_start, void
> *fixup_end)
> +{
> + unsigned int *start, *end, *dest;
> +
> + if (!(value & CPU_FTR_LWSYNC))
> + return ;
> +
> + start = fixup_start;
> + end = fixup_end;
> +
> + for (; start < end; start++) {
> + dest = (void *)start + *start;
> + patch_instruction(dest, PPC_LWSYNC_INSTR);
> + }
> +}
> +
> #ifdef CONFIG_FTR_FIXUP_SELFTEST
...
No tests? :)
> diff --git a/include/asm-powerpc/synch.h b/include/asm-powerpc/synch.h
> index 42a1ef5..4737c61 100644
> --- a/include/asm-powerpc/synch.h
> +++ b/include/asm-powerpc/synch.h
> @@ -3,20 +3,37 @@
> #ifdef __KERNEL__
>
> #include <linux/stringify.h>
> +#include <asm/feature-fixups.h>
>
> -#if defined(__powerpc64__) || defined(CONFIG_PPC_E500MC)
> -#define __SUBARCH_HAS_LWSYNC
> -#endif
> +#ifndef __ASSEMBLY__
> +extern unsigned int __start___lwsync_fixup, __stop___lwsync_fixup;
> +extern void do_lwsync_fixups(unsigned long value, void *fixup_start,
> + void *fixup_end);
> +#endif /* __ASSEMBLY__ */
> +
> +#define START_LWSYNC_SECTION(label) label##1:
> +#define MAKE_LWSYNC_SECTION_ENTRY(label, sect) \
> +label##4: \
> + .pushsection sect,"a"; \
> + .align 2; \
> +label##5: \
> + .long label##1b-label##5b; \
> + .popsection;
I'd rather this was in feature-fixups.h, seeing as I just moved all the
feature-fixup related macros in there :)
It might make more sense to use label##2 and label##3.
>
> -#ifdef __SUBARCH_HAS_LWSYNC
> +#if defined(__powerpc64__)
> # define LWSYNC lwsync
> +#elif defined(CONFIG_E500)
> +# define LWSYNC \
> + START_LWSYNC_SECTION(97); \
> + sync; \
> + MAKE_LWSYNC_SECTION_ENTRY(97, __lwsync_fixup);
> #else
> # define LWSYNC sync
> #endif
Using 97 means you can't nest an LWSYNC inside a feature or firmware
feature section, if you use say 96 then nesting should work.
cheers
--
Michael Ellerman
OzLabs, IBM Australia Development Lab
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
