From mboxrd@z Thu Jan 1 00:00:00 1970 From: Razvan Cojocaru Subject: Re: [PATCH] ARM: Support for guest-request vm-events Date: Thu, 28 Jan 2016 14:49:49 +0200 Message-ID: <56AA0E6D.5040406@bitdefender.com> References: <1453979874-20037-1-git-send-email-czuzu@bitdefender.com> <1453980216.26591.69.camel@citrix.com> <56AA0B3E.9030106@bitdefender.com> <1453985101.26591.74.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1453985101.26591.74.camel@citrix.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: Ian Campbell , CORNELIU ZUZU , xen-devel@lists.xen.org Cc: Tamas K Lengyel , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On 01/28/2016 02:45 PM, Ian Campbell wrote: > On Thu, 2016-01-28 at 14:36 +0200, CORNELIU ZUZU wrote: >> On 1/28/2016 1:23 PM, Ian Campbell wrote: >>> On Thu, 2016-01-28 at 13:17 +0200, Corneliu ZUZU wrote: >>>> This patch implements ARM support for guest-request vm-events. >>>> The code has been ported from x86 side w/ minor adjustments. >>> I've not looked at the patch yet, but if it only involves minor >>> adjustments >>> from the x86 side can some amount of it not be refactored into common >>> code? >>> >>> Ian. >>> >> >> At a first glance it seems to me that parts of monitor vm-events code >> could be moved to common. >> But it also seems that it would require a bit of effort and I'm not sure >> yet if the end result >> won't actually complicate implementation of monitor vm-events for other >> architectures in the future. >> Some of the monitor vm-events implemented are strictly architecture >> specific, >> e.g. VM_EVENT_REASON_MOV_TO_MSR will always be an x86-only vm-event, unless >> it is somehow generalized (maybe somehow merged w/ >> VM_EVENT_REASON_WRITE_CTRLREG?). >> But *most* of them indeed don't directly have this kind of specificity, >> so it would make sense to make >> most of the code common, if possible. > > That the sort of thing we would usually handle by having the common code > call into a arch_foo() to decode any non-common options. > >> To me personally this seems like a good idea and I'd be willing to give >> it a try, but as I said, >> it might require some other changes of the code, including x86 changes. >> I was about to release another patch after this one to implement >> control-register writes >> vm-events for ARM, but I anticipate that doing this move first will >> actually benefit my effort in that >> direction as well (I think the patch code will get to be cleaner). >> >> So, shall I try it? > > From my POV as an ARM maintainer I think this is the way to go. I've also > copied the VM EVENT maintainers in case they object for some reason. > > I'd recommend always CCing Razvan and Tamas on this series, and considering > adding any new ARM files to the entry in MAINTAINERS (assuming they don't > object of course) No objection from me on either factoring out the common code (where, and if possible) or adding the new files to MAINTAINERS. Thanks, Razvan