| Message ID | 20230220110439.1518972-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/asm: ELF metadata for simple cases | expand |
> From: Andrew Cooper <andrew.cooper3@citrix.com> > Sent: Monday, February 20, 2023 11:04 AM > To: Xen-devel <xen-devel@lists.xenproject.org> > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Jan Beulich <JBeulich@suse.com>; Roger Pau Monne <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; Ross Lagerwall <ross.lagerwall@citrix.com> > Subject: [PATCH] x86/asm: ELF metadata for simple cases > > This is generally good practice, and necessary for livepatch binary diffing to > work. > > With this, livepatching of the SVM entry path works. The only complication is > with svm_stgi_label which is only used by oprofile to guestimate (not > completely correctly) when an NMI hit guest context. > > Livepatching of VMX is still an open question, because the logic doesn't form > anything remotely resembling functions. Both code fragments jump into each > other so need to be updated in tandem. Also, both code fragment entries need > trampolines in the case that patching actually occurs. For now, just treat it > as a single function. If it is treated as two functions and both functions are always included in the live patch, would that work? > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > CC: Wei Liu <wl@xen.org> > CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > CC: Ross Lagerwall <ross.lagerwall@citrix.com> > --- > xen/arch/x86/clear_page.S | 3 +++ > xen/arch/x86/copy_page.S | 3 +++ > xen/arch/x86/hvm/svm/entry.S | 3 +++ > xen/arch/x86/hvm/vmx/entry.S | 3 +++ > 4 files changed, 12 insertions(+) > > diff --git a/xen/arch/x86/clear_page.S b/xen/arch/x86/clear_page.S > index d9d524c79ecd..5b5622cc526f 100644 > --- a/xen/arch/x86/clear_page.S > +++ b/xen/arch/x86/clear_page.S > @@ -16,3 +16,6 @@ ENTRY(clear_page_sse2) > > sfence > ret > + > + .type clear_page_sse2, @function > + .size clear_page_sse2, . - clear_page_sse2 Would it be worth wrapping this pattern in a macro? Ross
On 20.02.2023 12:51, Ross Lagerwall wrote: >> --- a/xen/arch/x86/clear_page.S >> +++ b/xen/arch/x86/clear_page.S >> @@ -16,3 +16,6 @@ ENTRY(clear_page_sse2) >> >> sfence >> ret >> + >> + .type clear_page_sse2, @function >> + .size clear_page_sse2, . - clear_page_sse2 > > Would it be worth wrapping this pattern in a macro? Funny you should ask this: Yes, certainly, but we can't quite agree what shape that macro (or really set of macros) is to take. Hence we did agree on this minimalistic approach as an intermediate solution. Jan
On 20.02.2023 12:04, Andrew Cooper wrote: > This is generally good practice, and necessary for livepatch binary diffing to > work. > > With this, livepatching of the SVM entry path works. The only complication is > with svm_stgi_label which is only used by oprofile to guestimate (not > completely correctly) when an NMI hit guest context. > > Livepatching of VMX is still an open question, because the logic doesn't form > anything remotely resembling functions. Both code fragments jump into each > other so need to be updated in tandem. Also, both code fragment entries need > trampolines in the case that patching actually occurs. For now, just treat it > as a single function. I agree. > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
On 20/02/2023 11:51 am, Ross Lagerwall wrote: >> From: Andrew Cooper <andrew.cooper3@citrix.com> >> Sent: Monday, February 20, 2023 11:04 AM >> To: Xen-devel <xen-devel@lists.xenproject.org> >> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Jan Beulich <JBeulich@suse.com>; Roger Pau Monne <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; Ross Lagerwall <ross.lagerwall@citrix.com> >> Subject: [PATCH] x86/asm: ELF metadata for simple cases >> >> This is generally good practice, and necessary for livepatch binary diffing to >> work. >> >> With this, livepatching of the SVM entry path works. The only complication is >> with svm_stgi_label which is only used by oprofile to guestimate (not >> completely correctly) when an NMI hit guest context. >> >> Livepatching of VMX is still an open question, because the logic doesn't form >> anything remotely resembling functions. Both code fragments jump into each >> other so need to be updated in tandem. Also, both code fragment entries need >> trampolines in the case that patching actually occurs. For now, just treat it >> as a single function. > If it is treated as two functions and both functions are always included in > the live patch, would that work? I think so, but only because the first jumped-to label in vmx_asm_do_vmentry is beyond the trampoline. But I guess the question is how to tie the two symbols together. We don't want to be hardcoding this in livepatch-build-tools IMO. Perhaps we want a CONFIG_LIVEPATCH build of Xen to include a section/note/something identifying "grouped symbols", meaning "if there's a delta in one, include all even if they haven't changed" ? I'm getting the distinct impression that we're going to need it it for the PV entry/exit paths too. ~Andrew
diff --git a/xen/arch/x86/clear_page.S b/xen/arch/x86/clear_page.S index d9d524c79ecd..5b5622cc526f 100644 --- a/xen/arch/x86/clear_page.S +++ b/xen/arch/x86/clear_page.S @@ -16,3 +16,6 @@ ENTRY(clear_page_sse2) sfence ret + + .type clear_page_sse2, @function + .size clear_page_sse2, . - clear_page_sse2 diff --git a/xen/arch/x86/copy_page.S b/xen/arch/x86/copy_page.S index 2da81126c5fa..ddb6e0ebbb6e 100644 --- a/xen/arch/x86/copy_page.S +++ b/xen/arch/x86/copy_page.S @@ -41,3 +41,6 @@ ENTRY(copy_page_sse2) sfence ret + + .type copy_page_sse2, @function + .size copy_page_sse2, . - copy_page_sse2 diff --git a/xen/arch/x86/hvm/svm/entry.S b/xen/arch/x86/hvm/svm/entry.S index 981cd82e7c0b..9e3dab768027 100644 --- a/xen/arch/x86/hvm/svm/entry.S +++ b/xen/arch/x86/hvm/svm/entry.S @@ -142,3 +142,6 @@ GLOBAL(svm_stgi_label) sti call do_softirq jmp .Lsvm_do_resume + + .type svm_asm_do_resume, @function + .size svm_asm_do_resume, . - svm_asm_do_resume diff --git a/xen/arch/x86/hvm/vmx/entry.S b/xen/arch/x86/hvm/vmx/entry.S index 5f5de45a1309..e3f60d5a82f7 100644 --- a/xen/arch/x86/hvm/vmx/entry.S +++ b/xen/arch/x86/hvm/vmx/entry.S @@ -150,3 +150,6 @@ ENTRY(vmx_asm_do_vmentry) sti call do_softirq jmp .Lvmx_do_vmentry + + .type vmx_asm_vmexit_handler, @function + .size vmx_asm_vmexit_handler, . - vmx_asm_vmexit_handler
This is generally good practice, and necessary for livepatch binary diffing to work. With this, livepatching of the SVM entry path works. The only complication is with svm_stgi_label which is only used by oprofile to guestimate (not completely correctly) when an NMI hit guest context. Livepatching of VMX is still an open question, because the logic doesn't form anything remotely resembling functions. Both code fragments jump into each other so need to be updated in tandem. Also, both code fragment entries need trampolines in the case that patching actually occurs. For now, just treat it as a single function. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Wei Liu <wl@xen.org> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> CC: Ross Lagerwall <ross.lagerwall@citrix.com> --- xen/arch/x86/clear_page.S | 3 +++ xen/arch/x86/copy_page.S | 3 +++ xen/arch/x86/hvm/svm/entry.S | 3 +++ xen/arch/x86/hvm/vmx/entry.S | 3 +++ 4 files changed, 12 insertions(+)