Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1189831 > unrolled thread

[PATCH v3 0/5] arm64: alternative: Provide if/else/endif assembler macros

Started byDaniel Thompson <daniel.thompson@linaro.org>
First post2015-07-22 13:30 +0200
Last post2015-07-22 13:30 +0200
Articles 7 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v3 0/5] arm64: alternative: Provide if/else/endif assembler macros Daniel Thompson <daniel.thompson@linaro.org> - 2015-07-22 13:30 +0200
    [PATCH v3 4/5] arm64: kvm: Adopt new alternative assembler macros Daniel Thompson <daniel.thompson@linaro.org> - 2015-07-22 13:30 +0200
      Re: [PATCH v3 4/5] arm64: kvm: Adopt new alternative assembler macros Will Deacon <will.deacon@arm.com> - 2015-07-22 17:20 +0200
        Re: [PATCH v3 4/5] arm64: kvm: Adopt new alternative assembler macros Marc Zyngier <marc.zyngier@arm.com> - 2015-07-22 18:00 +0200
    [PATCH v3 2/5] arm64: mm: Adopt new alternative assembler macros Daniel Thompson <daniel.thompson@linaro.org> - 2015-07-22 13:30 +0200
    [PATCH v3 5/5] arm64: alternative: Remove alternative_insn macro Daniel Thompson <daniel.thompson@linaro.org> - 2015-07-22 13:30 +0200
    [PATCH v3 1/5] arm64: alternative: Provide if/else/endif assembler macros Daniel Thompson <daniel.thompson@linaro.org> - 2015-07-22 13:30 +0200

#1189831 — [PATCH v3 0/5] arm64: alternative: Provide if/else/endif assembler macros

FromDaniel Thompson <daniel.thompson@linaro.org>
Date2015-07-22 13:30 +0200
Subject[PATCH v3 0/5] arm64: alternative: Provide if/else/endif assembler macros
Message-ID<pP0nL-5o8-7@gated-at.bofh.it>
Will: This is a split out version of previous patch, as requested. I have
      retained a patch at the send of the series to nix alternative_insn
      but there's no need to take this one for 4.3. Likewise I've fully
      split out all the switch-over patches so you can just drop them
      if they bring any merge trouble.

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.

This patchset introduces new macros that allow instructions to be
presented to the assembler as normal and overcomes these limitations,
together with patches to switch all existing users to the new macros.

My view is that these if_not/else/endif macros are more readable
than the original macro and that alone might be enough to justify them.
However below is an concrete example of an alterntive sequence that is
needlessly hard to write without them because ICC_PMR_EL1 is a C
pre-processor macro.

 	.macro	disable_irq, tmp
 	mov	\tmp, #ICC_PMR_EL1_MASKED
alternative_if_not ARM64_HAS_SYSREG_GIC_CPUIF
	msr	daifset, #2
alternative_else
 	msr_s	ICC_PMR_EL1, \tmp
alternative_endif
	.endm

The new macros have received a fair degree of testing because I have
based my (not published since March) pseudo-NMI patch set on them.

v3:

 * Corrected some technical and spelling errors in the comments
   (Will Deacon).

v2:

 * Split big patch out into atomized components (Will Deacon).
   To show where I would like to go I have retained a patch to remove
   assembler_insn() from the code base although, based on Will's email
   I don't anticipate this patch being taken for 4.3.

 * Add some comments to make clear the constraints imposed on alternative
   sequences.

Daniel Thompson (5):
  arm64: alternative: Provide if/else/endif assembler macros
  arm64: mm: Adopt new alternative assembler macros
  arm64: kernel: Adopt new alternative assembler macros
  arm64: kvm: Adopt new alternative assembler macros
  arm64: alternative: Remove alternative_insn macro

 arch/arm64/include/asm/alternative.h | 41 ++++++++++++++++++++++++++++++------
 arch/arm64/kernel/entry.S            | 29 ++++++++++++-------------
 arch/arm64/kvm/hyp.S                 | 12 +++++++++--
 arch/arm64/mm/cache.S                |  7 +++++-
 4 files changed, 64 insertions(+), 25 deletions(-)

--
2.4.3

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

[toc] | [next] | [standalone]

#1189832 — [PATCH v3 4/5] arm64: kvm: Adopt new alternative assembler macros

FromDaniel Thompson <daniel.thompson@linaro.org>
Date2015-07-22 13:30 +0200
Subject[PATCH v3 4/5] arm64: kvm: Adopt new alternative assembler macros
Message-ID<pP0nM-5o8-17@gated-at.bofh.it>
In reply to#1189831
Convert the dynamic patching for ARM64_HAS_SYSREG_GIC_CPUIF over to
the newly added alternative assembler macros.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
---
 arch/arm64/kvm/hyp.S | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
index 17a8fb14f428..10915aaf0b01 100644
--- a/arch/arm64/kvm/hyp.S
+++ b/arch/arm64/kvm/hyp.S
@@ -810,7 +810,11 @@
  * Call into the vgic backend for state saving
  */
 .macro save_vgic_state
-	alternative_insn "bl __save_vgic_v2_state", "bl __save_vgic_v3_state", ARM64_HAS_SYSREG_GIC_CPUIF
+alternative_if_not ARM64_HAS_SYSREG_GIC_CPUIF
+	bl	__save_vgic_v2_state
+alternative_else
+	bl	__save_vgic_v3_state
+alternative_endif
 	mrs	x24, hcr_el2
 	mov	x25, #HCR_INT_OVERRIDE
 	neg	x25, x25
@@ -827,7 +831,11 @@
 	orr	x24, x24, #HCR_INT_OVERRIDE
 	orr	x24, x24, x25
 	msr	hcr_el2, x24
-	alternative_insn "bl __restore_vgic_v2_state", "bl __restore_vgic_v3_state", ARM64_HAS_SYSREG_GIC_CPUIF
+alternative_if_not ARM64_HAS_SYSREG_GIC_CPUIF
+	bl	__restore_vgic_v2_state
+alternative_else
+	bl	__restore_vgic_v3_state
+alternative_endif
 .endm
 
 .macro save_timer_state
-- 
2.4.3

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

[toc] | [prev] | [next] | [standalone]

#1190051 — Re: [PATCH v3 4/5] arm64: kvm: Adopt new alternative assembler macros

FromWill Deacon <will.deacon@arm.com>
Date2015-07-22 17:20 +0200
SubjectRe: [PATCH v3 4/5] arm64: kvm: Adopt new alternative assembler macros
Message-ID<pP3Ym-2d9-17@gated-at.bofh.it>
In reply to#1189832
Marc, Christoffer,

On Wed, Jul 22, 2015 at 12:21:04PM +0100, Daniel Thompson wrote:
> Convert the dynamic patching for ARM64_HAS_SYSREG_GIC_CPUIF over to
> the newly added alternative assembler macros.

Do you mind if I take this via the arm64 tree? It won't apply in
isolation and I'd expect conflicts to be small/trivial to resolve.

Cheers,

Will

> ---
>  arch/arm64/kvm/hyp.S | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
> index 17a8fb14f428..10915aaf0b01 100644
> --- a/arch/arm64/kvm/hyp.S
> +++ b/arch/arm64/kvm/hyp.S
> @@ -810,7 +810,11 @@
>   * Call into the vgic backend for state saving
>   */
>  .macro save_vgic_state
> -	alternative_insn "bl __save_vgic_v2_state", "bl __save_vgic_v3_state", ARM64_HAS_SYSREG_GIC_CPUIF
> +alternative_if_not ARM64_HAS_SYSREG_GIC_CPUIF
> +	bl	__save_vgic_v2_state
> +alternative_else
> +	bl	__save_vgic_v3_state
> +alternative_endif
>  	mrs	x24, hcr_el2
>  	mov	x25, #HCR_INT_OVERRIDE
>  	neg	x25, x25
> @@ -827,7 +831,11 @@
>  	orr	x24, x24, #HCR_INT_OVERRIDE
>  	orr	x24, x24, x25
>  	msr	hcr_el2, x24
> -	alternative_insn "bl __restore_vgic_v2_state", "bl __restore_vgic_v3_state", ARM64_HAS_SYSREG_GIC_CPUIF
> +alternative_if_not ARM64_HAS_SYSREG_GIC_CPUIF
> +	bl	__restore_vgic_v2_state
> +alternative_else
> +	bl	__restore_vgic_v3_state
> +alternative_endif
>  .endm
>  
>  .macro save_timer_state
> -- 
> 2.4.3
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1190077 — Re: [PATCH v3 4/5] arm64: kvm: Adopt new alternative assembler macros

FromMarc Zyngier <marc.zyngier@arm.com>
Date2015-07-22 18:00 +0200
SubjectRe: [PATCH v3 4/5] arm64: kvm: Adopt new alternative assembler macros
Message-ID<pP4B4-2Wr-19@gated-at.bofh.it>
In reply to#1190051
On 22/07/15 16:17, Will Deacon wrote:

