Re: [Xen-devel] [PATCH V3] x86/vm_event: Added support for VM_EVENT_REASON_INTERRUPT

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.11.16 at 11:32, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> On 11/11/2016 12:26 PM, Jan Beulich wrote:
>>>>> On 11.11.16 at 11:15, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>> > On 11/11/2016 12:02 PM, Jan Beulich wrote:
>>>>>>> >>>>> On 11.11.16 at 09:06, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>>>> >>> --- a/xen/include/asm-x86/domain.h
>>>>> >>> +++ b/xen/include/asm-x86/domain.h
>>>>> >>> @@ -576,6 +576,10 @@ struct arch_vcpu
>>>>> >>>      XEN_GUEST_HANDLE(vcpu_time_info_t) time_info_guest;
>>>>> >>>  
>>>>> >>>      struct arch_vm_event *vm_event;
>>>>> >>> +
>>>>> >>> +    struct {
>>>>> >>> +        unsigned int next_interrupt_enabled : 1;
>>>> >> 
>>>> >> bool? Stray spaces. And then (sorry for thinking of this only now) - is
>>>> >> this really usefully an arch-specific flag? I guess there's nothing
>>>> >> precluding this from also being implemented on ARM eventually?
>>> > 
>>> > Stray spaces? Do you mean the newline between "struct arch_vm_event
>>> > *vm_event;" and "struct {"?
>> No. I mean the ones around the colon.
> 
> I'm sorry, I don't follow. The examples I've pasted in the previous
> reply make similar use of the colon:
> 
> 399     /* Arch-specific monitor options */
> 400     struct {
> 401         unsigned int write_ctrlreg_enabled       : 4;
> 402         unsigned int write_ctrlreg_sync          : 4;
> 403         unsigned int write_ctrlreg_onchangeonly  : 4;
> 404         unsigned int singlestep_enabled          : 1;
> 405         unsigned int software_breakpoint_enabled : 1;
> 406         unsigned int debug_exception_enabled     : 1;
> 407         unsigned int debug_exception_sync        : 1;
> 408         unsigned int cpuid_enabled               : 1;
> 409         struct monitor_msr_bitmap *msr_bitmap;
> 410     } monitor;
> 
> and
> 
> 130     /* Monitor options */
> 131     struct {
> 132         uint8_t privileged_call_enabled : 1;
> 133     } monitor;
> 
> I take that you would prefer this?
> 
> unsigned int next_interrupt_enabled:1;
> 
> I have nothing against the change, I'm just confused about what the
> proper and consistent way of writing that is.

grep-ing the include/ subtree I see that there are (apart from the
quoted ones) examples of all kinds, so I guess it can as well stay as
is, even if I personally consider the blanks stray here.

Jan


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