On 26/02/2024 9:25 pm, Yamahata, Isaku wrote:
+struct tdx_enabled {
+ cpumask_var_t enabled;
+ atomic_t err;
+};
+
+static void __init tdx_on(void *_enable)
+{
+ struct tdx_enabled *enable = _enable;
+ int r;
+
+ r = vmx_hardware_enable();
+ if (!r) {
+ cpumask_set_cpu(smp_processor_id(), enable->enabled);
+ r = tdx_cpu_enable();
+ }
+ if (r)
+ atomic_set(&enable->err, r);
+}
+
+static void __init vmx_off(void *_enabled)
+{
+ cpumask_var_t *enabled = (cpumask_var_t *)_enabled;
+
+ if (cpumask_test_cpu(smp_processor_id(), *enabled))
+ vmx_hardware_disable();
+}
+
+int __init tdx_hardware_setup(struct kvm_x86_ops *x86_ops)
+{
+ struct tdx_enabled enable = {
+ .err = ATOMIC_INIT(0),
+ };
+ int r = 0;
+
+ if (!enable_ept) {
+ pr_warn("Cannot enable TDX with EPT disabled\n");
+ return -EINVAL;
+ }
+
+ if (!zalloc_cpumask_var(&enable.enabled, GFP_KERNEL)) {
+ r = -ENOMEM;
+ goto out;
+ }
+
+ /* tdx_enable() in tdx_module_setup() requires cpus lock. */
+ cpus_read_lock();
+ on_each_cpu(tdx_on, &enable, true); /* TDX requires vmxon. */
+ r = atomic_read(&enable.err);
+ if (!r)
+ r = tdx_module_setup();
+ else
+ r = -EIO;
I was thinking why do we need to convert to -EIO.
Convert error code to -EIO unconditionally would cause the original
error code being lost. Although given tdx_on() is called on all online
cpus in parallel, the @enable.err could be imprecise anyway, the
explicit conversion seems not quite reasonable to be done _here_.
I think it would be more reasonable to explicitly set the error code to
-EIO in tdx_on(), where we _exactly_ know what went wrong and can still
possibly do something before losing the error code.
E.g., we can dump the error code to the user, but looks both
vmx_hardware_enable() and tdx_cpu_enable() will do so already so we can
safely lose the error code there.
We can perhaps add a comment to point this out before losing the error
code if that's better:
/*
* Both vmx_hardware_enable() and tdx_cpu_enable() print error
* message when they fail. Just convert the error code to -EIO
* when multiple cpu fault the @err cannot be used to precisely
* record the error code for them anyway.
*/
+ on_each_cpu(vmx_off, &enable.enabled, true);
+ cpus_read_unlock();
+ free_cpumask_var(enable.enabled);
+
+out:
+ return r;
+}
