Xen-Devel Archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86/asm: ELF metadata for simple cases
@ 2023-02-20 11:04 Andrew Cooper
  2023-02-20 11:51 ` Ross Lagerwall
  2023-02-21 11:05 ` Jan Beulich
  0 siblings, 2 replies; 5+ messages in thread
From: Andrew Cooper @ 2023-02-20 11:04 UTC (permalink / raw
  To: Xen-devel
  Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu,
	Konrad Rzeszutek Wilk, Ross Lagerwall

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(+)

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
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] x86/asm: ELF metadata for simple cases
  2023-02-20 11:04 [PATCH] x86/asm: ELF metadata for simple cases Andrew Cooper
@ 2023-02-20 11:51 ` Ross Lagerwall
  2023-02-21 10:56   ` Jan Beulich
  2023-02-24 16:34   ` Andrew Cooper
  2023-02-21 11:05 ` Jan Beulich
  1 sibling, 2 replies; 5+ messages in thread
From: Ross Lagerwall @ 2023-02-20 11:51 UTC (permalink / raw
  To: Andrew Cooper, Xen-devel
  Cc: Jan Beulich, Roger Pau Monne, Wei Liu, Konrad Rzeszutek Wilk

> 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] x86/asm: ELF metadata for simple cases
  2023-02-20 11:51 ` Ross Lagerwall
@ 2023-02-21 10:56   ` Jan Beulich
  2023-02-24 16:34   ` Andrew Cooper
  1 sibling, 0 replies; 5+ messages in thread
From: Jan Beulich @ 2023-02-21 10:56 UTC (permalink / raw
  To: Ross Lagerwall
  Cc: Roger Pau Monne, Wei Liu, Konrad Rzeszutek Wilk, Andrew Cooper,
	Xen-devel

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] x86/asm: ELF metadata for simple cases
  2023-02-20 11:04 [PATCH] x86/asm: ELF metadata for simple cases Andrew Cooper
  2023-02-20 11:51 ` Ross Lagerwall
@ 2023-02-21 11:05 ` Jan Beulich
  1 sibling, 0 replies; 5+ messages in thread
From: Jan Beulich @ 2023-02-21 11:05 UTC (permalink / raw
  To: Andrew Cooper
  Cc: Roger Pau Monné, Wei Liu, Konrad Rzeszutek Wilk,
	Ross Lagerwall, Xen-devel

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>




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] x86/asm: ELF metadata for simple cases
  2023-02-20 11:51 ` Ross Lagerwall
  2023-02-21 10:56   ` Jan Beulich
@ 2023-02-24 16:34   ` Andrew Cooper
  1 sibling, 0 replies; 5+ messages in thread
From: Andrew Cooper @ 2023-02-24 16:34 UTC (permalink / raw
  To: Ross Lagerwall, Xen-devel
  Cc: Jan Beulich, Roger Pau Monne, Wei Liu, Konrad Rzeszutek Wilk

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-02-24 16:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-20 11:04 [PATCH] x86/asm: ELF metadata for simple cases Andrew Cooper
2023-02-20 11:51 ` Ross Lagerwall
2023-02-21 10:56   ` Jan Beulich
2023-02-24 16:34   ` Andrew Cooper
2023-02-21 11:05 ` Jan Beulich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).