From mboxrd@z Thu Jan 1 00:00:00 1970 From: Razvan Cojocaru Subject: Re: [PATCH V2] mem_event: Add support for MEM_EVENT_REASON_MSR Date: Wed, 02 Jan 2013 15:56:15 +0200 Message-ID: <50E43C7F.6000909@gmail.com> References: <1357133451.5668.23.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1357133451.5668.23.camel@zakaz.uk.xensource.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 Cc: "xen-devel@lists.xensource.com" , "Tim (Xen.org)" List-Id: xen-devel@lists.xenproject.org >> +#define MEM_EVENT_REASON_MSR 7 /* MSR was hit: gfn is MSR value, gla is MSR type; >> + does NOT honour HVMPME_onchangeonly */ > > Why doesn't/shouldn't it support onchangeonly? It doesn't support onchangeonly because it is unreasonably complicated to get the old value (which we need to compare to the new one, and thus establish if onchangeonly applies or not). If you look at hvm_msr_write_intercept() (it's at line 2917 in xen/arch/x86/hvm/hvm.c), depending on the value of the "msr" parameter there are several ways of setting the value: hvm_set_efer() if msr is MSR_EFER, hvm_set_guest_tsc() if msr is MSR_IA32_TSC, and so on. So rather than having a getter call for the current value for each of these MSR types, I found it cleaner to just notify the event channel that a value is being written, regardless of whether it's different from the currently stored value or not, and regardless of how the actual write is being handle depending on the MSR type. > HVM_PARAM_MEMORY_EVENT_INT3 and HVM_PARAM_MEMORY_EVENT_SINGLE_STEP seem > not to support it either but they reject attempts to use them with it > (in do_hvm_op). I think this new value should do the same. Understood, I agree. Will add this new modification and submit V3 of the patch tomorrow (no access to my development machine now). > By "MSR type" do you mean the address? I mean one of the following: MSR_EFER, MSR_IA32_TSC, etc. Would you prefer that I change the "MSR type" expression in the comment to something else? What would make more sense? Thanks, Razvan Cojocaru