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

Huang, Kai
2023-02-14T22:53:42Z

> >
> > 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;
+}