From: Catalin Marinas <catalin.marinas@arm.com>
To: Andre Przywara <andre.przywara@arm.com>
Cc: Will Deacon <will.deacon@arm.com>,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/6] arm64: alternatives: drop enable parameter from _else and _endif macro
Date: Thu, 23 Jun 2016 18:17:33 +0100 [thread overview]
Message-ID: <20160623171733.GR6521@e104818-lin.cambridge.arm.com> (raw)
In-Reply-To: <1462812590-4494-2-git-send-email-andre.przywara@arm.com>
On Mon, May 09, 2016 at 05:49:45PM +0100, Andre Przywara wrote:
> Commit 77ee306c0aea9 ("arm64: alternatives: add enable parameter to
> conditional asm macros") extended the alternative assembly macros.
> Unfortunately this does not really work as one would expect, as the
> enable parameter in fact correctly protects the alternative section
> magic, but not the actual code sequences.
I don't remember how we ended up with this code because it's not used
anywhere.
> So if enable is false, we will have the original instruction(s) _and_
> the alternative ones in the file, which is just wrong.
> To make this work one would need to additionally protect the
> alternative sequence with extra .if directives, which makes
> the intention of the enable parameter rather pointless.
> Instead users should directly guard the whole "_else; insn; _endif"
> sequence with .if directives.
> Add a comment describing this usage and drop the enable parameter from
> the alternative_else and alternative_endif macros.
>
> This reverts parts of commit 77ee306c0aea ("arm64: alternatives: add
> enable parameter to conditional asm macros").
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
> arch/arm64/include/asm/alternative.h | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
> index beccbde..502c9ef 100644
> --- a/arch/arm64/include/asm/alternative.h
> +++ b/arch/arm64/include/asm/alternative.h
> @@ -94,6 +94,8 @@ void apply_alternatives(void *start, size_t length);
> *
> * The code that follows this macro will be assembled and linked as
> * normal. There are no restrictions on this code.
> + * If you use the enable parameter, see the comments below for _else
> + * and _endif.
> */
> .macro alternative_if_not cap, enable = 1
> .if \enable
> @@ -117,23 +119,33 @@ void apply_alternatives(void *start, size_t length);
> * 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).
> + *
> + * If you used the optional enable parameter in the opening
> + * alternative_if_not macro above, please protect the whole _else
> + * branch with an .if directive:
> + * alternative_if_not CAP_SOMETHING, condition
> + * orig_insn
> + * .if condition
> + * alternative_else
> + * repl_insn
> + * alternative_endif
> + * .endif
I think the intention was to that in the !condition case, both
alternatives are dropped. That includes the original code.
I propose we revert the original commit, I don't think it is strictly
needed for your patches (I'll try to fix them up and see how it goes).
--
Catalin
next prev parent reply other threads:[~2016-06-23 17:17 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-09 16:49 [PATCH 0/6] arm64: Extend Cortex-A53 errata workaround Andre Przywara
2016-05-09 16:49 ` [PATCH 1/6] arm64: alternatives: drop enable parameter from _else and _endif macro Andre Przywara
2016-06-23 17:17 ` Catalin Marinas [this message]
2016-05-09 16:49 ` [PATCH 2/6] arm64: fix "dc cvau" cache operation on errata-affected core Andre Przywara
2016-05-09 16:49 ` [PATCH 3/6] arm64: include alternative handling in dcache_by_line_op Andre Przywara
2016-06-24 15:32 ` Catalin Marinas
2016-05-09 16:49 ` [PATCH 4/6] arm64: errata: Calling enable functions for CPU errata too Andre Przywara
2016-06-10 15:31 ` Suzuki K Poulose
2016-06-24 15:34 ` Catalin Marinas
2016-05-09 16:49 ` [PATCH 5/6] arm64: consolidate signal injection on emulation errors Andre Przywara
2016-05-09 16:49 ` [PATCH 6/6] arm64: trap userspace "dc cvau" cache operation on errata-affected core Andre Przywara
2016-06-14 16:16 ` Suzuki K Poulose
2016-06-17 17:20 ` Andre Przywara
2016-06-17 17:25 ` Suzuki K Poulose
2016-06-24 16:25 ` Catalin Marinas
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=20160623171733.GR6521@e104818-lin.cambridge.arm.com \
--to=catalin.marinas@arm.com \
--cc=andre.przywara@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=will.deacon@arm.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).