From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1UMcAt-0004bG-8a for mharc-grub-devel@gnu.org; Mon, 01 Apr 2013 06:41:15 -0400 Received: from eggs.gnu.org ([208.118.235.92]:38033) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UMcAn-0004Sq-Ft for grub-devel@gnu.org; Mon, 01 Apr 2013 06:41:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UMcAk-0005pD-Cn for grub-devel@gnu.org; Mon, 01 Apr 2013 06:41:09 -0400 Received: from mail-ee0-f49.google.com ([74.125.83.49]:43271) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UMcAk-0005oz-3n for grub-devel@gnu.org; Mon, 01 Apr 2013 06:41:06 -0400 Received: by mail-ee0-f49.google.com with SMTP id d41so961417eek.36 for ; Mon, 01 Apr 2013 03:41:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:message-id:date:from:user-agent:mime-version:to:subject :references:in-reply-to:content-type:content-transfer-encoding; bh=zm25zx4+CcZcum+Bh1FMdGfwEwRI+Lzg1CcA3X1Ha1M=; b=eP8ra2W8b69hNCTd+riQJImYGC8oBFWKcOItbM7Jk9nn3uAFvVZD5nGQ4hpHH4RqUs ce6AZc3PfG+vECsaaDCN1mQmmPZ7uujeXN+o2LatSqZGulYWe/ADKN+hK3WaJtl5DBW2 Rtvmhps8nKtWbrzYEdgtq6/idKvMSP+fhpHrZ+qnlZJ3xdGMmt1rShhuxrTh4WSR4RWR Iu0QOJeZz49R/bgU9DNfx2dXkekRMed7v//ErGucMRv1icAadxSjFRUm1FfJ6CCoIynq G447r7KlkL9ns0NiHLlClns4crtt14o+A0UR6ovHpEWXL50BRQ9ymr21F6kDWNbmLNUk Mj1w== X-Received: by 10.14.184.68 with SMTP id r44mr24061253eem.40.1364812865036; Mon, 01 Apr 2013 03:41:05 -0700 (PDT) Received: from [192.168.56.2] (adsl-ull-215-77.47-151.net24.it. [151.47.77.215]) by mx.google.com with ESMTPS id h5sm20452601eem.1.2013.04.01.03.41.02 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 01 Apr 2013 03:41:04 -0700 (PDT) Message-ID: <51596446.3030709@gmail.com> Date: Mon, 01 Apr 2013 12:41:10 +0200 From: Francesco Lavra User-Agent: Mozilla/5.0 (X11; Linux i686; rv:7.0.1) Gecko/20110929 Thunderbird/7.0.1 MIME-Version: 1.0 To: grub-devel@gnu.org Subject: Re: [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms References: <5158F171.6030702@gmail.com> In-Reply-To: <5158F171.6030702@gmail.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [fuzzy] X-Received-From: 74.125.83.49 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Apr 2013 10:41:13 -0000 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