From mboxrd@z Thu Jan 1 00:00:00 1970 From: Razvan Cojocaru Subject: Re: [PATCH] xen/vm_event: Clean up control-register-write vm_events Date: Tue, 12 May 2015 21:38:01 +0300 Message-ID: <55524889.3020604@bitdefender.com> References: <1431442725-18395-1-git-send-email-rcojocaru@bitdefender.com> <555239EE020000780007998A@mail.emea.novell.com> <5552211A.3040706@bitdefender.com> <5552214E.1010004@citrix.com> <55522214.3030807@citrix.com> <55522FDA.7030704@bitdefender.com> <55523E39.9080904@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <55523E39.9080904@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: Andrew Cooper , Jan Beulich Cc: kevin.tian@intel.com, wei.liu2@citrix.com, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, eddie.dong@intel.com, tim@xen.org, xen-devel@lists.xen.org, jun.nakajima@intel.com, keir@xen.org, ian.jackson@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On 05/12/2015 08:54 PM, Andrew Cooper wrote: > On 12/05/15 17:52, Razvan Cojocaru wrote: >> On 05/12/2015 06:53 PM, Andrew Cooper wrote: >>> On 12/05/15 16:50, Andrew Cooper wrote: >>>> On 12/05/15 16:49, Razvan Cojocaru wrote: >>>>> On 05/12/2015 06:35 PM, Jan Beulich wrote: >>>>>>>>> On 12.05.15 at 16:58, wrote: >>>>>>> +/* Supported values for the vm_event_write_ctrlreg index. */ >>>>>>> +#define X86_CR0 (1 << 0) >>>>>>> +#define X86_CR3 (1 << 1) >>>>>>> +#define X86_CR4 (1 << 2) >>>>>>> +#define X86_XCR0 (1 << 3) >>>>>> These names, being put in the public interface, are way too generic. >>>>> I've copied them from Andrew Cooper's suggestion in the previous thread, >>>>> hopefully he'll chime in. I'm happy to rename / move them if so desired. >>>> I intended something like VM_EVENT_ARCH_X86_CR0 etc. >>> or perhaps slightly more specific to ctrl_reg read/write. >> VM_EVENT_ARCH_X86_MOV_TO_CR0? Or maybe VM_EVENT_WRITE_ARCH_X86_CR0? > > Are there any other events which could plausibly use the same indicies? > Monitoring ctrl_reg reads perhaps ? I see what you mean, those could be reused for reads and result in less verbosity in the vm_event.h header. I'll stick to VM_EVENT_ARCH_X86_CR0 and so on (unless requested otherwise), I don't have a better naming convention (I guess if we need to emphasize that something is being done to said control register we could call them VM_EVENT_ACCESS_ARCH_X86_CR0, but that's more verbose and IMHO not much better). >> There's something else that should also be clarified: I've renamed the >> vm_event.h struct to vm_event_write_ctrlreg, and changed >> monitor.mov_to_cr* to monitor.write_ctrlreg*, but left >> xc_monitor_mov_to_cr() in libxc, and mov_to_cr in the domctl part (I >> thought it fit with mov_to_msr(), etc.). Should I change that to >> write_ctrlreg as well? On the one hand, mov_to_cr seems to be consistent >> with the previous interface, and on the other it's not consistent with >> the name changes the patch has made. > > Any API/ABI changes in this area are fine until 4.6 is released, and a > consistent interface is much preferred. Understood. Then I'll rename xc_monitor_mov_to_cr() to xc_monitor_write_ctrlreg(), since vm_event_write_ctrlreg has been previously requested. I'll also rename XEN_DOMCTL_MONITOR_EVENT_MOV_TO_CR to XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG, and VM_EVENT_REASON_MOV_TO_CR to VM_EVENT_REASON_WRITE_CTRLREG. I'm reading your reply to mean that I should also rename xc_monitor_mov_to_msr() to xc_monitor_write_msr(), and do similar things to the respective vm_event struct, VM_EVENT_REASON_MOV_TO_MSR and so on. Is that correct? Thanks, Razvan