From: Thomas Huth <thuth@redhat.com>
To: kvm@vger.kernel.org
Cc: kvm-ppc@vger.kernel.org, "Laurent Vivier" <lvivier@redhat.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Radim Krčmář" <rkrcmar@redhat.com>
Subject: [kvm-unit-tests PATCH] powerpc: Rework the rtas_token() function
Date: Wed, 31 May 2017 11:15:38 +0000 [thread overview]
Message-ID: <1496229338-7113-1-git-send-email-thuth@redhat.com> (raw)
RTAS tokens can have any value, also 0xffffffff is theoretically
allowed (which is currently used for the RTAS_UNKNOWN_SERVICE error
code). Thus we should not mix error codes and tokens in the return
value here and return the token value via a pointer parameter
instead.
This patch also adds a check to rtas_token() to test whether the device
tree is available at all. This fixes a possible endless loop that
happens without device tree, spamming the console with "rtas_node:
/rtas: FDT_ERR_BADMAGIC" messages: Somewhere the code calls abort()
due to the missing device tree, and abort() calls exit() which in
turn tries to shut down the VM with rtas_power_off(). rtas_power_off()
needs the device tree again to look up the right RTAS token, where we
then end up in the next iteration.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
lib/powerpc/asm/rtas.h | 2 +-
lib/powerpc/rtas.c | 30 ++++++++++++++++++++++--------
lib/powerpc/smp.c | 11 ++++++-----
powerpc/rtas.c | 24 +++++++++++++-----------
4 files changed, 42 insertions(+), 25 deletions(-)
diff --git a/lib/powerpc/asm/rtas.h b/lib/powerpc/asm/rtas.h
index 9012a1e..6fb407a 100644
--- a/lib/powerpc/asm/rtas.h
+++ b/lib/powerpc/asm/rtas.h
@@ -21,7 +21,7 @@ struct rtas_args {
};
extern void rtas_init(void);
-extern int rtas_token(const char *service);
+extern int rtas_token(const char *service, uint32_t *token);
extern int rtas_call(int token, int nargs, int nret, int *outputs, ...);
extern void rtas_power_off(void);
diff --git a/lib/powerpc/rtas.c b/lib/powerpc/rtas.c
index 3407e25..2e7e0da 100644
--- a/lib/powerpc/rtas.c
+++ b/lib/powerpc/rtas.c
@@ -69,17 +69,22 @@ void rtas_init(void)
}
}
-int rtas_token(const char *service)
+int rtas_token(const char *service, uint32_t *token)
{
const struct fdt_property *prop;
- u32 *token;
+ u32 *data;
+
+ if (!dt_available())
+ return RTAS_UNKNOWN_SERVICE;
prop = fdt_get_property(dt_fdt(), rtas_node(), service, NULL);
- if (prop) {
- token = (u32 *)prop->data;
- return fdt32_to_cpu(*token);
- }
- return RTAS_UNKNOWN_SERVICE;
+ if (!prop)
+ return RTAS_UNKNOWN_SERVICE;
+
+ data = (u32 *)prop->data;
+ *token = fdt32_to_cpu(*data);
+
+ return 0;
}
int rtas_call(int token, int nargs, int nret, int *outputs, ...)
@@ -116,6 +121,15 @@ int rtas_call(int token, int nargs, int nret, int *outputs, ...)
void rtas_power_off(void)
{
- int ret = rtas_call(rtas_token("power-off"), 2, 1, NULL, -1, -1);
+ uint32_t token;
+ int ret;
+
+ ret = rtas_token("power-off", &token);
+ if (ret) {
+ puts("RTAS power-off not available\n");
+ return;
+ }
+
+ ret = rtas_call(token, 2, 1, NULL, -1, -1);
printf("RTAS power-off returned %d\n", ret);
}
diff --git a/lib/powerpc/smp.c b/lib/powerpc/smp.c
index e18894b..afe4361 100644
--- a/lib/powerpc/smp.c
+++ b/lib/powerpc/smp.c
@@ -27,12 +27,13 @@ struct secondary_entry_data {
*/
int start_thread(int cpu_id, secondary_entry_fn entry, uint32_t r3)
{
- int query_token, start_token, outputs[1], ret;
+ uint32_t query_token, start_token;
+ int outputs[1], ret;
- query_token = rtas_token("query-cpu-stopped-state");
- start_token = rtas_token("start-cpu");
- assert(query_token != RTAS_UNKNOWN_SERVICE &&
- start_token != RTAS_UNKNOWN_SERVICE);
+ ret = rtas_token("query-cpu-stopped-state", &query_token);
+ assert(ret = 0);
+ ret = rtas_token("start-cpu", &start_token);
+ assert(ret = 0);
ret = rtas_call(query_token, 1, 2, outputs, cpu_id);
if (ret) {
diff --git a/powerpc/rtas.c b/powerpc/rtas.c
index 1b1e9c7..5d43f33 100644
--- a/powerpc/rtas.c
+++ b/powerpc/rtas.c
@@ -39,14 +39,14 @@ static unsigned long mktime(int year, int month, int day,
static void check_get_time_of_day(unsigned long start)
{
- int token;
+ uint32_t token;
int ret;
int now[8];
unsigned long t1, t2, count;
- token = rtas_token("get-time-of-day");
- report("token available", token != RTAS_UNKNOWN_SERVICE);
- if (token = RTAS_UNKNOWN_SERVICE)
+ ret = rtas_token("get-time-of-day", &token);
+ report("token available", ret = 0);
+ if (ret)
return;
ret = rtas_call(token, 0, 8, now);
@@ -74,23 +74,25 @@ static void check_get_time_of_day(unsigned long start)
static void check_set_time_of_day(void)
{
- int token;
+ uint32_t stod_token, gtod_token;
int ret;
int date[8];
unsigned long t1, t2, count;
- token = rtas_token("set-time-of-day");
- report("token available", token != RTAS_UNKNOWN_SERVICE);
- if (token = RTAS_UNKNOWN_SERVICE)
+ ret = rtas_token("set-time-of-day", &stod_token);
+ report("token available", ret = 0);
+ if (ret)
return;
/* 23:59:59 28/2/2000 */
- ret = rtas_call(token, 7, 1, NULL, 2000, 2, 28, 23, 59, 59);
+ ret = rtas_call(stod_token, 7, 1, NULL, 2000, 2, 28, 23, 59, 59);
report("execution", ret = 0);
/* check it has worked */
- ret = rtas_call(rtas_token("get-time-of-day"), 0, 8, date);
+ ret = rtas_token("get-time-of-day", >od_token);
+ assert(ret = 0);
+ ret = rtas_call(gtod_token, 0, 8, date);
report("re-read", ret = 0);
t1 = mktime(2000, 2, 28, 23, 59, 59);
t2 = mktime(date[0], date[1], date[2],
@@ -100,7 +102,7 @@ static void check_set_time_of_day(void)
/* check it is running */
count = 0;
do {
- ret = rtas_call(rtas_token("get-time-of-day"), 0, 8, date);
+ ret = rtas_call(gtod_token, 0, 8, date);
t2 = mktime(date[0], date[1], date[2],
date[3], date[4], date[5]);
count++;
--
1.8.3.1
WARNING: multiple messages have this Message-ID (diff)
From: Thomas Huth <thuth@redhat.com>
To: kvm@vger.kernel.org
Cc: kvm-ppc@vger.kernel.org, "Laurent Vivier" <lvivier@redhat.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Radim Krčmář" <rkrcmar@redhat.com>
Subject: [kvm-unit-tests PATCH] powerpc: Rework the rtas_token() function
Date: Wed, 31 May 2017 13:15:38 +0200 [thread overview]
Message-ID: <1496229338-7113-1-git-send-email-thuth@redhat.com> (raw)
RTAS tokens can have any value, also 0xffffffff is theoretically
allowed (which is currently used for the RTAS_UNKNOWN_SERVICE error
code). Thus we should not mix error codes and tokens in the return
value here and return the token value via a pointer parameter
instead.
This patch also adds a check to rtas_token() to test whether the device
tree is available at all. This fixes a possible endless loop that
happens without device tree, spamming the console with "rtas_node:
/rtas: FDT_ERR_BADMAGIC" messages: Somewhere the code calls abort()
due to the missing device tree, and abort() calls exit() which in
turn tries to shut down the VM with rtas_power_off(). rtas_power_off()
needs the device tree again to look up the right RTAS token, where we
then end up in the next iteration.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
lib/powerpc/asm/rtas.h | 2 +-
lib/powerpc/rtas.c | 30 ++++++++++++++++++++++--------
lib/powerpc/smp.c | 11 ++++++-----
powerpc/rtas.c | 24 +++++++++++++-----------
4 files changed, 42 insertions(+), 25 deletions(-)
diff --git a/lib/powerpc/asm/rtas.h b/lib/powerpc/asm/rtas.h
index 9012a1e..6fb407a 100644
--- a/lib/powerpc/asm/rtas.h
+++ b/lib/powerpc/asm/rtas.h
@@ -21,7 +21,7 @@ struct rtas_args {
};
extern void rtas_init(void);
-extern int rtas_token(const char *service);
+extern int rtas_token(const char *service, uint32_t *token);
extern int rtas_call(int token, int nargs, int nret, int *outputs, ...);
extern void rtas_power_off(void);
diff --git a/lib/powerpc/rtas.c b/lib/powerpc/rtas.c
index 3407e25..2e7e0da 100644
--- a/lib/powerpc/rtas.c
+++ b/lib/powerpc/rtas.c
@@ -69,17 +69,22 @@ void rtas_init(void)
}
}
-int rtas_token(const char *service)
+int rtas_token(const char *service, uint32_t *token)
{
const struct fdt_property *prop;
- u32 *token;
+ u32 *data;
+
+ if (!dt_available())
+ return RTAS_UNKNOWN_SERVICE;
prop = fdt_get_property(dt_fdt(), rtas_node(), service, NULL);
- if (prop) {
- token = (u32 *)prop->data;
- return fdt32_to_cpu(*token);
- }
- return RTAS_UNKNOWN_SERVICE;
+ if (!prop)
+ return RTAS_UNKNOWN_SERVICE;
+
+ data = (u32 *)prop->data;
+ *token = fdt32_to_cpu(*data);
+
+ return 0;
}
int rtas_call(int token, int nargs, int nret, int *outputs, ...)
@@ -116,6 +121,15 @@ int rtas_call(int token, int nargs, int nret, int *outputs, ...)
void rtas_power_off(void)
{
- int ret = rtas_call(rtas_token("power-off"), 2, 1, NULL, -1, -1);
+ uint32_t token;
+ int ret;
+
+ ret = rtas_token("power-off", &token);
+ if (ret) {
+ puts("RTAS power-off not available\n");
+ return;
+ }
+
+ ret = rtas_call(token, 2, 1, NULL, -1, -1);
printf("RTAS power-off returned %d\n", ret);
}
diff --git a/lib/powerpc/smp.c b/lib/powerpc/smp.c
index e18894b..afe4361 100644
--- a/lib/powerpc/smp.c
+++ b/lib/powerpc/smp.c
@@ -27,12 +27,13 @@ struct secondary_entry_data {
*/
int start_thread(int cpu_id, secondary_entry_fn entry, uint32_t r3)
{
- int query_token, start_token, outputs[1], ret;
+ uint32_t query_token, start_token;
+ int outputs[1], ret;
- query_token = rtas_token("query-cpu-stopped-state");
- start_token = rtas_token("start-cpu");
- assert(query_token != RTAS_UNKNOWN_SERVICE &&
- start_token != RTAS_UNKNOWN_SERVICE);
+ ret = rtas_token("query-cpu-stopped-state", &query_token);
+ assert(ret == 0);
+ ret = rtas_token("start-cpu", &start_token);
+ assert(ret == 0);
ret = rtas_call(query_token, 1, 2, outputs, cpu_id);
if (ret) {
diff --git a/powerpc/rtas.c b/powerpc/rtas.c
index 1b1e9c7..5d43f33 100644
--- a/powerpc/rtas.c
+++ b/powerpc/rtas.c
@@ -39,14 +39,14 @@ static unsigned long mktime(int year, int month, int day,
static void check_get_time_of_day(unsigned long start)
{
- int token;
+ uint32_t token;
int ret;
int now[8];
unsigned long t1, t2, count;
- token = rtas_token("get-time-of-day");
- report("token available", token != RTAS_UNKNOWN_SERVICE);
- if (token == RTAS_UNKNOWN_SERVICE)
+ ret = rtas_token("get-time-of-day", &token);
+ report("token available", ret == 0);
+ if (ret)
return;
ret = rtas_call(token, 0, 8, now);
@@ -74,23 +74,25 @@ static void check_get_time_of_day(unsigned long start)
static void check_set_time_of_day(void)
{
- int token;
+ uint32_t stod_token, gtod_token;
int ret;
int date[8];
unsigned long t1, t2, count;
- token = rtas_token("set-time-of-day");
- report("token available", token != RTAS_UNKNOWN_SERVICE);
- if (token == RTAS_UNKNOWN_SERVICE)
+ ret = rtas_token("set-time-of-day", &stod_token);
+ report("token available", ret == 0);
+ if (ret)
return;
/* 23:59:59 28/2/2000 */
- ret = rtas_call(token, 7, 1, NULL, 2000, 2, 28, 23, 59, 59);
+ ret = rtas_call(stod_token, 7, 1, NULL, 2000, 2, 28, 23, 59, 59);
report("execution", ret == 0);
/* check it has worked */
- ret = rtas_call(rtas_token("get-time-of-day"), 0, 8, date);
+ ret = rtas_token("get-time-of-day", >od_token);
+ assert(ret == 0);
+ ret = rtas_call(gtod_token, 0, 8, date);
report("re-read", ret == 0);
t1 = mktime(2000, 2, 28, 23, 59, 59);
t2 = mktime(date[0], date[1], date[2],
@@ -100,7 +102,7 @@ static void check_set_time_of_day(void)
/* check it is running */
count = 0;
do {
- ret = rtas_call(rtas_token("get-time-of-day"), 0, 8, date);
+ ret = rtas_call(gtod_token, 0, 8, date);
t2 = mktime(date[0], date[1], date[2],
date[3], date[4], date[5]);
count++;
--
1.8.3.1
next reply other threads:[~2017-05-31 11:15 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-31 11:15 Thomas Huth [this message]
2017-05-31 11:15 ` [kvm-unit-tests PATCH] powerpc: Rework the rtas_token() function Thomas Huth
2017-05-31 11:58 ` Andrew Jones
2017-05-31 11:58 ` Andrew Jones
2017-05-31 12:07 ` Laurent Vivier
2017-05-31 12:07 ` Laurent Vivier
2017-05-31 12:31 ` Paolo Bonzini
2017-05-31 12:31 ` Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1496229338-7113-1-git-send-email-thuth@redhat.com \
--to=thuth@redhat.com \
--cc=kvm-ppc@vger.kernel.org \
--cc=kvm@vger.kernel.org \
--cc=lvivier@redhat.com \
--cc=pbonzini@redhat.com \
--cc=rkrcmar@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.