Re: [XEN PATCH v2 3/5] x86/spec-ctrl: configurable Intlel/AMD-specific calculations

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.08.2024 21:25, Andrew Cooper wrote:
> On 16/08/2024 12:14 pm, Sergiy Kibrik wrote:
>> Put platforms-specific code under #ifdef CONFIG_{AMD,INTEL} so that when
>> corresponding CPU support is disabled by configuration less dead code will 
>> end
>> up in the build.
>>
>> This includes re-ordering of calls to ibpb_calculations() & 
>> div_calculations(),
>> but since they don't access common variables or feature bits it should be
>> safe to do.
>>
>> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@xxxxxxxx>
>> CC: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Sorry, but no.
> 
> This logic is security critical, highly fragile, gets chopped/changed
> multiple times a year (as researchers keep on finding new things), and
> all major work is done to it under embargo.
> 
> Just look at the history of the file.
> 
> The ifdefary around the tsx_init() call is bad enough and I intend to
> revert it and do that differently.  I would have objected if I'd got to
> the patch in time.
> 
> 
> The only relevant cost in this file is whether I (and the other security
> team members) can edit it correctly or not in staging and all prior
> in-support branches.  You really don't want to know how many times
> there's been a bug in backports...
> 
> Saving 451 lines from certification is not cheaper than the
> problems/risks you're introducing with this change.

Did you see my earlier reply? I don't think the issue is with hiding source
lines. We want to have the compiler DCE stuff wherever possible, hence why
I did respond asking to switch to IS_ENABLED(). That imo fits pretty well
with the vendor checks we have there already anyway.

Jan