On 7/1/22 5:18 PM, C. Erastus Toe wrote: > On Wed, Jun 29, 2022 at 11:16 AM Joe Lawrence <joe.lawrence@xxxxxxxxxx > <mailto:joe.lawrence@xxxxxxxxxx>> wrote: > > On 6/27/22 8:50 AM, Vasily Gorbik wrote: > > Hi Joe, > > > > sorry for late reply. > > > >> I couldn't find the upstream patch post for 1d2ad084800e > ("s390/nospec: > >> add an option to use thunk-extern"), so replying off-list here. Feel > >> free to cc the appropriate list. > >> > >> Regarding this change, as I understand it, when > CONFIG_EXPOLINE_EXTERN=y > >> out-of-tree kernel modules will need to link against > >> arch/s390x/lib/expoline.o, right? > >> > >> And if so, shouldn't the top level 'prepare_modules' target create > >> expoline.o for this purpose? > > > > Thanks for bringing this up. I definitely missed out-of-tree > kernel modules > > build case without a prebuilt kernel. On the other hand this > post-linking > > trick is a rip off from powerpc: > > > > KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o > > > > So, now I wonder why powerpc doesn't have crtsavres.o in > 'prepare_modules'. > > > > Anyhow, below is couple of patches to consider. The first one is > > meant to be backportable, as the second one requires 4efd417f298b. > > > > I had to move expoline.S to a separate directory to be able to > call into > > its Makefile for 'prepare_modules' and avoid warnings for other > targets > > defined in the same Makefile. Not sure if there are better kbuild > tricks > > I could use. Another option I thought about is to keep expoline.S > where > > it is and add a condition into that Makefile: > > expoline_prepare: prepare0 > > $(Q)$(MAKE) $(build)=arch/s390/lib expoline_prepare=1 > arch/s390/lib/expoline.o > > > > arch/s390/lib/Makefile: > > # first target defined > > obj-$(CONFIG_EXPOLINE_EXTERN) += expoline.o > > ifndef expoline_prepare > > # ...other targets... > > > > Vasily Gorbik (2): > > s390/nospec: build expoline.o for modules_prepare target > > s390/nospec: remove unneeded header includes > > > > arch/s390/Makefile | 8 +++++++- > > arch/s390/include/asm/nospec-insn.h | 2 -- > > arch/s390/lib/Makefile | 3 ++- > > arch/s390/lib/expoline/Makefile | 3 +++ > > arch/s390/lib/{ => expoline}/expoline.S | 0 > > 5 files changed, 12 insertions(+), 4 deletions(-) > > create mode 100644 arch/s390/lib/expoline/Makefile > > rename arch/s390/lib/{ => expoline}/expoline.S (100%) > > > > Thanks, Vasily. We'll test these with OOT and the original gitlab > pipeline where we spotted potential issue with packaging and report > back. > > Hi, > > Successfully tested the first patch in a rhel-9 backport. (had to skip > the second as it has dependencies on other patches like [1] that > deprecated symbols like __LC_BR_R1. Without those, the build resulted in > a flood of: depmod: WARNING: <module>.ko needs unknown symbol __LC_BR_R1.) > > For ("s390/nospec: build expoline.o for modules_prepare target"), > Tested-by: C. Erastus Toe <ctoe@xxxxxxxxxx <mailto:ctoe@xxxxxxxxxx>> > > [1] 4efd417f298b ("s390: raise minimum supported machine generation to z10") > And then for the entire series (tested on top of v5.19-rc4), Tested-by: Joe Lawrence <joe.lawrence@xxxxxxxxxx> -- Joe