Skip to content

Refactor x86_64 crate #516

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

Closed
andreeaflorescu opened this issue Oct 9, 2018 · 2 comments
Closed

Refactor x86_64 crate #516

andreeaflorescu opened this issue Oct 9, 2018 · 2 comments
Labels
Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled `
Milestone

Comments

@andreeaflorescu
Copy link
Member

This crate is currently used in the kernel_loader and the vmm.

The kernel_loader only needs the constant HIMEM_START and this can be passed as a parameter to load_kernel instead of importing a whole crate.

The vmm uses the functions configure_system, get_32bit_gap_start and arch_memory_regions and the following constants: BOOT_STACK_POINTER, CMDLINE_MAX_SIZE, CMDLINE_START.
The vstate (from vmm) uses only the regs and the interrupts modules.

The configure_system function receives as a parameter the address where the command line was loaded as a GuestAddress object. This object is created in vmm/src/lib.rs as GuestAddress(x86_64::layout::CMDLINE_START). In configure_system is then unwrapped and used as the usize value of CMDLINE_START. We should probably sent the command line address directly as a usize.

We don't need to save the command line address in the KernelConfig. It is used only in the load_kernel function from vmm/src/lib.rs to load the command line at the specified address.

The cpuid should not be a module of x86_64, but rather an independent crate.

@dianpopa
Copy link
Contributor

dianpopa commented Nov 1, 2018

Another problem with this crate lies in the regs.rs module where the 4 public function for setting up the needed registers (i.e FPU, base registers, segment registers and MSRs) are hardcoding (some of them entirely and some only partially) the parameters received. For example, see the configuration of the FPU where the kvm_fpu structure is created inside the function by hardcoding some of its fields.
This approach makes the x86_64 crate very firecracker specific and burdens our initial desire to make it a standalone crate.

@andreeaflorescu andreeaflorescu added the Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled ` label Nov 22, 2018
@raduweiss raduweiss modified the milestone: ARM Support Feb 15, 2019
@dianpopa
Copy link
Contributor

Fixed by #759

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled `
Projects
None yet
Development

No branches or pull requests

3 participants