Skip to content

Abstract away boot modules and memory maps#149

Merged
SamTebbs33 merged 1 commit intodevelopfrom
feature/arch-boot-payload
May 30, 2020
Merged

Abstract away boot modules and memory maps#149
SamTebbs33 merged 1 commit intodevelopfrom
feature/arch-boot-payload

Conversation

@SamTebbs33
Copy link
Collaborator

Closes #147
This patch removes the dependence on multiboot since not all architectures will use a multiboot-compliant bootloader (the Raspberry Pi for example). This meant allowing the architecture to decide what regions of memory are reserved and which are available for use by the VMM, which had the consequence of making the VMM's allocation of reserved memory cleaner.

@SamTebbs33 SamTebbs33 requested a review from DrDeano May 15, 2020 13:58
// Reserve the unavailable sections from the multiboot memory map
for (mem_map) |entry| {
if (entry.@"type" != multiboot.MULTIBOOT_MEMORY_AVAILABLE) {
const end: usize = if (entry.addr + entry.len > std.math.maxInt(usize)) std.math.maxInt(usize) else @intCast(usize, entry.addr + entry.len);
Copy link
Member

Choose a reason for hiding this comment

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

This if can cause a integer overflow, would be better: entry.addr > std.math.maxInt(usize) - entry.len. Also, what happens if the end is too long as this would lead to the OS using unavailable memory as it couldn't fit it in a uzise. Maybe best to error here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nice idea on the if statement. Regarding the other point, what do you mean by the end being too long?

Copy link
Member

Choose a reason for hiding this comment

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

if entry.addr + entry.len > std.math.maxInt(usize) this is true, then you just use std.math.maxInt(usize) so there there will be memory that won't be mapped out.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't quite see how using maxInt(usize) is a problem. Since x86 is 32-bit it can't address the memory beyond maxInt(usize) anyway so ignoring whatever comes after that in memory map entry should be fine.

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes, good point, maybe add a comment :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure thing :)

@SamTebbs33 SamTebbs33 force-pushed the feature/arch-boot-payload branch from 042bcca to e8838ff Compare May 25, 2020 13:59
Copy link
Member

@DrDeano DrDeano left a comment

Choose a reason for hiding this comment

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

Bam, don't forget to squash :P

@SamTebbs33
Copy link
Collaborator Author

Bam, don't forget to squash :P

Noice, now I can do some more work on the aarch64 port :D

@SamTebbs33 SamTebbs33 force-pushed the feature/arch-boot-payload branch from 0d6213a to 554b970 Compare May 30, 2020 22:24
@SamTebbs33 SamTebbs33 merged commit 122adab into develop May 30, 2020
@SamTebbs33 SamTebbs33 deleted the feature/arch-boot-payload branch May 30, 2020 22:40
@SamTebbs33 SamTebbs33 mentioned this pull request Jul 20, 2020
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.

Remove dependence on multiboot for all architectures

2 participants