linux-kernel - Re: [PATCH V4 23/23] crypto: hisilicon/qm: Workaround to enable build with RISC-V clang

Open Source and information security mailing list archives

Date:   Tue, 4 Apr 2023 22:59:41 +0100
From:   Conor Dooley <conor@...nel.org>
To:     Sunil V L <sunilvl@...tanamicro.com>
Cc:     linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-riscv@...ts.infradead.org, linux-acpi@...r.kernel.org,
        linux-crypto@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        llvm@...ts.linux.dev, Weili Qian <qianweili@...wei.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Tom Rix <trix@...hat.com>, Jonathan Corbet <corbet@....net>,
        Marc Zyngier <maz@...nel.org>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Mark Gross <markgross@...nel.org>,
        Hans de Goede <hdegoede@...hat.com>,
        Zhou Wang <wangzhou1@...ilicon.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        "Rafael J . Wysocki" <rafael@...nel.org>,
        Nathan Chancellor <nathan@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Maximilian Luz <luzmaximilian@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Len Brown <lenb@...nel.org>
Subject: Re: [PATCH V4 23/23] crypto: hisilicon/qm: Workaround to enable
 build with RISC-V clang

Hey Sunil,

This one made me scratch my head for a bit..

On Tue, Apr 04, 2023 at 11:50:37PM +0530, Sunil V L wrote:
> With CONFIG_ACPI enabled for RISC-V, this driver gets enabled in
> allmodconfig build. The gcc tool chain builds this driver removing the
> inline arm64 assembly code. However, clang for RISC-V tries to build
> the arm64 assembly and below error is seen.

There's actually nothing RISC-V specific about that behaviour, that's
just how clang works. Quoting Nathan:
"Clang performs semantic analysis (i.e., validates assembly) before
dead code elimination, so IS_ENABLED() is not sufficient for avoiding
that error."

> drivers/crypto/hisilicon/qm.c:627:10: error: invalid output constraint '+Q' in asm
>                        "+Q" (*((char __iomem *)fun_base))
>                        ^
> It appears that RISC-V clang is not smart enough to detect
> IS_ENABLED(CONFIG_ARM64) and remove the dead code.

So I think this statement is just not true, it can remove dead code, but
only after it has done the semantic analysis.

The reason that this has not been seen before, again quoting Nathan, is:
"arm64 and x86_64 both support the Q constraint, we cannot build
LoongArch yet (although it does not have support for Q either so same
boat as RISC-V), and ia64 is dead/unsupported in LLVM. Those are the
only architectures that support ACPI, so I guess that explains why we
have seen no issues aside from RISC-V so far."

> As a workaround, move this check to preprocessing stage which works
> with the RISC-V clang tool chain.

I don't think there's much else you can do!
Reviewed-by: Conor Dooley <conor.dooley@...rochip.com>

Perhaps it is also worth adding:
Link: https://github.com/ClangBuiltLinux/linux/issues/999

Cheers,
Conor.

> Signed-off-by: Sunil V L <sunilvl@...tanamicro.com>
> ---
>  drivers/crypto/hisilicon/qm.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/crypto/hisilicon/qm.c b/drivers/crypto/hisilicon/qm.c
> index e4c84433a88a..a5f521529ab2 100644
> --- a/drivers/crypto/hisilicon/qm.c
> +++ b/drivers/crypto/hisilicon/qm.c
> @@ -611,13 +611,9 @@ EXPORT_SYMBOL_GPL(hisi_qm_wait_mb_ready);
>  static void qm_mb_write(struct hisi_qm *qm, const void *src)
>  {
>  	void __iomem *fun_base = qm->io_base + QM_MB_CMD_SEND_BASE;
> -	unsigned long tmp0 = 0, tmp1 = 0;
>  
> -	if (!IS_ENABLED(CONFIG_ARM64)) {
> -		memcpy_toio(fun_base, src, 16);
> -		dma_wmb();
> -		return;
> -	}
> +#if IS_ENABLED(CONFIG_ARM64)
> +	unsigned long tmp0 = 0, tmp1 = 0;
>  
>  	asm volatile("ldp %0, %1, %3\n"
>  		     "stp %0, %1, %2\n"
> @@ -627,6 +623,11 @@ static void qm_mb_write(struct hisi_qm *qm, const void *src)
>  		       "+Q" (*((char __iomem *)fun_base))
>  		     : "Q" (*((char *)src))
>  		     : "memory");
> +#else
> +	memcpy_toio(fun_base, src, 16);
> +	dma_wmb();
> +#endif
> +
>  }
>  
>  static int qm_mb_nolock(struct hisi_qm *qm, struct qm_mailbox *mailbox)
> -- 
> 2.34.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives