From mboxrd@z Thu Jan 1 00:00:00 1970 From: Corneliu ZUZU Subject: Re: [PATCH v4 2/2] xen/vm-events: Move parts of monitor_domctl code to common-side. Date: Tue, 16 Feb 2016 10:13:14 +0200 Message-ID: <56C2DA1A.5040004@bitdefender.com> References: <1455606432-18648-1-git-send-email-czuzu@bitdefender.com> <1455606514-18705-1-git-send-email-czuzu@bitdefender.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1455606514-18705-1-git-send-email-czuzu@bitdefender.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: xen-devel@lists.xen.org Cc: Tamas K Lengyel , Keir Fraser , Ian Campbell , Razvan Cojocaru , Andrew Cooper , Stefano Stabellini , Jan Beulich List-Id: xen-devel@lists.xenproject.org On 2/16/2016 9:08 AM, Corneliu ZUZU wrote: > This patch moves monitor_domctl to common-side. > Purpose: move what's common to common, prepare for implementation > of such vm-events on ARM. > > * move get_capabilities to arch-side => arch_monitor_get_capabilities. > * add arch-side monitor op handling function => arch_monitor_domctl_op. > e.g. X86-side handles XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP op > * add arch-side monitor event handling function => arch_monitor_domctl_event. > e.g. X86-side handles XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR event enable/disable > * remove status_check > > Signed-off-by: Corneliu ZUZU > > --- > Changed since v3: > * monitor_domctl @ common/monitor.c: > - remove unused requested_status > - sanity check mop->event range to avoid left-shift undefined behavior Due to left-shift undefined behavior situations, shouldn't I also: * in X86 arch_monitor_get_capabilities: replace '1 <<' w/ '1U <<' * in X86 arch_monitor_domctl_event, XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG case add a sanity check of mop->u.mov_to_cr.index before: unsigned int ctrlreg_bitmask = monitor_ctrlreg_bitmask(mop->u.mov_to_cr.index); , which basically translates to: unsigned int ctrlreg_bitmask = (1U << mop->u.mov_to_cr.index); ? (especially since mop->u.mov_to_cr.index is set by the caller). Would have been good if I'd thought of that before sending this patch series :). Thanks, Corneliu.