On Mon, Jan 22, 2024 at 11:50:08AM +0100, Jan Beulich wrote: > On 19.01.2024 11:36, Roger Pau Monné wrote: > > On Mon, Jan 15, 2024 at 03:40:19PM +0100, Jan Beulich wrote: > >> Leverage the new infrastructure in xen/linkage.h to also switch to per- > >> function sections (when configured), deriving the specific name from the > >> "base" section in use at the time FUNC() is invoked. > >> > >> Signed-off-by: Jan Beulich <jbeul...@suse.com> > >> --- > >> TBD: Since we use .subsection in UNLIKELY_START(), a perhaps not really > >> wanted side effect of this change is that respective out-of-line > >> code now moves much closer to its original (invoking) code. > > > > Hm, I'm afraid I don't have much useful suggestions here. > > > >> TBD: Of course something with the same overall effect, but less > >> impactful might do in Config.mk. E.g. $(filter-out -D%,$(3)) > >> instead of $(firstword (3)). > > > > I don't have a strong opinion re those two options My preference > > however (see below for reasoning) would be to put this detection in > > Kconfig. > > > >> TBD: On top of Roger's respective patch (for livepatch), also respect > >> CONFIG_FUNCTION_ALIGNMENT. > > > > I think you can drop that, as the series is blocked. > > Considering the series here has been pending for quite some time, too, > I guess I'd like to keep it just in case that other functionality > becomes unblocked or available by some other means, even if only to > remind myself.
So as you have seen I've posted a new version of just the function alignment patch, that I expected wasn't controversial. > >> --- a/xen/Makefile > >> +++ b/xen/Makefile > >> @@ -409,6 +409,9 @@ AFLAGS += -D__ASSEMBLY__ > >> > >> $(call cc-option-add,AFLAGS,CC,-Wa$$(comma)--noexecstack) > >> > >> +# Check to see whether the assmbler supports the --sectname-subst option. > >> +$(call cc-option-add,AFLAGS,CC,-Wa$$(comma)--sectname-subst > >> -DHAVE_AS_SECTNAME_SUBST) > > > > I guess you already know what I'm going to comment on. I think this > > would be clearer if it was a Kconfig option. For once because I think > > we should gate livapatch support on the option being available, and > > secondly it would avoid having to pass the extra -D around. > > > > I think it's relevant to have a consistent set of build tool options > > requirements for livepatch support, so that when enabled the support > > is consistent across builds. With this approach livepatch could be > > enabled in Kconfig, but depending on the tools support the resulting > > binary might or might not support live patching of assembly code. > > Such behavior is IMO unhelpful from a user PoV, and can lead to > > surprises down the road. > > I can see the desire to have LIVEPATCH grow such a dependency. Yet there > is the bigger still open topic of the criteria towards what, if anything, > to probe for in Kconfig, what, if anything, to probe for in Makefile, and > which of the probing perhaps do in both places. I'm afraid my attempts to > move us closer to a resolution (topic on summit, making proposals on > list) have utterly failed. IOW I don't currently see how the existing > disagreement there can be resolved, which will result in me to continue > following the (traditional) Makefile approach unless I clearly view > Kconfig superior in a particular case. I could perhaps be talked into > following a "mixed Kconfig/Makefile model", along the lines of "x86: > convert CET tool chain feature checks to mixed Kconfig/Makefile model", > albeit I'm sure you're aware there are issues to sort out there, which I > see no value in putting time into as long as I can't expect things to > make progress subsequently. I think there are more subtle cases where it's indeed arguable that putting it in Kconfig or a Makefile makes no difference from a user experience PoV, hence in the context here I do believe it wants to be in Kconfig as LIVEPATCH can be make dependent on it. > Dropping this patch, while an option, would seem undesirable to me, since > even without Kconfig probing using new enough tool chains the splitting > would yield reliable results, and provide - imo - an improvement over > what we have right now. I could send a followup afterwards to arrange for the check to be in Kconfig, but it would feel a bit odd to me this is not done in the first place. I don't want to block the patch because I think it's useful, so worst case I'm willing to give my Ack and provide an alternative Kconfig based patch afterwards. Thanks, Roger.
