[PATCH] efi: rtc-efi: use correct EFI 'epoch'

[PATCH] efi: rtc-efi: use correct EFI 'epoch'

[Ard Biesheuvel]

The rtc-efi driver declares that the EFI 'epoch' is 1/1/1998, but
the UEFI spec does not define it at all. It does define a minimum
of 1900 for the 'Year' member of the EFI_TIME struct, so let's use
1900 as the minimum year and not 1998.

This prevents rtc_read_time() failures when the RTC that backs the
EFI time services is set to a date before 1998.

Cc: Alessandro Zummo <a.zummo-BfzFCNDTiLLj+vYz1yj4TQ@public.gmane.org>
Cc: Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/rtc/rtc-efi.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
index cb989cd00b14..4a93b0bbf22c 100644
--- a/drivers/rtc/rtc-efi.c
+++ b/drivers/rtc/rtc-efi.c
@@ -25,9 +25,12 @@
 
 #define EFI_ISDST (EFI_TIME_ADJUST_DAYLIGHT|EFI_TIME_IN_DAYLIGHT)
 /*
- * EFI Epoch is 1/1/1998
+ * The UEFI spec does not literally define an epoch, but it does
+ * define EFI_TIME::Year as having a value between 1900 and 9999.
+ * (UEFI spec v2.5 paragraph 7.3)
+ * So let's use 1900 as the EFI epoch year.
  */
-#define EFI_RTC_EPOCH		1998
+#define EFI_RTC_EPOCH		1900
 
 /*
  * returns day of the year [0-365]
@@ -40,8 +43,6 @@ compute_yday(efi_time_t *eft)
 }
 /*
  * returns day of the week [0-6] 0=Sunday
- *
- * Don't try to provide a year that's before 1998, please !
  */
 static int
 compute_wday(efi_time_t *eft)
@@ -60,9 +61,9 @@ compute_wday(efi_time_t *eft)
 	ndays += compute_yday(eft);
 
 	/*
-	 * 4=1/1/1998 was a Thursday
+	 * 1=1/1/1900 was a Monday
 	 */
-	return (ndays + 4) % 7;
+	return (ndays + 1) % 7;
 }
 
 static void
-- 
1.9.1

[rtc-linux] [PATCH] efi: rtc-efi: use correct EFI 'epoch'

[Ard Biesheuvel]

The rtc-efi driver declares that the EFI 'epoch' is 1/1/1998, but
the UEFI spec does not define it at all. It does define a minimum
of 1900 for the 'Year' member of the EFI_TIME struct, so let's use
1900 as the minimum year and not 1998.

This prevents rtc_read_time() failures when the RTC that backs the
EFI time services is set to a date before 1998.

Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: rtc-linux@googlegroups.com
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/rtc/rtc-efi.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
index cb989cd00b14..4a93b0bbf22c 100644
--- a/drivers/rtc/rtc-efi.c
+++ b/drivers/rtc/rtc-efi.c
@@ -25,9 +25,12 @@
 
 #define EFI_ISDST (EFI_TIME_ADJUST_DAYLIGHT|EFI_TIME_IN_DAYLIGHT)
 /*
- * EFI Epoch is 1/1/1998
+ * The UEFI spec does not literally define an epoch, but it does
+ * define EFI_TIME::Year as having a value between 1900 and 9999.
+ * (UEFI spec v2.5 paragraph 7.3)
+ * So let's use 1900 as the EFI epoch year.
  */
-#define EFI_RTC_EPOCH		1998
+#define EFI_RTC_EPOCH		1900
 
 /*
  * returns day of the year [0-365]
@@ -40,8 +43,6 @@ compute_yday(efi_time_t *eft)
 }
 /*
  * returns day of the week [0-6] 0=Sunday
- *
- * Don't try to provide a year that's before 1998, please !
  */
 static int
 compute_wday(efi_time_t *eft)
@@ -60,9 +61,9 @@ compute_wday(efi_time_t *eft)
 	ndays += compute_yday(eft);
 
 	/*
-	 * 4=1/1/1998 was a Thursday
+	 * 1=1/1/1900 was a Monday
 	 */
-	return (ndays + 4) % 7;
+	return (ndays + 1) % 7;
 }
 
 static void
-- 
1.9.1

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH] efi: rtc-efi: use correct EFI 'epoch'

[James Bottomley]

On Mon, 2015-06-08 at 13:27 +0200, Ard Biesheuvel wrote:
> The rtc-efi driver declares that the EFI 'epoch' is 1/1/1998, but
> the UEFI spec does not define it at all. It does define a minimum
> of 1900 for the 'Year' member of the EFI_TIME struct, so let's use
> 1900 as the minimum year and not 1998.
> 
> This prevents rtc_read_time() failures when the RTC that backs the
> EFI time services is set to a date before 1998.
> 
> Cc: Alessandro Zummo <a.zummo-BfzFCNDTiLLj+vYz1yj4TQ@public.gmane.org>
> Cc: Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/rtc/rtc-efi.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
> index cb989cd00b14..4a93b0bbf22c 100644
> --- a/drivers/rtc/rtc-efi.c
> +++ b/drivers/rtc/rtc-efi.c
> @@ -25,9 +25,12 @@
>  
>  #define EFI_ISDST (EFI_TIME_ADJUST_DAYLIGHT|EFI_TIME_IN_DAYLIGHT)
>  /*
> - * EFI Epoch is 1/1/1998
> + * The UEFI spec does not literally define an epoch, but it does
> + * define EFI_TIME::Year as having a value between 1900 and 9999.
> + * (UEFI spec v2.5 paragraph 7.3)
> + * So let's use 1900 as the EFI epoch year.
>   */
> -#define EFI_RTC_EPOCH		1998
> +#define EFI_RTC_EPOCH		1900
>  
>  /*
>   * returns day of the year [0-365]
> @@ -40,8 +43,6 @@ compute_yday(efi_time_t *eft)
>  }
>  /*
>   * returns day of the week [0-6] 0=Sunday
> - *
> - * Don't try to provide a year that's before 1998, please !
>   */
>  static int
>  compute_wday(efi_time_t *eft)
> @@ -60,9 +61,9 @@ compute_wday(efi_time_t *eft)
>  	ndays += compute_yday(eft);
>  
>  	/*
> -	 * 4=1/1/1998 was a Thursday
> +	 * 1=1/1/1900 was a Monday
>  	 */
> -	return (ndays + 4) % 7;
> +	return (ndays + 1) % 7;
>  }

