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

PR for x86_64 port merging into loaderarm #430

Closed
wants to merge 132 commits into from

Conversation

betahxy
Copy link
Contributor

@betahxy betahxy commented Oct 31, 2021

Summary of this Pull Request (PR)

Add description here.

Intent for your PR

Choose one (Mandatory):

  • This PR is for a code-review and is intended to get feedback, but not to be pulled yet.
  • This PR is mature, and ready to be integrated into the repo.

Reviewers (Mandatory):

(Specify @<github.com username(s)> of the reviewers. Ex: @user1, @user2)

Code Quality

As part of this pull request, I've considered the following:

Style:

  • Comments adhere to the Style Guide (SG)
  • Spacing adhere's to the SG
  • Naming adhere's to the SG
  • All other aspects of the SG are adhered to, or exceptions are justified in this pull request
  • [ ] I have run the auto formatter on my code before submitting this PR (see doc/auto_formatter.md for instructions)

Code Craftsmanship:

  • I've made an attempt to remove all redundant code
  • I've considered ways in which my changes might impact existing code, and cleaned it up
  • I've formatted the code in an effort to make it easier to read (proper error handling, function use, etc...)
  • I've commented appropriately where code is tricky
  • I agree that there is no "throw-away" code, and that code in this PR is of high quality

Testing

I've tested the code using the following test programs (provide list here):

  • micro_booter
  • unit_pingpong
  • unit_schedtests
  • ...(add others here)

betahxy added 30 commits February 11, 2021 02:53
- Finish setup tss, gdt, idt
- kernel now begins at 0xffff800000000000
-x86_64 higher VA needs a different mask to calculate PGD's offset
-do_div macro in printk doesn't work on x86_64, provide a different implementation
-chal_init works
- fix chal cpu init settings on x86_64
- qemu does not check validity of tss structure in memory, but real machine does check, fix the wrong tss setting in gdt_init
- make it pass cap_captbl related assertion in x64
- previous global retype table has a walk[-1] entry when page order is 20,
  changed it to 22 to walk the correct global page entry
1. setup caps for booter component
2. split CAS into 32-bit and 64-bit version
3. rearrange capability index for booter component
1. keep using 32-bit CAS by default, since other architectures are using
   the same cos_cas() interface
2. support cos_cas_64(), but it needs to be disscussed before we use it
- .boottext section needs to be aligned to generate correct bootable image
- the stack in loader.S needs to be set to .data section, the original .comm will generate a .bss section that could have random address after compiling
- fix the linker script so it can correctly compile a bootable image
- add -nmagic flag to $(LD) in the Makefile so $(LD) does not add additional blank information to the bootable image
- enable smp booting into long mode and jump into kmain
cos Show resolved Hide resolved
Copy link
Collaborator

@gparmer gparmer left a comment

Choose a reason for hiding this comment

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

I'm not done (124/172 files), but I have to run to dinner. Want to make these visible in case I can't come back to it tonight.

Summary: this is amazing. A ton of work.

.gitignore Show resolved Hide resolved
cos Outdated Show resolved Hide resolved
cos Show resolved Hide resolved
cos Outdated Show resolved Hide resolved
src/components/cidl/calculate_dependencies.py Show resolved Hide resolved
src/platform/i386/chal_cpu.h Outdated Show resolved Hide resolved
src/platform/i386/chal_pgtbl.c Outdated Show resolved Hide resolved
src/platform/i386/chal_pgtbl.c Outdated Show resolved Hide resolved
src/platform/i386/chal_pgtbl.c Show resolved Hide resolved
src/platform/i386/lapic.c Show resolved Hide resolved
Copy link
Collaborator

@gparmer gparmer left a comment

Choose a reason for hiding this comment

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

This is fantastic, and I have very very little feedback on things to change.

A higher-order bit: I'm concerned about how much this breaks arm. @msdx321 might have some input.

That said, I think that I'm suggesting very small changes. We likely want more of the tests working if we're all going to move to this branch, which we all want to!!!

src/platform/x86_64/boot_comp.c Outdated Show resolved Hide resolved
tools/init_env.sh Outdated Show resolved Hide resolved
1. set pgtbl/comp cap size to 4 in composer
2. fix memory init bug in __cos_meminfo_populate
3. fix stack pointer alignment bug in cos_asm_stub_indirect stub
- add build_iso.sh and run.sh to help cos finish build and run
- soft linked boot_comp.c, share it with both i386 and x86_64
- other minor fixes
@gparmer
Copy link
Collaborator

gparmer commented Nov 20, 2021

If you think that there is discussion left, or you want me to see a comment, you have to "unresolve the conversation". Currently there are no unresolved conversations, so I don't see any comments.

@betahxy
Copy link
Contributor Author

betahxy commented Nov 20, 2021

@gparmer
Just added a word size definition, to resolve some places that really need this.

@gparmer
Copy link
Collaborator

gparmer commented Nov 20, 2021

Should we just be using __WORDSIZE in userlevel? https://github.com/runtimejs/musl-libc/blob/master/arch/x86_64/bits/user.h#L2

- fix debug_run function
@betahxy betahxy closed this Apr 18, 2022
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