linux-kernel - Re: [patch 02/23] GTOD: persistent clock support, core

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <200610030653.12411.david-b@pacbell.net>
Date:	Tue, 3 Oct 2006 06:53:11 -0700
From:	David Brownell <david-b@...bell.net>
To:	john stultz <johnstul@...ibm.com>
Cc:	Linux Kernel list <linux-kernel@...r.kernel.org>
Subject: Re: [patch 02/23] GTOD: persistent clock support, core

> Implement generic timekeeping suspend/resume accounting by introducing 
> the read_persistent_clock() interface. This is an arch specific 
> function that returns the seconds since the epoch using the arch 
> defined battery backed clock.

I remain unclear what's expected to happen when there IS no such
architcture-defined clock ... but where the system itself still
has one, e.g. a board may access one through I2C or SPI once IRQs
are working normally.

You'll recall that I had pointed out that the drivers/rtc framework
provides CONFIG_RTC_HCTOSYS, which already unifies quite a lot of
the "persistent" clocks in the way you described above, but without
that nasty requirement of working without IRQs enabled.

> +/**
> + * read_persistent_clock -  Return time in seconds from the persistent clock.
> + *
> + * Weak dummy function for arches that do not yet support it.
> + * Returns seconds from epoch using the battery backed persistent clock.
> + * Returns zero if unsupported.
> + *
> + *  XXX - Do be sure to remove it once all arches implement it.

But not all architectures **CAN** support this notion ...

> + */
> +unsigned long __attribute__((weak)) read_persistent_clock(void)
> +{
> +       return 0;
> +}
> +
>
>  /* 
>   * timekeeping_init - Initializes the clocksource and common timekeeping values
>   */
>  void __init timekeeping_init(void)
>  {
>         unsigned long flags;
> +       unsigned long sec = read_persistent_clock();

... and timekeeping_init() is called before I2C or SPI could be used,
since IRQs aren't enabled yet and accessing those busses can't be
done in general without IRQs enabled.

> @@ -774,13 +801,23 @@ static int timekeeping_suspended;
>  static int timekeeping_resume(struct sys_device *dev)
>  {
>         unsigned long flags;
> +       unsigned long now = read_persistent_clock();

Again: sys_device resume() is called with IRQs disabled, which
prevents access to many systems' persistent clocks.  In fact,
after posting this example patch

  http://marc.theaimsgroup.com/?l=linux-kernel&m=115600629813751&w=2

I never heard anything more from you on this issue.  Given this
particular patch (in $SUBJECT) should I assume you're going to
just ignore the issues whereby RTCs ("persistent clocks") can't
always meet the no-IRQs-needed assumptions being made here?  Or
address isssues like using pointer-to-function instead of using
linker tricks?

  http://marc.theaimsgroup.com/?l=linux-kernel&m=115600629825461&w=2

Those class suspend/resume hooks are now merged to kernel.org, by
the way, so that example patch is now pretty much deployable.

- Dave

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/