This is a bit nuts: increasing the for loop by a huge amount every time
we call the date just so we can cope with the odd early date.  Why not
just count backwards for the exception case?  That way the epoch becomes
an optimisation point, every year actually works, but years far before
the epoch get penalised in the loop, while our current "efficiency"
remains ... perhaps plus points if we want to move the epoch to 2015?

James

---

diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
index cb989cd..73f1ba6 100644
--- a/drivers/rtc/rtc-efi.c
+++ b/drivers/rtc/rtc-efi.c
@@ -40,8 +40,6 @@ compute_yday(efi_time_t *eft)
 }
 /*
  * returns day of the week [0-6] 0=Sunday
- *
- * Don't try to provide a year that's before 1998, please !
  */
 static int
 compute_wday(efi_time_t *eft)
@@ -49,16 +47,25 @@ compute_wday(efi_time_t *eft)
 	int y;
 	int ndays = 0;
 
-	if (eft->year < EFI_RTC_EPOCH) {
-		pr_err("EFI year < " __stringify(EFI_RTC_EPOCH) ", invalid date\n");
-		return -1;
+	if (unlikely(eft->year < EFI_RTC_EPOCH)) {
+		/* unlikely event, just count backwards */
+		for (y = eft->year; y < EFI_RTC_EPOCH; y++)
+			ndays -= 365 + (is_leap_year(y) ? 1 : 0);
+	} else {
+		for (y = EFI_RTC_EPOCH; y < eft->year; y++)
+			ndays += 365 + (is_leap_year(y) ? 1 : 0);
 	}
 
-	for (y = EFI_RTC_EPOCH; y < eft->year; y++)
-		ndays += 365 + (is_leap_year(y) ? 1 : 0);
-
 	ndays += compute_yday(eft);
 
+	if (ndays < 0)
+		/* 
+		 * make ndays positive by adding a multiple of seven because
+		 * clock arithmetic using % doesn't work with negative
+		 * numbers
+		 */
+		ndays += ((-ndays)/7 + 1)*7;
+
 	/*
 	 * 4=1/1/1998 was a Thursday
 	 */

[rtc-linux] Re: [PATCH] efi: rtc-efi: use correct EFI 'epoch'

[James Bottomley]

On Mon, 2015-06-08 at 13:27 +0200, Ard Biesheuvel wrote:
> The rtc-efi driver declares that the EFI 'epoch' is 1/1/1998, but
> the UEFI spec does not define it at all. It does define a minimum
> of 1900 for the 'Year' member of the EFI_TIME struct, so let's use
> 1900 as the minimum year and not 1998.
> 
> This prevents rtc_read_time() failures when the RTC that backs the
> EFI time services is set to a date before 1998.
> 
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Cc: rtc-linux@googlegroups.com
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  drivers/rtc/rtc-efi.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
> index cb989cd00b14..4a93b0bbf22c 100644
> --- a/drivers/rtc/rtc-efi.c
> +++ b/drivers/rtc/rtc-efi.c
> @@ -25,9 +25,12 @@
>  
>  #define EFI_ISDST (EFI_TIME_ADJUST_DAYLIGHT|EFI_TIME_IN_DAYLIGHT)
>  /*
> - * EFI Epoch is 1/1/1998
> + * The UEFI spec does not literally define an epoch, but it does
> + * define EFI_TIME::Year as having a value between 1900 and 9999.
> + * (UEFI spec v2.5 paragraph 7.3)
> + * So let's use 1900 as the EFI epoch year.
>   */
> -#define EFI_RTC_EPOCH		1998
> +#define EFI_RTC_EPOCH		1900
>  
>  /*
>   * returns day of the year [0-365]
> @@ -40,8 +43,6 @@ compute_yday(efi_time_t *eft)
>  }
>  /*
>   * returns day of the week [0-6] 0=Sunday
> - *
> - * Don't try to provide a year that's before 1998, please !
>   */
>  static int
>  compute_wday(efi_time_t *eft)
> @@ -60,9 +61,9 @@ compute_wday(efi_time_t *eft)
>  	ndays += compute_yday(eft);
>  
>  	/*
> -	 * 4=1/1/1998 was a Thursday
> +	 * 1=1/1/1900 was a Monday
>  	 */
> -	return (ndays + 4) % 7;
> +	return (ndays + 1) % 7;
>  }

This is a bit nuts: increasing the for loop by a huge amount every time
we call the date just so we can cope with the odd early date.  Why not
just count backwards for the exception case?  That way the epoch becomes
an optimisation point, every year actually works, but years far before
the epoch get penalised in the loop, while our current "efficiency"
remains ... perhaps plus points if we want to move the epoch to 2015?

James

---

diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
index cb989cd..73f1ba6 100644
--- a/drivers/rtc/rtc-efi.c
+++ b/drivers/rtc/rtc-efi.c
@@ -40,8 +40,6 @@ compute_yday(efi_time_t *eft)
 }
 /*
  * returns day of the week [0-6] 0=Sunday
- *
- * Don't try to provide a year that's before 1998, please !
  */
 static int
 compute_wday(efi_time_t *eft)
@@ -49,16 +47,25 @@ compute_wday(efi_time_t *eft)
 	int y;
 	int ndays = 0;
 
-	if (eft->year < EFI_RTC_EPOCH) {
-		pr_err("EFI year < " __stringify(EFI_RTC_EPOCH) ", invalid date\n");
-		return -1;
+	if (unlikely(eft->year < EFI_RTC_EPOCH)) {
+		/* unlikely event, just count backwards */
+		for (y = eft->year; y < EFI_RTC_EPOCH; y++)
+			ndays -= 365 + (is_leap_year(y) ? 1 : 0);
+	} else {
+		for (y = EFI_RTC_EPOCH; y < eft->year; y++)
+			ndays += 365 + (is_leap_year(y) ? 1 : 0);
 	}
 
-	for (y = EFI_RTC_EPOCH; y < eft->year; y++)
-		ndays += 365 + (is_leap_year(y) ? 1 : 0);
-
 	ndays += compute_yday(eft);
 
+	if (ndays < 0)
+		/* 
+		 * make ndays positive by adding a multiple of seven because
+		 * clock arithmetic using % doesn't work with negative
+		 * numbers
+		 */
+		ndays += ((-ndays)/7 + 1)*7;
+
 	/*
 	 * 4=1/1/1998 was a Thursday
 	 */


-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH] efi: rtc-efi: use correct EFI 'epoch'

[Ard Biesheuvel]

On 8 June 2015 at 21:26, James Bottomley
<James.Bottomley-JuX6DAaQMKPCXq6kfMZ53/egYHeGw8Jk@public.gmane.org> wrote:
> On Mon, 2015-06-08 at 13:27 +0200, Ard Biesheuvel wrote:
>> The rtc-efi driver declares that the EFI 'epoch' is 1/1/1998, but
>> the UEFI spec does not define it at all. It does define a minimum
>> of 1900 for the 'Year' member of the EFI_TIME struct, so let's use
>> 1900 as the minimum year and not 1998.
>>
>> This prevents rtc_read_time() failures when the RTC that backs the
>> EFI time services is set to a date before 1998.
>>
>> Cc: Alessandro Zummo <a.zummo-BfzFCNDTiLLj+vYz1yj4TQ@public.gmane.org>
>> Cc: Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>> Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>  drivers/rtc/rtc-efi.c | 13 +++++++------
>>  1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
>> index cb989cd00b14..4a93b0bbf22c 100644
>> --- a/drivers/rtc/rtc-efi.c
>> +++ b/drivers/rtc/rtc-efi.c
>> @@ -25,9 +25,12 @@
>>
>>  #define EFI_ISDST (EFI_TIME_ADJUST_DAYLIGHT|EFI_TIME_IN_DAYLIGHT)
>>  /*
>> - * EFI Epoch is 1/1/1998
>> + * The UEFI spec does not literally define an epoch, but it does
>> + * define EFI_TIME::Year as having a value between 1900 and 9999.
>> + * (UEFI spec v2.5 paragraph 7.3)
>> + * So let's use 1900 as the EFI epoch year.
>>   */
>> -#define EFI_RTC_EPOCH                1998
>> +#define EFI_RTC_EPOCH                1900
>>
>>  /*
>>   * returns day of the year [0-365]
>> @@ -40,8 +43,6 @@ compute_yday(efi_time_t *eft)
>>  }
>>  /*
>>   * returns day of the week [0-6] 0=Sunday
>> - *
>> - * Don't try to provide a year that's before 1998, please !
>>   */
>>  static int
>>  compute_wday(efi_time_t *eft)
>> @@ -60,9 +61,9 @@ compute_wday(efi_time_t *eft)
>>       ndays += compute_yday(eft);
>>
>>       /*
>> -      * 4=1/1/1998 was a Thursday
>> +      * 1=1/1/1900 was a Monday
>>        */
>> -     return (ndays + 4) % 7;
>> +     return (ndays + 1) % 7;
>>  }
>
> This is a bit nuts: increasing the for loop by a huge amount every time
> we call the date just so we can cope with the odd early date.  Why not
> just count backwards for the exception case?  That way the epoch becomes
> an optimisation point, every year actually works, but years far before
> the epoch get penalised in the loop, while our current "efficiency"
> remains ... perhaps plus points if we want to move the epoch to 2015?
>

The 'epoch' is defined by the UEFI spec as starting at 1900, and
changing it to 2015 to optimize this for loop is equally silly imo.
Note that the core purpose of the patch is to change the cutoff point
below which years are rejected, but that context does not appear in
the diff.

rtc_time64_to_tm() already has a better way of counting the leap days
between two years. I'd be happy to propose a followup patch that gets
rid of the for loop entirely.
But those are two separate issues imo.

-- 
Ard.

[rtc-linux] Re: [PATCH] efi: rtc-efi: use correct EFI 'epoch'

[Ard Biesheuvel]

On 8 June 2015 at 21:26, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Mon, 2015-06-08 at 13:27 +0200, Ard Biesheuvel wrote:
>> The rtc-efi driver declares that the EFI 'epoch' is 1/1/1998, but
>> the UEFI spec does not define it at all. It does define a minimum
>> of 1900 for the 'Year' member of the EFI_TIME struct, so let's use
>> 1900 as the minimum year and not 1998.
>>
>> This prevents rtc_read_time() failures when the RTC that backs the
>> EFI time services is set to a date before 1998.
>>
>> Cc: Alessandro Zummo <a.zummo@towertech.it>
>> Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>> Cc: rtc-linux@googlegroups.com
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  drivers/rtc/rtc-efi.c | 13 +++++++------
>>  1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
>> index cb989cd00b14..4a93b0bbf22c 100644
>> --- a/drivers/rtc/rtc-efi.c
>> +++ b/drivers/rtc/rtc-efi.c
>> @@ -25,9 +25,12 @@
>>
>>  #define EFI_ISDST (EFI_TIME_ADJUST_DAYLIGHT|EFI_TIME_IN_DAYLIGHT)
>>  /*
>> - * EFI Epoch is 1/1/1998
>> + * The UEFI spec does not literally define an epoch, but it does
>> + * define EFI_TIME::Year as having a value between 1900 and 9999.
>> + * (UEFI spec v2.5 paragraph 7.3)
>> + * So let's use 1900 as the EFI epoch year.
>>   */
>> -#define EFI_RTC_EPOCH                1998
>> +#define EFI_RTC_EPOCH                1900
>>
>>  /*
>>   * returns day of the year [0-365]
>> @@ -40,8 +43,6 @@ compute_yday(efi_time_t *eft)
>>  }
>>  /*
>>   * returns day of the week [0-6] 0=Sunday
>> - *
>> - * Don't try to provide a year that's before 1998, please !
>>   */
>>  static int
>>  compute_wday(efi_time_t *eft)
>> @@ -60,9 +61,9 @@ compute_wday(efi_time_t *eft)
>>       ndays += compute_yday(eft);
>>
>>       /*
>> -      * 4=1/1/1998 was a Thursday
>> +      * 1=1/1/1900 was a Monday
>>        */
>> -     return (ndays + 4) % 7;
>> +     return (ndays + 1) % 7;
>>  }
>
> This is a bit nuts: increasing the for loop by a huge amount every time
> we call the date just so we can cope with the odd early date.  Why not
> just count backwards for the exception case?  That way the epoch becomes
> an optimisation point, every year actually works, but years far before
> the epoch get penalised in the loop, while our current "efficiency"
> remains ... perhaps plus points if we want to move the epoch to 2015?
>

The 'epoch' is defined by the UEFI spec as starting at 1900, and
changing it to 2015 to optimize this for loop is equally silly imo.
Note that the core purpose of the patch is to change the cutoff point
below which years are rejected, but that context does not appear in
the diff.

rtc_time64_to_tm() already has a better way of counting the leap days
between two years. I'd be happy to propose a followup patch that gets
rid of the for loop entirely.
But those are two separate issues imo.

-- 
Ard.

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH] efi: rtc-efi: use correct EFI 'epoch'

[James Bottomley]

On Mon, 2015-06-08 at 22:15 +0200, Ard Biesheuvel wrote:
> On 8 June 2015 at 21:26, James Bottomley
> <James.Bottomley-JuX6DAaQMKPCXq6kfMZ53/egYHeGw8Jk@public.gmane.org> wrote:
> > On Mon, 2015-06-08 at 13:27 +0200, Ard Biesheuvel wrote:
> >> The rtc-efi driver declares that the EFI 'epoch' is 1/1/1998, but
> >> the UEFI spec does not define it at all. It does define a minimum
> >> of 1900 for the 'Year' member of the EFI_TIME struct, so let's use
> >> 1900 as the minimum year and not 1998.
> >>
> >> This prevents rtc_read_time() failures when the RTC that backs the
> >> EFI time services is set to a date before 1998.
> >>
> >> Cc: Alessandro Zummo <a.zummo-BfzFCNDTiLLj+vYz1yj4TQ@public.gmane.org>
> >> Cc: Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> >> Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >> ---
> >>  drivers/rtc/rtc-efi.c | 13 +++++++------
> >>  1 file changed, 7 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
> >> index cb989cd00b14..4a93b0bbf22c 100644
> >> --- a/drivers/rtc/rtc-efi.c
> >> +++ b/drivers/rtc/rtc-efi.c
> >> @@ -25,9 +25,12 @@
> >>
> >>  #define EFI_ISDST (EFI_TIME_ADJUST_DAYLIGHT|EFI_TIME_IN_DAYLIGHT)
> >>  /*
> >> - * EFI Epoch is 1/1/1998
> >> + * The UEFI spec does not literally define an epoch, but it does
> >> + * define EFI_TIME::Year as having a value between 1900 and 9999.
> >> + * (UEFI spec v2.5 paragraph 7.3)
> >> + * So let's use 1900 as the EFI epoch year.
> >>   */
> >> -#define EFI_RTC_EPOCH                1998
> >> +#define EFI_RTC_EPOCH                1900
> >>
> >>  /*
> >>   * returns day of the year [0-365]
> >> @@ -40,8 +43,6 @@ compute_yday(efi_time_t *eft)
> >>  }
> >>  /*
> >>   * returns day of the week [0-6] 0=Sunday
> >> - *
> >> - * Don't try to provide a year that's before 1998, please !
> >>   */
> >>  static int
> >>  compute_wday(efi_time_t *eft)
> >> @@ -60,9 +61,9 @@ compute_wday(efi_time_t *eft)
> >>       ndays += compute_yday(eft);
> >>
> >>       /*
> >> -      * 4=1/1/1998 was a Thursday
> >> +      * 1=1/1/1900 was a Monday
> >>        */
> >> -     return (ndays + 4) % 7;
> >> +     return (ndays + 1) % 7;
> >>  }
> >
> > This is a bit nuts: increasing the for loop by a huge amount every time
> > we call the date just so we can cope with the odd early date.  Why not
> > just count backwards for the exception case?  That way the epoch becomes
> > an optimisation point, every year actually works, but years far before
> > the epoch get penalised in the loop, while our current "efficiency"
> > remains ... perhaps plus points if we want to move the epoch to 2015?
> >
> 
> The 'epoch' is defined by the UEFI spec as starting at 1900, and
> changing it to 2015 to optimize this for loop is equally silly imo.

The spec doesn't define an epoch in the sense we usually mean it, it
just says the year in EFI_TIME should be between 1900 and 9999.  So I
don't think EFI_RTC_EPOCH in the driver is really related.

> Note that the core purpose of the patch is to change the cutoff point
> below which years are rejected, but that context does not appear in
> the diff.

Both patches achieve that ... I just think doing a for loop from 1900 is
very inefficient.

> rtc_time64_to_tm() already has a better way of counting the leap days
> between two years. I'd be happy to propose a followup patch that gets
> rid of the for loop entirely.
> But those are two separate issues imo.

Getting rid of the loop is better ... but creating a problem and then
fixing it is still not really good operating procedure.  Just propose
the final fix and we can apply that.

James

[rtc-linux] Re: [PATCH] efi: rtc-efi: use correct EFI 'epoch'

[James Bottomley]

