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

Correct FspInfoheader Assumption #10678

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

praravin
Copy link
Contributor

@praravin praravin commented Jan 27, 2025

It is assumed that the address of FspInfoheader is present
in rsi while reading ImageAttirbute. Reading and retrieving
the FspInfoheader to correct this.

Signed-off-by: Aravind P R [email protected]

  • Breaking change?
    • Breaking change - Does this PR cause a break in build or boot behavior?
    • Examples: Does it add a new library class or move a module to a different repo.
  • Impacts security?
    • Security - Does this PR have a direct security impact?
    • Examples: Crypto algorithm change or buffer overflow fix.
  • Includes tests?
    • Tests - Does this PR include any explicit test code?
    • Examples: Unit tests or integration tests.

How This Was Tested

<Describe the test(s) that were run to verify the changes.>

Integration Instructions

<Describe how these changes should be integrated. Use N/A if nothing is required.>

;
CALL_RDI ASM_PFX(AsmGetFspInfoHeaderNoStack)
xor rsi, rsi
mov rsi, rax
Copy link
Contributor

Choose a reason for hiding this comment

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

mov esi, rax

Copy link
Contributor Author

Choose a reason for hiding this comment

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

corrected

; Get FspInfoHeader address
;
CALL_RDI ASM_PFX(AsmGetFspInfoHeaderNoStack)
xor rsi, rsi
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need the xor operation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed as it is not necessary

@praravin praravin force-pushed the correct_fspinfoheader_assumption branch from 4234dce to 438cc3a Compare January 29, 2025 04:18
@praravin praravin requested a review from AshrafAliS January 29, 2025 04:21
@praravin praravin force-pushed the correct_fspinfoheader_assumption branch 3 times, most recently from d0a92b1 to 7ef43c7 Compare January 30, 2025 06:12
;
; Get FspInfoHeader address
;
CALL_EDI ASM_PFX(AsmGetFspInfoHeaderNoStack)
Copy link
Contributor

Choose a reason for hiding this comment

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

EAX holds FSP info header after LoadUpdPointerToECX called at line#602. We can save EAX to ESI at line#603 instead of calling AsmGetFspInfoHeaderNoStack here.

;
; Get FspInfoHeader address
;
CALL_RDI ASM_PFX(AsmGetFspInfoHeaderNoStack)
Copy link
Contributor

Choose a reason for hiding this comment

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

We can call AsmGetFspInfoHeaderNoStack and save RAX to RSI at line#450. By doing so, calling AsmGetFspInfoHeaderNoStack can be removed from line#460.

@praravin praravin force-pushed the correct_fspinfoheader_assumption branch from 7ef43c7 to b1884b1 Compare February 4, 2025 04:44
It is assumed that the address of FspInfoheader is
present in rsi while reading ImageAttribute. Reading
and retrieving the FspInfoheader address to correct
this.

Signed-off-by: Aravind P R <[email protected]>
@praravin praravin force-pushed the correct_fspinfoheader_assumption branch from b1884b1 to bff24b8 Compare February 5, 2025 05:32
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