Re: CVS commit: src/sys/arch

[Christoph Badura <bad%bsd.de@localhost>]

On Tue, Dec 25, 2018 at 11:45:42AM +0530, Cherry G.Mathew wrote:
> Joerg Sonnenberger <joerg%bec.de@localhost> writes:
> > On Sat, Dec 22, 2018 at 09:27:22PM +0000, Cherry G. Mathew wrote:
> >> Modified Files:
> >> 	src/sys/arch/amd64/amd64: cpufunc.S
> >> 	src/sys/arch/i386/i386: cpufunc.S i386func.S
> >> 	src/sys/arch/xen/x86: xenfunc.c
> >> Log Message:
> >> Introduce a weak alias method of exporting different implementations
> >> of the same API.
> > Why? As in: what's the advantage of making them weak.
> I'd posted separately before this commit explaining the rationale.

It took me a while to check the most obvious places and figure out which
message it might have been.  A more precise reference would have helped.

> Here's the scenario above:
> 
> let's take lcr3(). On native this is a ring3 operation, implemented in
> assembler. On xen, this is a PV call. Currently there's no need to have
> both implementations in a single binary, since PV and native binaries
> are distinct. With PVHVM, we have a situation where at boot time, the
> native version is required, and after XEN is detected, we can use the
> XEN version of the call. I've taken the approach of naming the
> individual functions distinctly, eg: i386_lcr3(), or xen_lcr3(), while
> and exporting them weakly as the consumed version, ie; lcr3().
> 
> What happens is that when the individual binary is compiled, the
> appropriate weakly exported symbol takes over, and things work as
> expected. When  the combined binary for PVHVM is required, we write a
> strongly exported "switch" function, called lcr3() (I haven't committed
> this yet) which overrides both the weak symbols. It can then do the
> runtime calls by calling into the appropriate i386_lcr3() or xen_lcr3(),
> depending on the mode in which it is running.

I don't find this argument for weak aliases convincing.  You plan to
write the function that makes a runtime decision between the
implemenation anyway.  You might as well write that function now and avoid
another bit of hidden magic.

You can have multiple versions of that "switch" function and select the
appropriate one with config(8) logic.  Or you can select with #ifdef.
Somewhere you have to place the logic for conditional compilation/linking.
Having that logic concentrated in a single place instead of N seems
preferable to me.

--chris