Messages in this thread | | | Subject | Re: WARNING: can't access registers at asm_common_interrupt | From | Andrew Cooper <> | Date | Wed, 11 Nov 2020 20:25:36 +0000 |
| |
On 11/11/2020 20:15, Josh Poimboeuf wrote: > On Wed, Nov 11, 2020 at 09:07:30PM +0100, Peter Zijlstra wrote: >> On Wed, Nov 11, 2020 at 01:59:00PM -0600, Josh Poimboeuf wrote: >>> On Wed, Nov 11, 2020 at 08:42:06PM +0100, Peter Zijlstra wrote: >>>>> Would objtool have an easier time coping if this were implemented in >>>>> terms of a static call? >>>> I doubt it, the big problem is that there is no visibility into the >>>> actual alternative text. Runtime patching fragments into static call >>>> would have the exact same problem. >>>> >>>> Something that _might_ maybe work is trying to morph the immediate >>>> fragments into an alternative. That is, instead of this: >>>> >>>> static inline notrace unsigned long arch_local_save_flags(void) >>>> { >>>> return PVOP_CALLEE0(unsigned long, irq.save_fl); >>>> } >>>> >>>> Write it something like: >>>> >>>> static inline notrace unsigned long arch_local_save_flags(void) >>>> { >>>> PVOP_CALL_ARGS; >>>> PVOP_TEST_NULL(irq.save_fl); >>>> asm_inline volatile(ALTERNATIVE(paravirt_alt(PARAVIRT_CALL), >>>> "PUSHF; POP _ASM_AX", >>>> X86_FEATURE_NATIVE) >>>> : CLBR_RET_REG, ASM_CALL_CONSTRAINT >>>> : paravirt_type(irq.save_fl.func), >>>> paravirt_clobber(PVOP_CALLEE_CLOBBERS) >>>> : "memory", "cc"); >>>> return __eax; >>>> } >>>> >>>> And then we have to teach objtool how to deal with conflicting >>>> alternatives... >>>> >>>> That would remove most (all, if we can figure out a form that deals with >>>> the spinlock fragments) of paravirt_patch.c >>>> >>>> Hmm? >>> I was going to suggest something similar. Though I would try to take it >>> further and replace paravirt_patch_default() with static calls. >> Possible, we just need to be _really_ careful to not allow changing >> those static_call()s. So maybe we need DEFINE_STATIC_CALL_RO() which >> does a __ro_after_init on the whole thing. > But what if you want to live migrate to another hypervisor ;-)
The same as what happens currently. The user gets to keep all the resulting pieces ;)
>>> Either way it doesn't make objtool's job much easier. But it would be >>> nice to consolidate runtime patching mechanisms and get rid of >>> .parainstructions. >> I think the above (combining alternative and paravirt/static_call) does >> make objtool's job easier, since then we at least have the actual >> alternative instructions available to inspect, or am I mis-understanding >> things? > Right, it makes objtool's job a _little_ easier, since it already knows > how to read alternatives. But it still has to learn to deal with the > conflicting stack layouts.
I suppose the needed abstraction is "these blocks will start and end with the same stack layout", while allowing the internals to diverge.
~Andrew
| |