Re: [PATCH] mips: export icache_flush_range

[Kees Cook]

On Sat, Mar 22, 2014 at 3:54 PM, Sergei Shtylyov
<sergei.shtylyov@xxxxxxxxxxxxxxxxxx> wrote:
> On 03/23/2014 12:48 AM, Sergei Shtylyov wrote:
>
>>>> The lkdtm module performs tests against executable memory ranges, so
>>>> it needs to flush the icache for proper behaviors. Other architectures
>>>> already export this, so do the same for MIPS.
>
>
>>>> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
>>>> ---
>>>> This is currently untested! I'm building a MIPS cross-compiler now...
>>>> If someone can validate this fixes the build when lkdtm is a module,
>>>> that would be appreciated. :)
>>>> ---
>>>>   arch/mips/mm/cache.c |    1 +
>>>>   1 file changed, 1 insertion(+)
>
>
>>>> diff --git a/arch/mips/mm/cache.c b/arch/mips/mm/cache.c
>>>> index fde7e56d13fe..b3f1df13d9f6 100644
>>>> --- a/arch/mips/mm/cache.c
>>>> +++ b/arch/mips/mm/cache.c
>>>> @@ -38,6 +38,7 @@ void (*__flush_kernel_vmap_range)(unsigned long vaddr,
>>>> int
>>>> size);
>>>>   void (*__invalidate_kernel_vmap_range)(unsigned long vaddr, int size);
>>>>
>>>>   EXPORT_SYMBOL_GPL(__flush_kernel_vmap_range);
>>>> +EXPORT_SYMBOL_GPL(flush_icache_range);
>
>
>>>     Have you run this thru scripts/checkpatch.pl? It would have told you
>>> that
>>> an export should immediately follow the corresponding function body,
>>> AFAIK.
>
>
>>     Hm, it doesn't now but definitely used to...
>
>
>    Decided to check Documentation/CodingStyle, and it still codifies this:
>
> In source files, separate functions with one blank line.  If the function is
> exported, the EXPORT* macro for it should follow immediately after the
> closing
> function brace line.  E.g.:
>
> int system_is_up(void)
> {
>         return system_state == SYSTEM_RUNNING;
> }
> EXPORT_SYMBOL(system_is_up);
>
> WBR, Sergei

Yup, thanks. I know the style, but it seemed from the source file that
the style was to declare the function pointers in bulk, and then
explicitly export them in the next section, so I continued that style.
For example:

void (*flush_cache_sigtramp)(unsigned long addr);
void (*local_flush_data_cache_page)(void * addr);
void (*flush_data_cache_page)(unsigned long addr);
void (*flush_icache_all)(void);

EXPORT_SYMBOL_GPL(local_flush_data_cache_page);
EXPORT_SYMBOL(flush_data_cache_page);
EXPORT_SYMBOL(flush_icache_all);

Regardless, I'm happy to stick it in the middle of the function
pointers if that's preferred.

-Kees

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/