Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protectli vault ehl/pop up fix #362

Merged
merged 5 commits into from
Jul 19, 2023

Conversation

miczyg1
Copy link
Contributor

@miczyg1 miczyg1 commented Jul 19, 2023

No description provided.

Signed-off-by: Michał Żygowski <[email protected]>
coreboot offers two vboot schemes VBOOT_SLOTS_RW_A and
VBOOT_SLOTS_RW_AB. When VBOOT_SLOTS_RW_AB is not selected then the
resulting image is rather not expected to have RW_B flash regions.
When only RW_A partition is used, vboot does additional full_reset
cycles to try RW_B, even though it does not exist / the build was not
configured for two RW partitions. To avoid it, a new vboot context
flag has been introduced, VB2_CONTEXT_SLOT_A_ONLY, which can be set
right after context initialization to inform vboot about absence of
slot B. This will result in less full_reset cycles when vboot runs
out of available slots and cause vboot to switch to recovery mode
faster.

Signed-off-by: Michał Żygowski <[email protected]>
@miczyg1 miczyg1 temporarily deployed to Protectli July 19, 2023 12:28 — with GitHub Actions Inactive
@miczyg1 miczyg1 requested a review from mkopec July 19, 2023 12:32
@pietrushnic
Copy link
Contributor

@miczyg1 @mkopec, just for my information, because maybe I don't understand something. If MR/PR is approved by the reviewer, then who is responsible for merging it?

@miczyg1
Copy link
Contributor Author

miczyg1 commented Jul 19, 2023

@miczyg1 @mkopec, just for my information, because maybe I don't understand something. If MR/PR is approved by the reviewer, then who is responsible for merging it?

I guess whoever first (reviewer or author) does a local FF merge and pushes it in case of the code repos, as we don't use merge button here (signatures get lost, and we want to omit merge commits done by GH)

@miczyg1 miczyg1 merged commit e467a0a into protectli_vault_ehl/develop Jul 19, 2023
1 check passed
@miczyg1 miczyg1 deleted the protectli_vault_ehl/pop_up_fix branch July 19, 2023 15:17
@miczyg1 miczyg1 temporarily deployed to Protectli July 19, 2023 15:17 — with GitHub Actions Inactive
@pietrushnic
Copy link
Contributor

@miczyg1 @mkopec, just for my information, because maybe I don't understand something. If MR/PR is approved by the reviewer, then who is responsible for merging it?

I guess whoever first (reviewer or author) does a local FF merge and pushes it in case of the code repos, as we don't use merge button here (signatures get lost, and we want to omit merge commits done by GH)

I wonder if @mkopec knew that since he approved but did not merged for some reason.

@miczyg1
Copy link
Contributor Author

miczyg1 commented Jul 20, 2023

I wonder if @mkopec knew that since he approved but did not merged for some reason.

Doesn't really matter IMO. As long as it is approved, I am free to merge it as well. And I did it so I could finish the task, because I care. And whoever cares, should act in similar fashion. It is not only reviewer's interest to merge PRs that are approved.

@mkopec
Copy link
Member

mkopec commented Jul 20, 2023

In cases where I see a lot of PRs to the same repo from the same person I tend to leave it up to them so that they can merge in their preferred order. In fact just an hour ago I merged Dasharo/edk2#64 and caused the rest of the PRs to need a rebase.

@pietrushnic
Copy link
Contributor

It may make sense; I just wanted to know the reasoning. I doubt this is clearly described in our processes. It is all plain logic, but for a new beginner, it may not be that obvious.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants