Skip to content

QemuSbsaPkg: Update RT Code bucket and Disable Mem Type Info Reset [Rebase & FF]#1371

Merged
makubacki merged 2 commits intomicrosoft:mainfrom
makubacki:update_sbsa_rt_code_bucket
Mar 31, 2026
Merged

QemuSbsaPkg: Update RT Code bucket and Disable Mem Type Info Reset [Rebase & FF]#1371
makubacki merged 2 commits intomicrosoft:mainfrom
makubacki:update_sbsa_rt_code_bucket

Conversation

@makubacki
Copy link
Copy Markdown
Member

@makubacki makubacki commented Mar 31, 2026

Description

Two memory type info related changes for SBSA.


QemuSbsaPkg: Update RT Code bucket

Currently memory type info causes resets because the RT Code bucket is too small. Current value is 0x300 needed value is 0x425, this provides some buffer at 0x450.


Disable Memory Type Information Change Reset

Memory Type Information sets the size of memory buckets used for
runtime memory types for S4 memory map stability. The actual values
are platform-specific based on allocations of those memory types
cumulatively performed by the platform firmware.

The reset helps maintain a consistent runtime memory map across S4
resume in the field but it is not a very effective method for
detecting runtime memory map changes as they resets often go unnoticed
and the reset extends CI boot time until the reset is noticed.

This change removes the reset for SBSA since PEI is being removed
and the memory map is expected to be stable across S4 resume without
the reset. A future mechanism to detect bucket size changes is being
tracked in mu_basecore.


  • Impacts functionality?
  • Impacts security?
  • Breaking change?
  • Includes tests?
  • Includes documentation?

How This Was Tested

  • QemuSbsaPkg boot to EFI shell
    • No memory type info reset after change

Integration Instructions

  • N/A

Currently memory type info causes resets because the RT Code bucket
is too small. Current value is 0x300 needed value is 0x425, this
provides some buffer at 0x450.

Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
@makubacki makubacki self-assigned this Mar 31, 2026
Copy link
Copy Markdown
Contributor

@os-d os-d left a comment

Choose a reason for hiding this comment

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

In patina-qemu we disabled the reset pcd. I would suggest we do so here as well. If we want to catch bin size changing, we can have a test app that does what bds does. We do need a holistic solution for peiless platforms.

@makubacki
Copy link
Copy Markdown
Member Author

In patina-qemu we disabled the reset pcd. I would suggest we do so here as well. If we want to catch bin size changing, we can have a test app that does what bds does. We do need a holistic solution for peiless platforms.

I don't have a problem with disabling the reset here as well. I'll leave the updated value though because there is no reason to knowingly have it too small.

@kuqin12
Copy link
Copy Markdown
Contributor

kuqin12 commented Mar 31, 2026

In patina-qemu we disabled the reset pcd. I would suggest we do so here as well. If we want to catch bin size changing, we can have a test app that does what bds does. We do need a holistic solution for peiless platforms.

Why is a test app better than the BDS reset?

@kuqin12
Copy link
Copy Markdown
Contributor

kuqin12 commented Mar 31, 2026

But why is it broken? This PR does not have pairing changes and memory map updated spontaneously?

@os-d
Copy link
Copy Markdown
Contributor

os-d commented Mar 31, 2026

But why is it broken? This PR does not have pairing changes and memory map updated spontaneously?

It’s broken because someone removed PEI :). So now the variable can’t be read and you’ll endlessly reboot. Folks have not liked and been confused by the endless reboot (although it does tell you why) and in CI endless rebooting wastes pipeline cycles. The test app can exit with a known reason. For Q35, we should leave the autotuning flow. For PEIless platforms, we need a solution.

@kuqin12
Copy link
Copy Markdown
Contributor

kuqin12 commented Mar 31, 2026

It’s broken because someone removed PEI :). So now the variable can’t be read and you’ll endlessly reboot. Folks have not liked and been confused by the endless reboot (although it does tell you why) and in CI endless rebooting wastes pipeline cycles.

PEI less change has been in for a couple of months and ARM did not have auto tune even before that change..

I do not disagree that we need something for PEI less. But the question is why the original change did not trigger pipeline hang.

Memory Type Information sets the size of memory buckets used for
runtime memory types for S4 memory map stability. The actual values
are platform-specific based on allocations of those memory types
cumulatively performed by the platform firmware.

The reset helps maintain a consistent runtime memory map across S4
resume in the field but it is not a very effective method for
detecting runtime memory map changes as they resets often go unnoticed
and the reset extends CI boot time until the reset is noticed.

This change removes the reset for SBSA since PEI is being removed
and the memory map is expected to be stable across S4 resume without
the reset. A future mechanism to detect bucket size changes is being
tracked in mu_basecore.

Signed-off-by: Michael Kubacki <michael.kubaacki@microsoft.com>
@os-d
Copy link
Copy Markdown
Contributor

os-d commented Mar 31, 2026

It’s broken because someone removed PEI :). So now the variable can’t be read and you’ll endlessly reboot. Folks have not liked and been confused by the endless reboot (although it does tell you why) and in CI endless rebooting wastes pipeline cycles.

PEI less change has been in for a couple of months and ARM did not have auto tune even before that change..

Yes, but I have hit it locally a number of times since then.

I do not disagree that we need something for PEI less. But the question is why the original change did not trigger pipeline hang.

True, I don't know what changed the bucket size and why it is causing an issue now and not when it was checked in. I thought this had stemmed from a local change that was attempting to bring in the TPM. But @makubacki this is addressing a current failure?

@makubacki
Copy link
Copy Markdown
Member Author

But why is it broken? This PR does not have pairing changes and memory map updated spontaneously?

My new "Mu PR Validation Workflow" that I'm testing did break (time out) which is what led me to make the change.

image image

@kuqin12 & @os-d, are you both okay with having a separate commit to turn off the reset just for SBSA? I'll file an issue in mu_basecore to track an alternate mechanism.

@makubacki
Copy link
Copy Markdown
Member Author

But @makubacki this is addressing a current failure?

Yes

@os-d
Copy link
Copy Markdown
Contributor

os-d commented Mar 31, 2026

Thanks. A separate commit is fine with me. And that explains, it is a current failure, but with a new basecore, which is why the pipeline is now failing. It is not a failure with the current repo.

@kuqin12
Copy link
Copy Markdown
Contributor

kuqin12 commented Mar 31, 2026

Sure, let's file a separate issue to support auto tune for sbsa. That will likely mean to support variable usage in sec ... ;( but, that is just to libify the mm pei variable module.

@makubacki
Copy link
Copy Markdown
Member Author

makubacki commented Mar 31, 2026

Two issues created for the future:

@makubacki makubacki changed the title QemuSbsaPkg: Update RT Code bucket QemuSbsaPkg: Update RT Code bucket and Disable Mem Type Info Reset [Rebase & FF] Mar 31, 2026
@makubacki makubacki enabled auto-merge (rebase) March 31, 2026 18:13
@makubacki makubacki merged commit b911bf8 into microsoft:main Mar 31, 2026
36 checks passed
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.

4 participants