Hi Will,

> On Wed, Jul 22, 2015 at 12:21:04PM +0100, Daniel Thompson wrote:
>> Convert the dynamic patching for ARM64_HAS_SYSREG_GIC_CPUIF over to
>> the newly added alternative assembler macros.
> 
> Do you mind if I take this via the arm64 tree? It won't apply in
> isolation and I'd expect conflicts to be small/trivial to resolve.

Looks perfectly sensible to me, and I'm happy for you to take this. I
don't believe we have anything queued for 4.3 that would clash with this
anyway.

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
-- 
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1189833 — [PATCH v3 2/5] arm64: mm: Adopt new alternative assembler macros

FromDaniel Thompson <daniel.thompson@linaro.org>
Date2015-07-22 13:30 +0200
Subject[PATCH v3 2/5] arm64: mm: Adopt new alternative assembler macros
Message-ID<pP0nM-5o8-21@gated-at.bofh.it>
In reply to#1189831
Convert the dynamic patching for ARM64_WORKAROUND_CLEAN_CACHE over to
the newly added alternative assembler macros.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
---
 arch/arm64/mm/cache.S | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/mm/cache.S b/arch/arm64/mm/cache.S
index bdeb5d38c2dd..eb48d5df4a0f 100644
--- a/arch/arm64/mm/cache.S
+++ b/arch/arm64/mm/cache.S
@@ -143,7 +143,12 @@ __dma_clean_range:
 	dcache_line_size x2, x3
 	sub	x3, x2, #1
 	bic	x0, x0, x3
-1:	alternative_insn "dc cvac, x0", "dc civac, x0", ARM64_WORKAROUND_CLEAN_CACHE
+1:
+alternative_if_not ARM64_WORKAROUND_CLEAN_CACHE
+	dc	cvac, x0
+alternative_else
+	dc	civac, x0
+alternative_endif
 	add	x0, x0, x2
 	cmp	x0, x1
 	b.lo	1b
-- 
2.4.3

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

[toc] | [prev] | [next] | [standalone]

#1189834 — [PATCH v3 5/5] arm64: alternative: Remove alternative_insn macro

FromDaniel Thompson <daniel.thompson@linaro.org>
Date2015-07-22 13:30 +0200
Subject[PATCH v3 5/5] arm64: alternative: Remove alternative_insn macro
Message-ID<pP0nM-5o8-23@gated-at.bofh.it>
In reply to#1189831
This macro has been superceded by the alterntive_if_not/else/endif
macro family and is no longer used anywhere in the kerne. Remove it.

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

diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
index e86681ad0931..81c7369eb358 100644
--- a/arch/arm64/include/asm/alternative.h
+++ b/arch/arm64/include/asm/alternative.h
@@ -65,18 +65,6 @@ void free_alternatives_memory(void);
 	.byte \alt_len
 .endm
 
-.macro alternative_insn insn1 insn2 cap
-661:	\insn1
-662:	.pushsection .altinstructions, "a"
-	altinstruction_entry 661b, 663f, \cap, 662b-661b, 664f-663f
-	.popsection
-	.pushsection .altinstr_replacement, "ax"
-663:	\insn2
-664:	.popsection
-	.org	. - (664b-663b) + (662b-661b)
-	.org	. - (662b-661b) + (664b-663b)
-.endm
-
 /*
  * Begin an alternative code sequence.
  *
-- 
2.4.3

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

[toc] | [prev] | [next] | [standalone]

#1189835 — [PATCH v3 1/5] arm64: alternative: Provide if/else/endif assembler macros

FromDaniel Thompson <daniel.thompson@linaro.org>
Date2015-07-22 13:30 +0200
Subject[PATCH v3 1/5] arm64: alternative: Provide if/else/endif assembler macros
Message-ID<pP0nM-5o8-27@gated-at.bofh.it>
In reply to#1189831
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 | 41 ++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
index c385a0c4057f..e86681ad0931 100644
--- a/arch/arm64/include/asm/alternative.h
+++ b/arch/arm64/include/asm/alternative.h
@@ -77,6 +77,47 @@ void free_alternatives_memory(void);
 	.org	. - (662b-661b) + (664b-663b)
 .endm
 
+/*
+ * Begin an alternative code sequence.
+ *
+ * The code that follows this macro 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 contain a branch target that is used outside of the
+ *    alternative sequence it is defined in (branches into an
+ *    alternative sequence are not fixed up).
+ */
+.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 */
-- 
2.4.3

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web