Re: [GIT PULL] EFI changes for v3.18

[Ingo Molnar <mingo@kernel.org>]

* Matt Fleming <matt@console-pimps.org> wrote:

>  * Implement new EFI runtime lock which is required by the UEFI
>    specification - Ard Biesheuvel

Firstly, under what circumstances can EFI call parallelism happen 
currently? Most of the EFI code runs during early bootup, which 
is serialized.

Secondly, this locking pattern looks pretty disgusting:

@@ -94,7 +187,17 @@ static efi_status_t virt_efi_set_variable(efi_char16_t *name,
                                          unsigned long data_size,
                                          void *data)
 {
-       return efi_call_virt(set_variable, name, vendor, attr, data_size, data);
+       unsigned long flags;
+       efi_status_t status;
+       bool __in_nmi = efi_in_nmi();
+
+       if (!__in_nmi)
+               spin_lock_irqsave(&efi_runtime_lock, flags);
+       status = efi_call_virt(set_variable, name, vendor, attr, data_size,
+                              data);
+       if (!__in_nmi)
+               spin_unlock_irqrestore(&efi_runtime_lock, flags);
+       return status;
 }

and that's repeated in virt_efi_query_variable_info() as well.

and that's the explanation given:

+/*
+ * Some runtime services calls can be reentrant under NMI, even if the table
+ * above says they are not. (source: UEFI Specification v2.4A)
+ *
+ * Table 32. Functions that may be called after Machine Check, INIT and NMI
+ * 
+----------------------------+------------------------------------------+
+ * | Function                  | Called after Machine Check, INIT and NMI |
+ * 
+----------------------------+------------------------------------------+
+ * | GetTime()                 | Yes, even if previously busy.            |
+ * | GetVariable()             | Yes, even if previously busy             |
+ * | GetNextVariableName()     | Yes, even if previously busy             |
+ * | QueryVariableInfo()       | Yes, even if previously busy             |
+ * | SetVariable()             | Yes, even if previously busy             |
+ * | UpdateCapsule()           | Yes, even if previously busy             |
+ * | QueryCapsuleCapabilities()| Yes, even if previously busy             |
+ * | ResetSystem()             | Yes, even if previously busy             |
+ * 
+----------------------------+------------------------------------------+
+ *
+ * In order to prevent deadlocks under NMI, the wrappers for these functions
+ * may only grab the efi_runtime_lock or rtc_lock spinlocks if !efi_in_nmi().
+ * However, not all of the services listed are reachable through NMI code paths,
+ * so the the special handling as suggested by the UEFI spec is only implemented
+ * for QueryVariableInfo() and SetVariable(), as these can be reached in NMI
+ * context through efi_pstore_write().

Are pstore calls into the EFI runtime reentrant?

Thanks,

	Ingo