On Mon, Jul 6, 2015 at 6:26 AM, Jan Beulich <jbeul...@suse.com> wrote:
> >>> On 30.06.15 at 16:48, <tleng...@novetta.com> wrote:
> >> > --- a/xen/include/asm-x86/domain.h
> >
> >> >> +++ b/xen/include/asm-x86/domain.h
> >> >> @@ -342,13 +342,15 @@ struct arch_domain
> >> >>
> >> >> /* Monitor options */
> >> >> struct {
> >> >> - uint16_t write_ctrlreg_enabled : 4;
> >> >> - uint16_t write_ctrlreg_sync : 4;
> >> >> - uint16_t write_ctrlreg_onchangeonly : 4;
> >> >> - 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 : 4;
> >> >> + uint32_t write_ctrlreg_sync : 4;
> >> >> + uint32_t write_ctrlreg_onchangeonly : 4;
> >> >> + uint32_t mov_to_msr_enabled : 1;
> >> >> + uint32_t mov_to_msr_extended : 1;
> >> >> + uint32_t singlestep_enabled : 1;
> >> >> + uint32_t software_breakpoint_enabled : 1;
> >> >> + uint32_t request_enabled : 1;
> >> >> + uint32_t request_sync : 1;
> >> >
> >> > Can you please switch to plain unsigned int if you already have to
> >> > touch this? There's no reason I can see to use a fixed width integer
> >> > type here.
> >>
> >> Ack, will make it plain int.
> >
> >
> > IMHO having it fix-width is easier to read when adding new elements to
> see
> > how many bits we have left free. I would not want this changed unless
> there
> > is a clear benefit to doing so.
>
> I can't see what benefit using fixed widht types here is: Best
> demonstrated with uint64_t, gcc doesn't honor the type of the
> field anyway. And hence counting the number of left bits (with
> some unknown definition of "left") is quite pointless too - it's an
> internal structure, and when a new bit field needs to be added,
> it doesn't really matter whether it grows the structure (at least
> no more than any other field addition).
>
> Bottom line - no fixed width types when not really warranted,
> please.
>
> Jan
>
SGTM.
Thanks,
Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
