[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5027DAAF.2060406@ti.com>
Date: Sun, 12 Aug 2012 12:32:47 -0400
From: Cyril Chemparathy <cyril@...com>
To: Nicolas Pitre <nicolas.pitre@...aro.org>
CC: <linux-kernel@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>, <arnd@...db.de>,
<catalin.marinas@....com>, <grant.likely@...retlab.ca>,
<linux@....linux.org.uk>, <will.deacon@....com>
Subject: Re: [PATCH v2 02/22] ARM: add self test for runtime patch mechanism
On 08/11/12 22:35, Nicolas Pitre wrote:
> On Fri, 10 Aug 2012, Cyril Chemparathy wrote:
>
>> This patch adds basic sanity tests to ensure that the instruction patching
>> results in valid instruction encodings. This is done by verifying the output
>> of the patch process against a vector of assembler generated instructions at
>> init time.
>>
>> Signed-off-by: Cyril Chemparathy <cyril@...com>
>> ---
[...]
>> + __asm__ __volatile__ (
>> + " .irp shift1, 0, 6, 12, 18\n"
>> + " .irp shift2, 0, 1, 2, 3, 4, 5\n"
>> + " add r1, r2, #(0x41 << (\\shift1 + \\shift2))\n"
>> + " .endr\n"
>> + " .endr\n"
>
>
> Maybe adding a "add r1, r2 #0x81 << 24" here might be a good thing since
> this is the most used case but missing from the above.
>
Indeed. I've now replaced this with something a bit more extensive.
Does the following look better?
+struct patch_test_imm8 {
+ u16 imm;
+ u16 shift;
+ u32 insn;
+};
+
+static void __init __used __naked __patch_test_code_imm8(void)
+{
+ __asm__ __volatile__ (
+
+ /* a single test case */
+ " .macro test_one, imm, sft\n"
+ " .hword \\imm\n"
+ " .hword \\sft\n"
+ " add r1, r2, #(\\imm << \\sft)\n"
+ " .endm\n"
+
+ /* a sequence of tests at 'inc' increments of shift */
+ " .macro test_seq, imm, sft, max, inc\n"
+ " test_one \\imm, \\sft\n"
+ " .if \\sft < \\max\n"
+ " test_seq \\imm, (\\sft + \\inc), \\max,
\\inc\n"
+ " .endif\n"
+ " .endm\n"
+
+ /* an empty record to mark the end */
+ " .macro test_end\n"
+ " .hword 0, 0\n"
+ " .word 0\n"
+ " .endm\n"
+
+ /* finally generate the test sequences */
+ " test_seq 0x41, 0, 24, 1\n"
+ " test_seq 0x81, 0, 24, 2\n"
+ " test_end\n"
+ : : :
+ );
+}
+
[...]
>> + u32 test_code_addr = (u32)(&__patch_test_code_imm8);
>> + u32 *test_code = (u32 *)(test_code_addr & ~0x3);
>
> Why this masking? With Thumb2 you may find functions starting at
> halfword aligned addresses. Only the LSB (indicating thumb mode) should
> be masked out.
>
Fixed. Thanks.
Thanks
-- Cyril.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists