| 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
| ||
|
Date: Wed, 6 Apr 2022 12:27:48 +0200
From: Daniel Lezcano <daniel.lezcano@...aro.org>
To: Laurent Vivier <laurent@...ier.eu>
Cc: Jiaxun Yang <jiaxun.yang@...goat.com>,
Alessandro Zummo <a.zummo@...ertech.it>,
linux-rtc@...r.kernel.org, Stephen Boyd <sboyd@...nel.org>,
Arnd Bergmann <arnd@...db.de>,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
linux-m68k@...ts.linux-m68k.org,
Geert Uytterhoeven <geert@...ux-m68k.org>,
Thomas Gleixner <tglx@...utronix.de>,
John Stultz <john.stultz@...aro.org>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v15 4/5] clocksource/drivers: Add a goldfish-timer
clocksource
Hi Laurent,
On 01/04/2022 12:20, Laurent Vivier wrote:
>
> Daniel ?
>
> Thanks,
> Laurent
>
> Le 10/03/2022 à 10:00, Laurent Vivier a écrit :
>> Add a clocksource based on the goldfish-rtc device.
>>
>> Move the timer register definition to <clocksource/timer-goldfish.h>
>>
>> This kernel implementation is based on the QEMU upstream implementation:
>>
>> https://git.qemu.org/?p=qemu.git;a=blob_plain;f=hw/rtc/goldfish_rtc.c
>>
>> goldfish-timer is a high-precision signed 64-bit nanosecond timer.
>> It is part of the 'goldfish' virtual hardware platform used to run
>> some emulated Android systems under QEMU.
>> This timer only supports oneshot event.
>>
>> Signed-off-by: Laurent Vivier <laurent@...ier.eu>
>> ---
>> drivers/clocksource/Kconfig | 7 ++
>> drivers/clocksource/Makefile | 1 +
>> drivers/clocksource/timer-goldfish.c | 153 +++++++++++++++++++++++++++
>> drivers/rtc/rtc-goldfish.c | 13 +--
>> include/clocksource/timer-goldfish.h | 31 ++++++
>> 5 files changed, 193 insertions(+), 12 deletions(-)
>> create mode 100644 drivers/clocksource/timer-goldfish.c
>> create mode 100644 include/clocksource/timer-goldfish.h
>>
>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>> index cfb8ea0df3b1..94f00374cebb 100644
>> --- a/drivers/clocksource/Kconfig
>> +++ b/drivers/clocksource/Kconfig
>> @@ -721,4 +721,11 @@ config MICROCHIP_PIT64B
>> modes and high resolution. It is used as a clocksource
>> and a clockevent.
>> +config GOLDFISH_TIMER
>> + bool "Clocksource using goldfish-rtc"
>> + depends on M68K || COMPILE_TEST
>> + depends on RTC_DRV_GOLDFISH
>> + help
>> + Support for the timer/counter of goldfish-rtc
>> +
>> endmenu
>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
>> index fa5f624eadb6..12f5d7e8cc2d 100644
>> --- a/drivers/clocksource/Makefile
>> +++ b/drivers/clocksource/Makefile
>> @@ -89,3 +89,4 @@ obj-$(CONFIG_GX6605S_TIMER) += timer-gx6605s.o
>> obj-$(CONFIG_HYPERV_TIMER) += hyperv_timer.o
>> obj-$(CONFIG_MICROCHIP_PIT64B) += timer-microchip-pit64b.o
>> obj-$(CONFIG_MSC313E_TIMER) += timer-msc313e.o
>> +obj-$(CONFIG_GOLDFISH_TIMER) += timer-goldfish.o
>> diff --git a/drivers/clocksource/timer-goldfish.c
>> b/drivers/clocksource/timer-goldfish.c
>> new file mode 100644
>> index 000000000000..0512d5eabc82
>> --- /dev/null
>> +++ b/drivers/clocksource/timer-goldfish.c
>> @@ -0,0 +1,153 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include <linux/interrupt.h>
>> +#include <linux/ioport.h>
>> +#include <linux/clocksource.h>
>> +#include <linux/clockchips.h>
>> +#include <linux/module.h>
>> +#include <linux/slab.h>
>> +#include <linux/goldfish.h>
Not related to this patch: would it make sense to move it to linux/soc/... ?
>> +#include <clocksource/timer-goldfish.h>
>> +
>> +struct goldfish_timer {
>> + struct clocksource cs;
>> + struct clock_event_device ced;
>> + struct resource res;
>> + void __iomem *base;
>> +};
>> +
>> +static struct goldfish_timer *ced_to_gf(struct clock_event_device *ced)
>> +{
>> + return container_of(ced, struct goldfish_timer, ced);
>> +}
>> +
>> +static struct goldfish_timer *cs_to_gf(struct clocksource *cs)
>> +{
>> + return container_of(cs, struct goldfish_timer, cs);
>> +}
>> +
>> +static u64 goldfish_timer_read(struct clocksource *cs)
>> +{
>> + struct goldfish_timer *timerdrv = cs_to_gf(cs);
>> + void __iomem *base = timerdrv->base;
>> + u32 time_low, time_high;
>> + u64 ticks;
>> +
>> + /*
>> + * time_low: get low bits of current time and update time_high
>> + * time_high: get high bits of time at last time_low read
>> + */
>> + time_low = gf_ioread32(base + TIMER_TIME_LOW);
>> + time_high = gf_ioread32(base + TIMER_TIME_HIGH);
There is a risk here to have the counter rolling over between low and
high reading, no ?
If it is the case, you may consider using 32bits instead of 64bits,
otherwise handle the counter wrapping around.
>> + ticks = ((u64)time_high << 32) | time_low;
>> +
>> + return ticks;
>> +}
>> +
[ ... ]
>> +/*
>> + * TIMER_TIME_LOW get low bits of current time and update
>> TIMER_TIME_HIGH
>> + * TIMER_TIME_HIGH get high bits of time at last TIMER_TIME_LOW read
>> + * TIMER_ALARM_LOW set low bits of alarm and activate it
>> + * TIMER_ALARM_HIGH set high bits of next alarm
>> + * TIMER_IRQ_ENABLED enable alarm interrupt
>> + * TIMER_CLEAR_ALARM disarm an existin alarm
typo: s/existin/existing/
>> + * TIMER_ALARM_STATUS alarm status (running or not)
>> + * TIMER_CLEAR_INTERRUPT clear interrupt
>> + */
>> +#define TIMER_TIME_LOW 0x00
>> +#define TIMER_TIME_HIGH 0x04
>> +#define TIMER_ALARM_LOW 0x08
>> +#define TIMER_ALARM_HIGH 0x0c
>> +#define TIMER_IRQ_ENABLED 0x10
>> +#define TIMER_CLEAR_ALARM 0x14
>> +#define TIMER_ALARM_STATUS 0x18
>> +#define TIMER_CLEAR_INTERRUPT 0x1c
>> +
>> +extern int goldfish_timer_init(int irq, void __iomem *base);
>> +
>> +#endif /* _CLOCKSOURCE_TIMER_GOLDFISH_H */
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
Powered by blists - more mailing lists