On Tue, 08 Jul, at 10:54:13AM, Ard Biesheuvel wrote: > > After doing a bit more research, I still think there is work needed if > we aim to adhere to the UEFI spec, or at least be safe from the > hazards it points out. Note that I never claimed there wasn't a need for an EFI runtime lock, I was just pointing out that you need to consider the efi-pstore scenario, and that a mutex isn't suitable in that case. I did in fact make a half-arsed attempt at introducing a runtime lock here, http://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/commit/?h=capsule-blkdev&id=c0a88ac5b21f3c837121748be2e59e995126a6cb Provided we can get away with a single EFI runtime lock like that patch, your recent efi_call_virt() changes actually make the required patch much simpler, at least for arm64 and x86. > So the current status is: > - get/set time calls are serialized with respect to one another using > rtc_lock at the wrapper level The time functions are completely unused on x86, which is why no proper runtime locking exists. It's basically dead code. > - get/set variable calls are serialized using the efivars->lock in the > efivars module > - get_next_variable() calls use the BKL It uses __efivars->lock just like the other variable services. Is that what you meant by BKL? > The two things I am most concerned with are: > - reset system while other calls are in flight; is this handled > implicitly in some other way? No, it isn't handled, so yeah, it needs fixing. I think on x86 we actually wait for other cpus to shutdown before issuing the reset but it's obviously not possible to make that guarantee across architectures. > - things like settime()/setwakeuptime() and setvariable() poking into > the flash at the same time. > > Perhaps it would be sufficient to have a single spinlock cover all > these cases? Or just let efivars grab the rtc_lock as well? I think we need to introduce a separate lock, logically below all other subsystem specific ones (rtc_lock, __efivars->lock, etc). It needs to be the final lock you grab before invoking the runtime services. I don't think the additional complexity of introducing multiple locks to parallelise access to, say, GetTime() and GetVariable(), is really worth the headache. Definitely not without someone making a really compelling case for why they need to squeeze every ounce of performance out of that scenario. -- Matt Fleming, Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
