Re: [Xen-devel] [PATCH v2 2/2] x86/spec-ctrl: add support for modifying SSBD VIA LS_CFG MSR

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.08.18 at 20:45, <brian.woods@xxxxxxx> wrote:
> On Fri, Aug 17, 2018 at 12:59:28AM -0600, Jan Beulich wrote:
>> >>> On 16.08.18 at 22:02, <brian.woods@xxxxxxx> wrote:
>> > On Wed, Aug 15, 2018 at 10:00:48AM -0600, Jan Beulich wrote:
>> >> > +bool __read_mostly ssbd_amd_smt_en = false;
>> >> > +bool __read_mostly default_xen_ssbd_amd_ls_cfg_en = false;
>> >> >  uint64_t __read_mostly ssbd_amd_ls_cfg_mask = 0ull;
>> >> > +struct ssbd_amd_ls_cfg_smt_status 
>> >> > *ssbd_amd_smt_status[SSBD_AMD_MAX_SOCKET] = {NULL};
>> >> 
>> >> Several further pointless initializers.
>> > 
>> > ssbd_amd_ls_cfg_mask -> needs to be initialized, due to how it gets set
>> > ssbd_amd_ls_cfg_smt_status -> needs to be initialized, unless xalloc
>> >                               sets it as NULL on failure to alloc
>> > default_xen_ssbd_amd_ls_cfg_en -> needs to be initialized of an else
>> >                                   added to an if statement
>> > ssbd_amd_smt_en -> like the above
>> > 
>> > If you want default_xen_ssbd_amd_ls_cfg_en and ssbd_amd_smt_en to be
>> > not be initialized, I can add code to do that.
>> 
>> I don't understand: Add code? The initializers here are all pointless
>> because the values you supply are what the variables would be
>> initialized with anyway if you omitted the initializers. That's what
>> the C standard specifies.
> 
> Leaving values which determine the behavior of the hypervisor to
> defaults of the compiler's implementation of the standard seems like it
> would be a possible source of subtle bugs when the compiler doesn't do
> something just right.  I'd much rather have an initialized value or
> have it set in the code before use.
> 
> If you strongly feel that they shouldn't be initialized or set in code,
> I'll change them though.

Yes, I do feel strongly about this: We omit pointless initializers
elsewhere, and the comment I've given here is a pretty common one
to be made in reviews. If we were to write code being prepared for
compiler bugs (which standards-non-conformance is), we could as
well stop writing any code in the first place. Workarounds are
acceptable only for _known_ compiler defects.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel