From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH V8] xen/vm_event: Clean up control-register-write vm_events and add XCR0 event Date: Tue, 2 Jun 2015 16:54:52 +0100 Message-ID: <1433260492.15036.335.camel@citrix.com> References: <1432907159-21899-1-git-send-email-rcojocaru@bitdefender.com> <1433259579.15036.327.camel@citrix.com> <556DCF0E.8080105@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <556DCF0E.8080105@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 Cc: kevin.tian@intel.com, wei.liu2@citrix.com, jun.nakajima@intel.com, Razvan Cojocaru , stefano.stabellini@eu.citrix.com, ian.jackson@eu.citrix.com, tim@xen.org, xen-devel@lists.xen.org, eddie.dong@intel.com, jbeulich@suse.com, keir@xen.org List-Id: xen-devel@lists.xenproject.org On Tue, 2015-06-02 at 16:43 +0100, Andrew Cooper wrote: > On 02/06/15 16:39, Ian Campbell wrote: > > On Fri, 2015-05-29 at 16:45 +0300, Razvan Cojocaru wrote: > >> As suggested by Andrew Cooper, this patch attempts to remove > >> some redundancy and allow for an easier time when adding vm_events > >> for new control registers in the future, by having a single > >> VM_EVENT_REASON_WRITE_CTRLREG vm_event type, meant to serve CR0, > >> CR3, CR4 and (newly introduced) XCR0. The actual control register > >> will be deduced by the new .index field in vm_event_write_ctrlreg > >> (renamed from vm_event_mov_to_cr). The patch has also modified > >> the xen-access.c test - it is now able to log CR3 events. > >> > >> Signed-off-by: Razvan Cojocaru > >> Acked-by: Jan Beulich > > Seems ok from the tools and arm side, so long as the xen-access.c test > > is likely to build on ARM despite the new x86-isms (it looks like it to > > me) and the following Q: > > > >> +/* Supported values for the vm_event_write_ctrlreg index. */ > >> +#define VM_EVENT_X86_CR0 0 > >> +#define VM_EVENT_X86_CR3 1 > >> +#define VM_EVENT_X86_CR4 2 > >> +#define VM_EVENT_X86_XCR0 3 > > Is the intention for different architectures to use non-overlapping > > number spaces? (i.e. ARM would start from 0x10000 or something)? > > > > If not then the usages in xen-access.c are a little more problematic. I > > can just about tolerate a tool on arm which asks "monitor x86 cr3" and > > then never sees anything, but to get events for whatever ARM reg happens > > to share the index instead would be wrong I think. > > When I suggested this, I expected each arch to maintain its own index > space and for these spaces to all start at 0, as there is very little > which is truly common at this level. That's fine too, but implies that tests/xen-access.c therefore needs some additional #ifdeffery. Ian.