Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: "Wieczorkiewicz, Pawel" <wipawel@amazon.de>
To: Julien Grall <julien.grall@arm.com>
Cc: "Tim Deegan" <tim@xen.org>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Wei Liu" <wl@xen.org>,
	"Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>,
	"George Dunlap" <George.Dunlap@eu.citrix.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Ross Lagerwall" <ross.lagerwall@citrix.com>,
	"Ian Jackson" <ian.jackson@eu.citrix.com>,
	xen-devel <xen-devel@lists.xen.org>,
	"Pohlack, Martin" <mpohlack@amazon.de>,
	"Wieczorkiewicz, Pawel" <wipawel@amazon.de>,
	"Jan Beulich" <jbeulich@suse.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH 09/14] livepatch: Add per-function applied/reverted state tracking marker
Date: Thu, 22 Aug 2019 11:02:18 +0000
Message-ID: <DB27F58E-DA27-4A5D-BE23-4CDB5D9736A2@amazon.com> (raw)
In-Reply-To: <d5aca603-3b30-e3ea-1256-51de8ca40eaf@arm.com>

[-- Attachment #1.1: Type: text/plain, Size: 10764 bytes --]



On 22. Aug 2019, at 12:29, Julien Grall <julien.grall@arm.com<mailto:julien.grall@arm.com>> wrote:

Hi Pawel,

Hi Julien,

Thank for looking at this!


On 21/08/2019 09:19, Pawel Wieczorkiewicz wrote:
Livepatch only tracks an entire payload applied/reverted state. But,
with an option to supply the apply_payload() and/or revert_payload()
functions as optional hooks, it becomes possible to intermix the
execution of the original apply_payload()/revert_payload() functions
with their dynamically supplied counterparts.
It is important then to track the current state of every function
being patched and prevent situations of unintentional double-apply
or unapplied revert.
To support that, it is necessary to extend public interface of the
livepatch. The struct livepatch_func gets additional field holding
the applied/reverted state marker.
To reflect the livepatch payload ABI change, bump the version flag
LIVEPATCH_PAYLOAD_VERSION up to 2.
The above solution only applies to x86 architecture for now.
Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de<mailto:wipawel@amazon.de>>
Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com<mailto:andraprs@amazon.com>>
Reviewed-by: Bjoern Doebel <doebel@amazon.de<mailto:doebel@amazon.de>>
Reviewed-by: Martin Pohlack <mpohlack@amazon.de<mailto:mpohlack@amazon.de>>
---
 xen/arch/x86/livepatch.c    | 20 +++++++++++++++++++-
 xen/common/livepatch.c      | 35 +++++++++++++++++++++++++++++++++++
 xen/include/public/sysctl.h | 11 ++++++++++-
 xen/include/xen/livepatch.h |  2 +-
 4 files changed, 65 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
index 436ee40fe1..76fa91a082 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -61,6 +61,14 @@ void noinline arch_livepatch_apply(struct livepatch_func *func)
     if ( !len )
         return;
 +    /* If the apply action has been already executed on this function, do nothing... */
+    if ( func->applied == LIVEPATCH_FUNC_APPLIED )
+    {
+        printk(XENLOG_WARNING LIVEPATCH "%s: %s has been already applied before\n",
+                __func__, func->name);
+        return;
+    }

Does this need to in arch specific code?

As a matter of fact it does not.
Let me enable this feature for Arm as well and make this code a common inline.


+
     memcpy(func->opaque, old_ptr, len);
     if ( func->new_addr )
     {
@@ -77,15 +85,25 @@ void noinline arch_livepatch_apply(struct livepatch_func *func)
         add_nops(insn, len);
       memcpy(old_ptr, insn, len);
+    func->applied = LIVEPATCH_FUNC_APPLIED;
 }
   /*
  * "noinline" to cause control flow change and thus invalidate I$ and
  * cause refetch after modification.
  */
-void noinline arch_livepatch_revert(const struct livepatch_func *func)
+void noinline arch_livepatch_revert(struct livepatch_func *func)
 {
+    /* If the apply action hasn't been executed on this function, do nothing... */
+    if ( !func->old_addr || func->applied == LIVEPATCH_FUNC_NOT_APPLIED )
+    {
+        printk(XENLOG_WARNING LIVEPATCH "%s: %s has not been applied before\n",
+                __func__, func->name);
+        return;
+    }
+
     memcpy(func->old_addr, func->opaque, livepatch_insn_len(func));
+    func->applied = LIVEPATCH_FUNC_NOT_APPLIED;
 }
   /*
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 585ec9819a..090a48977b 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -1242,6 +1242,29 @@ static inline void revert_payload_tail(struct payload *data)
     data->state = LIVEPATCH_STATE_CHECKED;
 }
 +/*
+ * Check if an action has applied the same state to all payload's functions consistently.
+ */
+static inline bool was_action_consistent(const struct payload *data, livepatch_func_state_t expected_state)
+{
+    int i;
+
+    for ( i = 0; i < data->nfuncs; i++ )
+    {
+        struct livepatch_func *f = &(data->funcs[i]);
+
+        if ( f->applied != expected_state )

Per the definition of livepath_func, the field "applied" only exists for x86. So this will not compiled on Arm.

I will enable Arm too.


+        {
+            printk(XENLOG_ERR LIVEPATCH "%s: Payload has a function: '%s' with inconsistent applied state.\n",
+                   data->name, f->name ?: "noname");
+
+            return false;
+        }
+    }
+
+    return true;
+}
+
 /*
  * This function is executed having all other CPUs with no deep stack (we may
  * have cpu_idle on it) and IRQs disabled.
@@ -1268,6 +1291,9 @@ static void livepatch_do_action(void)
         else
             rc = apply_payload(data);
 +        if ( !was_action_consistent(data, rc ? LIVEPATCH_FUNC_NOT_APPLIED : LIVEPATCH_FUNC_APPLIED) )

Regardless the compilation issue above, none of the common code will set the state. So for Arm, this would likely mean the function return false.

True, I should have disabled this code for Arm too.
But now, let me just make it common for all archs.


+            panic("livepatch: partially applied payload '%s'!\n", data->name);

I can see at least a case where the panic can be reached here. Per the changes in arch_livepatch_apply(), f->applied will only be set to LIVEPATCH_FUNC_APPLIED if livepatch_insn_len() is not 0.

I don't know whether livepatch_insn_len() can ever return 0 as my livepatch knowledge is limited. But the code live little doubt this is a theoritical possibility that after this patch will turn into crashing Xen.


I suppose the livepatch_insn_len() returning 0 (i.e. func->new_size being 0, which is valid AFAICS) is the to-be-implemented feature of noping out code:
From the livepatch doc:
"""
## TODO Goals

 * NOP out the code sequence if `new_size` is zero.
"""
So, in theory it can be 0 (it has to be introduced to the livepatch.funcs). Thus, you’re right.

I will fix that, but marking such function (new_size==0) as applied.

Thanks!

More generally, I am not very comfortable to see panic() in the middle of the code. Could you explain why panic is the best solution over reverting the work?


Yes. Production-ready hotpatches must not contain inconsistent hooks or functions-to-be-applied/reverted.
The goal here is to detect such hotpatches and fail hard immediately highlighting the fact that such hotpatch
is broken.

The inconsistent application of a hotpatch (some function applied, some reverted while other left behined) leaves
the system in a very bad state. I think the best what we could do here is panic() to enable post-mortem analysis
of what went wrong and avoid leaking such system into production.

Mis-detecting such conditions and leaving such inconsistent system running is a very bad idea.

My question applies for all the other panic() below.

+
         if ( rc == 0 )
             apply_payload_tail(data);
         break;
@@ -1282,6 +1308,9 @@ static void livepatch_do_action(void)
         else
             rc = revert_payload(data);
 +        if ( !was_action_consistent(data, rc ? LIVEPATCH_FUNC_APPLIED : LIVEPATCH_FUNC_NOT_APPLIED) )
+            panic("livepatch: partially reverted payload '%s'!\n", data->name);
+
         if ( rc == 0 )
             revert_payload_tail(data);
         break;
@@ -1304,6 +1333,9 @@ static void livepatch_do_action(void)
                 other->rc = revert_payload(other);
   +            if ( !was_action_consistent(other, rc ? LIVEPATCH_FUNC_APPLIED : LIVEPATCH_FUNC_NOT_APPLIED) )
+                panic("livepatch: partially reverted payload '%s'!\n", other->name);
+
             if ( other->rc == 0 )
                 revert_payload_tail(other);
             else
@@ -1324,6 +1356,9 @@ static void livepatch_do_action(void)
             else
                 rc = apply_payload(data);
 +            if ( !was_action_consistent(data, rc ? LIVEPATCH_FUNC_NOT_APPLIED : LIVEPATCH_FUNC_APPLIED) )
+                panic("livepatch: partially applied payload '%s'!\n", data->name);
+
             if ( rc == 0 )
                 apply_payload_tail(data);
         }
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 1b2b165a6d..b55ad6d050 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -818,7 +818,7 @@ struct xen_sysctl_cpu_featureset {
  *     If zero exit with success.
  */
 -#define LIVEPATCH_PAYLOAD_VERSION 1
+#define LIVEPATCH_PAYLOAD_VERSION 2
 /*
  * .livepatch.funcs structure layout defined in the `Payload format`
  * section in the Live Patch design document.
@@ -826,6 +826,11 @@ struct xen_sysctl_cpu_featureset {
  * We guard this with __XEN__ as toolstacks SHOULD not use it.
  */
 #ifdef __XEN__
+typedef enum livepatch_func_state {
+    LIVEPATCH_FUNC_NOT_APPLIED = 0,
+    LIVEPATCH_FUNC_APPLIED = 1

AFAIK, enum will always start counting from 0, so is it really necessary to specify the exact values?

It’s probably my (or some earlier reviewer's) OCD. I am happy to remove these.


+} livepatch_func_state_t;
+
 struct livepatch_func {
     const char *name;       /* Name of function to be patched. */
     void *new_addr;
@@ -834,6 +839,10 @@ struct livepatch_func {
     uint32_t old_size;
     uint8_t version;        /* MUST be LIVEPATCH_PAYLOAD_VERSION. */
     uint8_t opaque[31];
+#if defined CONFIG_X86
+    uint8_t applied;
+    uint8_t _pad[7];
+#endif

Above, you increase the version of the payload here even for Arm when there are no modification in Arm. This raises the question on whether we would need to increase the version when Arm is going to be supported?

I think the best way forward is to add Arm support.
This feature is simple enough and it does not make much sense to complicate it by adding support spaghetti.


 };
 typedef struct livepatch_func livepatch_func_t;
 #endif
diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
index 2aec532ee2..a93126f631 100644
--- a/xen/include/xen/livepatch.h
+++ b/xen/include/xen/livepatch.h
@@ -117,7 +117,7 @@ int arch_livepatch_quiesce(void);
 void arch_livepatch_revive(void);
   void arch_livepatch_apply(struct livepatch_func *func);
-void arch_livepatch_revert(const struct livepatch_func *func);
+void arch_livepatch_revert(struct livepatch_func *func);
 void arch_livepatch_post_action(void);
   void arch_livepatch_mask(void);

Cheers,

--
Julien Grall

Best Regards,
Pawel Wieczorkiewicz






Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879



[-- Attachment #1.2: Type: text/html, Size: 20833 bytes --]

<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">
<br class="">
<div><br class="">
<blockquote type="cite" class="">
<div class="">On 22. Aug 2019, at 12:29, Julien Grall &lt;<a href="mailto:julien.grall@arm.com" class="">julien.grall@arm.com</a>&gt; wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div class="">Hi Pawel,<br class="">
</div>
</div>
</blockquote>
<div><br class="">
</div>
<div>Hi Julien,</div>
<div><br class="">
</div>
<div>Thank for looking at this!</div>
<br class="">
<blockquote type="cite" class="">
<div class="">
<div class=""><br class="">
On 21/08/2019 09:19, Pawel Wieczorkiewicz wrote:<br class="">
<blockquote type="cite" class="">Livepatch only tracks an entire payload applied/reverted state. But,<br class="">
with an option to supply the apply_payload() and/or revert_payload()<br class="">
functions as optional hooks, it becomes possible to intermix the<br class="">
execution of the original apply_payload()/revert_payload() functions<br class="">
with their dynamically supplied counterparts.<br class="">
It is important then to track the current state of every function<br class="">
being patched and prevent situations of unintentional double-apply<br class="">
or unapplied revert.<br class="">
To support that, it is necessary to extend public interface of the<br class="">
livepatch. The struct livepatch_func gets additional field holding<br class="">
the applied/reverted state marker.<br class="">
To reflect the livepatch payload ABI change, bump the version flag<br class="">
LIVEPATCH_PAYLOAD_VERSION up to 2.<br class="">
The above solution only applies to x86 architecture for now.<br class="">
Signed-off-by: Pawel Wieczorkiewicz &lt;<a href="mailto:wipawel@amazon.de" class="">wipawel@amazon.de</a>&gt;<br class="">
Reviewed-by: Andra-Irina Paraschiv &lt;<a href="mailto:andraprs@amazon.com" class="">andraprs@amazon.com</a>&gt;<br class="">
Reviewed-by: Bjoern Doebel &lt;<a href="mailto:doebel@amazon.de" class="">doebel@amazon.de</a>&gt;<br class="">
Reviewed-by: Martin Pohlack &lt;<a href="mailto:mpohlack@amazon.de" class="">mpohlack@amazon.de</a>&gt;<br class="">
---<br class="">
&nbsp;xen/arch/x86/livepatch.c &nbsp;&nbsp;&nbsp;| 20 &#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;-<br class="">
&nbsp;xen/common/livepatch.c &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;| 35 &#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;<br class="">
&nbsp;xen/include/public/sysctl.h | 11 &#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;-<br class="">
&nbsp;xen/include/xen/livepatch.h | &nbsp;2 &#43;-<br class="">
&nbsp;4 files changed, 65 insertions(&#43;), 3 deletions(-)<br class="">
diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c<br class="">
index 436ee40fe1..76fa91a082 100644<br class="">
--- a/xen/arch/x86/livepatch.c<br class="">
&#43;&#43;&#43; b/xen/arch/x86/livepatch.c<br class="">
@@ -61,6 +61,14 @@ void noinline arch_livepatch_apply(struct livepatch_func *func)<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if ( !len )<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;return;<br class="">
&nbsp;&#43; &nbsp;&nbsp;&nbsp;/* If the apply action has been already executed on this function, do nothing... */<br class="">
&#43; &nbsp;&nbsp;&nbsp;if ( func-&gt;applied == LIVEPATCH_FUNC_APPLIED )<br class="">
&#43; &nbsp;&nbsp;&nbsp;{<br class="">
&#43; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;printk(XENLOG_WARNING LIVEPATCH &quot;%s: %s has been already applied before\n&quot;,<br class="">
&#43; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;__func__, func-&gt;name);<br class="">
&#43; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;return;<br class="">
&#43; &nbsp;&nbsp;&nbsp;}<br class="">
</blockquote>
<br class="">
Does this need to in arch specific code?<br class="">
</div>
</div>
</blockquote>
<div><br class="">
</div>
<div>As a matter of fact it does not.</div>
<div>Let me enable this feature for Arm as well and make this code a common inline.</div>
<br class="">
<blockquote type="cite" class="">
<div class="">
<div class=""><br class="">
<blockquote type="cite" class="">&#43;<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;memcpy(func-&gt;opaque, old_ptr, len);<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if ( func-&gt;new_addr )<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;{<br class="">
@@ -77,15 +85,25 @@ void noinline arch_livepatch_apply(struct livepatch_func *func)<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;add_nops(insn, len);<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;memcpy(old_ptr, insn, len);<br class="">
&#43; &nbsp;&nbsp;&nbsp;func-&gt;applied = LIVEPATCH_FUNC_APPLIED;<br class="">
&nbsp;}<br class="">
&nbsp;&nbsp;&nbsp;/*<br class="">
&nbsp;&nbsp;* &quot;noinline&quot; to cause control flow change and thus invalidate I$ and<br class="">
&nbsp;&nbsp;* cause refetch after modification.<br class="">
&nbsp;&nbsp;*/<br class="">
-void noinline arch_livepatch_revert(const struct livepatch_func *func)<br class="">
&#43;void noinline arch_livepatch_revert(struct livepatch_func *func)<br class="">
&nbsp;{<br class="">
&#43; &nbsp;&nbsp;&nbsp;/* If the apply action hasn't been executed on this function, do nothing... */<br class="">
&#43; &nbsp;&nbsp;&nbsp;if ( !func-&gt;old_addr || func-&gt;applied == LIVEPATCH_FUNC_NOT_APPLIED )<br class="">
&#43; &nbsp;&nbsp;&nbsp;{<br class="">
&#43; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;printk(XENLOG_WARNING LIVEPATCH &quot;%s: %s has not been applied before\n&quot;,<br class="">
&#43; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;__func__, func-&gt;name);<br class="">
&#43; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;return;<br class="">
&#43; &nbsp;&nbsp;&nbsp;}<br class="">
&#43;<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;memcpy(func-&gt;old_addr, func-&gt;opaque, livepatch_insn_len(func));<br class="">
&#43; &nbsp;&nbsp;&nbsp;func-&gt;applied = LIVEPATCH_FUNC_NOT_APPLIED;<br class="">
&nbsp;}<br class="">
&nbsp;&nbsp;&nbsp;/*<br class="">
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c<br class="">
index 585ec9819a..090a48977b 100644<br class="">
--- a/xen/common/livepatch.c<br class="">
&#43;&#43;&#43; b/xen/common/livepatch.c<br class="">
@@ -1242,6 +1242,29 @@ static inline void revert_payload_tail(struct payload *data)<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;data-&gt;state = LIVEPATCH_STATE_CHECKED;<br class="">
&nbsp;}<br class="">
&nbsp;&#43;/*<br class="">
&#43; * Check if an action has applied the same state to all payload's functions consistently.<br class="">
&#43; */<br class="">
&#43;static inline bool was_action_consistent(const struct payload *data, livepatch_func_state_t expected_state)<br class="">
&#43;{<br class="">
&#43; &nbsp;&nbsp;&nbsp;int i;<br class="">
&#43;<br class="">
&#43; &nbsp;&nbsp;&nbsp;for ( i = 0; i &lt; data-&gt;nfuncs; i&#43;&#43; )<br class="">
&#43; &nbsp;&nbsp;&nbsp;{<br class="">
&#43; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;struct livepatch_func *f = &amp;(data-&gt;funcs[i]);<br class="">
&#43;<br class="">
&#43; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if ( f-&gt;applied != expected_state )<br class="">
</blockquote>
<br class="">
Per the definition of livepath_func, the field &quot;applied&quot; only exists for x86. So this will not compiled on Arm.<br class="">
</div>
</div>
</blockquote>
<div><br class="">
</div>
<div>I will enable Arm too.</div>
<br class="">
<blockquote type="cite" class="">
<div class="">
<div class=""><br class="">
<blockquote type="cite" class="">&#43; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;{<br class="">
&#43; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;printk(XENLOG_ERR LIVEPATCH &quot;%s: Payload has a function: '%s' with inconsistent applied state.\n&quot;,<br class="">
&#43; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;data-&gt;name, f-&gt;name ?: &quot;noname&quot;);<br class="">
&#43;<br class="">
&#43; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;return false;<br class="">
&#43; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;}<br class="">
&#43; &nbsp;&nbsp;&nbsp;}<br class="">
&#43;<br class="">
&#43; &nbsp;&nbsp;&nbsp;return true;<br class="">
&#43;}<br class="">
&#43;<br class="">
&nbsp;/*<br class="">
&nbsp;&nbsp;* This function is executed having all other CPUs with no deep stack (we may<br class="">
&nbsp;&nbsp;* have cpu_idle on it) and IRQs disabled.<br class="">
@@ -1268,6 +1291,9 @@ static void livepatch_do_action(void)<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;else<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;rc = apply_payload(data);<br class="">
&nbsp;&#43; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if ( !was_action_consistent(data, rc ? LIVEPATCH_FUNC_NOT_APPLIED : LIVEPATCH_FUNC_APPLIED) )<br class="">
</blockquote>
<br class="">
Regardless the compilation issue above, none of the common code will set the state. So for Arm, this would likely mean the function return false.<br class="">
</div>
</div>
</blockquote>
<div><br class="">
</div>
<div>True, I should have disabled this code for Arm too.</div>
<div>But now, let me just make it common for all archs.</div>
<br class="">
<blockquote type="cite" class="">
<div class="">
<div class=""><br class="">
<blockquote type="cite" class="">&#43; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;panic(&quot;livepatch: partially applied payload '%s'!\n&quot;, data-&gt;name);<br class="">
</blockquote>
<br class="">
I can see at least a case where the panic can be reached here. Per the changes in arch_livepatch_apply(), f-&gt;applied will only be set to LIVEPATCH_FUNC_APPLIED if livepatch_insn_len() is not 0.<br class="">
<br class="">
I don't know whether livepatch_insn_len() can ever return 0 as my livepatch knowledge is limited. But the code live little doubt this is a theoritical possibility that after this patch will turn into crashing Xen.<br class="">
<br class="">
</div>
</div>
</blockquote>
<div><br class="">
</div>
<div>I suppose the livepatch_insn_len() returning 0 (i.e. func-&gt;new_size being 0, which is valid AFAICS) is the to-be-implemented feature of noping out code:</div>
<div>From the livepatch doc:</div>
<div>
<div>&quot;&quot;&quot;</div>
<div>## TODO Goals</div>
<div class=""><br class="">
</div>
</div>
<div>&nbsp;* NOP out the code sequence if `new_size` is zero.</div>
&quot;&quot;&quot;</div>
<div>So, in theory it can be 0 (it has to be introduced to the livepatch.funcs). Thus, you’re right.</div>
<div><br class="">
</div>
<div>I will fix that, but marking such function (new_size==0) as applied.</div>
<div><br class="">
</div>
<div>Thanks!&nbsp;</div>
<div><br class="">
<blockquote type="cite" class="">
<div class="">
<div class="">More generally, I am not very comfortable to see panic() in the middle of the code. Could you explain why panic is the best solution over reverting the work?<br class="">
<br class="">
</div>
</div>
</blockquote>
<div><br class="">
</div>
<div>Yes. Production-ready hotpatches must not contain inconsistent hooks or functions-to-be-applied/reverted.</div>
<div>The goal here is to detect such hotpatches and fail hard immediately highlighting the fact that such hotpatch</div>
<div>is broken.</div>
<div><br class="">
</div>
<div>The inconsistent application of a hotpatch (some function applied, some reverted while other left behined) leaves</div>
<div>the system in a very bad state. I think the best what we could do here is panic() to enable post-mortem analysis</div>
<div>of what went wrong and avoid leaking such system into production.</div>
<div><br class="">
</div>
<div>Mis-detecting such conditions and leaving such inconsistent system running is a very bad idea.&nbsp;</div>
<br class="">
<blockquote type="cite" class="">
<div class="">
<div class="">My question applies for all the other panic() below.<br class="">
<br class="">
<blockquote type="cite" class="">&#43;<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if ( rc == 0 )<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;apply_payload_tail(data);<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;break;<br class="">
@@ -1282,6 +1308,9 @@ static void livepatch_do_action(void)<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;else<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;rc = revert_payload(data);<br class="">
&nbsp;&#43; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if ( !was_action_consistent(data, rc ? LIVEPATCH_FUNC_APPLIED : LIVEPATCH_FUNC_NOT_APPLIED) )<br class="">
&#43; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;panic(&quot;livepatch: partially reverted payload '%s'!\n&quot;, data-&gt;name);<br class="">
&#43;<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if ( rc == 0 )<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;revert_payload_tail(data);<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;break;<br class="">
@@ -1304,6 +1333,9 @@ static void livepatch_do_action(void)<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;other-&gt;rc = revert_payload(other);<br class="">
&nbsp;&nbsp;&nbsp;&#43; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if ( !was_action_consistent(other, rc ? LIVEPATCH_FUNC_APPLIED : LIVEPATCH_FUNC_NOT_APPLIED) )<br class="">
&#43; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;panic(&quot;livepatch: partially reverted payload '%s'!\n&quot;, other-&gt;name);<br class="">
&#43;<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if ( other-&gt;rc == 0 )<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;revert_payload_tail(other);<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;else<br class="">
@@ -1324,6 +1356,9 @@ static void livepatch_do_action(void)<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;else<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;rc = apply_payload(data);<br class="">
&nbsp;&#43; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if ( !was_action_consistent(data, rc ? LIVEPATCH_FUNC_NOT_APPLIED : LIVEPATCH_FUNC_APPLIED) )<br class="">
&#43; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;panic(&quot;livepatch: partially applied payload '%s'!\n&quot;, data-&gt;name);<br class="">
&#43;<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if ( rc == 0 )<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;apply_payload_tail(data);<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;}<br class="">
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h<br class="">
index 1b2b165a6d..b55ad6d050 100644<br class="">
--- a/xen/include/public/sysctl.h<br class="">
&#43;&#43;&#43; b/xen/include/public/sysctl.h<br class="">
@@ -818,7 +818,7 @@ struct xen_sysctl_cpu_featureset {<br class="">
&nbsp;&nbsp;* &nbsp;&nbsp;&nbsp;&nbsp;If zero exit with success.<br class="">
&nbsp;&nbsp;*/<br class="">
&nbsp;-#define LIVEPATCH_PAYLOAD_VERSION 1<br class="">
&#43;#define LIVEPATCH_PAYLOAD_VERSION 2<br class="">
&nbsp;/*<br class="">
&nbsp;&nbsp;* .livepatch.funcs structure layout defined in the `Payload format`<br class="">
&nbsp;&nbsp;* section in the Live Patch design document.<br class="">
@@ -826,6 +826,11 @@ struct xen_sysctl_cpu_featureset {<br class="">
&nbsp;&nbsp;* We guard this with __XEN__ as toolstacks SHOULD not use it.<br class="">
&nbsp;&nbsp;*/<br class="">
&nbsp;#ifdef __XEN__<br class="">
&#43;typedef enum livepatch_func_state {<br class="">
&#43; &nbsp;&nbsp;&nbsp;LIVEPATCH_FUNC_NOT_APPLIED = 0,<br class="">
&#43; &nbsp;&nbsp;&nbsp;LIVEPATCH_FUNC_APPLIED = 1<br class="">
</blockquote>
<br class="">
AFAIK, enum will always start counting from 0, so is it really necessary to specify the exact values?<br class="">
</div>
</div>
</blockquote>
<div><br class="">
</div>
<div>It’s probably my (or some earlier reviewer's) OCD. I am happy to remove these.</div>
<br class="">
<blockquote type="cite" class="">
<div class="">
<div class=""><br class="">
<blockquote type="cite" class="">&#43;} livepatch_func_state_t;<br class="">
&#43;<br class="">
&nbsp;struct livepatch_func {<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;const char *name; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/* Name of function to be patched. */<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;void *new_addr;<br class="">
@@ -834,6 +839,10 @@ struct livepatch_func {<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;uint32_t old_size;<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;uint8_t version; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/* MUST be LIVEPATCH_PAYLOAD_VERSION. */<br class="">
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;uint8_t opaque[31];<br class="">
&#43;#if defined CONFIG_X86<br class="">
&#43; &nbsp;&nbsp;&nbsp;uint8_t applied;<br class="">
&#43; &nbsp;&nbsp;&nbsp;uint8_t _pad[7];<br class="">
&#43;#endif<br class="">
</blockquote>
<br class="">
Above, you increase the version of the payload here even for Arm when there are no modification in Arm. This raises the question on whether we would need to increase the version when Arm is going to be supported?<br class="">
</div>
</div>
</blockquote>
<div><br class="">
</div>
<div>I think the best way forward is to add Arm support.</div>
<div>This feature is simple enough and it does not make much sense to complicate it by adding support spaghetti.</div>
<br class="">
<blockquote type="cite" class="">
<div class="">
<div class=""><br class="">
<blockquote type="cite" class="">&nbsp;};<br class="">
&nbsp;typedef struct livepatch_func livepatch_func_t;<br class="">
&nbsp;#endif<br class="">
diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h<br class="">
index 2aec532ee2..a93126f631 100644<br class="">
--- a/xen/include/xen/livepatch.h<br class="">
&#43;&#43;&#43; b/xen/include/xen/livepatch.h<br class="">
@@ -117,7 +117,7 @@ int arch_livepatch_quiesce(void);<br class="">
&nbsp;void arch_livepatch_revive(void);<br class="">
&nbsp;&nbsp;&nbsp;void arch_livepatch_apply(struct livepatch_func *func);<br class="">
-void arch_livepatch_revert(const struct livepatch_func *func);<br class="">
&#43;void arch_livepatch_revert(struct livepatch_func *func);<br class="">
&nbsp;void arch_livepatch_post_action(void);<br class="">
&nbsp;&nbsp;&nbsp;void arch_livepatch_mask(void);<br class="">
</blockquote>
<br class="">
Cheers,<br class="">
<br class="">
-- <br class="">
Julien Grall<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
<div class="">
<div dir="auto" style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">
<div style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">
Best Regards,<br class="">
Pawel Wieczorkiewicz</div>
<br class="Apple-interchange-newline">
</div>
<br class="Apple-interchange-newline">
</div>
<br class="">
<br><br><br>Amazon Development Center Germany GmbH
<br>Krausenstr. 38
<br>10117 Berlin
<br>Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
<br>Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
<br>Sitz: Berlin
<br>Ust-ID: DE 289 237 879
<br><br><br>
</body>
</html>

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

  reply index

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-21  8:19 [Xen-devel] [PATCH 00/14] livepatch: new features and fixes Pawel Wieczorkiewicz
2019-08-21  8:19 ` [Xen-devel] [PATCH 01/14] livepatch: Always check hypervisor build ID upon hotpatch upload Pawel Wieczorkiewicz
2019-08-21 18:16   ` Konrad Rzeszutek Wilk
2019-08-21  8:19 ` [Xen-devel] [PATCH 02/14] livepatch: Allow to override inter-modules buildid dependency Pawel Wieczorkiewicz
2019-08-21  8:19 ` [Xen-devel] [PATCH 03/14] python: Add XC binding for Xen build ID Pawel Wieczorkiewicz
2019-08-21  8:19 ` [Xen-devel] [PATCH 04/14] livepatch: Export payload structure via livepatch_payload.h Pawel Wieczorkiewicz
2019-08-21  8:19 ` [Xen-devel] [PATCH 05/14] livepatch: Implement pre-|post- apply|revert hooks Pawel Wieczorkiewicz
2019-08-21  8:19 ` [Xen-devel] [PATCH 06/14] livepatch: Add support for apply|revert action replacement hooks Pawel Wieczorkiewicz
2019-08-21 18:31   ` Konrad Rzeszutek Wilk
2019-08-21 19:06     ` Wieczorkiewicz, Pawel
2019-08-26 14:30       ` Konrad Rzeszutek Wilk
2019-08-21  8:19 ` [Xen-devel] [PATCH 07/14] livepatch: Do not enforce ELF_LIVEPATCH_FUNC section presence Pawel Wieczorkiewicz
2019-08-21  8:19 ` [Xen-devel] [PATCH 08/14] livepatch: always print XENLOG_ERR information Pawel Wieczorkiewicz
2019-08-21  8:19 ` [Xen-devel] [PATCH 09/14] livepatch: Add per-function applied/reverted state tracking marker Pawel Wieczorkiewicz
2019-08-21 18:28   ` Konrad Rzeszutek Wilk
2019-08-21 19:00     ` Wieczorkiewicz, Pawel
2019-08-21 21:34   ` Julien Grall
2019-08-22  7:44     ` Wieczorkiewicz, Pawel
2019-08-22 10:07       ` Julien Grall
2019-08-22 10:20         ` Wieczorkiewicz, Pawel
2019-08-22 10:43           ` Julien Grall
2019-08-22 11:15             ` Wieczorkiewicz, Pawel
2019-08-22 15:02               ` Julien Grall
2019-08-22 10:29   ` Julien Grall
2019-08-22 11:02     ` Wieczorkiewicz, Pawel [this message]
2019-08-22 15:30       ` Julien Grall
2019-08-22 15:42         ` Wieczorkiewicz, Pawel
2019-08-21  8:19 ` [Xen-devel] [PATCH 10/14] livepatch: Add support for inline asm hotpatching expectations Pawel Wieczorkiewicz
2019-08-21 18:30   ` Konrad Rzeszutek Wilk
2019-08-21 19:02     ` Wieczorkiewicz, Pawel
2019-08-22 10:31   ` Julien Grall
2019-08-22 11:03     ` Wieczorkiewicz, Pawel
2019-08-21  8:19 ` [Xen-devel] [PATCH 11/14] livepatch: Add support for modules .modinfo section metadata Pawel Wieczorkiewicz
2019-08-21  8:19 ` [Xen-devel] [PATCH 12/14] livepatch: Handle arbitrary size names with the list operation Pawel Wieczorkiewicz
2019-08-21  8:19 ` [Xen-devel] [PATCH 13/14] livepatch: Add metadata runtime retrieval mechanism Pawel Wieczorkiewicz
2019-08-21  8:19 ` [Xen-devel] [PATCH 14/14] livepatch: Add python bindings for livepatch operations Pawel Wieczorkiewicz
2019-08-22 21:55   ` Marek Marczykowski-Górecki
2019-08-27  8:46 ` [Xen-devel] [PATCH v2 00/12] livepatch: new features and fixes Pawel Wieczorkiewicz
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 01/12] livepatch: Always check hypervisor build ID upon hotpatch upload Pawel Wieczorkiewicz
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 02/12] livepatch: Allow to override inter-modules buildid dependency Pawel Wieczorkiewicz
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 03/12] livepatch: Export payload structure via livepatch_payload.h Pawel Wieczorkiewicz
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 04/12] livepatch: Implement pre-|post- apply|revert hooks Pawel Wieczorkiewicz
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 05/12] livepatch: Add support for apply|revert action replacement hooks Pawel Wieczorkiewicz
2019-08-27 16:58     ` Konrad Rzeszutek Wilk
2019-08-28  7:37       ` Wieczorkiewicz, Pawel
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 06/12] livepatch: Do not enforce ELF_LIVEPATCH_FUNC section presence Pawel Wieczorkiewicz
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 07/12] livepatch: Add per-function applied/reverted state tracking marker Pawel Wieczorkiewicz
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 08/12] livepatch: Add support for inline asm hotpatching expectations Pawel Wieczorkiewicz
2019-08-29 14:34     ` Konrad Rzeszutek Wilk
2019-08-29 15:29       ` Wieczorkiewicz, Pawel
2019-08-29 15:58     ` Konrad Rzeszutek Wilk
2019-08-29 16:16       ` Wieczorkiewicz, Pawel
2019-08-29 17:49         ` Konrad Rzeszutek Wilk
2019-08-29 19:07           ` Wieczorkiewicz, Pawel
2019-08-29 20:48             ` Konrad Rzeszutek Wilk
2019-09-05 18:05     ` Konrad Rzeszutek Wilk
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 09/12] livepatch: Add support for modules .modinfo section metadata Pawel Wieczorkiewicz
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 10/12] livepatch: Handle arbitrary size names with the list operation Pawel Wieczorkiewicz
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 11/12] livepatch: Add metadata runtime retrieval mechanism Pawel Wieczorkiewicz
2019-08-29 20:48     ` Konrad Rzeszutek Wilk
2019-08-27  8:46   ` [Xen-devel] [PATCH v2 12/12] livepatch: Add python bindings for livepatch operations Pawel Wieczorkiewicz
2019-08-28 13:21     ` Marek Marczykowski-Górecki
2019-08-29 19:23   ` [Xen-devel] [PATCH v2 00/12] livepatch: new features and fixes Konrad Rzeszutek Wilk
2019-09-05 19:13   ` Konrad Rzeszutek Wilk
2019-09-06 22:52     ` Julien Grall
2019-09-06 22:42   ` Julien Grall

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DB27F58E-DA27-4A5D-BE23-4CDB5D9736A2@amazon.com \
    --to=wipawel@amazon.de \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=jbeulich@suse.com \
    --cc=julien.grall@arm.com \
    --cc=konrad.wilk@oracle.com \
    --cc=mpohlack@amazon.de \
    --cc=roger.pau@citrix.com \
    --cc=ross.lagerwall@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=tim@xen.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git