Re: [PATCH V2] xen/vm_event: Clean up control-register-write vm_events

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 05/14/2015 08:31 PM, Tamas K Lengyel wrote:
> On Thu, May 14, 2015 at 7:11 PM, Razvan Cojocaru
> <rcojocaru@bitdefender.com> wrote:
>> On 05/14/2015 07:55 PM, Tamas K Lengyel wrote:
>>>> diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
>>>> index 45b5283..1dd49dd 100644
>>>> --- a/xen/include/asm-x86/domain.h
>>>> +++ b/xen/include/asm-x86/domain.h
>>>> @@ -341,19 +341,13 @@ struct arch_domain
>>>>
>>>>      /* Monitor options */
>>>>      struct {
>>>> -        uint16_t mov_to_cr0_enabled          : 1;
>>>> -        uint16_t mov_to_cr0_sync             : 1;
>>>> -        uint16_t mov_to_cr0_onchangeonly     : 1;
>>>> -        uint16_t mov_to_cr3_enabled          : 1;
>>>> -        uint16_t mov_to_cr3_sync             : 1;
>>>> -        uint16_t mov_to_cr3_onchangeonly     : 1;
>>>> -        uint16_t mov_to_cr4_enabled          : 1;
>>>> -        uint16_t mov_to_cr4_sync             : 1;
>>>> -        uint16_t mov_to_cr4_onchangeonly     : 1;
>>>> -        uint16_t mov_to_msr_enabled          : 1;
>>>> -        uint16_t mov_to_msr_extended         : 1;
>>>> -        uint16_t singlestep_enabled          : 1;
>>>> -        uint16_t software_breakpoint_enabled : 1;
>>>> +        uint32_t write_ctrlreg_enabled       : 8;
>>>> +        uint32_t write_ctrlreg_sync          : 8;
>>>> +        uint32_t write_ctrlreg_onchangeonly  : 8;
>>>
>>> Any particular reason why you have these bitmaps 8-bits wide? There
>>> are only 4 events defined at the moment that would use these.
>>
>> ARM control registers have been mentioned, so I thought I would leave
>> some space for a few more events. Other than that, they don't _need_ to
>> be 8-bits wide. If compactness matters more I'll change them to 4.
>>
>>
>> Thanks,
>> Razvan
> 
> IMHO it's better to widen the field when there is an actual need for
> it. For now I think 4 would be appropriate.

Fair enough. I'll change it to 4 and we'll just have to be careful when
patches touching these events come in.


Thanks,
Razvan