diff mbox

[v2,1/5] arm64: alternative: Provide if/else/endif assembler macros

Message ID 1437405004-1296-2-git-send-email-daniel.thompson@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Thompson July 20, 2015, 3:10 p.m. UTC
The existing alternative_insn macro has some limitations that make it
hard to work with. In particular the fact it takes instructions from it
own macro arguments means it doesn't play very nicely with C pre-processor
macros because the macro arguments look like a string to the C
pre-processor. Workarounds are (probably) possible but things start to
look ugly.

Introduce an alternative set of macros that allows instructions to be
presented to the assembler as normal and switch everything over to the
new macros.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
---
 arch/arm64/include/asm/alternative.h | 40 ++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

Comments

Will Deacon July 20, 2015, 5:12 p.m. UTC | #1
Hi Daniel,

Couple of really small comments.

On Mon, Jul 20, 2015 at 04:10:00PM +0100, Daniel Thompson wrote:
> The existing alternative_insn macro has some limitations that make it
> hard to work with. In particular the fact it takes instructions from it
> own macro arguments means it doesn't play very nicely with C pre-processor
> macros because the macro arguments look like a string to the C
> pre-processor. Workarounds are (probably) possible but things start to
> look ugly.
> 
> Introduce an alternative set of macros that allows instructions to be
> presented to the assembler as normal and switch everything over to the
> new macros.
> 
> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> ---
>  arch/arm64/include/asm/alternative.h | 40 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
> index c385a0c4057f..31b19ad18f7e 100644
> --- a/arch/arm64/include/asm/alternative.h
> +++ b/arch/arm64/include/asm/alternative.h
> @@ -77,6 +77,46 @@ void free_alternatives_memory(void);
>  	.org	. - (662b-661b) + (664b-663b)
>  .endm
>  
> +/*
> + * Begin an alternative code sequence.
> + *
> + * The code that follows this marco will be assembled and linked as
> + * normal. There are no restrictions on this code.

s/marco/macro/

> + */
> +.macro alternative_if_not cap
> +	.pushsection .altinstructions, "a"
> +	altinstruction_entry 661f, 663f, \cap, 662f-661f, 664f-663f
> +	.popsection
> +661:
> +.endm
> +
> +/*
> + * Provide the alternative code sequence.
> + *
> + * The code that follows this macro is assembled into a special
> + * section to be used for dynamic patching. Code that follows this
> + * macro must:
> + *
> + * 1. Be exactly the same length (in bytes) as the default code
> + *    sequence.
> + *
> + * 2. Not jump to local labels defined outside of the alternative
> + *    sequence.

Actually, we fix up the branch target during patching. What you can't do
is jump into *another* alternative sequence.

Will
Daniel Thompson July 22, 2015, 11:08 a.m. UTC | #2
On 20/07/15 18:12, Will Deacon wrote:
>> diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
>> index c385a0c4057f..31b19ad18f7e 100644
>> --- a/arch/arm64/include/asm/alternative.h
>> +++ b/arch/arm64/include/asm/alternative.h
>> @@ -77,6 +77,46 @@ void free_alternatives_memory(void);
>>   	.org	. - (662b-661b) + (664b-663b)
>>   .endm
>>
>> +/*
>> + * Begin an alternative code sequence.
>> + *
>> + * The code that follows this marco will be assembled and linked as
>> + * normal. There are no restrictions on this code.
>
> s/marco/macro/

Will fix this.


>
>> + */
>> +.macro alternative_if_not cap
>> +	.pushsection .altinstructions, "a"
>> +	altinstruction_entry 661f, 663f, \cap, 662f-661f, 664f-663f
>> +	.popsection
>> +661:
>> +.endm
>> +
>> +/*
>> + * Provide the alternative code sequence.
>> + *
>> + * The code that follows this macro is assembled into a special
>> + * section to be used for dynamic patching. Code that follows this
>> + * macro must:
>> + *
>> + * 1. Be exactly the same length (in bytes) as the default code
>> + *    sequence.
>> + *
>> + * 2. Not jump to local labels defined outside of the alternative
>> + *    sequence.
>
> Actually, we fix up the branch target during patching. What you can't do
> is jump into *another* alternative sequence.

Ok. I will update this.

I saw that there must be branch target patching (due to branches to 
__save_vgic_v3_state) but got it into my head that branches to local 
labels confused the assembler. I guess I must made an unrelated syntax 
error because I just done a few tests and can't reproduce anything like 
that.


Daniel.
diff mbox

Patch

diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
index c385a0c4057f..31b19ad18f7e 100644
--- a/arch/arm64/include/asm/alternative.h
+++ b/arch/arm64/include/asm/alternative.h
@@ -77,6 +77,46 @@  void free_alternatives_memory(void);
 	.org	. - (662b-661b) + (664b-663b)
 .endm
 
+/*
+ * Begin an alternative code sequence.
+ *
+ * The code that follows this marco will be assembled and linked as
+ * normal. There are no restrictions on this code.
+ */
+.macro alternative_if_not cap
+	.pushsection .altinstructions, "a"
+	altinstruction_entry 661f, 663f, \cap, 662f-661f, 664f-663f
+	.popsection
+661:
+.endm
+
+/*
+ * Provide the alternative code sequence.
+ *
+ * The code that follows this macro is assembled into a special
+ * section to be used for dynamic patching. Code that follows this
+ * macro must:
+ *
+ * 1. Be exactly the same length (in bytes) as the default code
+ *    sequence.
+ *
+ * 2. Not jump to local labels defined outside of the alternative
+ *    sequence.
+ */
+.macro alternative_else
+662:	.pushsection .altinstr_replacement, "ax"
+663:
+.endm
+
+/*
+ * Complete an alternative code sequence.
+ */
+.macro alternative_endif
+664:	.popsection
+	.org	. - (664b-663b) + (662b-661b)
+	.org	. - (662b-661b) + (664b-663b)
+.endm
+
 #endif  /*  __ASSEMBLY__  */
 
 #endif /* __ASM_ALTERNATIVE_H */