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

[FREELDR] Obtain Xbox memory map via multiboot spec #1971

Merged
merged 1 commit into from Dec 31, 2019

Conversation

@binarymaster
Copy link
Member

binarymaster commented Oct 15, 2019

Purpose

  • Obtain Xbox memory map via multiboot spec
  • Also obtain framebuffer memory size the same way

JIRA issue: CORE-16300

@binarymaster binarymaster force-pushed the binarymaster:xboxmem branch 3 times, most recently from 945c8f5 to 40c1c27 Oct 17, 2019
shr ecx, 2
rep movsd
mov dword ptr ds:[MultibootInfo + INITIAL_BASE - FREELDR_BASE + MB_INFO_MMAP_ADDR_OFFSET], 0
mov dword ptr ds:[_MultibootInfoPtr + INITIAL_BASE - FREELDR_BASE], offset MultibootInfo

This comment has been minimized.

Copy link
@HBelusca

HBelusca Oct 19, 2019

Contributor

I think that we can remove the MultibootInfoPtr variable, since we "know" where the MultibootInfo is: it is in MultibootInfo ! (so that in C code you could use something like &MultibootInfo ).

This comment has been minimized.

Copy link
@binarymaster

binarymaster Oct 19, 2019

Author Member

&MultibootInfo will be always defined, and MultibootInfoPtr will be NULL if we're booting non-multiboot way, it's more proper way I think.

This comment has been minimized.

Copy link
@HBelusca

HBelusca Oct 19, 2019

Contributor

ok you've a good point there.

@binarymaster binarymaster force-pushed the binarymaster:xboxmem branch from 40c1c27 to 07d112c Oct 23, 2019
@@ -91,6 +95,35 @@ MultibootEntry:
cmp eax, MULTIBOOT_BOOTLOADER_MAGIC
jne mbfail

/* Save multiboot info structure */

This comment has been minimized.

Copy link
@tkreuzer

tkreuzer Nov 15, 2019

Contributor

Why does all of this need to be in asm?

This comment has been minimized.

Copy link
@binarymaster

binarymaster Nov 15, 2019

Author Member

Asm code part is executed initially and then entire freeldr gets relocated. Relocation could rewrite some multiboot structures, that's why it's done here.

This comment has been minimized.

Copy link
@binarymaster

binarymaster Nov 22, 2019

Author Member

It probably could be rewritten in C, but it's question to the code maintainer. 🙂

This comment has been minimized.

Copy link
@tkreuzer

tkreuzer Dec 21, 2019

Contributor

I would suggest doing it in C. If it needs to be done here, add a single call to a helper function. That funtion will probably small and much simpler to understand than all of this.

This comment has been minimized.

Copy link
@binarymaster

binarymaster Dec 21, 2019

Author Member

I agree, but I'm not sure we can easily jump to C code from here at this intermediate stage, so it's better to convert this to C code later (hope @HBelusca will help).

@tkreuzer tkreuzer added this to New PRs in ReactOS PRs via automation Nov 27, 2019
@HBelusca HBelusca added this to the XBOX Boot milestone Dec 31, 2019
@HBelusca HBelusca merged commit 9669263 into reactos:master Dec 31, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
ReactOS PRs automation moved this from New PRs to Done Dec 31, 2019
@binarymaster binarymaster deleted the binarymaster:xboxmem branch Dec 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
ReactOS PRs
  
Done
4 participants
You can’t perform that action at this time.