-
-
Notifications
You must be signed in to change notification settings - Fork 180
Add set_watchdog_timer implementation and test #27
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,7 +50,7 @@ pub struct BootServices { | |
| // Misc services | ||
| get_next_monotonic_count: usize, | ||
| stall: extern "C" fn(usize) -> Status, | ||
| set_watchdog_timer: usize, | ||
| set_watchdog_timer: extern "C" fn(timeout: usize, watchdog_code: u64, data_size: usize, watchdog_data: *mut u16) -> Status, | ||
|
|
||
| // Driver support services | ||
| connect_controller: usize, | ||
|
|
@@ -236,6 +236,11 @@ impl BootServices { | |
| assert_eq!((self.stall)(time), Status::Success); | ||
| } | ||
|
|
||
| /// Set the watchdog timer. | ||
| pub fn set_watchdog_timer(&self, timeout: usize, watchdog_code: u64, data_size: usize, watchdog_data: *mut u16) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method should probably be unsafe since 1/it takes an unchecked pointer as a parameter and 2/data_size is not related in any way to the watchdog_data pointer so safety-critical caller mistakes can happen here as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A safe alternative would be to pass in a slice of u16s. |
||
| assert_eq!((self.set_watchdog_timer)(timeout, watchdog_code, data_size, watchdog_data), Status::Success); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method can return status codes other than Success if the wrong parameters are passed in or the system does not have a watchdog timer, therefore a Result should be propagated. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @GabrielMajeri Already applied some changes on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gotta go fast! 😄 |
||
| } | ||
|
|
||
| /// Copies memory from source to destination. The buffers can overlap. | ||
| pub fn memmove(&self, dest: *mut u8, src: *const u8, size: usize) { | ||
| (self.copy_mem)(dest, src, size); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| use uefi::table::boot::BootServices; | ||
| use core::ptr; | ||
|
|
||
| pub fn test(bt: &BootServices) { | ||
| test_watchdog(bt); | ||
| } | ||
|
|
||
| fn test_watchdog(bt: &BootServices) { | ||
| bt.set_watchdog_timer(0, 0, 0, ptr::null_mut()); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a more minor point, but I'm not sure if the UEFI spec allows you to use 0 as a watchdog code. It does say that codes from 0 to 0xffff are reserved by the implementation... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Firmware is required to set a 5-minute watchdog timer before running an image, setting the code 0 to 0 disables it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can find a more detailed explanation here: https://wiki.osdev.org/UEFI#My_UEFI_application_hangs.2Fresets_after_about_5_minutes There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. According to the UEFI spec, it is the timeout parameter that must be set to zero to disable the watchdog timer. The WatchdogCode is something that will be logged on a timeout event. Now, since we're disabling the timeout, that will never happen, but an UEFI implementation that carefully validates its inputs could still complain about our use of a reserved code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The rather hypothetical nature of an UEFI implementation that checks its inputs so carefully is why I classified this as a less pressing issue 😄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I opened #28 to resolve this. |
||
| } | ||