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_kib...@epam.com>
>> CC: Jan Beulich <jbeul...@suse.com>
>
> 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