On Mon, 2015-06-08 at 22:15 +0200, Ard Biesheuvel wrote:
> On 8 June 2015 at 21:26, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > On Mon, 2015-06-08 at 13:27 +0200, Ard Biesheuvel wrote:
> >> The rtc-efi driver declares that the EFI 'epoch' is 1/1/1998, but
> >> the UEFI spec does not define it at all. It does define a minimum
> >> of 1900 for the 'Year' member of the EFI_TIME struct, so let's use
> >> 1900 as the minimum year and not 1998.
> >>
> >> This prevents rtc_read_time() failures when the RTC that backs the
> >> EFI time services is set to a date before 1998.
> >>
> >> Cc: Alessandro Zummo <a.zummo@towertech.it>
> >> Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> >> Cc: rtc-linux@googlegroups.com
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> ---
> >>  drivers/rtc/rtc-efi.c | 13 +++++++------
> >>  1 file changed, 7 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
> >> index cb989cd00b14..4a93b0bbf22c 100644
> >> --- a/drivers/rtc/rtc-efi.c
> >> +++ b/drivers/rtc/rtc-efi.c
> >> @@ -25,9 +25,12 @@
> >>
> >>  #define EFI_ISDST (EFI_TIME_ADJUST_DAYLIGHT|EFI_TIME_IN_DAYLIGHT)
> >>  /*
> >> - * EFI Epoch is 1/1/1998
> >> + * The UEFI spec does not literally define an epoch, but it does
> >> + * define EFI_TIME::Year as having a value between 1900 and 9999.
> >> + * (UEFI spec v2.5 paragraph 7.3)
> >> + * So let's use 1900 as the EFI epoch year.
> >>   */
> >> -#define EFI_RTC_EPOCH                1998
> >> +#define EFI_RTC_EPOCH                1900
> >>
> >>  /*
> >>   * returns day of the year [0-365]
> >> @@ -40,8 +43,6 @@ compute_yday(efi_time_t *eft)
> >>  }
> >>  /*
> >>   * returns day of the week [0-6] 0=Sunday
> >> - *
> >> - * Don't try to provide a year that's before 1998, please !
> >>   */
> >>  static int
> >>  compute_wday(efi_time_t *eft)
> >> @@ -60,9 +61,9 @@ compute_wday(efi_time_t *eft)
> >>       ndays += compute_yday(eft);
> >>
> >>       /*
> >> -      * 4=1/1/1998 was a Thursday
> >> +      * 1=1/1/1900 was a Monday
> >>        */
> >> -     return (ndays + 4) % 7;
> >> +     return (ndays + 1) % 7;
> >>  }
> >
> > This is a bit nuts: increasing the for loop by a huge amount every time
> > we call the date just so we can cope with the odd early date.  Why not
> > just count backwards for the exception case?  That way the epoch becomes
> > an optimisation point, every year actually works, but years far before
> > the epoch get penalised in the loop, while our current "efficiency"
> > remains ... perhaps plus points if we want to move the epoch to 2015?
> >
> 
> The 'epoch' is defined by the UEFI spec as starting at 1900, and
> changing it to 2015 to optimize this for loop is equally silly imo.

The spec doesn't define an epoch in the sense we usually mean it, it
just says the year in EFI_TIME should be between 1900 and 9999.  So I
don't think EFI_RTC_EPOCH in the driver is really related.

> Note that the core purpose of the patch is to change the cutoff point
> below which years are rejected, but that context does not appear in
> the diff.

Both patches achieve that ... I just think doing a for loop from 1900 is
very inefficient.

> rtc_time64_to_tm() already has a better way of counting the leap days
> between two years. I'd be happy to propose a followup patch that gets
> rid of the for loop entirely.
> But those are two separate issues imo.

Getting rid of the loop is better ... but creating a problem and then
fixing it is still not really good operating procedure.  Just propose
the final fix and we can apply that.

James


-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH] efi: rtc-efi: use correct EFI 'epoch'

[Ard Biesheuvel]

On 8 June 2015 at 22:27, James Bottomley
<James.Bottomley-JuX6DAaQMKPCXq6kfMZ53/egYHeGw8Jk@public.gmane.org> wrote:
> On Mon, 2015-06-08 at 22:15 +0200, Ard Biesheuvel wrote:
>> On 8 June 2015 at 21:26, James Bottomley
>> <James.Bottomley-JuX6DAaQMKPCXq6kfMZ53/egYHeGw8Jk@public.gmane.org> wrote:
>> > On Mon, 2015-06-08 at 13:27 +0200, Ard Biesheuvel wrote:
>> >> The rtc-efi driver declares that the EFI 'epoch' is 1/1/1998, but
>> >> the UEFI spec does not define it at all. It does define a minimum
>> >> of 1900 for the 'Year' member of the EFI_TIME struct, so let's use
>> >> 1900 as the minimum year and not 1998.
>> >>
>> >> This prevents rtc_read_time() failures when the RTC that backs the
>> >> EFI time services is set to a date before 1998.
>> >>
>> >> Cc: Alessandro Zummo <a.zummo-BfzFCNDTiLLj+vYz1yj4TQ@public.gmane.org>
>> >> Cc: Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>> >> Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> >> ---
>> >>  drivers/rtc/rtc-efi.c | 13 +++++++------
>> >>  1 file changed, 7 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
>> >> index cb989cd00b14..4a93b0bbf22c 100644
>> >> --- a/drivers/rtc/rtc-efi.c
>> >> +++ b/drivers/rtc/rtc-efi.c
>> >> @@ -25,9 +25,12 @@
>> >>
>> >>  #define EFI_ISDST (EFI_TIME_ADJUST_DAYLIGHT|EFI_TIME_IN_DAYLIGHT)
>> >>  /*
>> >> - * EFI Epoch is 1/1/1998
>> >> + * The UEFI spec does not literally define an epoch, but it does
>> >> + * define EFI_TIME::Year as having a value between 1900 and 9999.
>> >> + * (UEFI spec v2.5 paragraph 7.3)
>> >> + * So let's use 1900 as the EFI epoch year.
>> >>   */
>> >> -#define EFI_RTC_EPOCH                1998
>> >> +#define EFI_RTC_EPOCH                1900
>> >>
>> >>  /*
>> >>   * returns day of the year [0-365]
>> >> @@ -40,8 +43,6 @@ compute_yday(efi_time_t *eft)
>> >>  }
>> >>  /*
>> >>   * returns day of the week [0-6] 0=Sunday
>> >> - *
>> >> - * Don't try to provide a year that's before 1998, please !
>> >>   */
>> >>  static int
>> >>  compute_wday(efi_time_t *eft)
>> >> @@ -60,9 +61,9 @@ compute_wday(efi_time_t *eft)
>> >>       ndays += compute_yday(eft);
>> >>
>> >>       /*
>> >> -      * 4=1/1/1998 was a Thursday
>> >> +      * 1=1/1/1900 was a Monday
>> >>        */
>> >> -     return (ndays + 4) % 7;
>> >> +     return (ndays + 1) % 7;
>> >>  }
>> >
>> > This is a bit nuts: increasing the for loop by a huge amount every time
>> > we call the date just so we can cope with the odd early date.  Why not
>> > just count backwards for the exception case?  That way the epoch becomes
>> > an optimisation point, every year actually works, but years far before
>> > the epoch get penalised in the loop, while our current "efficiency"
>> > remains ... perhaps plus points if we want to move the epoch to 2015?
>> >
>>
>> The 'epoch' is defined by the UEFI spec as starting at 1900, and
>> changing it to 2015 to optimize this for loop is equally silly imo.
>
> The spec doesn't define an epoch in the sense we usually mean it, it
> just says the year in EFI_TIME should be between 1900 and 9999.  So I
> don't think EFI_RTC_EPOCH in the driver is really related.
>

OK.

>> Note that the core purpose of the patch is to change the cutoff point
>> below which years are rejected, but that context does not appear in
>> the diff.
>
> Both patches achieve that ... I just think doing a for loop from 1900 is
> very inefficient.
>

Yours allows years < 1900. I know this is all of little importance,
just pointing it out since retaining the test but changing the limit
was the point of the whole patch. Allowing any year instead is also
fine by me, but I do think it makes sense to test against the UEFI
specified range. Or, since this is Linux, we could settle on 1970 as
well. Mind you, I am just trying to get rid of the errors when booting
on a machine whose RTC is uninitialized, e.g., when it has lost power,
so anything that achieves that is acceptable to me.

>> rtc_time64_to_tm() already has a better way of counting the leap days
>> between two years. I'd be happy to propose a followup patch that gets
>> rid of the for loop entirely.
>> But those are two separate issues imo.
>
> Getting rid of the loop is better ... but creating a problem and then
> fixing it is still not really good operating procedure.  Just propose
> the final fix and we can apply that.
>

Yep, makes sense. I will spin a v2 that gets rid of the 'epoch'
mention and the for loop.

-- 
Ard.

[rtc-linux] Re: [PATCH] efi: rtc-efi: use correct EFI 'epoch'

[Ard Biesheuvel]

On 8 June 2015 at 22:27, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Mon, 2015-06-08 at 22:15 +0200, Ard Biesheuvel wrote:
>> On 8 June 2015 at 21:26, James Bottomley
>> <James.Bottomley@hansenpartnership.com> wrote:
>> > On Mon, 2015-06-08 at 13:27 +0200, Ard Biesheuvel wrote:
>> >> The rtc-efi driver declares that the EFI 'epoch' is 1/1/1998, but
>> >> the UEFI spec does not define it at all. It does define a minimum
>> >> of 1900 for the 'Year' member of the EFI_TIME struct, so let's use
>> >> 1900 as the minimum year and not 1998.
>> >>
>> >> This prevents rtc_read_time() failures when the RTC that backs the
>> >> EFI time services is set to a date before 1998.
>> >>
>> >> Cc: Alessandro Zummo <a.zummo@towertech.it>
>> >> Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>> >> Cc: rtc-linux@googlegroups.com
>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >> ---
>> >>  drivers/rtc/rtc-efi.c | 13 +++++++------
>> >>  1 file changed, 7 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
>> >> index cb989cd00b14..4a93b0bbf22c 100644
>> >> --- a/drivers/rtc/rtc-efi.c
>> >> +++ b/drivers/rtc/rtc-efi.c
>> >> @@ -25,9 +25,12 @@
>> >>
>> >>  #define EFI_ISDST (EFI_TIME_ADJUST_DAYLIGHT|EFI_TIME_IN_DAYLIGHT)
>> >>  /*
>> >> - * EFI Epoch is 1/1/1998
>> >> + * The UEFI spec does not literally define an epoch, but it does
>> >> + * define EFI_TIME::Year as having a value between 1900 and 9999.
>> >> + * (UEFI spec v2.5 paragraph 7.3)
>> >> + * So let's use 1900 as the EFI epoch year.
>> >>   */
>> >> -#define EFI_RTC_EPOCH                1998
>> >> +#define EFI_RTC_EPOCH                1900
>> >>
>> >>  /*
>> >>   * returns day of the year [0-365]
>> >> @@ -40,8 +43,6 @@ compute_yday(efi_time_t *eft)
>> >>  }
>> >>  /*
>> >>   * returns day of the week [0-6] 0=Sunday
>> >> - *
>> >> - * Don't try to provide a year that's before 1998, please !
>> >>   */
>> >>  static int
>> >>  compute_wday(efi_time_t *eft)
>> >> @@ -60,9 +61,9 @@ compute_wday(efi_time_t *eft)
>> >>       ndays += compute_yday(eft);
>> >>
>> >>       /*
>> >> -      * 4=1/1/1998 was a Thursday
>> >> +      * 1=1/1/1900 was a Monday
>> >>        */
>> >> -     return (ndays + 4) % 7;
>> >> +     return (ndays + 1) % 7;
>> >>  }
>> >
>> > This is a bit nuts: increasing the for loop by a huge amount every time
>> > we call the date just so we can cope with the odd early date.  Why not
>> > just count backwards for the exception case?  That way the epoch becomes
>> > an optimisation point, every year actually works, but years far before
>> > the epoch get penalised in the loop, while our current "efficiency"
>> > remains ... perhaps plus points if we want to move the epoch to 2015?
>> >
>>
>> The 'epoch' is defined by the UEFI spec as starting at 1900, and
>> changing it to 2015 to optimize this for loop is equally silly imo.
>
> The spec doesn't define an epoch in the sense we usually mean it, it
> just says the year in EFI_TIME should be between 1900 and 9999.  So I
> don't think EFI_RTC_EPOCH in the driver is really related.
>

OK.

>> Note that the core purpose of the patch is to change the cutoff point
>> below which years are rejected, but that context does not appear in
>> the diff.
>
> Both patches achieve that ... I just think doing a for loop from 1900 is
> very inefficient.
>

Yours allows years < 1900. I know this is all of little importance,
just pointing it out since retaining the test but changing the limit
was the point of the whole patch. Allowing any year instead is also
fine by me, but I do think it makes sense to test against the UEFI
specified range. Or, since this is Linux, we could settle on 1970 as
well. Mind you, I am just trying to get rid of the errors when booting
on a machine whose RTC is uninitialized, e.g., when it has lost power,
so anything that achieves that is acceptable to me.

>> rtc_time64_to_tm() already has a better way of counting the leap days
>> between two years. I'd be happy to propose a followup patch that gets
>> rid of the for loop entirely.
>> But those are two separate issues imo.
>
> Getting rid of the loop is better ... but creating a problem and then
> fixing it is still not really good operating procedure.  Just propose
> the final fix and we can apply that.
>

Yep, makes sense. I will spin a v2 that gets rid of the 'epoch'
mention and the for loop.

-- 
Ard.

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH v2] efi: rtc-efi: use correct EFI 'epoch'

[Ard Biesheuvel]

The rtc-efi driver declares that the EFI 'epoch' is 1/1/1998, but
the UEFI spec does not define it at all. It does define a range of
[1900, 9999] for the 'Year' member of the EFI_TIME struct, so let's
use 1900 as the minimum year and not 1998.
Also, move the validation of the year to the convert_from_efi_time()
routine where all other EFI_TIME fields are validated as well.

This prevents rtc_read_time() failures when the RTC that backs the
EFI time services is set to a date before 1998, e.g., when it has
lost power.

This also optimizes the compute_wday() routine, by replacing the for
loop with a simple arithmetic expression, and by reusing the yearday
value that we need to compute anyway when populating the
rtc_time::tm_yday field.

Cc: Alessandro Zummo <a.zummo-BfzFCNDTiLLj+vYz1yj4TQ@public.gmane.org>
Cc: Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
v2:
- optimize compute_wday() to prevent adding latency to the common case to
  accommodate the exceptional case (i.e., RTC date << present date)
- move year validation to convert_from_efi_time() and check the upper bound
  as well (<= 9999)

 drivers/rtc/rtc-efi.c | 39 ++++++++++++++-------------------------
 1 file changed, 14 insertions(+), 25 deletions(-)

diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
index cb989cd00b14..6e917e2f5e86 100644
--- a/drivers/rtc/rtc-efi.c
+++ b/drivers/rtc/rtc-efi.c
@@ -24,10 +24,6 @@
 #include <linux/efi.h>
 
 #define EFI_ISDST (EFI_TIME_ADJUST_DAYLIGHT|EFI_TIME_IN_DAYLIGHT)
-/*
- * EFI Epoch is 1/1/1998
- */
-#define EFI_RTC_EPOCH		1998
 
 /*
  * returns day of the year [0-365]
@@ -38,31 +34,24 @@ compute_yday(efi_time_t *eft)
 	/* efi_time_t.month is in the [1-12] so, we need -1 */
 	return rtc_year_days(eft->day, eft->month - 1, eft->year);
 }
+
 /*
  * returns day of the week [0-6] 0=Sunday
- *
- * Don't try to provide a year that's before 1998, please !
  */
 static int
-compute_wday(efi_time_t *eft)
+compute_wday(efi_time_t *eft, int yday)
 {
-	int y;
-	int ndays = 0;
-
-	if (eft->year < EFI_RTC_EPOCH) {
-		pr_err("EFI year < " __stringify(EFI_RTC_EPOCH) ", invalid date\n");
-		return -1;
-	}
-
-	for (y = EFI_RTC_EPOCH; y < eft->year; y++)
-		ndays += 365 + (is_leap_year(y) ? 1 : 0);
-
-	ndays += compute_yday(eft);
+	int ndays = eft->year * (365 % 7)
+		    + (eft->year - 1) / 4
+		    - (eft->year - 1) / 100
+		    + (eft->year - 1) / 400
+		    + yday;
 
 	/*
-	 * 4=1/1/1998 was a Thursday
+	 * 1/1/0000 may or may not have been a Sunday (if it ever existed at
+	 * all) but assuming it was makes this calculation work correctly.
 	 */
-	return (ndays + 4) % 7;
+	return ndays % 7;
 }
 
 static void
@@ -103,16 +92,16 @@ convert_from_efi_time(efi_time_t *eft, struct rtc_time *wtime)
 	if (!eft->month || eft->month > 12)
 		return false;
 	wtime->tm_mon  = eft->month - 1;
-	wtime->tm_year = eft->year - 1900;
 
-	/* day of the week [0-6], Sunday=0 */
-	wtime->tm_wday = compute_wday(eft);
-	if (wtime->tm_wday < 0)
+	if (eft->year < 1900 || eft->year > 9999)
 		return false;
+	wtime->tm_year = eft->year - 1900;
 
 	/* day in the year [1-365]*/
 	wtime->tm_yday = compute_yday(eft);
 
+	/* day of the week [0-6], Sunday=0 */
+	wtime->tm_wday = compute_wday(eft, wtime->tm_yday);
 
 	switch (eft->daylight & EFI_ISDST) {
 	case EFI_ISDST:
-- 
1.9.1

[rtc-linux] [PATCH v2] efi: rtc-efi: use correct EFI 'epoch'

[Ard Biesheuvel]

The rtc-efi driver declares that the EFI 'epoch' is 1/1/1998, but
the UEFI spec does not define it at all. It does define a range of
[1900, 9999] for the 'Year' member of the EFI_TIME struct, so let's
use 1900 as the minimum year and not 1998.
Also, move the validation of the year to the convert_from_efi_time()
routine where all other EFI_TIME fields are validated as well.

This prevents rtc_read_time() failures when the RTC that backs the
EFI time services is set to a date before 1998, e.g., when it has
lost power.

This also optimizes the compute_wday() routine, by replacing the for
loop with a simple arithmetic expression, and by reusing the yearday
value that we need to compute anyway when populating the
rtc_time::tm_yday field.

Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: rtc-linux@googlegroups.com
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
v2:
- optimize compute_wday() to prevent adding latency to the common case to
  accommodate the exceptional case (i.e., RTC date << present date)
- move year validation to convert_from_efi_time() and check the upper bound
  as well (<= 9999)

 drivers/rtc/rtc-efi.c | 39 ++++++++++++++-------------------------
 1 file changed, 14 insertions(+), 25 deletions(-)

diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
index cb989cd00b14..6e917e2f5e86 100644
--- a/drivers/rtc/rtc-efi.c
+++ b/drivers/rtc/rtc-efi.c
@@ -24,10 +24,6 @@
 #include <linux/efi.h>
 
 #define EFI_ISDST (EFI_TIME_ADJUST_DAYLIGHT|EFI_TIME_IN_DAYLIGHT)
-/*
- * EFI Epoch is 1/1/1998
- */
-#define EFI_RTC_EPOCH		1998
 
 /*
  * returns day of the year [0-365]
@@ -38,31 +34,24 @@ compute_yday(efi_time_t *eft)
 	/* efi_time_t.month is in the [1-12] so, we need -1 */
 	return rtc_year_days(eft->day, eft->month - 1, eft->year);
 }
+
 /*
  * returns day of the week [0-6] 0=Sunday
- *
- * Don't try to provide a year that's before 1998, please !
  */
 static int
-compute_wday(efi_time_t *eft)
+compute_wday(efi_time_t *eft, int yday)
 {
-	int y;
-	int ndays = 0;
-
-	if (eft->year < EFI_RTC_EPOCH) {
-		pr_err("EFI year < " __stringify(EFI_RTC_EPOCH) ", invalid date\n");
-		return -1;
-	}
-
-	for (y = EFI_RTC_EPOCH; y < eft->year; y++)
-		ndays += 365 + (is_leap_year(y) ? 1 : 0);
-
-	ndays += compute_yday(eft);
+	int ndays = eft->year * (365 % 7)
+		    + (eft->year - 1) / 4
+		    - (eft->year - 1) / 100
+		    + (eft->year - 1) / 400
+		    + yday;
 
 	/*
-	 * 4=1/1/1998 was a Thursday
+	 * 1/1/0000 may or may not have been a Sunday (if it ever existed at
+	 * all) but assuming it was makes this calculation work correctly.
 	 */
-	return (ndays + 4) % 7;
+	return ndays % 7;
 }
 
 static void
@@ -103,16 +92,16 @@ convert_from_efi_time(efi_time_t *eft, struct rtc_time *wtime)
 	if (!eft->month || eft->month > 12)
 		return false;
 	wtime->tm_mon  = eft->month - 1;
-	wtime->tm_year = eft->year - 1900;
 
-	/* day of the week [0-6], Sunday=0 */
-	wtime->tm_wday = compute_wday(eft);
-	if (wtime->tm_wday < 0)
+	if (eft->year < 1900 || eft->year > 9999)
 		return false;
+	wtime->tm_year = eft->year - 1900;
 
 	/* day in the year [1-365]*/
 	wtime->tm_yday = compute_yday(eft);
 
+	/* day of the week [0-6], Sunday=0 */
+	wtime->tm_wday = compute_wday(eft, wtime->tm_yday);
 
 	switch (eft->daylight & EFI_ISDST) {
 	case EFI_ISDST:
-- 
1.9.1

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH v2] efi: rtc-efi: use correct EFI 'epoch'

[Alexandre Belloni]

On 09/06/2015 at 11:15:35 +0200, Ard Biesheuvel wrote :
> The rtc-efi driver declares that the EFI 'epoch' is 1/1/1998, but
> the UEFI spec does not define it at all. It does define a range of
> [1900, 9999] for the 'Year' member of the EFI_TIME struct, so let's
> use 1900 as the minimum year and not 1998.
> Also, move the validation of the year to the convert_from_efi_time()
> routine where all other EFI_TIME fields are validated as well.
> 
> This prevents rtc_read_time() failures when the RTC that backs the
> EFI time services is set to a date before 1998, e.g., when it has
> lost power.
> 
> This also optimizes the compute_wday() routine, by replacing the for
> loop with a simple arithmetic expression, and by reusing the yearday
> value that we need to compute anyway when populating the
> rtc_time::tm_yday field.
> 
> Cc: Alessandro Zummo <a.zummo-BfzFCNDTiLLj+vYz1yj4TQ@public.gmane.org>
> Cc: Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Applied, thanks.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[rtc-linux] Re: [PATCH v2] efi: rtc-efi: use correct EFI 'epoch'

[Alexandre Belloni]

On 09/06/2015 at 11:15:35 +0200, Ard Biesheuvel wrote :
> The rtc-efi driver declares that the EFI 'epoch' is 1/1/1998, but
> the UEFI spec does not define it at all. It does define a range of
> [1900, 9999] for the 'Year' member of the EFI_TIME struct, so let's
> use 1900 as the minimum year and not 1998.
> Also, move the validation of the year to the convert_from_efi_time()
> routine where all other EFI_TIME fields are validated as well.
> 
> This prevents rtc_read_time() failures when the RTC that backs the
> EFI time services is set to a date before 1998, e.g., when it has
> lost power.
> 
> This also optimizes the compute_wday() routine, by replacing the for
> loop with a simple arithmetic expression, and by reusing the yearday
> value that we need to compute anyway when populating the
> rtc_time::tm_yday field.
> 
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Cc: rtc-linux@googlegroups.com
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Applied, thanks.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.