Re: [Xen-devel] [PATCH 09/14] livepatch: Add per-function applied/reverted state tracking marker

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 07:44:50 +0000	[thread overview]
Message-ID: <BEEC1E7D-4A4D-406E-97C3-D5433BAE8CAF@amazon.com> (raw)
In-Reply-To: <680c5b24-b3fd-97b6-c048-49a2bdba4a3d@arm.com>


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


On 21. Aug 2019, at 23:34, Julien Grall <julien.grall@arm.com<mailto:julien.grall@arm.com>> wrote:

Hi Pawel,

Hi Julien,


On 8/21/19 9:19 AM, 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.

Is this going to break livepatch on Arm? If so, do you have any plan to fix it?


No, I do not think it is. But, I am unable to test on Arm (No access to HW and SW),
so I took the conservative approach here.

[...]

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);

I doubt livepatch on Arm will compile after this change.


What would be you suggestion then?
Shall I limit the change to X86 everywhere
 Or maybe drop the compilation flag completely?

 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: 4570 bytes --]

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