On 23/05/2024 18:38:06-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@xxxxxxxxxx>
> Reviewed-by: Matthew R. Ochs <mochs@xxxxxxxxxx>
> Signed-off-by: Joseph Jang <jjang@xxxxxxxxxx>
> ---
> 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../../../../usr/include/
Is this change actually needed?
> 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, ¶m);
> + 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
