Re: [Xen-devel] [PATCH V4] xen/vm_event: Clean up control-register-write vm_events

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.05.15 at 09:50, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> On 05/22/2015 10:36 AM, Jan Beulich wrote:
>>>>> On 21.05.15 at 15:00, <rcojocaru@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel