linux-kernel - Re: [PATCH v6 08/11] platform/x86/intel/ifs: Add scan test support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 06 May 2022 21:06:32 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     "Luck, Tony" <tony.luck@...el.com>
Cc:     hdegoede@...hat.com, markgross@...nel.org, mingo@...hat.com,
        bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org,
        hpa@...or.com, corbet@....net, gregkh@...uxfoundation.org,
        andriy.shevchenko@...ux.intel.com, jithu.joseph@...el.com,
        ashok.raj@...el.com, rostedt@...dmis.org, dan.j.williams@...el.com,
        linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
        platform-driver-x86@...r.kernel.org, patches@...ts.linux.dev,
        ravi.v.shankar@...el.com
Subject: Re: [PATCH v6 08/11] platform/x86/intel/ifs: Add scan test support

On Fri, May 06 2022 at 11:49, Luck, Tony wrote:
> On Fri, May 06, 2022 at 03:30:30PM +0200, Thomas Gleixner wrote:
>> 1) How is that supposed to work on a system which has HT enabled in BIOS,
>>    but disabled on the kernel command line or via /sys/..../smt/control or
>>    when a HT sibling is offlined temporarily?
>> 
>>    I assume it cannot work, but I can't see anything which handles those
>>    cases.
>
> Correct. If HT is disabled in BIOS, then there is no other thread, so
> core tests just use a single thread.
>
> If a logical CPU is "offline" due to Linux actions, then core test will
> fail. In an earlier version we did attempt to detect this before trying
> to run the test. But we didn't find a simple way to determine that a
> core has one thread online, and another offline. Rather than a bunch of
> code to detect an operator error it seemed better to let it run &
> fail.

Fair enough.

> GregKH wasn't a fan of this itty bitty driver cluttering up
> Documentation/x86. He said:
>
>    I don't know which is better, it's just that creating a whole new
>    documentation file for a single tiny driver feels very odd as it will
>    get out of date and is totally removed from the driver itself.
>
>    I'd prefer that drivers be self-contained, including the documentation,
>    as it is much more obvious what is happening with that.  Spreading stuff
>    around the tree only causes stuff to get out of sync easier.

Well, I agree to some extent, but the documentation which I want to see
is documentation for admins. I'm not sure whether we want them to search
the code. Those are consumers of Documentation/ AFAICT.

> So the documentation patch was dropped after v3. Last version here:
>
> https://lore.kernel.org/r/20220419163859.2228874-3-tony.luck@intel.com
>
> That doc would need pathnames updated to match the move from a platform
> device to a virtual misc device. But otherwise seems still accurate.
>
> Does that cover what you want from documentation for this driver
> (wherever it gets located in the tree)? Are you looking for more?

It's pretty detailed on the inner workings, but lacks a big fat warning
for the admin vs. the impact, i.e. that it makes the core go out for
lunch for a while, which has consequences on workloads and interrupts
directed at that core. Plus some explanation vs. the HT (SMT=off, soft
offline) case above. Similar to what we have e.g. for buslocks.

Thanks,

        tglx