Re: [PATCH v4 1/2] x86/unwind: add ORC unwinder

[Josh Poimboeuf]

On Wed, Aug 09, 2017 at 11:55:35AM +0200, Juergen Gross wrote:
> On 09/08/17 11:35, Peter Zijlstra wrote:
> > On Wed, Aug 09, 2017 at 11:24:07AM +0200, Juergen Gross wrote:
> >> On 09/08/17 11:16, Peter Zijlstra wrote:
> >>> On Wed, Aug 09, 2017 at 10:49:43AM +0200, Juergen Gross wrote:
> >>>>> ALTERNATIVE "pushfq; popq %rax", "callq *pv_irq_ops.save_fl",
> >>>>> X86_FEATURE_GODDAMN_PV_IRQ_OPS
> >>>>
> >>>> You are aware that at least some of the Xen irq pvops functionality is
> >>>> patched inline? Your modification would slow down pv guests quite a
> >>>> bit, I guess.
> >>>
> >>> Where does that live? I know of the inline patching for native, but
> >>> didn't know the guests did any of that too.
> >>
> >> See arch/x86/xen/enlighten_pv.c xen_patch().
> > 
> > 'obvious' name that :-) I see that the actual code that's patched in
> > lives in xen-asm.S which unlike the native case doesn't appear to have
> > its own section. So that might make things even more difficult.
> 
> I don't see why this couldn't be changed.

I'm wondering why xen_patch() even exists.  The main difference between
xen_patch() and native_patch() seems to be that xen_patch() does some
relocs when doing an inline patch after calling paravirt_patch_insns().

But I can't see how that code path would ever run, because the
replacement functions are all larger than the size of the call
instruction to be replaced (7 bytes).  So they would never fit, and
instead the paravirt_patch_default() case would always run.  Or am I
missing something?

If we could get rid of the hypervisor-specific patching functions
(pv_init_ops) -- including possibly removing the lguest and vsmp code,
if nobody cares about them anymore -- that might make it easier to
consolidate all the patching things into a single place.

For example, maybe the paravirt patching could just use alternatives
under the hood somehow (insert hand waving).

-- 
Josh