Re: [PATCH 01/10] modules: Provide macros making it easier to identify module exports

[Christophe de Dinechin <dinechin@redhat.com>]

On 2020-06-30 at 11:38 CEST, Michael S. Tsirkin wrote...
> On Fri, Jun 26, 2020 at 06:42:58PM +0200, Christophe de Dinechin wrote:
>> In order to facilitate the move of large chunks of functionality to
>> load modules, it is simpler to create a wrapper with the same name
>> that simply relays the implementation. For efficiency, this is
>> typically done using inline functions in the header for the
>> corresponding functionality. In that case, we rename the actual
>> implementation by appending _implementation to its name. This makes it
>> easier to select which function you want to put a breakpoint on.
>>
>> Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
>> ---
>>  include/qemu/module.h | 24 ++++++++++++++++++++++++
>>  1 file changed, 24 insertions(+)
>>
>> diff --git a/include/qemu/module.h b/include/qemu/module.h
>> index 011ae1ae76..1922a0293c 100644
>> --- a/include/qemu/module.h
>> +++ b/include/qemu/module.h
>> @@ -39,6 +39,30 @@ static void __attribute__((constructor)) do_qemu_init_ ## function(void)    \
>>  }
>>  #endif
>>
>> +#ifdef CONFIG_MODULES
>> +/* Identify which functions are replaced by a callback stub */
>> +#ifdef MODULE_STUBS
>> +#define MODIFACE(Ret,Name,Args)                                         \
>> +    Ret (*Name)Args;                                                    \
>> +    extern Ret Name##_implementation Args
>> +#else /* !MODULE_STUBS */
>> +#define MODIFACE(Ret,Name,Args)                                         \
>> +    extern Ret (*Name)Args;                                             \
>> +    extern Ret Name##_implementation Args
>> +#endif /* MODULE_STUBS */
>> +
>> +#define MODIMPL(Ret,Name,Args)                                          \
>> +    static void __attribute__((constructor)) Name##_register(void)      \
>> +    {                                                                   \
>> +        Name = Name##_implementation;                                   \
>> +    }                                                                   \
>> +    Ret Name##_implementation Args
>> +#else /* !CONFIG_MODULES */
>> +/* When not using a module, such functions are called directly */
>> +#define MODIFACE(Ret,Name,Args)         Ret Name Args
>> +#define MODIMPL(Ret,Name,Args)          Ret Name Args
>> +#endif /* CONFIG_MODULES */
>> +
>>  typedef enum {
>>      MODULE_INIT_MIGRATION,
>>      MODULE_INIT_BLOCK,
>
> Hmm that's quite a bit of overhead for each call across modules.

Do you mean runtime overhead, i.e. the difference between
foo(x) and (*foo)(x)?


> Can't code patching be used somehow?

I've considered it, but this seems like a bit of a hammer for half a dozen
calls that are mostly init, so not performance sensitive.

In the old times, ld.so used to do that for you. All you had to do was to
mark one symbol as weak. Apparently for security reasons, that feature was
dropped several years ago. At least, that put call patching where it
belonged, i.e. in the loader.

>
>
>> --
>> 2.26.2


--
Cheers,
Christophe de Dinechin (IRC c3d)