[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH RFC V5 3/5] xen: Force-enable relevant MSR events; optimize the number of sent MSR events
>>> On 06.08.14 at 17:58, <rcojocaru@xxxxxxxxxxxxxxx> wrote: > @@ -695,11 +696,30 @@ static void vmx_set_host_env(struct vcpu *v) > void vmx_disable_intercept_for_msr(struct vcpu *v, u32 msr, int type) > { > unsigned long *msr_bitmap = v->arch.hvm_vmx.msr_bitmap; > + struct domain *d = v->domain; > > /* VMX MSR bitmap supported? */ > if ( msr_bitmap == NULL ) > return; > > + if ( mem_event_check_ring(&d->mem_event->access) ) > + { > + /* Filter out MSR-s needed for memory introspection */ I continue to be unconvinced that this code block's surrounding conditional is as precise as possible: Your introspection code surely isn't the only mem-event based mechanism. Yet you'd impact guests in all other cases too. > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -1682,6 +1682,22 @@ void vmx_hypervisor_cpuid_leaf(uint32_t sub_idx, > *eax |= XEN_HVM_CPUID_X2APIC_VIRT; > } > > +static void vmx_enable_intro_msr_interception(struct domain *d) The "intro" in the name is surely odd: For one, it implies that _only_ introspection might be interested in doing this. And then it may (without reading the comments inside the function) well be an abbreviation for something else, e.g. "introduction". > +{ > + struct vcpu *v; > + > + /* Enable interception for MSRs needed for memory introspection. */ > + for_each_vcpu ( d, v ) > + { > + vmx_enable_intercept_for_msr(v, MSR_IA32_SYSENTER_EIP, MSR_TYPE_W); > + vmx_enable_intercept_for_msr(v, MSR_IA32_SYSENTER_ESP, MSR_TYPE_W); > + vmx_enable_intercept_for_msr(v, MSR_IA32_SYSENTER_CS, MSR_TYPE_W); > + vmx_enable_intercept_for_msr(v, MSR_IA32_MC0_CTL, MSR_TYPE_W); > + vmx_enable_intercept_for_msr(v, MSR_STAR, MSR_TYPE_W); > + vmx_enable_intercept_for_msr(v, MSR_LSTAR, MSR_TYPE_W); I also wonder whether the redundant enumeration of all these MSRs couldn't be abstracted to just a single place. > --- a/xen/arch/x86/mm/mem_event.c > +++ b/xen/arch/x86/mm/mem_event.c > @@ -600,6 +600,9 @@ int mem_event_domctl(struct domain *d, > xen_domctl_mem_event_op_t *mec, > rc = mem_event_enable(d, mec, med, _VPF_mem_access, > HVM_PARAM_ACCESS_RING_PFN, > mem_access_notification); > + > + if ( rc == 0 && hvm_funcs.enable_intro_msr_interception ) > + hvm_funcs.enable_intro_msr_interception(d); Isn't the sequence of operations wrong here (leaving a window in time where mem events are already enabled but the necessary MSRs aren't being intercepted yet? Or was it that guests are being paused while all this takes place? Plus of course the same remark regarding the insufficient conditional as above applies here. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |