linux-kernel - Re: [PATCH v9 07/18] x86/virt/tdx: Do TDX module per-cpu initialization

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:   Tue, 14 Feb 2023 22:53:26 +0000
From:   "Huang, Kai" <kai.huang@...el.com>
To:     "peterz@...radead.org" <peterz@...radead.org>
CC:     "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "Hansen, Dave" <dave.hansen@...el.com>,
        "david@...hat.com" <david@...hat.com>,
        "bagasdotme@...il.com" <bagasdotme@...il.com>,
        "ak@...ux.intel.com" <ak@...ux.intel.com>,
        "Wysocki, Rafael J" <rafael.j.wysocki@...el.com>,
        "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
        "Chatre, Reinette" <reinette.chatre@...el.com>,
        "Christopherson,, Sean" <seanjc@...gle.com>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "Yamahata, Isaku" <isaku.yamahata@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "Luck, Tony" <tony.luck@...el.com>,
        "Shahar, Sagi" <sagis@...gle.com>,
        "imammedo@...hat.com" <imammedo@...hat.com>,
        "Gao, Chao" <chao.gao@...el.com>,
        "Brown, Len" <len.brown@...el.com>,
        "sathyanarayanan.kuppuswamy@...ux.intel.com" 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        "Huang, Ying" <ying.huang@...el.com>,
        "Williams, Dan J" <dan.j.williams@...el.com>
Subject: Re: [PATCH v9 07/18] x86/virt/tdx: Do TDX module per-cpu
 initialization


> > 
> > But just checking:
> > 
> > LP.INIT can actually be called in parallel on different cpus (doesn't have to,
> > of course), so we can actually just use on_each_cpu_cond() for LP.INIT:
> > 
> > 	on_each_cpu_cond(should_skip_cpu, smp_func_module_lp_init, NULL, true);
> > 
> > But IIUC Peter doesn't like using IPI and prefers using via work:
> > 
> > https://lore.kernel.org/lkml/Y30dujuXC8wlLwoQ@hirez.programming.kicks-ass.net/
> > 
> > So I used smp_call_on_cpu() here, which only calls @func on one cpu, but not a
> > cpumask.  For LP.INIT ideally we can have something like:
> > 
> > 	schedule_on_cpu(struct cpumask *cpus, work_func_t func);
> > 
> > to call @func on a cpu set, but that doesn't exist now, and I don't think it's
> > worth to introduce it?
> 
> schedule_on_each_cpu() exists and can easily be extended to take a cond
> function if you so please.
> 

Sure.  I just tried to do.  There are two minor things:

1) should I just use smp_cond_func_t directly as the cond function?
2) schedule_on_each_cpu() takes cpus_read_lock() internally.  However in my
case, tdx_enable() already takes that so I need a _locked_ version.

How does below look like? (Not tested)

+/**
+ * schedule_on_each_cpu_cond_locked - execute a function synchronously
+ *                                   on each online CPU for which the
+ *                                   condition function returns positive
+ * @func:      the function to call
+ * @cond_func:  the condition function to call
+ * @cond_data: the data passed to the condition function
+ *
+ * schedule_on_each_cpu_cond_locked() executes @func on each online CPU
+ * when @cond_func returns positive for that cpu, using the system
+ * workqueue and blocks until all CPUs have completed.
+ *
+ * schedule_on_each_cpu_cond_locked() doesn't hold read lock of CPU
+ * hotplug lock but depend on the caller to do.
+ *
+ * schedule_on_each_cpu_cond_locked() is very slow.
+ *
+ * Return:
+ * 0 on success, -errno on failure.
+ */
+int schedule_on_each_cpu_cond_locked(work_func_t func,
+                                    smp_cond_func_t cond_func,
+                                    void *cond_data)
+{
+       int cpu;
+       struct work_struct __percpu *works;
+
+       works = alloc_percpu(struct work_struct);
+       if (!works)
+               return -ENOMEM;
+
+       for_each_online_cpu(cpu) {
+               struct work_struct *work = per_cpu_ptr(works, cpu);
+
+               if (cond_func && !cond_func(cpu, cond_data))
+                       continue;
+
+               INIT_WORK(work, func);
+               schedule_work_on(cpu, work);
+       }
+
+       for_each_online_cpu(cpu)
+               flush_work(per_cpu_ptr(works, cpu));
+
+       free_percpu(works);
+       return 0;
+}
+
+/**
+ * schedule_on_each_cpu_cond - execute a function synchronously on each
+ *                            online CPU for which the condition
+ *                            function returns positive
+ * @func:      the function to call
+ * @cond_func:  the condition function to call
+ * @cond_data: the data passed to the condition function
+ *
+ * schedule_on_each_cpu_cond() executes @func on each online CPU
+ * when @cond_func returns positive for that cpu, using the system
+ * workqueue and blocks until all CPUs have completed.
+ *
+ * schedule_on_each_cpu_cond() is very slow.
+ *
+ * Return:
+ * 0 on success, -errno on failure.
+ */
+int schedule_on_each_cpu_cond(work_func_t func,
+                             smp_cond_func_t cond_func,
+                             void *cond_data)
+{
+       int ret;
+
+       cpus_read_lock();
+
+       ret = schedule_on_each_cpu_cond_locked(func, cond_func, cond_data);
+
+       cpus_read_unlock();
+
+       return ret;
+}