linux-kernel - Re: [PATCH v2 1/2] x86/fpu: Extend kernel_fpu_begin

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]

Message-ID: <62e2ebd9-4afa-4bb1-a30c-5ba34bc6b94e@intel.com>
Date: Wed, 8 May 2024 11:03:22 -0700
From: "Chang S. Bae" <chang.seok.bae@...el.com>
To: Dave Hansen <dave.hansen@...el.com>, <linux-kernel@...r.kernel.org>
CC: <x86@...nel.org>, <platform-driver-x86@...r.kernel.org>,
	<tglx@...utronix.de>, <mingo@...hat.com>, <bp@...en8.de>,
	<dave.hansen@...ux.intel.com>, <hdegoede@...hat.com>,
	<ilpo.jarvinen@...ux.intel.com>, <tony.luck@...el.com>,
	<ashok.raj@...el.com>, <jithu.joseph@...el.com>
Subject: Re: [PATCH v2 1/2] x86/fpu: Extend kernel_fpu_begin_mask() to
 initialize AMX state

On 5/8/2024 7:40 AM, Dave Hansen wrote:
> On 5/7/24 16:53, Chang S. Bae wrote:
> 
>> However, due to resource constraints in storage, AMX state is excluded
>> from the scope of state recovery. Consequently, AMX state must be in its
>> initialized state for the IFS test to run.
> 
> This doesn't mention how this issue got introduced.  Are we all bad at
> reading the SDM? :)

Ah, I'd rather zap out this SDM sentence.

>> When AMX workloads are running, an active user AMX state remains even
>> after a context switch, optimizing to reduce the state reload cost. In
>> such cases, the test cannot proceed if it is scheduled.
> 
> This is a bit out of the blue.  What does scheduling have do do with IFS?

$ echo <cpu#> > /sys/devices/virtual/misc/intel_ifs_0/run_test

Then,
run_test_store()
-> do_core_test()
   -> ifs_test_core()
     -> stop_core_cpuslocked()
       -> stop_cpus()
         -> queue_stop_cpus_work()
           -> cpu_stop_queue_work()
             -> wake_q_add()
             -> wake_up_q()

So, the CPU stopper threads for <cpu#> and its sibling to execute 
doscan() are queued up with the highest priority.

queue_stop_cpus_work() has

	for_each_cpu(cpu, cpumask) {
		work = &per_cpu(cpu_stopper.stop_work, cpu);
		work->fn = fn;
		work->arg = arg;
		work->done = done;
		work->caller = _RET_IP_;
		if (cpu_stop_queue_work(cpu, work))
			queued = true;
	}

Those threads are created during early boot via 
smpboot_register_percpu_thread().

> I'm not sure those last two paragraphs add much value.  I'd try to
> banish most of that content to *after* you talk about the solution.  Or
> maybe put it in the cover letter.

It looks like lots of distractions coming from bunch of alternatives in 
different levels.

Thanks,
Chang

PS: Let me respond the solution discussion separately. I do want to 
experiment the init-track behavior a bit.