[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v6 3/3] xen/livepatch: align functions to ensure minimal distance between entry points
On 27.02.2024 09:15, Roger Pau Monné wrote: > On Mon, Feb 26, 2024 at 01:36:32PM +0100, Jan Beulich wrote: >> On 26.02.2024 12:32, Roger Pau Monné wrote: >>> On Tue, Feb 13, 2024 at 04:58:38PM +0100, Jan Beulich wrote: >>>> On 07.02.2024 15:55, Roger Pau Monne wrote: >>>>> The minimal function size requirements for an x86 livepatch are either 5 >>>>> bytes >>>>> (for jmp) or 9 bytes (for endbr + jmp), and always 4 bytes on Arm. >>>>> Ensure that >>>>> distance between functions entry points is always at least of the minimal >>>>> required size for livepatch instruction replacement to be successful. >>>>> >>>>> Add an additional align directive to the linker scripts, in order to >>>>> ensure that >>>>> the next section placed after the .text.* (per-function sections) is also >>>>> aligned to the required boundary, so that the distance of the last >>>>> function >>>>> entry point with the next symbol is also of minimal size. >>>> >>>> Perhaps "... minimal required size"? >>> >>> Yes. >>> >>>>> --- a/xen/common/Kconfig >>>>> +++ b/xen/common/Kconfig >>>>> @@ -395,8 +395,11 @@ config CRYPTO >>>>> config LIVEPATCH >>>>> bool "Live patching support" >>>>> default X86 >>>>> - depends on "$(XEN_HAS_BUILD_ID)" = "y" >>>>> + depends on "$(XEN_HAS_BUILD_ID)" = "y" && CC_HAS_FUNCTION_ALIGNMENT >>>>> select CC_SPLIT_SECTIONS >>>>> + select FUNCTION_ALIGNMENT_16B if XEN_IBT >>>>> + select FUNCTION_ALIGNMENT_8B if X86 >>>>> + select FUNCTION_ALIGNMENT_4B if ARM >>>> >>>> This isn't strictly needed, is it? Would be nice to avoid re-selection >>>> of what the default for an arch is anyway, as otherwise this will start >>>> looking clumsy when a couple more architectures are added. >>> >>> My worry was that the default per-arch could change, ie: for example >>> x86 moving from 16 to 8 and then it would hamper livepatch support if >>> IBT is also enabled. I however think it's very unlikely to reduce the >>> default alignment, and in any case we would hit a build time assert if >>> that ever happens. >>> >>> So yes, I'm fine with dropping those. >> >> Oh, no - not "those", only "that", i.e. only the last (Arm) one. > > Oh, I see what you mean, even x86 selects the default one when IBT is > enabled, and when not the requirement for livepatch is < than the > default anyway. That's why I said that we could even drop all of them > and just rely on the build time assert to catch any changes here. Just to clarify: The default I mean is the architecture imposed one. Leaving aside Thumb mode, Arm instructions are all 32-bit words, and hence less than 4-byte alignment makes no sense (and may even be disallowed by the architecture). Whereas for x86 what you're talking about is just a compiler default, which isn't really guaranteed to never be lower (with -Os for example I'd expect it to be perhaps as low as 1). Jan > Feel free to drop the ARM one. > > Thanks, Roger.
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |