[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms
From: |
Francesco Lavra |
Subject: |
Re: [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms |
Date: |
Mon, 01 Apr 2013 12:41:10 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux i686; rv:7.0.1) Gecko/20110929 Thunderbird/7.0.1 |
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
- Re: [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms,
Francesco Lavra <=
Re: [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms, Leif Lindholm, 2013/04/03