Re: [Xen-devel] [PATCH V5] vm_event: Allow subscribing to write events for specific MSR-s

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 04/26/2016 06:28 PM, Tamas K Lengyel wrote:
> 
>     > @@ -55,6 +61,9 @@ void vm_event_cleanup_domain(struct domain *d)
>     >          v->arch.vm_event = NULL;
>     >      }
>     >
>     > +    xfree(d->arch.monitor_msr_bitmap);
>     > +    d->arch.monitor_msr_bitmap = NULL;
>     > +
>     >      d->arch.mem_access_emulate_each_rep = 0;
>     >      memset(&d->arch.monitor, 0, sizeof(d->arch.monitor));
>     >      memset(&d->monitor, 0, sizeof(d->monitor));
> 
>     Aside from accidentally duplicating the alloc() / free() code here, I
>     should have probably moved the following monitor-related memset()s in
>     arch_monitor_init_domain() as well. I'll do that in V6.
> 
> 
> Yes, +1 for moving the memsets to the new init function as well. It may
> be cleaner if you break this patch into two, one where you introduce the
> new monitor init and move these memsets there, and then do the msr patch.

You're right, and had this been planned from the start this would have
been better as a two-part series, but at this point I think the benefits
are offset by having to re-version the new series and the extra time
taken by the split, so if it's not too bothersome I'll send V6 as a
single patch - with an updated description.


Thanks,
Razvan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel