[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



>>> On 20.05.15 at 18:05, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> On 05/20/2015 06:48 PM, Jan Beulich wrote:
>>>>> On 20.05.15 at 17:24, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>> On 05/20/2015 05:53 PM, Jan Beulich wrote:
>>>>>>> On 19.05.15 at 10:31, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>>>> +/* Supported values for the vm_event_write_ctrlreg index. */
>>>>> +#define VM_EVENT_X86_CR0     (1 << 0)
>>>>> +#define VM_EVENT_X86_CR3     (1 << 1)
>>>>> +#define VM_EVENT_X86_CR4     (1 << 2)
>>>>> +#define VM_EVENT_X86_XCR0    (1 << 3)
>>>>
>>>> Why bit masks rather than an enumeration like thing?
>>>
>>> Ack, will change it to an enum. That would have been my first preference
>>> too, but the header just seemed to be more #define-oriented and I tried
>>> to follow suit.
>> 
>> And I didn't say use an enum, I intentionally said enum like.
> 
> I see. They're bitmasks because it makes it easy to use them with the
> X-bit wide (where X is the number of control register event indices)
> flags write_ctrlreg_enabled, write_ctrlreg_sync and
> write_ctrlreg_onchangeonly. It makes no difference for the .index field
> of the actual event, you are right indeed that it would not make sense
> for .index to consist of OR-ed bit masks.
> 
> I can change them to 0, 1, 2, and so on, and use individual single-bit
> bitfields for write_ctrlreg_enabled & friends, but this way makes it
> trivial to add a new control register vm_event: just use 1 << next
> available position and widen the proper domain bitfields by one, and the
> new vm_event is ready to use.

And I don't mind that - have an index where an index is needed,
and use (1 << index) when you want a bit mask.

Jan


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.