>>> On 22.05.15 at 09:50, <rcojoc...@bitdefender.com> wrote: > On 05/22/2015 10:36 AM, Jan Beulich wrote: >>>>> On 21.05.15 at 15:00, <rcojoc...@bitdefender.com> wrote: > Should I drop the introduction of the XR0 event from this patch and come > back to it in the following iteration of the series?
Or simply adjust the title of the patch. >> Also, for readability I wonder whether an identically named macro >> wrapper around hvm_event_cr() wouldn't be a good idea, eliminating >> the need to spell out VM_EVENT_X86_ at each use site. > > #define hvm_event_cr0(new, old) hvm_event_cr(VM_EVENT_X86_XCR0, new, > old)? Sure. Maybe this way. But specifically said "identically named", i.e. envisioned #define hvm_event_cr(what, new, old) hvm_event_cr(VM_EVENT_X86_##what, new, old) >>> --- a/xen/include/asm-x86/domain.h >>> +++ b/xen/include/asm-x86/domain.h >>> @@ -249,6 +249,8 @@ struct pv_domain >>> struct mapcache_domain mapcache; >>> }; >>> >>> +#define monitor_ctrlreg_bitmask(ctrlreg_index) (1 << ctrlreg_index) >> >> (1U << (ctrlreg_index)) would seem better to me. Also - does this >> need to be in this header (which gets included basically everywhere)? > > Ack. As for the header, that's where the fields are > (write_ctrlreg_enabled, sync and onchangeonly), and since several .c > files use the fields it seemed the natural place to put the macros. A monitor/event specific header would seem more natural to me. If everything relating to field in struct domain and struct vcpu got put in this one header, we'd end up with a mess. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel