On 04/01/2013 04:31 AM, Vladimir 'φ-coder/phcoder' Serbinenko wrote: >> +static grub_uint64_t > >> +grub_efi_get_time_ms(void) >> +{ >> + grub_efi_time_t now; >> + grub_uint64_t retval; >> + grub_efi_status_t status; >> + >> + status = efi_call_2 (grub_efi_system_table->runtime_services->get_time, >> + &now, NULL); >> + if (status != GRUB_EFI_SUCCESS) >> + { >> + grub_printf("No time!\n"); >> + return 0; > > This is about the worse thing you can do. It will make any timeout go wrong.
How would you handle such a case? I guess a machine which can't provide this runtime service would need some more work in its EFI firmware before being ready for GRUB, so perhaps this is a moot point. > >> + } >> + retval = now.year * 365 * 24 * 60 * 60 * 1000; >> + retval += now.month * 30 * 24 * 60 * 60 * 1000; >> + retval += now.day * 24 * 60 * 60 * 1000; >> + retval += now.hour * 60 * 60 * 1000; >> + retval += now.minute * 60 * 1000; >> + retval += now.second * 1000; >> + retval += now.nanosecond / 1000; >> + >> + grub_dprintf("timer", "timestamp: 0x%llx\n", retval); >> + >> + return retval; > > This is almost a verbatim copy of what we had for i386 efi before but it > went haywire in many ways. Like jumps forward or backward around end of > month or when one sets datetime. Or around the summer/winter timezone > transition. I propose to re-use the existing function grub_datetime2unixtime() (which handles correctly the number of days of each month, as well as leap years), instead of doing the calculations here. And take into account the time_zone member of grub_efi_time_t as well. Here is how I would write it: grub_uint64_t grub_efi_get_time_ms (void) { grub_efi_time_t efi_time; struct grub_datetime datetime; grub_int32_t unixtime; grub_uint64_t retval; if (efi_call_2 (grub_efi_system_table->runtime_services->get_time, &efi_time, 0) != GRUB_EFI_SUCCESS) { grub_printf("No time!\n"); return 0; } datetime.year = efi_time.year; datetime.month = efi_time.month; datetime.day = efi_time.day; datetime.hour = efi_time.hour; datetime.minute = efi_time.minute; datetime.second = efi_time.second; if (!grub_datetime2unixtime (&datetime, &unixtime)) return 0; unixtime += 60 * efi_time.time_zone; retval = (grub_uint64_t)unixtime * 1000 + efi_time.nanosecond / 1000000; grub_dprintf("timer", "timestamp: 0x%llx\n", retval); return retval; } Also, there is nothing ARM-specific in this function, so I would put it in a generic EFI file like kern/efi/efi.c. > Does ARM have anything like TSC? ARMv7 does not have an architecturally-defined mechanism of retrieving the timestamp, it's up to the system peripherals to provide time-keeping capabilities. -- Francesco _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel