[PATCH v2 0/2] EFI: Reset system after capsule-on-disk
Hi,
Here is the patch to reset after capsule-on-disk. This version fixes some bugs and remove kconfig for the reset (which uses panic).
The reset after completing the capsule-on-disk is stated in the UEFI specification 2.9, section 8.5.5 "Delivery of Capsules via file on Mass Storage device" as below,
In all cases that a capsule is identified for processing the system is restarted after capsule processing is completed.
Thank you,
---
Masami Hiramatsu (2): efi_loader: Avoid using efi_update_capsule() from update capsule on disk efi_loader: Reset system after CapsuleUpdate on disk
lib/efi_loader/efi_capsule.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
-- Masami Hiramatsu masami.hiramatsu@linaro.org
The efi_update_capsule() may have to handle the capsule flags as an UEFI runtime and boottime service, but the capsule-on-disk process doesn't. Thus, the capsule-on-disk should use the efi_capsule_update_firmware() directly instead of efi_update_capsule().
Suggested-by: AKASHI Takahiro takahiro.akashi@linaro.org Signed-off-by: Masami Hiramatsu masami.hiramatsu@linaro.org --- Changes in v2: - Fix to pass correct pointer to efi_capsule_update_firmware - Remove ESRT generation, because this part anyway will be removed next patch. --- lib/efi_loader/efi_capsule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 4463ae00fd..1ec7ea29ff 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -1118,7 +1118,7 @@ efi_status_t efi_launch_capsules(void) index = 0; ret = efi_capsule_read_file(files[i], &capsule); if (ret == EFI_SUCCESS) { - ret = EFI_CALL(efi_update_capsule(&capsule, 1, 0)); + ret = efi_capsule_update_firmware(capsule); if (ret != EFI_SUCCESS) log_err("Applying capsule %ls failed\n", files[i]);
hi Masami,
On Tue, 1 Feb 2022 at 14:03, Masami Hiramatsu masami.hiramatsu@linaro.org wrote:
The efi_update_capsule() may have to handle the capsule flags as an UEFI runtime and boottime service, but the capsule-on-disk process doesn't. Thus, the capsule-on-disk should use the efi_capsule_update_firmware() directly instead of efi_update_capsule().
Suggested-by: AKASHI Takahiro takahiro.akashi@linaro.org Signed-off-by: Masami Hiramatsu masami.hiramatsu@linaro.org
Changes in v2:
- Fix to pass correct pointer to efi_capsule_update_firmware
- Remove ESRT generation, because this part anyway will be removed next patch.
lib/efi_loader/efi_capsule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 4463ae00fd..1ec7ea29ff 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -1118,7 +1118,7 @@ efi_status_t efi_launch_capsules(void) index = 0; ret = efi_capsule_read_file(files[i], &capsule); if (ret == EFI_SUCCESS) {
ret = EFI_CALL(efi_update_capsule(&capsule, 1, 0));
ret = efi_capsule_update_firmware(capsule);
I believe this is not fixing any issue as such. If so, I would vote for keeping the call to efi_update_capsule. With the FWU Multi Bank feature enabled, the checks for capsule acceptance and revert are being done in this function. The reason I have put this code in the function is that it caters to both scenarios of capsule-on-disk and the runtime functionality. In addition, the FWU bootup checks are also done in this function through a call to fwu_update_checks_pass. So if this is not a fix, which I don't think it is, I would prefer this call to remain.
-sughosh
if (ret != EFI_SUCCESS) log_err("Applying capsule %ls failed\n", files[i]);
Am 1. Februar 2022 16:42:43 MEZ schrieb Sughosh Ganu sughosh.ganu@linaro.org:
hi Masami,
On Tue, 1 Feb 2022 at 14:03, Masami Hiramatsu masami.hiramatsu@linaro.org wrote:
The efi_update_capsule() may have to handle the capsule flags as an UEFI runtime and boottime service, but the capsule-on-disk process doesn't. Thus, the capsule-on-disk should use the efi_capsule_update_firmware() directly instead of efi_update_capsule().
Suggested-by: AKASHI Takahiro takahiro.akashi@linaro.org Signed-off-by: Masami Hiramatsu masami.hiramatsu@linaro.org
Changes in v2:
- Fix to pass correct pointer to efi_capsule_update_firmware
- Remove ESRT generation, because this part anyway will be removed next patch.
lib/efi_loader/efi_capsule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 4463ae00fd..1ec7ea29ff 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -1118,7 +1118,7 @@ efi_status_t efi_launch_capsules(void) index = 0; ret = efi_capsule_read_file(files[i], &capsule); if (ret == EFI_SUCCESS) {
ret = EFI_CALL(efi_update_capsule(&capsule, 1, 0));
ret = efi_capsule_update_firmware(capsule);I believe this is not fixing any issue as such. If so, I would vote for keeping the call to efi_update_capsule.
No, this is just about reducing code size by avoiding the EFI_CALL(). It should not change behaviour.
Best regards
Heinrich
With the FWU Multi Bank
feature enabled, the checks for capsule acceptance and revert are being done in this function. The reason I have put this code in the function is that it caters to both scenarios of capsule-on-disk and the runtime functionality. In addition, the FWU bootup checks are also done in this function through a call to fwu_update_checks_pass. So if this is not a fix, which I don't think it is, I would prefer this call to remain.
-sughosh
if (ret != EFI_SUCCESS) log_err("Applying capsule %ls failed\n", files[i]);
On Tue, 1 Feb 2022 at 22:14, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 1. Februar 2022 16:42:43 MEZ schrieb Sughosh Ganu sughosh.ganu@linaro.org:
hi Masami,
On Tue, 1 Feb 2022 at 14:03, Masami Hiramatsu masami.hiramatsu@linaro.org wrote:
The efi_update_capsule() may have to handle the capsule flags as an UEFI runtime and boottime service, but the capsule-on-disk process doesn't. Thus, the capsule-on-disk should use the efi_capsule_update_firmware() directly instead of efi_update_capsule().
Suggested-by: AKASHI Takahiro takahiro.akashi@linaro.org Signed-off-by: Masami Hiramatsu masami.hiramatsu@linaro.org
Changes in v2:
- Fix to pass correct pointer to efi_capsule_update_firmware
- Remove ESRT generation, because this part anyway will be removed next patch.
lib/efi_loader/efi_capsule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 4463ae00fd..1ec7ea29ff 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -1118,7 +1118,7 @@ efi_status_t efi_launch_capsules(void) index = 0; ret = efi_capsule_read_file(files[i], &capsule); if (ret == EFI_SUCCESS) {
ret = EFI_CALL(efi_update_capsule(&capsule, 1, 0));
ret = efi_capsule_update_firmware(capsule);I believe this is not fixing any issue as such. If so, I would vote for keeping the call to efi_update_capsule.
No, this is just about reducing code size by avoiding the EFI_CALL(). It should not change behaviour.
Okay, in that case, I will put a check for the FWU Multi Banks feature being enabled -- with the feature enabled, the call will be to efi_update_capsule, and with the feature disabled, the call will be made to efi_capsule_update_firmware. The compiler should compile out the code whenever the FWU feature is disabled and that will not impact the code size.
-sughosh
Best regards
Heinrich
With the FWU Multi Bank
feature enabled, the checks for capsule acceptance and revert are being done in this function. The reason I have put this code in the function is that it caters to both scenarios of capsule-on-disk and the runtime functionality. In addition, the FWU bootup checks are also done in this function through a call to fwu_update_checks_pass. So if this is not a fix, which I don't think it is, I would prefer this call to remain.
-sughosh
if (ret != EFI_SUCCESS) log_err("Applying capsule %ls failed\n", files[i]);
On Tue, Feb 01, 2022 at 10:33:20PM +0530, Sughosh Ganu wrote:
On Tue, 1 Feb 2022 at 22:14, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 1. Februar 2022 16:42:43 MEZ schrieb Sughosh Ganu sughosh.ganu@linaro.org:
hi Masami,
On Tue, 1 Feb 2022 at 14:03, Masami Hiramatsu masami.hiramatsu@linaro.org wrote:
The efi_update_capsule() may have to handle the capsule flags as an UEFI runtime and boottime service, but the capsule-on-disk process doesn't. Thus, the capsule-on-disk should use the efi_capsule_update_firmware() directly instead of efi_update_capsule().
Suggested-by: AKASHI Takahiro takahiro.akashi@linaro.org Signed-off-by: Masami Hiramatsu masami.hiramatsu@linaro.org
Changes in v2:
- Fix to pass correct pointer to efi_capsule_update_firmware
- Remove ESRT generation, because this part anyway will be removed next patch.
lib/efi_loader/efi_capsule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 4463ae00fd..1ec7ea29ff 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -1118,7 +1118,7 @@ efi_status_t efi_launch_capsules(void) index = 0; ret = efi_capsule_read_file(files[i], &capsule); if (ret == EFI_SUCCESS) {
ret = EFI_CALL(efi_update_capsule(&capsule, 1, 0));
ret = efi_capsule_update_firmware(capsule);I believe this is not fixing any issue as such. If so, I would vote for keeping the call to efi_update_capsule.
No, this is just about reducing code size by avoiding the EFI_CALL(). It should not change behaviour.
Okay, in that case, I will put a check for the FWU Multi Banks feature being enabled -- with the feature enabled, the call will be to efi_update_capsule, and with the feature disabled, the call will be made to efi_capsule_update_firmware.
Please don't do that. Instead, you should carve out a *common* function for UpdateCapsule api and capsule-on-disk. Please note, as I repeatedly said, that I didn't intend to implement the API with my initial commits. I think I should not have added efi_update_capsule() function to avoid any confusion.
-Takahiro Akashi
The compiler should compile out the code whenever the FWU feature is disabled and that will not impact the code size.
-sughosh
Best regards
Heinrich
With the FWU Multi Bank
feature enabled, the checks for capsule acceptance and revert are being done in this function. The reason I have put this code in the function is that it caters to both scenarios of capsule-on-disk and the runtime functionality. In addition, the FWU bootup checks are also done in this function through a call to fwu_update_checks_pass. So if this is not a fix, which I don't think it is, I would prefer this call to remain.
-sughosh
if (ret != EFI_SUCCESS) log_err("Applying capsule %ls failed\n", files[i]);
On Wed, 2 Feb 2022 at 05:17, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
On Tue, Feb 01, 2022 at 10:33:20PM +0530, Sughosh Ganu wrote:
On Tue, 1 Feb 2022 at 22:14, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 1. Februar 2022 16:42:43 MEZ schrieb Sughosh Ganu sughosh.ganu@linaro.org:
hi Masami,
On Tue, 1 Feb 2022 at 14:03, Masami Hiramatsu masami.hiramatsu@linaro.org wrote:
The efi_update_capsule() may have to handle the capsule flags as an UEFI runtime and boottime service, but the capsule-on-disk process doesn't. Thus, the capsule-on-disk should use the efi_capsule_update_firmware() directly instead of efi_update_capsule().
Suggested-by: AKASHI Takahiro takahiro.akashi@linaro.org Signed-off-by: Masami Hiramatsu masami.hiramatsu@linaro.org
Changes in v2:
- Fix to pass correct pointer to efi_capsule_update_firmware
- Remove ESRT generation, because this part anyway will be removed next patch.
lib/efi_loader/efi_capsule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 4463ae00fd..1ec7ea29ff 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -1118,7 +1118,7 @@ efi_status_t efi_launch_capsules(void) index = 0; ret = efi_capsule_read_file(files[i], &capsule); if (ret == EFI_SUCCESS) {
ret = EFI_CALL(efi_update_capsule(&capsule, 1, 0));
ret = efi_capsule_update_firmware(capsule);I believe this is not fixing any issue as such. If so, I would vote for keeping the call to efi_update_capsule.
No, this is just about reducing code size by avoiding the EFI_CALL(). It should not change behaviour.
Okay, in that case, I will put a check for the FWU Multi Banks feature being enabled -- with the feature enabled, the call will be to efi_update_capsule, and with the feature disabled, the call will be made to efi_capsule_update_firmware.
Please don't do that. Instead, you should carve out a *common* function for UpdateCapsule api and capsule-on-disk.
Can you also point out the issue you see with having the FWU checks in the efi_update_capsule. As I have said, having the checks here caters to both the scenarios -- capsule-on-disk update as well as secure world update. I think with the FWU feature enabled for secure world, the efi_update_capsule function will get called, before branching off to a different FMP.
Please note, as I repeatedly said, that I didn't intend to implement the API with my initial commits. I think I should not have added efi_update_capsule() function to avoid any confusion.
Maybe I missed this, but I don't know why you think the efi_update_capsule is superfluous. Also, if it really is superfluous, this commit from Masami should also be removing the function definition rather than just not calling the function.
-sughosh
-Takahiro Akashi
The compiler should compile out the code whenever the FWU feature is disabled and that will not impact the code size.
-sughosh
Best regards
Heinrich
With the FWU Multi Bank
feature enabled, the checks for capsule acceptance and revert are being done in this function. The reason I have put this code in the function is that it caters to both scenarios of capsule-on-disk and the runtime functionality. In addition, the FWU bootup checks are also done in this function through a call to fwu_update_checks_pass. So if this is not a fix, which I don't think it is, I would prefer this call to remain.
-sughosh
if (ret != EFI_SUCCESS) log_err("Applying capsule %ls failed\n", files[i]);
Hi Sughosh,
Could you tell me why do you need to do the FWU code in the efi_update_capsule? If you need to add some logic to both of the efi_update_capsule API and capsule-on-disk, it is better to be implemented in the efi_capsule_update_firmware() as a common part. Or, make an independent additional function and call it from both path. This is for decoupling the EFI boottime API wrapper (efi_capsule_update) from the capsule update logic itself.
Thank you,
2022年2月2日(水) 2:03 Sughosh Ganu sughosh.ganu@linaro.org:
On Tue, 1 Feb 2022 at 22:14, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 1. Februar 2022 16:42:43 MEZ schrieb Sughosh Ganu sughosh.ganu@linaro.org:
hi Masami,
On Tue, 1 Feb 2022 at 14:03, Masami Hiramatsu masami.hiramatsu@linaro.org wrote:
The efi_update_capsule() may have to handle the capsule flags as an UEFI runtime and boottime service, but the capsule-on-disk process doesn't. Thus, the capsule-on-disk should use the efi_capsule_update_firmware() directly instead of efi_update_capsule().
Suggested-by: AKASHI Takahiro takahiro.akashi@linaro.org Signed-off-by: Masami Hiramatsu masami.hiramatsu@linaro.org
Changes in v2:
- Fix to pass correct pointer to efi_capsule_update_firmware
- Remove ESRT generation, because this part anyway will be removed next patch.
lib/efi_loader/efi_capsule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 4463ae00fd..1ec7ea29ff 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -1118,7 +1118,7 @@ efi_status_t efi_launch_capsules(void) index = 0; ret = efi_capsule_read_file(files[i], &capsule); if (ret == EFI_SUCCESS) {
ret = EFI_CALL(efi_update_capsule(&capsule, 1, 0));
ret = efi_capsule_update_firmware(capsule);I believe this is not fixing any issue as such. If so, I would vote for keeping the call to efi_update_capsule.
No, this is just about reducing code size by avoiding the EFI_CALL(). It should not change behaviour.
Okay, in that case, I will put a check for the FWU Multi Banks feature being enabled -- with the feature enabled, the call will be to efi_update_capsule, and with the feature disabled, the call will be made to efi_capsule_update_firmware. The compiler should compile out the code whenever the FWU feature is disabled and that will not impact the code size.
-sughosh
Best regards
Heinrich
With the FWU Multi Bank
feature enabled, the checks for capsule acceptance and revert are being done in this function. The reason I have put this code in the function is that it caters to both scenarios of capsule-on-disk and the runtime functionality. In addition, the FWU bootup checks are also done in this function through a call to fwu_update_checks_pass. So if this is not a fix, which I don't think it is, I would prefer this call to remain.
-sughosh
if (ret != EFI_SUCCESS) log_err("Applying capsule %ls failed\n", files[i]);
hi Masami,
On Wed, 2 Feb 2022 at 05:39, Masami Hiramatsu masami.hiramatsu@linaro.org wrote:
Hi Sughosh,
Could you tell me why do you need to do the FWU code in the efi_update_capsule?
I thought I explained this in my previous email. Putting the FWU checks in efi_update_capsule caters to the scenario where FWU updates are being done in secure world. Even for such scenario, the efi_update_capsule function will get called. So having the checks in one single place is better.
If you need to add some logic to both of the efi_update_capsule API and capsule-on-disk, it is better to be implemented in the efi_capsule_update_firmware() as a common part. Or, make an independent additional function and call it from both path. This is for decoupling the EFI boottime API wrapper (efi_capsule_update) from the capsule update logic itself.
Like I asked Takahiro, I don't understand why you find the efi_update_capsule function superfluous. I do see it being called for secure world FWU updates. Also, if the function is indeed superfluous, you should also be removing the function definition as well as part of this patch.
-sughosh
Thank you,
2022年2月2日(水) 2:03 Sughosh Ganu sughosh.ganu@linaro.org:
On Tue, 1 Feb 2022 at 22:14, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 1. Februar 2022 16:42:43 MEZ schrieb Sughosh Ganu sughosh.ganu@linaro.org:
hi Masami,
On Tue, 1 Feb 2022 at 14:03, Masami Hiramatsu masami.hiramatsu@linaro.org wrote:
The efi_update_capsule() may have to handle the capsule flags as an UEFI runtime and boottime service, but the capsule-on-disk process doesn't. Thus, the capsule-on-disk should use the efi_capsule_update_firmware() directly instead of efi_update_capsule().
Suggested-by: AKASHI Takahiro takahiro.akashi@linaro.org Signed-off-by: Masami Hiramatsu masami.hiramatsu@linaro.org
Changes in v2:
- Fix to pass correct pointer to efi_capsule_update_firmware
- Remove ESRT generation, because this part anyway will be removed next patch.
lib/efi_loader/efi_capsule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 4463ae00fd..1ec7ea29ff 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -1118,7 +1118,7 @@ efi_status_t efi_launch_capsules(void) index = 0; ret = efi_capsule_read_file(files[i], &capsule); if (ret == EFI_SUCCESS) {
ret = EFI_CALL(efi_update_capsule(&capsule, 1, 0));
ret = efi_capsule_update_firmware(capsule);I believe this is not fixing any issue as such. If so, I would vote for keeping the call to efi_update_capsule.
No, this is just about reducing code size by avoiding the EFI_CALL(). It should not change behaviour.
Okay, in that case, I will put a check for the FWU Multi Banks feature being enabled -- with the feature enabled, the call will be to efi_update_capsule, and with the feature disabled, the call will be made to efi_capsule_update_firmware. The compiler should compile out the code whenever the FWU feature is disabled and that will not impact the code size.
-sughosh
Best regards
Heinrich
With the FWU Multi Bank
feature enabled, the checks for capsule acceptance and revert are being done in this function. The reason I have put this code in the function is that it caters to both scenarios of capsule-on-disk and the runtime functionality. In addition, the FWU bootup checks are also done in this function through a call to fwu_update_checks_pass. So if this is not a fix, which I don't think it is, I would prefer this call to remain.
-sughosh
if (ret != EFI_SUCCESS) log_err("Applying capsule %ls failed\n", files[i]);-- Masami Hiramatsu
Hi Sughosh,
2022年2月2日(水) 14:35 Sughosh Ganu sughosh.ganu@linaro.org:
hi Masami,
On Wed, 2 Feb 2022 at 05:39, Masami Hiramatsu masami.hiramatsu@linaro.org wrote:
Hi Sughosh,
Could you tell me why do you need to do the FWU code in the efi_update_capsule?
I thought I explained this in my previous email. Putting the FWU checks in efi_update_capsule caters to the scenario where FWU updates are being done in secure world. Even for such scenario, the efi_update_capsule function will get called. So having the checks in one single place is better.
Hmm, I'm not so sure the process flow of when the FWU update are being done in secure world. What will happen?
[OS] -> [UEFI UpdateCapsule()] -(SMC)> [secure FWU] -> [update firmware] ?
Or,
[OS] -(SMC)> [secure FWU] -> [UEFI UpdateCapsule()] -> [update firmware] ?
And anyway, if the FWU is done in secure world, will the FWU metadata be processed in the secure world too? (in this case, U-boot may not do anything about firmware update but just an interface, right?)
If you need to add some logic to both of the efi_update_capsule API and capsule-on-disk, it is better to be implemented in the efi_capsule_update_firmware() as a common part. Or, make an independent additional function and call it from both path. This is for decoupling the EFI boottime API wrapper (efi_capsule_update) from the capsule update logic itself.
Like I asked Takahiro, I don't understand why you find the efi_update_capsule function superfluous. I do see it being called for secure world FWU updates. Also, if the function is indeed superfluous, you should also be removing the function definition as well as part of this patch.
We don't said that the efi_update_capsule() is superfluous, but it has a different role (e.g. processing multiple capsules and handle the capsule flags) as UpdateCapsule() UEFI service API, which is defined in UEFI spec. This means we will allow user to run CapsuleApp.efi on U-Boot.
If it has to call secure world for FWU, I think that should be done in the efi_update_capsule_firmware(), so that that is called from *both* of UpdateCapsule() API and Capsule-on-disk.
Thank you,
-sughosh
Thank you,
2022年2月2日(水) 2:03 Sughosh Ganu sughosh.ganu@linaro.org:
On Tue, 1 Feb 2022 at 22:14, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 1. Februar 2022 16:42:43 MEZ schrieb Sughosh Ganu sughosh.ganu@linaro.org:
hi Masami,
On Tue, 1 Feb 2022 at 14:03, Masami Hiramatsu masami.hiramatsu@linaro.org wrote:
The efi_update_capsule() may have to handle the capsule flags as an UEFI runtime and boottime service, but the capsule-on-disk process doesn't. Thus, the capsule-on-disk should use the efi_capsule_update_firmware() directly instead of efi_update_capsule().
Suggested-by: AKASHI Takahiro takahiro.akashi@linaro.org Signed-off-by: Masami Hiramatsu masami.hiramatsu@linaro.org
Changes in v2:
- Fix to pass correct pointer to efi_capsule_update_firmware
- Remove ESRT generation, because this part anyway will be removed next patch.
lib/efi_loader/efi_capsule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 4463ae00fd..1ec7ea29ff 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -1118,7 +1118,7 @@ efi_status_t efi_launch_capsules(void) index = 0; ret = efi_capsule_read_file(files[i], &capsule); if (ret == EFI_SUCCESS) {
ret = EFI_CALL(efi_update_capsule(&capsule, 1, 0));
ret = efi_capsule_update_firmware(capsule);I believe this is not fixing any issue as such. If so, I would vote for keeping the call to efi_update_capsule.
No, this is just about reducing code size by avoiding the EFI_CALL(). It should not change behaviour.
Okay, in that case, I will put a check for the FWU Multi Banks feature being enabled -- with the feature enabled, the call will be to efi_update_capsule, and with the feature disabled, the call will be made to efi_capsule_update_firmware. The compiler should compile out the code whenever the FWU feature is disabled and that will not impact the code size.
-sughosh
Best regards
Heinrich
With the FWU Multi Bank
feature enabled, the checks for capsule acceptance and revert are being done in this function. The reason I have put this code in the function is that it caters to both scenarios of capsule-on-disk and the runtime functionality. In addition, the FWU bootup checks are also done in this function through a call to fwu_update_checks_pass. So if this is not a fix, which I don't think it is, I would prefer this call to remain.
-sughosh
if (ret != EFI_SUCCESS) log_err("Applying capsule %ls failed\n", files[i]);-- Masami Hiramatsu
hi Masami,
On Wed, 2 Feb 2022 at 12:33, Masami Hiramatsu masami.hiramatsu@linaro.org wrote:
Hi Sughosh,
2022年2月2日(水) 14:35 Sughosh Ganu sughosh.ganu@linaro.org:
hi Masami,
On Wed, 2 Feb 2022 at 05:39, Masami Hiramatsu masami.hiramatsu@linaro.org wrote:
Hi Sughosh,
Could you tell me why do you need to do the FWU code in the efi_update_capsule?
I thought I explained this in my previous email. Putting the FWU checks in efi_update_capsule caters to the scenario where FWU updates are being done in secure world. Even for such scenario, the efi_update_capsule function will get called. So having the checks in one single place is better.
Hmm, I'm not so sure the process flow of when the FWU update are being done in secure world. What will happen?
[OS] -> [UEFI UpdateCapsule()] -(SMC)> [secure FWU] -> [update firmware] ?
Yes, this would be the flow.
Or,
[OS] -(SMC)> [secure FWU] -> [UEFI UpdateCapsule()] -> [update firmware] ?
And anyway, if the FWU is done in secure world, will the FWU metadata be processed in the secure world too? (in this case, U-boot may not do anything about firmware update but just an interface, right?)
I think certain api's can be re-used, but I will re-check Jose's secure FWU implementation.
If you need to add some logic to both of the efi_update_capsule API and capsule-on-disk, it is better to be implemented in the efi_capsule_update_firmware() as a common part. Or, make an independent additional function and call it from both path. This is for decoupling the EFI boottime API wrapper (efi_capsule_update) from the capsule update logic itself.
Like I asked Takahiro, I don't understand why you find the efi_update_capsule function superfluous. I do see it being called for secure world FWU updates. Also, if the function is indeed superfluous, you should also be removing the function definition as well as part of this patch.
We don't said that the efi_update_capsule() is superfluous, but it has a different role (e.g. processing multiple capsules and handle the capsule flags) as UpdateCapsule() UEFI service API, which is defined in UEFI spec. This means we will allow user to run CapsuleApp.efi on U-Boot.
If it has to call secure world for FWU, I think that should be done in the efi_update_capsule_firmware(), so that that is called from *both* of UpdateCapsule() API and Capsule-on-disk.
Okay. In that case, I will put the checks in efi_update_capsule_firmware. Can you please expand your commit message a bit to explain why is the call to efi_update_capsule being bypassed. I believe there was a discussion between you and Takahiro where there is a more detailed explanation. It would help to have that in the commit message. Thanks.
-sughosh
Thank you,
-sughosh
Thank you,
2022年2月2日(水) 2:03 Sughosh Ganu sughosh.ganu@linaro.org:
On Tue, 1 Feb 2022 at 22:14, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 1. Februar 2022 16:42:43 MEZ schrieb Sughosh Ganu sughosh.ganu@linaro.org:
hi Masami,
On Tue, 1 Feb 2022 at 14:03, Masami Hiramatsu masami.hiramatsu@linaro.org wrote: > > The efi_update_capsule() may have to handle the capsule flags as an UEFI > runtime and boottime service, but the capsule-on-disk process doesn't. > Thus, the capsule-on-disk should use the efi_capsule_update_firmware() > directly instead of efi_update_capsule(). > > Suggested-by: AKASHI Takahiro takahiro.akashi@linaro.org > Signed-off-by: Masami Hiramatsu masami.hiramatsu@linaro.org > --- > Changes in v2: > - Fix to pass correct pointer to efi_capsule_update_firmware > - Remove ESRT generation, because this part anyway will be removed > next patch. > --- > lib/efi_loader/efi_capsule.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > index 4463ae00fd..1ec7ea29ff 100644 > --- a/lib/efi_loader/efi_capsule.c > +++ b/lib/efi_loader/efi_capsule.c > @@ -1118,7 +1118,7 @@ efi_status_t efi_launch_capsules(void) > index = 0; > ret = efi_capsule_read_file(files[i], &capsule); > if (ret == EFI_SUCCESS) { > - ret = EFI_CALL(efi_update_capsule(&capsule, 1, 0)); > + ret = efi_capsule_update_firmware(capsule);
I believe this is not fixing any issue as such. If so, I would vote for keeping the call to efi_update_capsule.
No, this is just about reducing code size by avoiding the EFI_CALL(). It should not change behaviour.
Okay, in that case, I will put a check for the FWU Multi Banks feature being enabled -- with the feature enabled, the call will be to efi_update_capsule, and with the feature disabled, the call will be made to efi_capsule_update_firmware. The compiler should compile out the code whenever the FWU feature is disabled and that will not impact the code size.
-sughosh
Best regards
Heinrich
With the FWU Multi Bank
feature enabled, the checks for capsule acceptance and revert are being done in this function. The reason I have put this code in the function is that it caters to both scenarios of capsule-on-disk and the runtime functionality. In addition, the FWU bootup checks are also done in this function through a call to fwu_update_checks_pass. So if this is not a fix, which I don't think it is, I would prefer this call to remain.
-sughosh
> if (ret != EFI_SUCCESS) > log_err("Applying capsule %ls failed\n", > files[i]); >
-- Masami Hiramatsu
-- Masami Hiramatsu
Add a config option to reset system soon after processing capsule update on disk. This is required in UEFI specification 2.9 Section 8.5.5 "Delivery of Capsules via file on Mass Storage device" as;
In all cases that a capsule is identified for processing the system is restarted after capsule processing is completed.
This also reports the result of each capsule update so that the user can notice that the capsule update has been succeeded or not from console log.
Signed-off-by: Masami Hiramatsu masami.hiramatsu@linaro.org --- Changes in v2: - Remove kconfig option to disable this feature. - Use panic() instead of do_reset() so that if the reset fails, the machine halt. - Log the result of each capsule update always. --- lib/efi_loader/efi_capsule.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 1ec7ea29ff..39bce714f7 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -1119,9 +1119,9 @@ efi_status_t efi_launch_capsules(void) ret = efi_capsule_read_file(files[i], &capsule); if (ret == EFI_SUCCESS) { ret = efi_capsule_update_firmware(capsule); - if (ret != EFI_SUCCESS) - log_err("Applying capsule %ls failed\n", - files[i]); + log_err("Applying capsule %ls %s\n", + files[i], + ret == EFI_SUCCESS ? "succeeded" : "failed");
/* create CapsuleXXXX */ set_capsule_result(index, capsule, ret); @@ -1142,6 +1142,12 @@ efi_status_t efi_launch_capsules(void) free(files[i]); free(files);
+ /* + * UEFI spec requires to reset system after complete processing capsule + * update on the storage. + */ + panic("Reboot after firmware update"); + return ret; } #endif /* CONFIG_EFI_CAPSULE_ON_DISK */
On Tue, Feb 01, 2022 at 05:33:09PM +0900, Masami Hiramatsu wrote:
Add a config option to reset system soon after processing capsule update on disk. This is required in UEFI specification 2.9 Section 8.5.5 "Delivery of Capsules via file on Mass Storage device" as;
In all cases that a capsule is identified for processing the system is restarted after capsule processing is completed.This also reports the result of each capsule update so that the user can notice that the capsule update has been succeeded or not from console log.
Signed-off-by: Masami Hiramatsu masami.hiramatsu@linaro.org
Changes in v2:
- Remove kconfig option to disable this feature.
- Use panic() instead of do_reset() so that if the reset fails, the machine halt.
- Log the result of each capsule update always.
lib/efi_loader/efi_capsule.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 1ec7ea29ff..39bce714f7 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -1119,9 +1119,9 @@ efi_status_t efi_launch_capsules(void) ret = efi_capsule_read_file(files[i], &capsule); if (ret == EFI_SUCCESS) { ret = efi_capsule_update_firmware(capsule);
if (ret != EFI_SUCCESS)log_err("Applying capsule %ls failed\n",files[i]);
log_err("Applying capsule %ls %s\n",files[i],ret == EFI_SUCCESS ? "succeeded" : "failed");
log_err()? log_info() is better, I think.
/* create CapsuleXXXX */ set_capsule_result(index, capsule, ret);@@ -1142,6 +1142,12 @@ efi_status_t efi_launch_capsules(void) free(files[i]); free(files);
- /*
* UEFI spec requires to reset system after complete processing capsule* update on the storage.*/- panic("Reboot after firmware update");
If CONFIG_PANIC_HANG is enabled, the system won't restart. It's not what we want here.
-Takahiro Akashi
- return ret;
} #endif /* CONFIG_EFI_CAPSULE_ON_DISK */
Hi Takahiro,
2022年2月1日(火) 20:38 AKASHI Takahiro takahiro.akashi@linaro.org:
On Tue, Feb 01, 2022 at 05:33:09PM +0900, Masami Hiramatsu wrote:
Add a config option to reset system soon after processing capsule update on disk. This is required in UEFI specification 2.9 Section 8.5.5 "Delivery of Capsules via file on Mass Storage device" as;
In all cases that a capsule is identified for processing the system is restarted after capsule processing is completed.This also reports the result of each capsule update so that the user can notice that the capsule update has been succeeded or not from console log.
Signed-off-by: Masami Hiramatsu masami.hiramatsu@linaro.org
Changes in v2:
- Remove kconfig option to disable this feature.
- Use panic() instead of do_reset() so that if the reset fails, the machine halt.
- Log the result of each capsule update always.
lib/efi_loader/efi_capsule.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 1ec7ea29ff..39bce714f7 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -1119,9 +1119,9 @@ efi_status_t efi_launch_capsules(void) ret = efi_capsule_read_file(files[i], &capsule); if (ret == EFI_SUCCESS) { ret = efi_capsule_update_firmware(capsule);
if (ret != EFI_SUCCESS)log_err("Applying capsule %ls failed\n",files[i]);
log_err("Applying capsule %ls %s\n",files[i],ret == EFI_SUCCESS ? "succeeded" : "failed");log_err()? log_info() is better, I think.
Hmm, would you think to use log_info() even if it is failed? Or should we have log_err(failure) and log_info(success)?
/* create CapsuleXXXX */ set_capsule_result(index, capsule, ret);@@ -1142,6 +1142,12 @@ efi_status_t efi_launch_capsules(void) free(files[i]); free(files);
/** UEFI spec requires to reset system after complete processing capsule* update on the storage.*/panic("Reboot after firmware update");If CONFIG_PANIC_HANG is enabled, the system won't restart. It's not what we want here.
Indeed. Heinrich, what would you think if do_reset() doesn't work? (I think it is OK to get it back here, but needs a warning)
Thank you,
-Takahiro Akashi
return ret;} #endif /* CONFIG_EFI_CAPSULE_ON_DISK */
-- Masami Hiramatsu
On Wed, Feb 02, 2022 at 10:53:05AM +0900, Masami Hiramatsu wrote:
Hi Takahiro,
2022年2月1日(火) 20:38 AKASHI Takahiro takahiro.akashi@linaro.org:
On Tue, Feb 01, 2022 at 05:33:09PM +0900, Masami Hiramatsu wrote:
Add a config option to reset system soon after processing capsule update on disk. This is required in UEFI specification 2.9 Section 8.5.5 "Delivery of Capsules via file on Mass Storage device" as;
In all cases that a capsule is identified for processing the system is restarted after capsule processing is completed.This also reports the result of each capsule update so that the user can notice that the capsule update has been succeeded or not from console log.
Signed-off-by: Masami Hiramatsu masami.hiramatsu@linaro.org
Changes in v2:
- Remove kconfig option to disable this feature.
- Use panic() instead of do_reset() so that if the reset fails, the machine halt.
- Log the result of each capsule update always.
lib/efi_loader/efi_capsule.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 1ec7ea29ff..39bce714f7 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -1119,9 +1119,9 @@ efi_status_t efi_launch_capsules(void) ret = efi_capsule_read_file(files[i], &capsule); if (ret == EFI_SUCCESS) { ret = efi_capsule_update_firmware(capsule);
if (ret != EFI_SUCCESS)log_err("Applying capsule %ls failed\n",files[i]);
log_err("Applying capsule %ls %s\n",files[i],ret == EFI_SUCCESS ? "succeeded" : "failed");log_err()? log_info() is better, I think.
Hmm, would you think to use log_info() even if it is failed? Or should we have log_err(failure) and log_info(success)?
It is what I meant :)
/* create CapsuleXXXX */ set_capsule_result(index, capsule, ret);@@ -1142,6 +1142,12 @@ efi_status_t efi_launch_capsules(void) free(files[i]); free(files);
/** UEFI spec requires to reset system after complete processing capsule* update on the storage.*/panic("Reboot after firmware update");If CONFIG_PANIC_HANG is enabled, the system won't restart. It's not what we want here.
Indeed. Heinrich, what would you think if do_reset() doesn't work? (I think it is OK to get it back here, but needs a warning)
If (CONFIG_IS_ENABLED(SYSRESET)) { puts ("resetting ...\n"); sysreset_reset_walk(SYSRESET_WARM); } else { do_reset(...) halt(); } /* not reach here */
-Takahiro Akashi
Thank you,
-Takahiro Akashi
return ret;} #endif /* CONFIG_EFI_CAPSULE_ON_DISK */
-- Masami Hiramatsu
Hi Takahiro,
2022年2月2日(水) 13:15 AKASHI Takahiro takahiro.akashi@linaro.org:
On Wed, Feb 02, 2022 at 10:53:05AM +0900, Masami Hiramatsu wrote:
Hi Takahiro,
2022年2月1日(火) 20:38 AKASHI Takahiro takahiro.akashi@linaro.org:
On Tue, Feb 01, 2022 at 05:33:09PM +0900, Masami Hiramatsu wrote:
Add a config option to reset system soon after processing capsule update on disk. This is required in UEFI specification 2.9 Section 8.5.5 "Delivery of Capsules via file on Mass Storage device" as;
In all cases that a capsule is identified for processing the system is restarted after capsule processing is completed.This also reports the result of each capsule update so that the user can notice that the capsule update has been succeeded or not from console log.
Signed-off-by: Masami Hiramatsu masami.hiramatsu@linaro.org
Changes in v2:
- Remove kconfig option to disable this feature.
- Use panic() instead of do_reset() so that if the reset fails, the machine halt.
- Log the result of each capsule update always.
lib/efi_loader/efi_capsule.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 1ec7ea29ff..39bce714f7 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -1119,9 +1119,9 @@ efi_status_t efi_launch_capsules(void) ret = efi_capsule_read_file(files[i], &capsule); if (ret == EFI_SUCCESS) { ret = efi_capsule_update_firmware(capsule);
if (ret != EFI_SUCCESS)log_err("Applying capsule %ls failed\n",files[i]);
log_err("Applying capsule %ls %s\n",files[i],ret == EFI_SUCCESS ? "succeeded" : "failed");log_err()? log_info() is better, I think.
Hmm, would you think to use log_info() even if it is failed? Or should we have log_err(failure) and log_info(success)?
It is what I meant :)
OK.
/* create CapsuleXXXX */ set_capsule_result(index, capsule, ret);@@ -1142,6 +1142,12 @@ efi_status_t efi_launch_capsules(void) free(files[i]); free(files);
/** UEFI spec requires to reset system after complete processing capsule* update on the storage.*/panic("Reboot after firmware update");If CONFIG_PANIC_HANG is enabled, the system won't restart. It's not what we want here.
Indeed. Heinrich, what would you think if do_reset() doesn't work? (I think it is OK to get it back here, but needs a warning)
If (CONFIG_IS_ENABLED(SYSRESET)) { puts ("resetting ...\n"); sysreset_reset_walk(SYSRESET_WARM); } else { do_reset(...) halt(); } /* not reach here */
OK, and in both case we should we puts() some messages before reboot, right?
Thank you,
-Takahiro Akashi
Thank you,
-Takahiro Akashi
return ret;} #endif /* CONFIG_EFI_CAPSULE_ON_DISK */
-- Masami Hiramatsu
-- Masami Hiramatsu
On 2/2/22 05:15, AKASHI Takahiro wrote:
On Wed, Feb 02, 2022 at 10:53:05AM +0900, Masami Hiramatsu wrote:
Hi Takahiro,
2022年2月1日(火) 20:38 AKASHI Takahiro takahiro.akashi@linaro.org:
On Tue, Feb 01, 2022 at 05:33:09PM +0900, Masami Hiramatsu wrote:
Add a config option to reset system soon after processing capsule update on disk. This is required in UEFI specification 2.9 Section 8.5.5 "Delivery of Capsules via file on Mass Storage device" as;
In all cases that a capsule is identified for processing the system is restarted after capsule processing is completed.This also reports the result of each capsule update so that the user can notice that the capsule update has been succeeded or not from console log.
Signed-off-by: Masami Hiramatsu masami.hiramatsu@linaro.org
Changes in v2:
- Remove kconfig option to disable this feature.
- Use panic() instead of do_reset() so that if the reset fails, the machine halt.
- Log the result of each capsule update always.
lib/efi_loader/efi_capsule.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 1ec7ea29ff..39bce714f7 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -1119,9 +1119,9 @@ efi_status_t efi_launch_capsules(void) ret = efi_capsule_read_file(files[i], &capsule); if (ret == EFI_SUCCESS) { ret = efi_capsule_update_firmware(capsule);
if (ret != EFI_SUCCESS)log_err("Applying capsule %ls failed\n",files[i]);
log_err("Applying capsule %ls %s\n",files[i],ret == EFI_SUCCESS ? "succeeded" : "failed");log_err()? log_info() is better, I think.
Hmm, would you think to use log_info() even if it is failed? Or should we have log_err(failure) and log_info(success)?
It is what I meant :)
/* create CapsuleXXXX */ set_capsule_result(index, capsule, ret);@@ -1142,6 +1142,12 @@ efi_status_t efi_launch_capsules(void) free(files[i]); free(files);
/** UEFI spec requires to reset system after complete processing capsule* update on the storage.*/panic("Reboot after firmware update");If CONFIG_PANIC_HANG is enabled, the system won't restart. It's not what we want here.
Indeed. Heinrich, what would you think if do_reset() doesn't work? (I think it is OK to get it back here, but needs a warning)
If (CONFIG_IS_ENABLED(SYSRESET)) { puts ("resetting ...\n"); sysreset_reset_walk(SYSRESET_WARM);
do_reset() is implemented in many 25 places. drivers/sysreset/sysreset-uclass.c is just one of them.
@Tom, @Simon: Is there a migration timeline to replace all other do_reset() implementations?
A dummy implementation like in arch/riscv/lib/reset.c should not exist. The sysreset uclass handles the case of no sysreset driver already.
Best regards
Heinrich
} else { do_reset(...) halt(); } /* not reach here */
-Takahiro Akashi
Thank you,
-Takahiro Akashi
return ret;-- Masami Hiramatsu
On Thu, Feb 03, 2022 at 06:32:50PM +0100, Heinrich Schuchardt wrote:
On 2/2/22 05:15, AKASHI Takahiro wrote:
On Wed, Feb 02, 2022 at 10:53:05AM +0900, Masami Hiramatsu wrote:
Hi Takahiro,
2022年2月1日(火) 20:38 AKASHI Takahiro takahiro.akashi@linaro.org:
On Tue, Feb 01, 2022 at 05:33:09PM +0900, Masami Hiramatsu wrote:
Add a config option to reset system soon after processing capsule update on disk. This is required in UEFI specification 2.9 Section 8.5.5 "Delivery of Capsules via file on Mass Storage device" as;
In all cases that a capsule is identified for processing the system is restarted after capsule processing is completed.This also reports the result of each capsule update so that the user can notice that the capsule update has been succeeded or not from console log.
Signed-off-by: Masami Hiramatsu masami.hiramatsu@linaro.org
Changes in v2:
- Remove kconfig option to disable this feature.
- Use panic() instead of do_reset() so that if the reset fails, the machine halt.
- Log the result of each capsule update always.
lib/efi_loader/efi_capsule.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 1ec7ea29ff..39bce714f7 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -1119,9 +1119,9 @@ efi_status_t efi_launch_capsules(void) ret = efi_capsule_read_file(files[i], &capsule); if (ret == EFI_SUCCESS) { ret = efi_capsule_update_firmware(capsule);
if (ret != EFI_SUCCESS)log_err("Applying capsule %ls failed\n",files[i]);
log_err("Applying capsule %ls %s\n",files[i],ret == EFI_SUCCESS ? "succeeded" : "failed");log_err()? log_info() is better, I think.
Hmm, would you think to use log_info() even if it is failed? Or should we have log_err(failure) and log_info(success)?
It is what I meant :)
/* create CapsuleXXXX */ set_capsule_result(index, capsule, ret);@@ -1142,6 +1142,12 @@ efi_status_t efi_launch_capsules(void) free(files[i]); free(files);
/** UEFI spec requires to reset system after complete processing capsule* update on the storage.*/panic("Reboot after firmware update");If CONFIG_PANIC_HANG is enabled, the system won't restart. It's not what we want here.
Indeed. Heinrich, what would you think if do_reset() doesn't work? (I think it is OK to get it back here, but needs a warning)
If (CONFIG_IS_ENABLED(SYSRESET)) { puts ("resetting ...\n"); sysreset_reset_walk(SYSRESET_WARM);
do_reset() is implemented in many 25 places. drivers/sysreset/sysreset-uclass.c is just one of them.
@Tom, @Simon: Is there a migration timeline to replace all other do_reset() implementations?
A dummy implementation like in arch/riscv/lib/reset.c should not exist. The sysreset uclass handles the case of no sysreset driver already.
Not yet, please feel free to propose something, if it can't just be done outright, right now.
participants (5)
-
AKASHI Takahiro -
Heinrich Schuchardt -
Masami Hiramatsu -
Sughosh Ganu -
Tom Rini