RE: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface
On Mar 5, 2015 1:19 AM, "Kweh, Hock Leong" wrote: > > > -Original Message- > > From: Andy Lutomirski [mailto:l...@amacapital.net] > > Sent: Wednesday, March 04, 2015 4:38 AM > > > > On Mon, Mar 2, 2015 at 9:56 PM, Kweh, Hock Leong > > wrote: > > > > > > Just to call out that using firmware class auto locate binary feature is > > > limited > > > to locations: > > > - "/lib/firmware/updates/" UTS_RELEASE, > > > - "/lib/firmware/updates", > > > - "/lib/firmware/" UTS_RELEASE, > > > - "/lib/firmware" > > > and one custom path which inputted during firmware_class module load > > > time or kernel boot up time. > > > > > > It is just not like the user helper interface which allow load the binary > > > at > > > any path/location. > > > > > > This really is not a big deal. User should cope with it. > > > > No, it's a big deal, and the user should not cope. > > > > The user *should not* be required to have write access to anything in > > /lib to install a UEFI capsule that they download from their > > motherboard vendor's website. /lib belongs to the distro, and UEFI > > capsules do not belong to the distro. In this regard, UEFI capsules > > are completely unlike your wireless card firmware, your cpu microcode, > > etc. > > > > Imagine systems using NFS root, Atomic-style systems (e.g. ostree), > > systems that boot off squashfs, etc. They should still be able to > > load capsules. The basic user interface that should work is: > > > > # uefi-load-capsule /path/to/capsule > > > > or: > > > > # uefi-load-capsule - > > > I don't really care how uefi-load-capsule is implemented, as long as > > it's straightforward, because people will screw it up if it isn't > > straightforward. > > > > Why is it so hard to have a file in sysfs that you write the capsule > > to using *cat* (not echo) and that will return an error code if cat > > fails? Is it because you don't know where the end of the capsule is? > > if so, ioctl is designed for exactly this purpose. > > > > TBH, I find this thread kind of ridiculous. The problem that you're > > trying to solve is extremely simple, the functionality that userspace > > needs is trivial, and all of these complex proposals for how it should > > work are an artifact of the fact that the kernel-internal interfaces > > you're using for it are not well suited to the problem at hand. > > > > --Andy > > Sorry, I may not catch your point correctly. Are you trying to tell that > a "normal" user can perform efi capsule update. But a "normal" user > does not have the right to install or copy the capsule binary into > "/lib/firmware/". So, there is a need to make this capsule module to > allow uploading the capsule binary at any path or location other than > "/lib/firmware/". > > Is this what you mean? No. Only root should be able to load capsules, but even root may not be able to write to /lib. --Andy > > > Regards, > Wilson -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64/efi: use UEFI ResetSystem() Runtime Service for system reset
Hi Ard,
On Thu, Mar 05, 2015 at 12:51:11PM +, Ard Biesheuvel wrote:
> If UEFI Runtime Services are available, the ResetSystem() service should
> be preferred over direct PSCI calls or other methods to reset the system.
> The reason is that the UpdateCapsule() UEFI Runtime Service, which is used
> to perform firmware updates, relies on this.
>
> Signed-off-by: Ard Biesheuvel
> ---
> I sent roughly the same patch ~6 months ago, but at the time, we were still
> waiting for the restart notifier call chain patches to land. Since that code
> got rejected, I am proposing this again. Note that efi_enabled(x) always
> evaluates to 'false' on !CONFIG_EFI.
Also, efi_reboot is a static inline for !CONFIG_EFI, so I can't see any
possibility of a build failure.
> This fixes reboot on my Seattle [although it doesn't make it any faster :-)]
>
> arch/arm64/kernel/process.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index fde9923af859..a52bc0c316a8 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -21,6 +21,7 @@
> #include
>
> #include
> +#include
> #include
> #include
> #include
> @@ -150,6 +151,14 @@ void machine_restart(char *cmd)
> local_irq_disable();
> smp_send_stop();
>
> + /*
> + * Prefer reboot via EFI if available, so that capsule updates [which
> + * rely on UEFI's ResetSystem() being called with the return value of
> + * UpdateCapsule()] have a chance of working as expected.
> + */
> + if (efi_enabled(EFI_RUNTIME_SERVICES))
> + efi_reboot(reboot_mode, NULL);
I expect that the particulars of the UpdateCapsule() will be handled
within efi_reboot and won't require any additions here. So the comment
could just be trimmed to say that UpdateCapsule() depends on the system
being reset with ResetSystem().
Also, we need to make sure we call efi_poweroff to make UpdateCapsule()
work when shutting the machine down (behind the scenes efi_poweroff
calls ResetSystem(EfiResetShutdown, ...)).
For that I think adding the following to arch/arm64/kernel/efi.c is
sufficient:
/*
* UpdateCapsule() depends on the system being shutdown via
* ResetSystem().
*/
bool efi_poweroff_required(void)
{
return efi_enabled(EFI_RUNTIME_SERVICES);
}
I've given the patch a spin (with and without that addition) on Juno and
Seattle. So with that folded in:
Tested-by: Mark Rutland
Reviewed-by: Mark Rutland
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] arm64/efi: use UEFI ResetSystem() Runtime Service for system reset
If UEFI Runtime Services are available, the ResetSystem() service should be preferred over direct PSCI calls or other methods to reset the system. The reason is that the UpdateCapsule() UEFI Runtime Service, which is used to perform firmware updates, relies on this. Signed-off-by: Ard Biesheuvel --- I sent roughly the same patch ~6 months ago, but at the time, we were still waiting for the restart notifier call chain patches to land. Since that code got rejected, I am proposing this again. Note that efi_enabled(x) always evaluates to 'false' on !CONFIG_EFI. This fixes reboot on my Seattle [although it doesn't make it any faster :-)] arch/arm64/kernel/process.c | 9 + 1 file changed, 9 insertions(+) diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index fde9923af859..a52bc0c316a8 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -150,6 +151,14 @@ void machine_restart(char *cmd) local_irq_disable(); smp_send_stop(); + /* +* Prefer reboot via EFI if available, so that capsule updates [which +* rely on UEFI's ResetSystem() being called with the return value of +* UpdateCapsule()] have a chance of working as expected. +*/ + if (efi_enabled(EFI_RUNTIME_SERVICES)) + efi_reboot(reboot_mode, NULL); + /* Now call the architecture specific reboot code. */ if (arm_pm_restart) arm_pm_restart(reboot_mode, cmd); -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface
> -Original Message- > From: Andy Lutomirski [mailto:l...@amacapital.net] > Sent: Wednesday, March 04, 2015 4:38 AM > > On Mon, Mar 2, 2015 at 9:56 PM, Kweh, Hock Leong > wrote: > > > > Just to call out that using firmware class auto locate binary feature is > > limited > > to locations: > > - "/lib/firmware/updates/" UTS_RELEASE, > > - "/lib/firmware/updates", > > - "/lib/firmware/" UTS_RELEASE, > > - "/lib/firmware" > > and one custom path which inputted during firmware_class module load > > time or kernel boot up time. > > > > It is just not like the user helper interface which allow load the binary at > > any path/location. > > > > This really is not a big deal. User should cope with it. > > No, it's a big deal, and the user should not cope. > > The user *should not* be required to have write access to anything in > /lib to install a UEFI capsule that they download from their > motherboard vendor's website. /lib belongs to the distro, and UEFI > capsules do not belong to the distro. In this regard, UEFI capsules > are completely unlike your wireless card firmware, your cpu microcode, > etc. > > Imagine systems using NFS root, Atomic-style systems (e.g. ostree), > systems that boot off squashfs, etc. They should still be able to > load capsules. The basic user interface that should work is: > > # uefi-load-capsule /path/to/capsule > > or: > > # uefi-load-capsule - > I don't really care how uefi-load-capsule is implemented, as long as > it's straightforward, because people will screw it up if it isn't > straightforward. > > Why is it so hard to have a file in sysfs that you write the capsule > to using *cat* (not echo) and that will return an error code if cat > fails? Is it because you don't know where the end of the capsule is? > if so, ioctl is designed for exactly this purpose. > > TBH, I find this thread kind of ridiculous. The problem that you're > trying to solve is extremely simple, the functionality that userspace > needs is trivial, and all of these complex proposals for how it should > work are an artifact of the fact that the kernel-internal interfaces > you're using for it are not well suited to the problem at hand. > > --Andy Sorry, I may not catch your point correctly. Are you trying to tell that a "normal" user can perform efi capsule update. But a "normal" user does not have the right to install or copy the capsule binary into "/lib/firmware/". So, there is a need to make this capsule module to allow uploading the capsule binary at any path or location other than "/lib/firmware/". Is this what you mean? Regards, Wilson
