RTC: add basic RTC support in OP-TEE #5179

clementleger · 2022-02-16T12:25:43Z

This PR adds support for the following:

RTC API
sama5d2 RTC driver
RTC PTA

On sama5d2, the RTC is contained in a SYS peripheral group which contains the WDT, the RSTC, the PIT and the RTC. This group can only be set secure as a whole and if doing so, the RTC won't be accessible anymore by the non-secure world. This PR adds support to expose the RTC through a PTA which will allow to communicate with a non-secure world driver (see [1]).

[1] (clementleger/linux@78c118b)

lib/libutils/ext/include/util.h

clementleger · 2022-02-17T10:45:24Z

New version:

Rebased on master
Remove DIV_ROUND_NEAREST commit
Use UDIV_ROUND_NEAREST function instead in RTC driver

core/drivers/atmel_rtc.c

core/pta/rtc.c

jenswi-linaro · 2022-02-17T13:31:49Z

core/pta/rtc.c

+	if (!time)
+		return TEE_ERROR_BAD_PARAMETERS;
+
+	rtc_set_time(time);


What happens if the info in time doesn't make sense?

It actually depends on the driver. The AT91 RTC will actually return an error while programming it. Maybe I could add a sanity check to at least avoid total non sense but it will probably be a bit difficult (leap year, february duration and so on)

core/include/drivers/rtc.h

clementleger · 2022-02-17T14:23:45Z

New version:

Use SHIFT_U32 were applicable in atmel_rtc driver
Removed all debug print in rtc_pta.c
Use MIN() in atmel_rtc driver
Update copyrights

core/pta/rtc.c

jenswi-linaro · 2022-02-17T16:28:42Z

lib/libutee/include/rtc_pta_client.h

+/*
+ * PTA_CMD_RTC_SET_OFFSET - Set RTC offset
+ *
+ * param[0] (in value) - value.a: RTC offset to be set


Can the offset be negative?

Yes the offset can be negative.

jenswi-linaro · 2022-02-18T06:33:05Z

core/pta/rtc.c

+		return TEE_ERROR_BAD_PARAMETERS;
+	}
+
+	rtc_set_offset(params[0].value.a);


Since an offset can be negative I suppose that even though the value is a uint32_t it should be casted into a int32_t in order to be properly upgraded to a long on 64bit platforms.

clementleger · 2022-02-21T13:18:51Z

New version:

Split RTC API definition and RTC PTA structs and defines into specific types for each.
Cast offset into int32_t
Packed PTA RTC structs to guarantee layout

etienne-lms

Some comments. The API looks good to me, only some descriptions are missing.
As for file rtc_pta_client.h. most PTA header file (where their API/ABI is defined) are named pta_foo.h. Prefer renaming it to pta_rtc.h?

core/include/drivers/rtc.h

core/pta/rtc.c

lib/libutee/include/rtc_pta_client.h

core/pta/rtc.c

lib/libutee/include/rtc_pta_client.h

core/drivers/atmel_rtc.c

clementleger · 2022-02-22T11:06:20Z

Some comments. The API looks good to me, only some descriptions are missing. As for file rtc_pta_client.h. most PTA header file (where their API/ABI is defined) are named pta_foo.h. Prefer renaming it to pta_rtc.h?

I named it after the rng pta one which is name rng_pta_client.h. I can rename it but it won't be coherent with what already exists.

Edit: But since all others are named differently, I will rename it

clementleger · 2022-02-22T13:54:41Z

New version:

Fixed Etienne comments
Rework RTC feature to be set based on set/get_offset presence
Add a check for rtc ops to be set in rtc_register()
Rename rtc_pta_client.h to pta_rtc.h

clementleger · 2022-02-23T08:48:24Z

New version:

Removed return value from rtc_register() and add assert for checking ops. Using a returned value does not really makes sense. The fact that a RTC driver provides the correct operations should be done during development and thus, there is no need to check that at runtime.

clementleger · 2022-02-23T10:52:50Z

Also, renamed CFG_RTC to CFG_DRIVERS_RTC to match what was done for other frameworks. (Unfortunately, I missed that for the watchdog)

etienne-lms

Hi @clementleger, I though I posted some comments but they were still drafts. Here there are. Only minor comments.

core/drivers/rtc/rtc.c

lib/libutee/include/pta_rtc.h

core/pta/rtc.c

core/drivers/atmel_rtc.c

etienne-lms

The ability the set date and offset is opened to any client application invoking the PTA. I wonder if should there be some identification passed to restrict such accesses upon 'root' or like login identification?

core/include/drivers/rtc.h

core/drivers/atmel_rtc.c

core/include/drivers/rtc.h

core/pta/rtc.c

etienne-lms · 2022-02-24T12:05:42Z

lib/libutee/include/pta_rtc.h

+	uint32_t tm_mon;
+	uint32_t tm_year;
+	uint32_t tm_wday;
+} __packed;


is __packed needed? I remember @jenswi-linaro worrying about such useless attribute.

No, it's not needed since both OP-TEE and the OS runs on the same platform. I added them because I was asked to on some other PR which add the same kind of communication pattern.

Will wait for @jenswi-linaro answer before removing them.

clementleger · 2022-02-24T16:12:36Z

The ability the set date and offset is opened to any client application invoking the PTA. I wonder if should there be some identification passed to restrict such accesses upon 'root' or like login identification?

Hum I'm not sure how I could do that properly. Do you have any example that I could look at if needed ?

etienne-lms · 2022-02-25T07:31:54Z

On Linux side, we can see 2 ways to set the RTC data. Either using a kernel driver plugged to /dev/rtc* standard tools or an application that would straight invoke the PTA.
If you plan to use the first case, the kernel driver can open the session with TEE_IOCTL_LOGIN_REE_KERNEL ID while the PTA would restrict set time/offset commands to the sessions opened with TEE_LOGIN_REE_KERNEL. The kernel uses CAP_SYS_TIME to identify applications allowed to alter time reference.
In the second case, I think the PTA needs some CFG_RTC_PTA_LOGIN_USER_ID or CFG_RTC_PTA_LOGIN_GROUP_ID or like configuration means.

clementleger · 2022-02-25T13:57:13Z

On Linux side, we can see 2 ways to set the RTC data. Either using a kernel driver plugged to /dev/rtc* standard tools or an application that would straight invoke the PTA. If you plan to use the first case, the kernel driver can open the session with TEE_IOCTL_LOGIN_REE_KERNEL ID while the PTA would restrict set time/offset commands to the sessions opened with TEE_LOGIN_REE_KERNEL. The kernel uses CAP_SYS_TIME to identify applications allowed to alter time reference.

Ok great, thanks for the explanations ! I already have a kernel driver for that RTC curently using TEE_IOCTL_LOGIN_PUBLIC so I will consider to open the session using TEE_IOCTL_LOGIN_REE_KERNEL.

clementleger · 2022-02-28T11:05:43Z

New version:

Fix trailing newline
Add open_session callback to allow only kernel clients.

core/include/drivers/rtc.h

clementleger · 2022-03-01T10:30:39Z

New version:

Remove TIME_INIT() macro and reorder optee_rtc_time struct members to be more logical

jenswi-linaro · 2022-03-01T10:45:09Z

Acked-by: Jens Wiklander <jens.wiklander@linaro.org>

clementleger · 2022-03-01T11:14:51Z

New version:

Added Acked-by Jens

etienne-lms

Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org> for commits
"drivers: rtc: add RTC API"
"pta: add PTA for RTC"

Acked-by: Etienne Carriere <etienne.carriere@linaro.org> for commits:
"driver: atmel_rtc: add driver for atmel RTC" (with minor comment)
"dts: sama5d2: set RTC as secure"
"plat-sam: enable RTC support"

etienne-lms · 2022-03-01T15:11:28Z

core/drivers/atmel_rtc.c

+static struct rtc atmel_rtc = {
+	.ops = &atmel_rtc_ops,
+	.range_min = {1900, 1, 1, 0, 0, 0, 0},
+	.range_max = {2099, 12, 31, 23, 59, 59, 0},


minor: add space char in braces:

.range_min = { 1900, 1, 1, 0, 0, 0, 0 }, .range_max = { 2099, 12, 31, 23, 59, 59, 0 },

This API allows to interact with a RTC registered as the system RTC. Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org> Acked-by: Jens Wiklander <jens.wiklander@linaro.org> Signed-off-by: Clément Léger <clement.leger@bootlin.com>

On some systems, when the RTC is secured, there is no way for the normal world to access it. This PTA uses the RTC API to allow a Linux OP-TEE based RTC driver to communicate with the RTC that is secured. Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org> Acked-by: Jens Wiklander <jens.wiklander@linaro.org> Signed-off-by: Clément Léger <clement.leger@bootlin.com>

On sama5d2, the RTC is included in a larger block of devices that can only be secured as a whole (RSTC, WDT, etc). Since these other peripherals needs to be secured, in order to still allow the RTC to be used from non-secure world, add a driver for the RTC which will be registered as the system RTC. The RTc PTA will then used this RTC to set/get time from Linux using a RTC driver that uses the TEE subsystem. Acked-by: Etienne Carriere <etienne.carriere@linaro.org> Acked-by: Jens Wiklander <jens.wiklander@linaro.org> Signed-off-by: Clément Léger <clement.leger@bootlin.com>

The RTC on sama5d2 is actually securing both RSTC, WDT and RTC register access. Enable secure mode for the RTC to ensure the WDT register accesses are secured. Acked-by: Etienne Carriere <etienne.carriere@linaro.org> Acked-by: Jens Wiklander <jens.wiklander@linaro.org> Signed-off-by: Clément Léger <clement.leger@bootlin.com>

Enable RTC API, RTC PTA and Atmel RTC driver for sama5d2. Acked-by: Etienne Carriere <etienne.carriere@linaro.org> Acked-by: Jens Wiklander <jens.wiklander@linaro.org> Signed-off-by: Clément Léger <clement.leger@bootlin.com>

clementleger · 2022-03-01T17:49:45Z

New version:

Added Acked/Reviewed-By Etienne
Added spaces before/after braces

jforissier · 2022-03-02T09:21:32Z

@clementleger thanks!

This drivers allows to communicate with a RTC PTA handled by OP-TEE [1]. This PTA allows to query RTC information, set/get time and set/get offset depending on the supported features. [1] OP-TEE/optee_os#5179 Signed-off-by: Clément Léger <clement.leger@bootlin.com>

This drivers allows to communicate with a RTC PTA handled by OP-TEE [1]. This PTA allows to query RTC information, set/get time and set/get offset depending on the supported features. [1] OP-TEE/optee_os#5179 Signed-off-by: Clément Léger <clement.leger@bootlin.com> Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> Link: https://lore.kernel.org/r/20220308133505.471601-1-clement.leger@bootlin.com

This drivers allows to communicate with a RTC PTA handled by OP-TEE [1]. This PTA allows to query RTC information, set/get time and set/get offset depending on the supported features. [1] OP-TEE/optee_os#5179 Signed-off-by: Clément Léger <clement.leger@bootlin.com> Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> Link: https://lore.kernel.org/r/20220308133505.471601-1-clement.leger@bootlin.com (cherry picked from commit 81c2f05) Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>

This drivers allows to communicate with a RTC PTA handled by OP-TEE [1]. This PTA allows to query RTC information, set/get time and set/get offset depending on the supported features. [1] OP-TEE/optee_os#5179 Signed-off-by: Clément Léger <clement.leger@bootlin.com> Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> Link: https://lore.kernel.org/r/20220308133505.471601-1-clement.leger@bootlin.com (cherry picked from commit 81c2f05) Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> Signed-off-by: Isaac True <isaac.true@canonical.com>

jenswi-linaro reviewed Feb 17, 2022

View reviewed changes

lib/libutils/ext/include/util.h Outdated Show resolved Hide resolved

clementleger force-pushed the rtc branch 2 times, most recently from d59cf83 to 335bc00 Compare February 17, 2022 10:44

clementleger force-pushed the rtc branch from 335bc00 to 1d98db2 Compare February 17, 2022 13:33

jenswi-linaro reviewed Feb 17, 2022

View reviewed changes

clementleger force-pushed the rtc branch 2 times, most recently from a9267ad to b3d791c Compare February 17, 2022 14:18

jenswi-linaro reviewed Feb 17, 2022

View reviewed changes

jenswi-linaro reviewed Feb 18, 2022

View reviewed changes

clementleger force-pushed the rtc branch from b3d791c to bc6f10a Compare February 21, 2022 13:17

etienne-lms reviewed Feb 22, 2022

View reviewed changes

clementleger force-pushed the rtc branch from bc6f10a to 0554c5c Compare February 22, 2022 10:55

clementleger force-pushed the rtc branch from 0554c5c to 2806e89 Compare February 22, 2022 11:08

clementleger force-pushed the rtc branch from 2806e89 to 9c534c0 Compare February 23, 2022 08:44

clementleger force-pushed the rtc branch from 9c534c0 to f30fbf6 Compare February 23, 2022 10:51

etienne-lms reviewed Feb 23, 2022

View reviewed changes

clementleger force-pushed the rtc branch from f30fbf6 to 2939651 Compare February 23, 2022 16:40

etienne-lms reviewed Feb 24, 2022

View reviewed changes

clementleger force-pushed the rtc branch from 2939651 to ccbe874 Compare February 24, 2022 16:11

clementleger force-pushed the rtc branch from ccbe874 to db90b5e Compare February 28, 2022 11:04

clementleger force-pushed the rtc branch from db90b5e to e2f5e4a Compare February 28, 2022 11:04

jenswi-linaro reviewed Feb 28, 2022

View reviewed changes

core/include/drivers/rtc.h Outdated Show resolved Hide resolved

clementleger force-pushed the rtc branch from e2f5e4a to 37d56a9 Compare March 1, 2022 10:30

clementleger force-pushed the rtc branch from 37d56a9 to 704e40b Compare March 1, 2022 11:14

etienne-lms reviewed Mar 1, 2022

View reviewed changes

clementleger added 5 commits March 1, 2022 18:34

drivers: rtc: add RTC API

816eba3

This API allows to interact with a RTC registered as the system RTC. Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org> Acked-by: Jens Wiklander <jens.wiklander@linaro.org> Signed-off-by: Clément Léger <clement.leger@bootlin.com>

plat-sam: enable RTC support

cbe530d

Enable RTC API, RTC PTA and Atmel RTC driver for sama5d2. Acked-by: Etienne Carriere <etienne.carriere@linaro.org> Acked-by: Jens Wiklander <jens.wiklander@linaro.org> Signed-off-by: Clément Léger <clement.leger@bootlin.com>

clementleger force-pushed the rtc branch from 704e40b to cbe530d Compare March 1, 2022 17:36

jforissier merged commit 0f6bd1d into OP-TEE:master Mar 2, 2022

OP-TEE deleted a comment Mar 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RTC: add basic RTC support in OP-TEE #5179

RTC: add basic RTC support in OP-TEE #5179

clementleger commented Feb 16, 2022

clementleger commented Feb 17, 2022

jenswi-linaro Feb 17, 2022

clementleger Feb 17, 2022

clementleger commented Feb 17, 2022

jenswi-linaro Feb 17, 2022

clementleger Feb 17, 2022

jenswi-linaro Feb 18, 2022

clementleger commented Feb 21, 2022 •

edited

Loading

etienne-lms left a comment

clementleger commented Feb 22, 2022 •

edited

Loading

clementleger commented Feb 22, 2022 •

edited

Loading

clementleger commented Feb 23, 2022

clementleger commented Feb 23, 2022

etienne-lms left a comment

etienne-lms left a comment

etienne-lms Feb 24, 2022

clementleger Feb 24, 2022 •

edited

Loading

clementleger Feb 24, 2022

clementleger commented Feb 24, 2022

etienne-lms commented Feb 25, 2022

clementleger commented Feb 25, 2022

clementleger commented Feb 28, 2022

clementleger commented Mar 1, 2022

jenswi-linaro commented Mar 1, 2022

clementleger commented Mar 1, 2022

etienne-lms left a comment

etienne-lms Mar 1, 2022

clementleger commented Mar 1, 2022 •

edited

Loading

jforissier commented Mar 2, 2022

RTC: add basic RTC support in OP-TEE #5179

RTC: add basic RTC support in OP-TEE #5179

Conversation

clementleger commented Feb 16, 2022

clementleger commented Feb 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clementleger commented Feb 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clementleger commented Feb 21, 2022 • edited Loading

etienne-lms left a comment

Choose a reason for hiding this comment

clementleger commented Feb 22, 2022 • edited Loading

clementleger commented Feb 22, 2022 • edited Loading

clementleger commented Feb 23, 2022

clementleger commented Feb 23, 2022

etienne-lms left a comment

Choose a reason for hiding this comment

etienne-lms left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clementleger Feb 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clementleger commented Feb 24, 2022

etienne-lms commented Feb 25, 2022

clementleger commented Feb 25, 2022

clementleger commented Feb 28, 2022

clementleger commented Mar 1, 2022

jenswi-linaro commented Mar 1, 2022

clementleger commented Mar 1, 2022

etienne-lms left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clementleger commented Mar 1, 2022 • edited Loading

jforissier commented Mar 2, 2022

clementleger commented Feb 21, 2022 •

edited

Loading

clementleger commented Feb 22, 2022 •

edited

Loading

clementleger commented Feb 22, 2022 •

edited

Loading

clementleger Feb 24, 2022 •

edited

Loading

clementleger commented Mar 1, 2022 •

edited

Loading