From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v3 09/23] xsplice: Add support for bug frames. (v4) Date: Wed, 24 Feb 2016 16:30:32 +0000 Message-ID: <56CDDAA8.1070002@citrix.com> References: <1455300361-13092-1-git-send-email-konrad.wilk@oracle.com> <1455300361-13092-10-git-send-email-konrad.wilk@oracle.com> <56C37A04.6030501@citrix.com> <20160224162218.GA25409@char.us.oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20160224162218.GA25409@char.us.oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Konrad Rzeszutek Wilk Cc: Keir Fraser , Ian Campbell , jinsong.liu@alibaba-inc.com, xen-devel@lists.xen.org, mpohlack@amazon.de, ross.lagerwall@citrix.com, Stefano Stabellini , Jan Beulich , xen-devel@lists.xenproject.org, sasha.levin@citrix.com List-Id: xen-devel@lists.xenproject.org On 24/02/16 16:22, Konrad Rzeszutek Wilk wrote: > . snip.. >> There is a neater way of doing this, which doesn't involve having "if ( >> regular ) else if ( xsplice )" logic chains through the code. > s/chains/chain/ > > There is only one that uses the 'xsplice' name in it:-) > > The other two are wrapped with the 'is_patch'. >> Given a >> >> struct virtual_region >> { >> struct list_head list; >> unsigned long start, size; >> >> struct bug_frame *foo; >> struct exception_table_entry *bar; >> }; >> >> The init code can construct one for the base hypervisor, and xsplice can >> add or remove entries from the list. Then, the trap routines search the >> virtual region list for [start, size) and follow the appropriate pointers. > You are suggesting that on bootup we parse the the __stop_bug_frames_[0-3] > (different on ARM), and create an linked list to contain those. Can probably manage this at compile time for the builtin ones. > > Then xSplice can call in this API to add their own - and on unload it can > unlink them and free them. > > If m understanding is correct - while it is certainly much nicer, it has drawbacks: > - Increases the code to now handle the linked list and all the code around it > (And correspondingly we may have now some extra bugs to track). > - Bigger memory consumption - we now to have to consume memory for this list - even > for the built-in ones. I mean having one struct virtual_region for Xen itself (perhaps two; one for .text and one for .init which gets removed later), and one extra for each xpatch. The extra memory consumption is a 6 pointers, traded against far cleaner logic for bugfames and extable redirections. ~Andrew