Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 07 Mar 2022 16:47:21 -0800 (PST)
Subject:     Re: [PATCH v6 07/14] riscv: prevent compressed instructions in alternatives
In-Reply-To: <20220209123800.269774-8-heiko@sntech.de>
CC:     Paul Walmsley <paul.walmsley@sifive.com>, aou@eecs.berkeley.edu,
        linux-riscv@lists.infradead.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, robh+dt@kernel.org, wefu@redhat.com,
        liush@allwinnertech.com, guoren@kernel.org, atishp@atishpatra.org,
        anup@brainfault.org, drew@beagleboard.org,
        Christoph Hellwig <hch@lst.de>, Arnd Bergmann <arnd@arndb.de>,
        wens@csie.org, maxime@cerno.tech, gfavor@ventanamicro.com,
        andrea.mondelli@huawei.com, behrensj@mit.edu, xinhaoqu@huawei.com,
        huffman@cadence.com, mick@ics.forth.gr,
        allen.baum@esperantotech.com, jscheid@ventanamicro.com,
        rtrauben@gmail.com, samuel@sholland.org, cmuellner@linux.com,
        philipp.tomsich@vrull.eu, heiko@sntech.de
From:   Palmer Dabbelt <palmer@dabbelt.com>
To:     heiko@sntech.de
Message-ID: <mhng-edc0da56-7fc8-4287-8856-9aac0f7465c1@palmer-ri-x1c9>
Mime-Version: 1.0 (MHng)
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Wed, 09 Feb 2022 04:37:53 PST (-0800), heiko@sntech.de wrote:
> Instructions are opportunistically compressed by the RISC-V assembler
> when possible, but in alternatives-blocks both the old and new content
> need to be the same size, so having the toolchain do somewhat random
> optimizations will cause strange side-effects like
> "attempt to move .org backwards" compile-time errors.
>
> Already a simple "and" used in alternatives assembly will cause these
> mismatched code sizes.

There should probably be a ".option norelax" in here as well, as 
relaxation will trigger exactly the same issues.  That, or we could just 
remove the constraint that these must be the same size (ie, 
automatically pad the smaller one with NOP/jump-to-end).

> So prevent compressed instructions to be generated in alternatives-
> code and use option-push and -pop to only limit this to the relevant
> code blocks
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  arch/riscv/include/asm/alternative-macros.h | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/arch/riscv/include/asm/alternative-macros.h b/arch/riscv/include/asm/alternative-macros.h
> index c0fb11fad631..3a52884bf23d 100644
> --- a/arch/riscv/include/asm/alternative-macros.h
> +++ b/arch/riscv/include/asm/alternative-macros.h
> @@ -19,7 +19,10 @@
>  	.popsection
>  	.subsection 1
>  888 :
> +	.option push
> +	.option norvc
>  	\new_c
> +	.option pop
>  889 :
>  	.previous
>  	.org    . - (889b - 888b) + (887b - 886b)
> @@ -29,7 +32,10 @@
>
>  .macro __ALTERNATIVE_CFG old_c, new_c, vendor_id, errata_id, enable
>  886 :
> +	.option push
> +	.option norvc
>  	\old_c
> +	.option pop
>  887 :
>  	ALT_NEW_CONTENT \vendor_id, \errata_id, \enable, \new_c
>  .endm
> @@ -40,7 +46,10 @@
>  .macro __ALTERNATIVE_CFG_2 old_c, new_c_1, vendor_id_1, errata_id_1, enable_1, \
>  				  new_c_2, vendor_id_2, errata_id_2, enable_2
>  886 :
> +	.option push
> +	.option norvc
>  	\old_c
> +	.option pop
>  887 :
>  	ALT_NEW_CONTENT \vendor_id_1, \errata_id_1, \enable_1, \new_c_1
>  	ALT_NEW_CONTENT \vendor_id_2, \errata_id_2, \enable_2, \new_c_2
> @@ -70,7 +79,10 @@
>  	".popsection\n"							\
>  	".subsection 1\n"						\
>  	"888 :\n"							\
> +	".option push\n"						\
> +	".option norvc\n"						\
>  	new_c "\n"							\
> +	".option pop\n"							\
>  	"889 :\n"							\
>  	".previous\n"							\
>  	".org	. - (887b - 886b) + (889b - 888b)\n"			\
> @@ -79,7 +91,10 @@
>
>  #define __ALTERNATIVE_CFG(old_c, new_c, vendor_id, errata_id, enable)	\
>  	"886 :\n"							\
> +	".option push\n"						\
> +	".option norvc\n"						\
>  	old_c "\n"							\
> +	".option pop\n"							\
>  	"887 :\n"							\
>  	ALT_NEW_CONTENT(vendor_id, errata_id, enable, new_c)
>
> @@ -89,7 +104,10 @@
>  #define __ALTERNATIVE_CFG_2(old_c, new_c_1, vendor_id_1, errata_id_1, enable_1, \
>  				  new_c_2, vendor_id_2, errata_id_2, enable_2) \
>  	"886 :\n"							\
> +	".option push\n"						\
> +	".option norvc\n"						\
>  	old_c "\n"							\
> +	".option pop\n"							\
>  	"887 :\n"							\
>  	ALT_NEW_CONTENT(vendor_id_1, errata_id_1, enable_1, new_c_1)	\
>  	ALT_NEW_CONTENT(vendor_id_2, errata_id_2, enable_2, new_c_2)