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

Update reset notification list to be notified before NVMe #80

Merged
merged 2 commits into from Nov 29, 2022

Conversation

mikeytdisco
Copy link
Contributor

Description

Reset notification was handled by the NVMe driver before the AdvLogger could write the log to disk.

  • [No] Breaking change?
    • Will this change break pre-existing builds or functionality without action being taken?

How This Was Tested

Tested on multiple systems.

Integration Instructions

N/A

@mikeytdisco mikeytdisco self-assigned this Oct 25, 2022
@makubacki
Copy link
Member

Is/was there a change to the NVMe code? If so, could you link that in the description for reference?

@mikeytdisco
Copy link
Contributor Author

There was no change to the NVMe code. The Reset System driver processes 3 queues. Both NVMe and AdvLogger were using the second queue processed by ResetSystemRuntimeDxe. AdvLogger was changed to use the first queue processed by ResetSystemRuntimeDxe.

@spbrogan
Copy link
Member

Not sure about this change. You are leveraging the "filter" list.
When we developed the new reset code we wrote some documentation about how it should be used. Do you know where that is? I don't see it in edk2.

Copy link
Member

@spbrogan spbrogan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to see the docs and confirm this is acceptable use

@spbrogan spbrogan added the type:bug Something isn't working label Oct 25, 2022
Copy link
Member

@spbrogan spbrogan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks like it should work and is aligned with the requirements of the api headers.

@mikeytdisco mikeytdisco merged commit b8b77aa into release/202208 Nov 29, 2022
@mikeytdisco mikeytdisco deleted the personal/miketur/AdvFileLoggerResetFix branch November 29, 2022 22:48
github-actions bot pushed a commit to Erich-McMillan/mu_plus that referenced this pull request Dec 9, 2022
## Description

Reset notification was handled by the NVMe driver before the AdvLogger
could write the log to disk.

- [No] Breaking change?
- Will this change break pre-existing builds or functionality without
action being taken?

## How This Was Tested

Tested on multiple systems.

## Integration Instructions

N/A
github-actions bot pushed a commit to Erich-McMillan/mu_plus that referenced this pull request Dec 9, 2022
## Description

Reset notification was handled by the NVMe driver before the AdvLogger
could write the log to disk.

- [No] Breaking change?
- Will this change break pre-existing builds or functionality without
action being taken?

## How This Was Tested

Tested on multiple systems.

## Integration Instructions

N/A
github-actions bot pushed a commit to TaylorBeebe/mu_plus that referenced this pull request Dec 9, 2022
## Description

Reset notification was handled by the NVMe driver before the AdvLogger
could write the log to disk.

- [No] Breaking change?
- Will this change break pre-existing builds or functionality without
action being taken?

## How This Was Tested

Tested on multiple systems.

## Integration Instructions

N/A
kenlautner pushed a commit that referenced this pull request May 14, 2023
## Description

Reset notification was handled by the NVMe driver before the AdvLogger
could write the log to disk.

- [No] Breaking change?
- Will this change break pre-existing builds or functionality without
action being taken?

## How This Was Tested

Tested on multiple systems.

## Integration Instructions

N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants