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

Refactor/add hal as datastructure #148

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

n1tram1
Copy link
Collaborator

@n1tram1 n1tram1 commented Jul 29, 2024

No description provided.

@n1tram1 n1tram1 requested review from Skallwar and CohenArthur July 29, 2024 16:51
@n1tram1 n1tram1 force-pushed the refactor/add-hal-as-datastructure branch from d91ff9d to 3388641 Compare July 29, 2024 17:05
@n1tram1 n1tram1 force-pushed the refactor/add-hal-as-datastructure branch 2 times, most recently from 5616c9b to bd7612f Compare October 2, 2024 21:23
@Skallwar Skallwar force-pushed the refactor/add-hal-as-datastructure branch 4 times, most recently from 120894e to 012f9aa Compare October 10, 2024 14:51
@n1tram1 n1tram1 force-pushed the refactor/add-hal-as-datastructure branch 2 times, most recently from 79d0437 to bf53293 Compare October 20, 2024 16:07
@Skallwar Skallwar force-pushed the refactor/add-hal-as-datastructure branch 2 times, most recently from 3382a7c to d1320cd Compare October 21, 2024 12:33
NOTES Outdated Show resolved Hide resolved
hal_aarch64/src/devices/gicv2.rs Outdated Show resolved Hide resolved

gic.init_distributor();

gic
}

pub fn disable_interrupts(&mut self) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

&mut encoded the fact that this would have side-effect in a way. Let's see how it goes in practice then

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 stopped doing that because having global mutables is a nightmare.

hal_aarch64/src/irq.rs Outdated Show resolved Hide resolved
hal_core/src/hal.rs Show resolved Hide resolved
let (gicd_base, gicc_base) = (0x800_0000, 0x801_0000);
HAL.kpt().lock().identity_map_range(
VAddr::new(gicd_base),
0x0001_0000 / HAL.page_size(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would rather have a calculus or a const than just 0x0001_0000

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A calculus ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like this comment was on an older version of this branch which was mistakenly pushed, I pushed the actual revision.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(te current code does this by looking up the gic node in the device tree)

kernel/src/generic_main.rs Outdated Show resolved Hide resolved
hal::mm::enable_paging();
log::trace!("going to enable paging...");
HAL.enable_paging()?;
log::trace!("enabled paging !");
Copy link
Collaborator

Choose a reason for hiding this comment

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

"paging enabled !"

@@ -7,7 +6,7 @@ use log::error;
fn panic(info: &core::panic::PanicInfo) -> ! {
error!("\x1b[31mkernel panic\x1b[0m: {}", info);

error!("hal panic info: {:X?}", hal::panic_info());
error!("hal panic info:"); // {:X?}", hal::panic_info());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why ':' if we don't print anything else

@n1tram1 n1tram1 force-pushed the refactor/add-hal-as-datastructure branch from d1320cd to 9a30ec0 Compare October 21, 2024 17:10
registers::set_sscratch(core_id);
}
fn core_id() -> usize {
// Early kernel code called Self::init and putthe core_id argument into the sscratch.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Early kernel code called Self::init and putthe core_id argument into the sscratch.
// Early kernel code called Self::init and put the core_id argument into the sscratch.

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