From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2DD1F72 for ; Fri, 15 Oct 2021 22:17:44 +0000 (UTC) From: Thomas Gleixner DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1634336261; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=GYaw9r3XwJHigQlTx9TPmuUJK0go4LDViScK7A+Dcl0=; b=RqZ72OBiPyLo1ca/1t9NFP9mA4cqqjUT7Xt3+zA29o50wA8Zh7OJDzVyO3B0gq3W32ml4W /ww7SavjZHXU2FQv951y5J4aOsLDhE4x3xmOUKLBR1uXAplmECmO8bgGdZXef9YTqM7T6b Bo3gLN+ft6Ib0BvIr19ynyI52Hsfq7IcHEzXnBEYyQIzZcG5mHJthdxi2UCHRciY15CLpK Kbf+HzoVfGtDu1wzbCWg0ktNCazV0r9dJHe5IVb8sC2gLabPlhhurFq5wWm6AU2SOR19P7 ohRMghdThdWpgbUewLW+J8g4JVU5+fWmPkERDUJOFNeW6+5WoGdLOHYoSsTNTQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1634336261; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=GYaw9r3XwJHigQlTx9TPmuUJK0go4LDViScK7A+Dcl0=; b=2U4hG/FdpmEehOILJSJiHiJYZBfrKOo3ubGzaQmIufXFD5KC+QcfjBCydQhSqAwlaTkzGw nukWOJ1+f/nauOCA== To: Sami Tolvanen Cc: Andy Lutomirski , the arch/x86 maintainers , Kees Cook , Josh Poimboeuf , "Peter Zijlstra (Intel)" , Nathan Chancellor , Nick Desaulniers , Sedat Dilek , Steven Rostedt , linux-hardening@vger.kernel.org, Linux Kernel Mailing List , llvm@lists.linux.dev Subject: Re: [PATCH v5 03/15] linkage: Add DECLARE_NOT_CALLED_FROM_C In-Reply-To: References: <20211013181658.1020262-1-samitolvanen@google.com> <20211013181658.1020262-4-samitolvanen@google.com> <7377e6b9-7130-4c20-a0c8-16de4620c995@www.fastmail.com> <8735p25llh.ffs@tglx> <87zgra41dh.ffs@tglx> Date: Sat, 16 Oct 2021 00:17:40 +0200 Message-ID: <87wnme3pbv.ffs@tglx> Precedence: bulk X-Mailing-List: llvm@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain On Fri, Oct 15 2021 at 11:42, Sami Tolvanen wrote: > On Fri, Oct 15, 2021 at 10:57 AM Thomas Gleixner wrote: >> Not beautiful, but it gives the information which is needed and it tells >> me clearly what this is about. While the above lumps everything together >> whatever it is. > > Sure, that makes sense. Ignoring the macro for a moment, how do you > feel about using incomplete structs for the non-C functions as Andy > suggested? I think I agreed with that back then when he suggested it the first time. That still allows me to do a classification: struct asm_exception struct asm_xen_hv_call .... >> Having __bikeshedme would allow to do: >> >> __hardware_call >> __xenhv_call >> __inline_asm_call >> >> or such, which clearly tells how the function should be used and it can >> even be validated by tooling. > > Previously you suggested adding a built-in function to the compiler: > > https://lore.kernel.org/lkml/877dl0sc2m.ffs@nanos.tec.linutronix.de/ > > I actually did implement this in Clang, but the feature wasn't > necessary with opaque types, so I never moved forward with those > patches. A built-in also won't make the code any cleaner, which was a > concern last time. > > I do agree that a function attribute would look cleaner, but it won't > stop anyone from mistakenly calling these functions from C code, which > was something Andy wanted to address at the same time. Do you still > prefer a function attribute over using an opaque type nevertheless? For actually callable functions, by some definition of callable, e.g. the clear_page_*() variants a proper attribute would be definitely preferred. That attribute should tell the compiler that the function is using the register arguments correctly but is not suitable for direct invocation because it clobbers registers. So the compiler can just refuse to call such a function if used directly without an inline asm wrapper which describes the clobbers, right? But thinking more about clobbers. The only "annotation" of clobbers we have today are the clobbers in the inline asm, which is fragile too. Something like __attribute__ ((clobbers ("rcx", "rax"))) might be useful by itself because it allows validation of the clobbers in the inline asm wrappers and also allows a analysis tool to look at the ASM code and check whether the above list is correct. Hmm? Thanks, tglx