[PATCH v2 1/2] selftest: rtc: Add to check rtc alarm status for alarm related test

[PATCH v2 1/2] selftest: rtc: Add to check rtc alarm status for alarm related test

[Joseph Jang]

In alarm_wkalm_set and alarm_wkalm_set_minute test, they use different
ioctl (RTC_ALM_SET/RTC_WKALM_SET) for alarm feature detection. They will
skip testing if RTC_ALM_SET/RTC_WKALM_SET ioctl returns an EINVAL error
code. This design may miss detecting real problems when the
efi.set_wakeup_time() return errors and then RTC_ALM_SET/RTC_WKALM_SET
ioctl returns an EINVAL error code with RTC_FEATURE_ALARM enabled.

In order to make rtctest more explicit and robust, we propose to use
RTC_PARAM_GET ioctl interface to check rtc alarm feature state before
running alarm related tests. If the kernel does not support RTC_PARAM_GET
ioctl interface, we will fallback to check the error number of
(RTC_ALM_SET/RTC_WKALM_SET) ioctl call for alarm feature detection.

Requires commit 101ca8d05913b ("rtc: efi: Enable SET/GET WAKEUP services
as optional")

Reviewed-by: Koba Ko <kobak@nvidia.com>
Reviewed-by: Matthew R. Ochs <mochs@nvidia.com>
Signed-off-by: Joseph Jang <jjang@nvidia.com>
---
Changes in v2:
- Changed to use $(top_srcdir) instead of hardcoding the path.

 tools/testing/selftests/rtc/Makefile  |  2 +-
 tools/testing/selftests/rtc/rtctest.c | 64 +++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/rtc/Makefile b/tools/testing/selftests/rtc/Makefile
index 55198ecc04db..6e3a98fb24ba 100644
--- a/tools/testing/selftests/rtc/Makefile
+++ b/tools/testing/selftests/rtc/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
-CFLAGS += -O3 -Wl,-no-as-needed -Wall
+CFLAGS += -O3 -Wl,-no-as-needed -Wall -I$(top_srcdir)/usr/include
 LDLIBS += -lrt -lpthread -lm
 
 TEST_GEN_PROGS = rtctest
diff --git a/tools/testing/selftests/rtc/rtctest.c b/tools/testing/selftests/rtc/rtctest.c
index 63ce02d1d5cc..2b12497eb30d 100644
--- a/tools/testing/selftests/rtc/rtctest.c
+++ b/tools/testing/selftests/rtc/rtctest.c
@@ -25,6 +25,12 @@
 
 static char *rtc_file = "/dev/rtc0";
 
+enum rtc_alarm_state {
+	RTC_ALARM_UNKNOWN,
+	RTC_ALARM_ENABLED,
+	RTC_ALARM_DISABLED,
+};
+
 FIXTURE(rtc) {
 	int fd;
 };
@@ -82,6 +88,24 @@ static void nanosleep_with_retries(long ns)
 	}
 }
 
+static enum rtc_alarm_state get_rtc_alarm_state(int fd)
+{
+	struct rtc_param param = { 0 };
+	int rc;
+
+	/* Validate kernel reflects unsupported RTC alarm state */
+	param.param = RTC_PARAM_FEATURES;
+	param.index = 0;
+	rc = ioctl(fd, RTC_PARAM_GET, &param);
+	if (rc < 0)
+		return RTC_ALARM_UNKNOWN;
+
+	if ((param.uvalue & _BITUL(RTC_FEATURE_ALARM)) == 0)
+		return RTC_ALARM_DISABLED;
+
+	return RTC_ALARM_ENABLED;
+}
+
 TEST_F_TIMEOUT(rtc, date_read_loop, READ_LOOP_DURATION_SEC + 2) {
 	int rc;
 	long iter_count = 0;
@@ -197,11 +221,16 @@ TEST_F(rtc, alarm_alm_set) {
 	fd_set readfds;
 	time_t secs, new;
 	int rc;
+	enum rtc_alarm_state alarm_state = RTC_ALARM_UNKNOWN;
 
 	if (self->fd == -1 && errno == ENOENT)
 		SKIP(return, "Skipping test since %s does not exist", rtc_file);
 	ASSERT_NE(-1, self->fd);
 
+	alarm_state = get_rtc_alarm_state(self->fd);
+	if (alarm_state == RTC_ALARM_DISABLED)
+		SKIP(return, "Skipping test since alarms are not supported.");
+
 	rc = ioctl(self->fd, RTC_RD_TIME, &tm);
 	ASSERT_NE(-1, rc);
 
@@ -210,6 +239,11 @@ TEST_F(rtc, alarm_alm_set) {
 
 	rc = ioctl(self->fd, RTC_ALM_SET, &tm);
 	if (rc == -1) {
+		/*
+		 * Report error if rtc alarm was enabled. Fallback to check ioctl
+		 * error number if rtc alarm state is unknown.
+		 */
+		ASSERT_EQ(RTC_ALARM_UNKNOWN, alarm_state);
 		ASSERT_EQ(EINVAL, errno);
 		TH_LOG("skip alarms are not supported.");
 		return;
@@ -255,11 +289,16 @@ TEST_F(rtc, alarm_wkalm_set) {
 	fd_set readfds;
 	time_t secs, new;
 	int rc;
+	enum rtc_alarm_state alarm_state = RTC_ALARM_UNKNOWN;
 
 	if (self->fd == -1 && errno == ENOENT)
 		SKIP(return, "Skipping test since %s does not exist", rtc_file);
 	ASSERT_NE(-1, self->fd);
 
+	alarm_state = get_rtc_alarm_state(self->fd);
+	if (alarm_state == RTC_ALARM_DISABLED)
+		SKIP(return, "Skipping test since alarms are not supported.");
+
 	rc = ioctl(self->fd, RTC_RD_TIME, &alarm.time);
 	ASSERT_NE(-1, rc);
 
@@ -270,6 +309,11 @@ TEST_F(rtc, alarm_wkalm_set) {
 
 	rc = ioctl(self->fd, RTC_WKALM_SET, &alarm);
 	if (rc == -1) {
+		/*
+		 * Report error if rtc alarm was enabled. Fallback to check ioctl
+		 * error number if rtc alarm state is unknown.
+		 */
+		ASSERT_EQ(RTC_ALARM_UNKNOWN, alarm_state);
 		ASSERT_EQ(EINVAL, errno);
 		TH_LOG("skip alarms are not supported.");
 		return;
@@ -307,11 +351,16 @@ TEST_F_TIMEOUT(rtc, alarm_alm_set_minute, 65) {
 	fd_set readfds;
 	time_t secs, new;
 	int rc;
+	enum rtc_alarm_state alarm_state = RTC_ALARM_UNKNOWN;
 
 	if (self->fd == -1 && errno == ENOENT)
 		SKIP(return, "Skipping test since %s does not exist", rtc_file);
 	ASSERT_NE(-1, self->fd);
 
+	alarm_state = get_rtc_alarm_state(self->fd);
+	if (alarm_state == RTC_ALARM_DISABLED)
+		SKIP(return, "Skipping test since alarms are not supported.");
+
 	rc = ioctl(self->fd, RTC_RD_TIME, &tm);
 	ASSERT_NE(-1, rc);
 
@@ -320,6 +369,11 @@ TEST_F_TIMEOUT(rtc, alarm_alm_set_minute, 65) {
 
 	rc = ioctl(self->fd, RTC_ALM_SET, &tm);
 	if (rc == -1) {
+		/*
+		 * Report error if rtc alarm was enabled. Fallback to check ioctl
+		 * error number if rtc alarm state is unknown.
+		 */
+		ASSERT_EQ(RTC_ALARM_UNKNOWN, alarm_state);
 		ASSERT_EQ(EINVAL, errno);
 		TH_LOG("skip alarms are not supported.");
 		return;
@@ -365,11 +419,16 @@ TEST_F_TIMEOUT(rtc, alarm_wkalm_set_minute, 65) {
 	fd_set readfds;
 	time_t secs, new;
 	int rc;
+	enum rtc_alarm_state alarm_state = RTC_ALARM_UNKNOWN;
 
 	if (self->fd == -1 && errno == ENOENT)
 		SKIP(return, "Skipping test since %s does not exist", rtc_file);
 	ASSERT_NE(-1, self->fd);
 
+	alarm_state = get_rtc_alarm_state(self->fd);
+	if (alarm_state == RTC_ALARM_DISABLED)
+		SKIP(return, "Skipping test since alarms are not supported.");
+
 	rc = ioctl(self->fd, RTC_RD_TIME, &alarm.time);
 	ASSERT_NE(-1, rc);
 
@@ -380,6 +439,11 @@ TEST_F_TIMEOUT(rtc, alarm_wkalm_set_minute, 65) {
 
 	rc = ioctl(self->fd, RTC_WKALM_SET, &alarm);
 	if (rc == -1) {
+		/*
+		 * Report error if rtc alarm was enabled. Fallback to check ioctl
+		 * error number if rtc alarm state is unknown.
+		 */
+		ASSERT_EQ(RTC_ALARM_UNKNOWN, alarm_state);
 		ASSERT_EQ(EINVAL, errno);
 		TH_LOG("skip alarms are not supported.");
 		return;
-- 
2.34.1

Re: [PATCH v2 1/2] selftest: rtc: Add to check rtc alarm status for alarm related test

[Alexandre Belloni]

On 20/10/2024 20:22:13-0700, Joseph Jang wrote:
> In alarm_wkalm_set and alarm_wkalm_set_minute test, they use different
> ioctl (RTC_ALM_SET/RTC_WKALM_SET) for alarm feature detection. They will
> skip testing if RTC_ALM_SET/RTC_WKALM_SET ioctl returns an EINVAL error
> code. This design may miss detecting real problems when the
> efi.set_wakeup_time() return errors and then RTC_ALM_SET/RTC_WKALM_SET
> ioctl returns an EINVAL error code with RTC_FEATURE_ALARM enabled.
> 
> In order to make rtctest more explicit and robust, we propose to use
> RTC_PARAM_GET ioctl interface to check rtc alarm feature state before
> running alarm related tests. If the kernel does not support RTC_PARAM_GET
> ioctl interface, we will fallback to check the error number of
> (RTC_ALM_SET/RTC_WKALM_SET) ioctl call for alarm feature detection.
> 
> Requires commit 101ca8d05913b ("rtc: efi: Enable SET/GET WAKEUP services
> as optional")
> 
> Reviewed-by: Koba Ko <kobak@nvidia.com>
> Reviewed-by: Matthew R. Ochs <mochs@nvidia.com>
> Signed-off-by: Joseph Jang <jjang@nvidia.com>

Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
> Changes in v2:
> - Changed to use $(top_srcdir) instead of hardcoding the path.
> 
>  tools/testing/selftests/rtc/Makefile  |  2 +-
>  tools/testing/selftests/rtc/rtctest.c | 64 +++++++++++++++++++++++++++
>  2 files changed, 65 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/rtc/Makefile b/tools/testing/selftests/rtc/Makefile
> index 55198ecc04db..6e3a98fb24ba 100644
> --- a/tools/testing/selftests/rtc/Makefile
> +++ b/tools/testing/selftests/rtc/Makefile
> @@ -1,5 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
> -CFLAGS += -O3 -Wl,-no-as-needed -Wall
> +CFLAGS += -O3 -Wl,-no-as-needed -Wall -I$(top_srcdir)/usr/include
>  LDLIBS += -lrt -lpthread -lm
>  
>  TEST_GEN_PROGS = rtctest
> diff --git a/tools/testing/selftests/rtc/rtctest.c b/tools/testing/selftests/rtc/rtctest.c
> index 63ce02d1d5cc..2b12497eb30d 100644
> --- a/tools/testing/selftests/rtc/rtctest.c
> +++ b/tools/testing/selftests/rtc/rtctest.c
> @@ -25,6 +25,12 @@
>  
>  static char *rtc_file = "/dev/rtc0";
>  
> +enum rtc_alarm_state {
> +	RTC_ALARM_UNKNOWN,
> +	RTC_ALARM_ENABLED,
> +	RTC_ALARM_DISABLED,
> +};
> +
>  FIXTURE(rtc) {
>  	int fd;
>  };
> @@ -82,6 +88,24 @@ static void nanosleep_with_retries(long ns)
>  	}
>  }
>  
> +static enum rtc_alarm_state get_rtc_alarm_state(int fd)
> +{
> +	struct rtc_param param = { 0 };
> +	int rc;
> +
> +	/* Validate kernel reflects unsupported RTC alarm state */
> +	param.param = RTC_PARAM_FEATURES;
> +	param.index = 0;
> +	rc = ioctl(fd, RTC_PARAM_GET, &param);
> +	if (rc < 0)
> +		return RTC_ALARM_UNKNOWN;
> +
> +	if ((param.uvalue & _BITUL(RTC_FEATURE_ALARM)) == 0)
> +		return RTC_ALARM_DISABLED;
> +
> +	return RTC_ALARM_ENABLED;
> +}
> +
>  TEST_F_TIMEOUT(rtc, date_read_loop, READ_LOOP_DURATION_SEC + 2) {
>  	int rc;
>  	long iter_count = 0;
> @@ -197,11 +221,16 @@ TEST_F(rtc, alarm_alm_set) {
>  	fd_set readfds;
>  	time_t secs, new;
>  	int rc;
> +	enum rtc_alarm_state alarm_state = RTC_ALARM_UNKNOWN;
>  
>  	if (self->fd == -1 && errno == ENOENT)
>  		SKIP(return, "Skipping test since %s does not exist", rtc_file);
>  	ASSERT_NE(-1, self->fd);
>  
> +	alarm_state = get_rtc_alarm_state(self->fd);
> +	if (alarm_state == RTC_ALARM_DISABLED)
> +		SKIP(return, "Skipping test since alarms are not supported.");
> +
>  	rc = ioctl(self->fd, RTC_RD_TIME, &tm);
>  	ASSERT_NE(-1, rc);
>  
> @@ -210,6 +239,11 @@ TEST_F(rtc, alarm_alm_set) {
>  
>  	rc = ioctl(self->fd, RTC_ALM_SET, &tm);
>  	if (rc == -1) {
> +		/*
> +		 * Report error if rtc alarm was enabled. Fallback to check ioctl
> +		 * error number if rtc alarm state is unknown.
> +		 */
> +		ASSERT_EQ(RTC_ALARM_UNKNOWN, alarm_state);
>  		ASSERT_EQ(EINVAL, errno);
>  		TH_LOG("skip alarms are not supported.");
>  		return;
> @@ -255,11 +289,16 @@ TEST_F(rtc, alarm_wkalm_set) {
>  	fd_set readfds;
>  	time_t secs, new;
>  	int rc;
> +	enum rtc_alarm_state alarm_state = RTC_ALARM_UNKNOWN;
>  
>  	if (self->fd == -1 && errno == ENOENT)
>  		SKIP(return, "Skipping test since %s does not exist", rtc_file);
>  	ASSERT_NE(-1, self->fd);
>  
> +	alarm_state = get_rtc_alarm_state(self->fd);
> +	if (alarm_state == RTC_ALARM_DISABLED)
> +		SKIP(return, "Skipping test since alarms are not supported.");
> +
>  	rc = ioctl(self->fd, RTC_RD_TIME, &alarm.time);
>  	ASSERT_NE(-1, rc);
>  
> @@ -270,6 +309,11 @@ TEST_F(rtc, alarm_wkalm_set) {
>  
>  	rc = ioctl(self->fd, RTC_WKALM_SET, &alarm);
>  	if (rc == -1) {
> +		/*
> +		 * Report error if rtc alarm was enabled. Fallback to check ioctl
> +		 * error number if rtc alarm state is unknown.
> +		 */
> +		ASSERT_EQ(RTC_ALARM_UNKNOWN, alarm_state);
>  		ASSERT_EQ(EINVAL, errno);
>  		TH_LOG("skip alarms are not supported.");
>  		return;
> @@ -307,11 +351,16 @@ TEST_F_TIMEOUT(rtc, alarm_alm_set_minute, 65) {
>  	fd_set readfds;
>  	time_t secs, new;
>  	int rc;
> +	enum rtc_alarm_state alarm_state = RTC_ALARM_UNKNOWN;
>  
>  	if (self->fd == -1 && errno == ENOENT)
>  		SKIP(return, "Skipping test since %s does not exist", rtc_file);
>  	ASSERT_NE(-1, self->fd);
>  
> +	alarm_state = get_rtc_alarm_state(self->fd);
> +	if (alarm_state == RTC_ALARM_DISABLED)
> +		SKIP(return, "Skipping test since alarms are not supported.");
> +
>  	rc = ioctl(self->fd, RTC_RD_TIME, &tm);
>  	ASSERT_NE(-1, rc);
>  
> @@ -320,6 +369,11 @@ TEST_F_TIMEOUT(rtc, alarm_alm_set_minute, 65) {
>  
>  	rc = ioctl(self->fd, RTC_ALM_SET, &tm);
>  	if (rc == -1) {
> +		/*
> +		 * Report error if rtc alarm was enabled. Fallback to check ioctl
> +		 * error number if rtc alarm state is unknown.
> +		 */
> +		ASSERT_EQ(RTC_ALARM_UNKNOWN, alarm_state);
>  		ASSERT_EQ(EINVAL, errno);
>  		TH_LOG("skip alarms are not supported.");
>  		return;
> @@ -365,11 +419,16 @@ TEST_F_TIMEOUT(rtc, alarm_wkalm_set_minute, 65) {
>  	fd_set readfds;
>  	time_t secs, new;
>  	int rc;
> +	enum rtc_alarm_state alarm_state = RTC_ALARM_UNKNOWN;
>  
>  	if (self->fd == -1 && errno == ENOENT)
>  		SKIP(return, "Skipping test since %s does not exist", rtc_file);
>  	ASSERT_NE(-1, self->fd);
>  
> +	alarm_state = get_rtc_alarm_state(self->fd);
> +	if (alarm_state == RTC_ALARM_DISABLED)
> +		SKIP(return, "Skipping test since alarms are not supported.");
> +
>  	rc = ioctl(self->fd, RTC_RD_TIME, &alarm.time);
>  	ASSERT_NE(-1, rc);
>  
> @@ -380,6 +439,11 @@ TEST_F_TIMEOUT(rtc, alarm_wkalm_set_minute, 65) {
>  
>  	rc = ioctl(self->fd, RTC_WKALM_SET, &alarm);
>  	if (rc == -1) {
> +		/*
> +		 * Report error if rtc alarm was enabled. Fallback to check ioctl
> +		 * error number if rtc alarm state is unknown.
> +		 */
> +		ASSERT_EQ(RTC_ALARM_UNKNOWN, alarm_state);
>  		ASSERT_EQ(EINVAL, errno);
>  		TH_LOG("skip alarms are not supported.");
>  		return;
> -- 
> 2.34.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 1/2] selftest: rtc: Add to check rtc alarm status for alarm related test

[Shuah Khan]

On 10/22/24 10:01, Alexandre Belloni wrote:
> On 20/10/2024 20:22:13-0700, Joseph Jang wrote:
>> In alarm_wkalm_set and alarm_wkalm_set_minute test, they use different
>> ioctl (RTC_ALM_SET/RTC_WKALM_SET) for alarm feature detection. They will
>> skip testing if RTC_ALM_SET/RTC_WKALM_SET ioctl returns an EINVAL error
>> code. This design may miss detecting real problems when the
>> efi.set_wakeup_time() return errors and then RTC_ALM_SET/RTC_WKALM_SET
>> ioctl returns an EINVAL error code with RTC_FEATURE_ALARM enabled.
>>
>> In order to make rtctest more explicit and robust, we propose to use
>> RTC_PARAM_GET ioctl interface to check rtc alarm feature state before
>> running alarm related tests. If the kernel does not support RTC_PARAM_GET
>> ioctl interface, we will fallback to check the error number of
>> (RTC_ALM_SET/RTC_WKALM_SET) ioctl call for alarm feature detection.
>>
>> Requires commit 101ca8d05913b ("rtc: efi: Enable SET/GET WAKEUP services
>> as optional")
>>
>> Reviewed-by: Koba Ko <kobak@nvidia.com>
>> Reviewed-by: Matthew R. Ochs <mochs@nvidia.com>
>> Signed-off-by: Joseph Jang <jjang@nvidia.com>
> 
> Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> 
>> ---
>> Changes in v2:
>> - Changed to use $(top_srcdir) instead of hardcoding the path.
>>

Thanks.

Applied to linux-kselftest next for Linux 6.13-rc1

thanks,
-- Shuah