On 1/19/23 17:47, Johan Hovold wrote:
On Wed, Jan 18, 2023 at 09:45:18PM +0100, Maximilian Luz wrote:
On 1/17/23 09:24, Johan Hovold wrote:
On Sun, Jul 24, 2022 at 12:49:48AM +0200, Maximilian Luz wrote:
+module_platform_driver(qcom_uefisecapp_driver);
I noticed that for efivarfs to work, you're currently relying on having
the firmware still claim that the variable services are supported in the
RT_PROP table so that efi core registers the default ops at subsys init
time (which are later overridden by this driver).
Otherwise efivarfs may fail to initialise when built in:
static __init int efivarfs_init(void)
{
if (!efivars_kobject())
return -ENODEV;
return register_filesystem(&efivarfs_type);
}
module_init(efivarfs_init);
With recent X13s firmware the corresponding bit in the RT_PROP table has
been cleared so that efivarfs would fail to initialise. Similar problem
when booting with 'efi=noruntime'.
One way to handle this is to register also the qcom_uefisecapp_driver at
subsys init time and prevent it from being built as a module (e.g. as is
done for the SCM driver). I'm using the below patch for this currently.
So I've had another look and I'm not sure this will work reliably:
First, you are correct in case the RT_PROP table is cleared. In that
case, using subsys_initcall() will move the efivar registration before
the efivarfs_init() call.
However, in case EFI indicates support for variables, we will then have
generic_ops_register() and the uefisecapp's driver call running both in
subsys_initcall(). So if I'm not mistaken, this could cause the generic
ops to be registered after the uefisecapp ones, which we want to avoid.
Good catch, I was using 'efi=noruntime' on the CRD so I did not notice
that race.
One solution is bumping uefisecapp to fs_initcall(). Or do you have any
other suggestions?
I think it would be best to avoid that if we can, but that should work.
The problem here is that the firmware claims to support the EFI variable
services even when it clearly does not and the corresponding callbacks
just return EFI_UNSUPPORTED. As far as I understand, this is still spec
compliant though so we just need to handle that.
One way to address this could be to have efi core not register the
default efivars ops in this case. That would require checking that the
services are indeed available by making one of those calls during
initialisation.
This would however expose the fact that the Google SMI implementation
implicitly relies on overriding the default ops, but I think that's a
good thing as what we have now is racy in multiple ways.
Instead I think we should move the efivarfs availability check from
module init to mount time. That should allow the Google driver, and your
SCM implementation, to continue to be built as modules.
Any consumers (e.g. the Qualcomm RTC driver) would instead need to
check if efivars is available or else defer probe.
Alternatively, it seems all efivars implementation would need to be
always-built in which is not ideal for generic kernels.
I just posted a series here as food for thought:
https://lore.kernel.org/r/20230119164255.28091-1-johan+linaro@xxxxxxxxxx
Thanks, I agree that those checks are probably the better option.
Regards,
Max
