On Fri, Oct 15 2021 at 11:42, Sami Tolvanen wrote:

On Fri, Oct 15, 2021 at 10:57 AM Thomas Gleixner [off-list ref] wrote:

quoted

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
....

quoted

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/ (local)

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