[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] x86/livepatch: enable livepatching assembly source files


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Wed, 19 Apr 2023 10:25:49 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=/tErtKDgMNZIdbfUMqXfkVAwKLd2PbJoAetA2bfFtK0=; b=AqAqhygo49TXUDCJDNGTzrTVF4WGCMc91ZEJe0ctKr7GWFWtb2Ha2NBHUEgIynFEV9Xhai4BwOV7atnc27ollQEph+sQywep0H8fonGKPJ2FQjYlBcQSkHq/kpty3b2OPSMVryMSm1/RMU24jTcxf5zCDiUKnPOfyNd9Leas5vbGh1V+15uD9rlLQ8XfW7UvNQB8SyXkeBM9usujxcxKPfc8CqUDrcoZ78jOEkbOGq/iCcVwqnl59FNKoAhJPh0vzUmLpoUQ+cEhzQ8NP9EmjCFjepMiDz7DP+9V6yPUrmp6NwT9QMzEfrRQh1jndnR2PQe1Nguo08f0By7kw0XpQg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Z+67UWvcAtHAbLiECx9RLYAB0iclVsj95rbTGbBJgZLIczzh+G5eIxJQb9ejAWfrz/q6bsW6c6SzdGdT6Hy5WFRu7jJ8zX9COOGVOjzu6XhWQrBsLpnZBJOIAcHewu4ha1wyT0JdvaY/46OMxxpAcYFT4IwCyH/D37kwPNHfHVXAoAQ5U40sluFXLei10rTIJNi4roeBd+qO9qcVXhI76KYM7m/WGMN8CwYXokTEveQwTB7onxXxcE2VDoUuazLPRM7pxOn3P7T87oq8ItvaR1hw77iZ9I9cq1gN3Pr0TiQSZBHROYTwfGcok55PgqFeZAiPtwKnkR3byNCvTs9Uww==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Wed, 19 Apr 2023 08:26:21 +0000
  • Ironport-data: A9a23:cFZNfquijxpDGfsogctdfIY73+fnVHFfMUV32f8akzHdYApBsoF/q tZmKTiEOayMYmf9fd8kbITkoRkDvMWDyoc3GwZlqysxFn9H+JbJXdiXEBz9bniYRiHhoOCLz O1FM4Wdc5pkJpP4jk3wWlQ0hSAkjclkfpKlVKiffHg3HVQ+IMsYoUoLs/YjhYJ1isSODQqIu Nfjy+XSI1bg0DNvWo4uw/vrRChH4bKj6Vv0gnRkPaoQ5AOHxyFOZH4iDfrZw0XQE9E88tGSH 44v/JnhlkvF8hEkDM+Sk7qTWiXmlZaLYGBiIlIPM0STqkAqSh4ai87XB9JFAatjsB2bnsgZ9 Tl4ncfYpTHFnEH7sL91vxFwS0mSNEDdkVPNCSDXXce7lyUqf5ZwqhnH4Y5f0YAwo45K7W9yG fMwDg4xckmSjuCN7ujgR+Vzq/slE9HsI9ZK0p1g5Wmx4fcOZ7nmG/+PyfoDmTA6i4ZJAOrUY NcfZXx3dhPcbhZTO1ARTpUjgOOvgXq5eDpdwL6XjfNvvy6Pk0osjv6xarI5efTTLSlRtlyfq W/cuXzwHzkRNcCFyCrD+XWp7gPKtXqjBd5LTuXprZaGhnWeyi8sBy00FmCLrOGyrFSecYJGN lI9r39GQa8asRbDosPGdx+yrWOAvxUcc8FNCOB84waIooLL5y6JC25CSSROAPQ2uclzSTE02 1uhm9LyGScpoLCTUWia9LqfsXW1Iyd9EIMZTSoNTA9A79y4pog21kjLVow7TPTzicDpEzbtx TzMtDI5m7gYkc8M0eO84EzDhDWv4JPOS2bZ+znqY45s1SshDKbNWmBiwQGzASpoRGpBcmS8g Q==
  • Ironport-hdrordr: A9a23:UvNz6a3kiOacGZEdu4R8bgqjBUtyeYIsimQD101hICG9Lfb0qy n+pp4mPEHP4wr5AEtQ4OxoS5PwOU80lKQFlbX5WI3PYOCIghrOEGgP1+rfKl7balrDH4xmpM FdmsFFYbWeY2STSK3BkW2F+r0bsbq6GdWT9ILjJgBWPGNXgs9bjztRO0K+KAlbVQNGDZ02GN 63/cxcvQetfnwRc4CSGmQFd/KrnayDqLvWJTo9QzI34giHij2lrJTgFQKD4xsYWzRThZ8/7G n+lRDj7KnLiYDy9vac7R6Z031loqqt9jJxPr3BtiEhEESntu/nXvUvZ1TIhkFPnAjm0idRrD CLmWZXAy1f0QKgQoiOm2qf5yDwlDI1r3Pyw16RhnXu5cT/WTIhEsJEwYZUaAHQ5UYstMx1lP sj5RPti7NHSRfb2Cjt7dnBUB9n0kKyvHo5iOYWy3hSS5EXZrNdpZEWuElVDJADFiTn751PKp gdMOjMoPJNNV+KZXHQuWdihNalW3g1Ex+cBlIPocyYyXxXm2plx0wTyIgekx47he4AorV/lp X52/5T5cxzp+ctHNxA7Mdoe7rJNlDw
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Wed, Apr 19, 2023 at 08:17:45AM +0200, Jan Beulich wrote:
> On 18.04.2023 15:06, Roger Pau Monné wrote:
> > On Tue, Apr 18, 2023 at 01:00:53PM +0200, Jan Beulich wrote:
> >> On 18.04.2023 11:24, Roger Pau Monne wrote:
> >>> --- a/xen/arch/x86/include/asm/config.h
> >>> +++ b/xen/arch/x86/include/asm/config.h
> >>> @@ -44,6 +44,20 @@
> >>>  /* Linkage for x86 */
> >>>  #ifdef __ASSEMBLY__
> >>>  #define ALIGN .align 16,0x90
> >>> +#ifdef CONFIG_LIVEPATCH
> >>> +#define START_LP(name)                          \
> >>> +  jmp name;                                     \
> >>> +  .pushsection .text.name, "ax", @progbits;     \
> >>> +  name:
> >>> +#define END_LP(name)                            \
> >>> +  .size name, . - name;                         \
> >>> +  .type name, @function;                        \
> >>> +  .popsection
> >>> +#else
> >>> +#define START_LP(name)                          \
> >>> +  name:
> >>> +#define END_LP(name)
> >>> +#endif
> >>>  #define ENTRY(name)                             \
> >>>    .globl name;                                  \
> >>>    ALIGN;                                        \
> >>
> >> Couldn't END_LP() set type and size unconditionally? (But see also
> >> below.)
> > 
> > I see, so that we could also use it for debug purposes.  I guess at
> > that point it might be better to use {START,END}_FUNC() to note that
> > the macros also have an effect beyond that of livepatching.
> > 
> > Maybe also introduce a START_ENTRY() that replaces ENTRY()?  Albeit I
> > find START_ENTRY a weird name.
> 
> So do I. {START,END}_FUNC() or whatever else are in principle fine, but
> I take it that you're aware that we meanwhile have two or even three
> concurring proposals on a general scheme of such annotations, and we
> don't seem to be able to agree on one. (I guess I'll make a design
> session proposal on this topic for Prague.)

Oh, I wasn't aware we had other proposals, I've been away on an off
quite a lot recently, and haven't been able to keep up with all
xen-devel email.  Do you have any references at hand?

> One thing needs to be clear though: Macros doing things solely needed
> for LP need to not have extra effects with it disabled, and such
> macros also better wouldn't e.g. insert stray JMP when not really
> needed. Hence I expect we still want (some) LP-specific macros besides
> whatever we settle on as the generic ones.

The stray jmp can be inserted only in the livepatch case, if we end up
having to add it.

Maybe we should just go with Linux names, so initially I would like to
use:

SYM_FUNC_START{_NOALIGN}(name)
SYM_FUNC_START_LOCAL{_NOALIGN}(name)
SYM_FUNC_END(name)

> >>> --- a/xen/arch/x86/x86_64/entry.S
> >>> +++ b/xen/arch/x86/x86_64/entry.S
> >>> @@ -660,7 +660,7 @@ ENTRY(early_page_fault)
> >>>  
> >>>          ALIGN
> >>>  /* No special register assumptions. */
> >>> -restore_all_xen:
> >>> +START_LP(restore_all_xen)
> >>>          /*
> >>>           * Check whether we need to switch to the per-CPU page tables, in
> >>>           * case we return to late PV exit code (from an NMI or #MC).
> >>> @@ -677,6 +677,7 @@ UNLIKELY_END(exit_cr3)
> >>>  
> >>>          RESTORE_ALL adj=8
> >>>          iretq
> >>> +END_LP(restore_all_xen)
> >>
> >> While I'm fine with this conversion, ...
> > 
> > So I take that overall you would agree to adding this extra
> > information using a pair of macros similar to the proposed ones.
> 
> Yes (with the above in mind, though).

Sure, thanks for the feedback.

Roger.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.