All of lore.kernel.org
 help / color / mirror / Atom feed
From: cyril@ti.com (Cyril Chemparathy)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 02/22] ARM: add self test for runtime patch mechanism
Date: Sun, 12 Aug 2012 12:32:47 -0400	[thread overview]
Message-ID: <5027DAAF.2060406@ti.com> (raw)
In-Reply-To: <alpine.LFD.2.02.1208112226310.5231@xanadu.home>

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@ti.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@'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.

WARNING: multiple messages have this Message-ID (diff)
From: Cyril Chemparathy <cyril@ti.com>
To: Nicolas Pitre <nicolas.pitre@linaro.org>
Cc: <linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>, <arnd@arndb.de>,
	<catalin.marinas@arm.com>, <grant.likely@secretlab.ca>,
	<linux@arm.linux.org.uk>, <will.deacon@arm.com>
Subject: Re: [PATCH v2 02/22] ARM: add self test for runtime patch mechanism
Date: Sun, 12 Aug 2012 12:32:47 -0400	[thread overview]
Message-ID: <5027DAAF.2060406@ti.com> (raw)
In-Reply-To: <alpine.LFD.2.02.1208112226310.5231@xanadu.home>

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@ti.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.

  reply	other threads:[~2012-08-12 16:32 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-11  1:24 [PATCH v2 00/22] Introducing the TI Keystone platform Cyril Chemparathy
2012-08-11  1:24 ` Cyril Chemparathy
2012-08-11  1:24 ` [PATCH v2 01/22] ARM: add mechanism for late code patching Cyril Chemparathy
2012-08-11  1:24   ` Cyril Chemparathy
2012-08-12  2:22   ` Nicolas Pitre
2012-08-12  2:22     ` Nicolas Pitre
2012-08-12 18:13     ` Cyril Chemparathy
2012-08-12 18:13       ` Cyril Chemparathy
2012-08-11  1:24 ` [PATCH v2 02/22] ARM: add self test for runtime patch mechanism Cyril Chemparathy
2012-08-11  1:24   ` Cyril Chemparathy
2012-08-12  2:35   ` Nicolas Pitre
2012-08-12  2:35     ` Nicolas Pitre
2012-08-12 16:32     ` Cyril Chemparathy [this message]
2012-08-12 16:32       ` Cyril Chemparathy
2012-08-13  3:19       ` Nicolas Pitre
2012-08-13  3:19         ` Nicolas Pitre
2012-08-11  1:24 ` [PATCH v2 03/22] ARM: use late patch framework for phys-virt patching Cyril Chemparathy
2012-08-11  1:24   ` Cyril Chemparathy
2012-08-12  3:03   ` Nicolas Pitre
2012-08-12  3:03     ` Nicolas Pitre
2012-08-12 17:34     ` Cyril Chemparathy
2012-08-12 17:34       ` Cyril Chemparathy
2012-08-13  3:32       ` Nicolas Pitre
2012-08-13  3:32         ` Nicolas Pitre
2012-08-11  1:24 ` [PATCH v2 04/22] ARM: LPAE: use phys_addr_t on virt <--> phys conversion Cyril Chemparathy
2012-08-11  1:24   ` Cyril Chemparathy
2012-08-12  3:04   ` Nicolas Pitre
2012-08-12  3:04     ` Nicolas Pitre
2012-08-11  1:24 ` [PATCH v2 05/22] ARM: LPAE: support 64-bit virt_to_phys patching Cyril Chemparathy
2012-08-11  1:24   ` Cyril Chemparathy
2012-08-12  3:39   ` Nicolas Pitre
2012-08-12  3:39     ` Nicolas Pitre
2012-08-12 23:27     ` Cyril Chemparathy
2012-08-12 23:27       ` Cyril Chemparathy
2012-08-13  4:03       ` Nicolas Pitre
2012-08-13  4:03         ` Nicolas Pitre
2012-08-11  1:24 ` [PATCH v2 06/22] ARM: LPAE: use signed arithmetic for mask definitions Cyril Chemparathy
2012-08-11  1:24   ` Cyril Chemparathy
2012-08-12  3:57   ` Nicolas Pitre
2012-08-12  3:57     ` Nicolas Pitre
2012-08-11  1:24 ` [PATCH v2 07/22] ARM: LPAE: use phys_addr_t in alloc_init_pud() Cyril Chemparathy
2012-08-11  1:24   ` Cyril Chemparathy
2012-08-11  1:24 ` [PATCH v2 08/22] ARM: LPAE: use phys_addr_t in free_memmap() Cyril Chemparathy
2012-08-11  1:24   ` Cyril Chemparathy
2012-08-11  1:24 ` [PATCH v2 09/22] ARM: LPAE: use phys_addr_t for initrd location and size Cyril Chemparathy
2012-08-11  1:24   ` Cyril Chemparathy
2012-08-12  3:58   ` Nicolas Pitre
2012-08-12  3:58     ` Nicolas Pitre
2012-08-11  1:24 ` [PATCH v2 10/22] ARM: LPAE: use phys_addr_t in switch_mm() Cyril Chemparathy
2012-08-11  1:24   ` Cyril Chemparathy
2012-08-12  4:04   ` Nicolas Pitre
2012-08-12  4:04     ` Nicolas Pitre
2012-08-11  1:24 ` [PATCH v2 11/22] ARM: LPAE: use 64-bit accessors for TTBR registers Cyril Chemparathy
2012-08-11  1:24   ` Cyril Chemparathy
2012-08-12  4:11   ` Nicolas Pitre
2012-08-12  4:11     ` Nicolas Pitre
2012-08-11  1:24 ` [PATCH v2 12/22] ARM: LPAE: define ARCH_LOW_ADDRESS_LIMIT for bootmem Cyril Chemparathy
2012-08-11  1:24   ` Cyril Chemparathy
2012-08-11  1:24 ` [PATCH v2 13/22] ARM: LPAE: factor out T1SZ and TTBR1 computations Cyril Chemparathy
2012-08-11  1:24   ` Cyril Chemparathy
2012-08-12  4:19   ` Nicolas Pitre
2012-08-12  4:19     ` Nicolas Pitre
2012-08-11  1:24 ` [PATCH v2 14/22] ARM: LPAE: accomodate >32-bit addresses for page table base Cyril Chemparathy
2012-08-11  1:24   ` Cyril Chemparathy
2012-08-11  1:24 ` [PATCH v2 15/22] ARM: mm: use physical addresses in highmem sanity checks Cyril Chemparathy
2012-08-11  1:24   ` Cyril Chemparathy
2012-08-12  4:29   ` Nicolas Pitre
2012-08-12  4:29     ` Nicolas Pitre
2012-08-11  1:24 ` [PATCH v2 16/22] ARM: mm: cleanup checks for membank overlap with vmalloc area Cyril Chemparathy
2012-08-11  1:24   ` Cyril Chemparathy
2012-08-12  4:36   ` Nicolas Pitre
2012-08-12  4:36     ` Nicolas Pitre
2012-09-10 17:43     ` Cyril Chemparathy
2012-09-10 17:43       ` Cyril Chemparathy
2012-09-10 18:07       ` Nicolas Pitre
2012-09-10 18:07         ` Nicolas Pitre
2012-08-11  1:25 ` [PATCH v2 17/22] ARM: mm: clean up membank size limit checks Cyril Chemparathy
2012-08-11  1:25   ` Cyril Chemparathy
2012-08-11  1:25 ` [PATCH v2 18/22] ARM: add virt_to_idmap for interconnect aliasing Cyril Chemparathy
2012-08-11  1:25   ` Cyril Chemparathy
2012-08-11  1:25 ` [PATCH v2 19/22] ARM: recreate kernel mappings in early_paging_init() Cyril Chemparathy
2012-08-11  1:25   ` Cyril Chemparathy
2012-08-11  1:25 ` [RFC v2 20/22] ARM: keystone: introducing TI Keystone platform Cyril Chemparathy
2012-08-11  1:26   ` Cyril Chemparathy
2012-08-11  1:25 ` [RFC v2 21/22] ARM: keystone: enable SMP on Keystone machines Cyril Chemparathy
2012-08-11  1:25   ` Cyril Chemparathy
2012-08-11  1:25 ` [RFC v2 22/22] ARM: keystone: add switch over to high physical address range Cyril Chemparathy
2012-08-11  1:25   ` Cyril Chemparathy

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=5027DAAF.2060406@ti.com \
    --to=cyril@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.