Re: git: 3a60869237b8 - main - Add assembly optimized code for OpenSSL on powerpc, powerpc64 and powerpc64le

On Thu, Nov 25, 2021 at 10:40:13AM -0300, Leandro Lupori wrote:
> On Wed, Nov 24, 2021 at 6:52 PM Piotr Kubaj <pkubaj@anongoth.pl> wrote:
> 
> > On 21-11-24 10:34:36, John Baldwin wrote:
> > > On 11/24/21 10:24 AM, Piotr Kubaj wrote:
> > > > OK, it looks like ossl uses fpu_kern functions and those are available
> > only an aarch64, amd64 and i386:
> > > > cc  -O2 -pipe -fno-common  -fno-strict-aliasing -Werror -D_KERNEL
> > -DKLD_MODULE -nostdinc   -include
> > /usr/obj/usr/src/powerpc.powerpc64/sys/modules/ossl/opt_global.h -I.
> > -I/usr/src/sys -I/usr/src/sys/contrib/ck/include -fno-common  -fPIC
> > -mlongcall -fno-omit-frame-pointer
> > -fdebug-prefix-map=./machine=/usr/src/sys/powerpc/include     -MD
> > -MF.depend.ossl.o -MTossl.o -mno-altivec -msoft-float -mabi=elfv2
> > -ffreestanding -fwrapv -fstack-protector -Wall -Wredundant-decls
> > -Wnested-externs -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith
> > -Wcast-qual -Wundef -Wno-pointer-sign -D__printf__=__freebsd_kprintf__
> > -Wmissing-include-dirs -fdiagnostics-show-option -Wno-unknown-pragmas
> > -Wno-error-tautological-compare -Wno-error-empty-body
> > -Wno-error-parentheses-equality -Wno-error-unused-function
> > -Wno-error-pointer-sign -Wno-error-shift-negative-value
> > -Wno-address-of-packed-member -Wno-format-zero-length     -std=iso9899:1999
> > -c /usr/src/sys/crypto/openssl/ossl.c -o ossl.o
> > > > /usr/src/sys/crypto/openssl/ossl.c:190:4: error: implicit declaration
> > of function 'fpu_kern_enter' is invalid in C99
> > [-Werror,-Wimplicit-function-declaration]
> > > >                          fpu_kern_enter(curthread, NULL,
> > FPU_KERN_NOCTX);
> > > >                          ^
> > > > /usr/src/sys/crypto/openssl/ossl.c:190:36: error: use of undeclared
> > identifier 'FPU_KERN_NOCTX'
> > > >                          fpu_kern_enter(curthread, NULL,
> > FPU_KERN_NOCTX);
> > > >                                                          ^
> > > > /usr/src/sys/crypto/openssl/ossl.c:193:4: error: implicit declaration
> > of function 'fpu_kern_leave' is invalid in C99
> > [-Werror,-Wimplicit-function-declaration]
> > > >                          fpu_kern_leave(curthread, NULL);
> > > >                          ^
> > > > /usr/src/sys/crypto/openssl/ossl.c:193:4: note: did you mean
> > 'fpu_kern_enter'?
> > > > /usr/src/sys/crypto/openssl/ossl.c:190:4: note: 'fpu_kern_enter'
> > declared here
> > > >                          fpu_kern_enter(curthread, NULL,
> > FPU_KERN_NOCTX);
> > > >                          ^
> > > > /usr/src/sys/crypto/openssl/ossl.c:214:6: error: implicit declaration
> > of function 'is_fpu_kern_thread' is invalid in C99
> > [-Werror,-Wimplicit-function-declaration]
> > > >          if (is_fpu_kern_thread(0)) {
> > > >              ^
> > > > /usr/src/sys/crypto/openssl/ossl.c:217:3: error: implicit declaration
> > of function 'fpu_kern_enter' is invalid in C99
> > [-Werror,-Wimplicit-function-declaration]
> > > >                  fpu_kern_enter(curthread, NULL, FPU_KERN_NOCTX);
> > > >                  ^
> > > > /usr/src/sys/crypto/openssl/ossl.c:217:35: error: use of undeclared
> > identifier 'FPU_KERN_NOCTX'
> > > >                  fpu_kern_enter(curthread, NULL, FPU_KERN_NOCTX);
> > > >                                                  ^
> > > > /usr/src/sys/crypto/openssl/ossl.c:263:3: error: implicit declaration
> > of function 'fpu_kern_leave' is invalid in C99
> > [-Werror,-Wimplicit-function-declaration]
> > > >                  fpu_kern_leave(curthread, NULL);
> > > >                  ^
> > > > 7 errors generated.
> > > >
> > > > AFAIK porting those is not easy and it's certainly above my pay grade.
> > >
> > > Do the powerpc instructions use additional registers that are not part
> > of the normal
> > > GPRs saved/restored on a context switch?  If so, then, yes, you will
> > need to implement
> > > something here (though the FPU_KERN_NOCTX variant is a bit simpler as it
> > disables
> > > context switches and it's sufficient just to init the registers and
> > permit accesses
> > > without raising a trap until fpu_kern_leave).  If not, then you can make
> > the use of
> > > those functions conditional on architecture.
> >
> > I guess bdragon, jhibbits, luporl or adalava would be better source of
> > information.
> >
> > That said, after looking at
> > https://community.nxp.com/t5/MPC5xxx/Example-code-for-context-switch-for-MPC56XX-PowerPC/m-p/732248/highlight/true,
> > it seems like indeed registers other than GPRs are saved and restored.
> >
> >
> > I tried looking at
> > https://cgit.freebsd.org/src/diff/?id=6ed982a221e024d64dd0ab776e6197643fa1530e.
> > fpu_kern_enter() and fpu_kern_leave() are quite scary :)
> >
> >
> AFAIK, non-integer registers are also saved/restored on a context switch on
> PowerPC, but only if there is an underlying user process that has used any
> FPU/Altivec/VSX instruction.
> I guess it will need some adjustments if the kernel itself starts using
> those registers.
Context switches are not the issue there.  Problem is that you need to save
e.g. user content of the FPU registers file if kernel is going to use it,
while user content is not yet spilled into the save area.  This is why the
calls to fpu_kern_enter()/leave() are required around regions in kernel
that would use FPU.

Of course, this save/restore operations need to coordinate with context
switches, which is why John pointed out FPU_KERN_NOCTX as simplest
variant, because it disables preemption around the FPU region.

AFAIR FPU on PPC is somewhat a mess, more like x86. There is FPU
registers file (FPR). Then there was altivec (older vector extension),
and now there is VSX, each with additional registers over the basic FPU,
and VSX is an extension over altivec.

Then there seems to be something completely incompatible on BookE, which
is the reason for our powerpcspe.