From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Vivier Subject: Re: [kvm-unit-tests PATCH] powerpc: Fix endless loop that occurs without a device tree Date: Wed, 31 May 2017 11:00:27 +0200 Message-ID: References: <1496156844-29196-1-git-send-email-thuth@redhat.com> <20170531082152.rid4563ogqeuqdhn@kamzik.brq.redhat.com> <2df7b234-9adb-dbec-6714-784a5e0b71f9@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, pbonzini@redhat.com, rkrcmar@redhat.com, kvm-ppc@vger.kernel.org To: Thomas Huth , Andrew Jones Return-path: Received: from mx1.redhat.com ([209.132.183.28]:56916 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751187AbdEaJAl (ORCPT ); Wed, 31 May 2017 05:00:41 -0400 In-Reply-To: <2df7b234-9adb-dbec-6714-784a5e0b71f9@redhat.com> Content-Language: en-US Sender: kvm-owner@vger.kernel.org List-ID: On 31/05/2017 10:32, Thomas Huth wrote: > On 31.05.2017 10:21, Andrew Jones wrote: >> On Tue, May 30, 2017 at 05:07:24PM +0200, Thomas Huth wrote: >>> When running a powerpc kvm-unit-test, and there is accidentially >>> no device tree available, the test ends up in an endless loop, >>> 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 and we >>> then end up in the next iteration. >>> Fix it by adding some proper checks to rtas_power_off() and >>> rtas_token(). >>> >>> Signed-off-by: Thomas Huth >>> --- >>> lib/powerpc/rtas.c | 13 ++++++++++++- >>> 1 file changed, 12 insertions(+), 1 deletion(-) >>> >>> diff --git a/lib/powerpc/rtas.c b/lib/powerpc/rtas.c >>> index 3407e25..a1c560b 100644 >>> --- a/lib/powerpc/rtas.c >>> +++ b/lib/powerpc/rtas.c >>> @@ -74,6 +74,9 @@ int rtas_token(const char *service) >>> const struct fdt_property *prop; >>> u32 *token; >>> >>> + if (!dt_available()) >>> + return RTAS_UNKNOWN_SERVICE; >>> + >>> prop = fdt_get_property(dt_fdt(), rtas_node(), service, NULL); >>> if (prop) { >>> token = (u32 *)prop->data; >>> @@ -116,6 +119,14 @@ 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); >>> + int token, ret; >>> + >>> + token = rtas_token("power-off"); >>> + if (token < 0) { >> >> token == RTAS_UNKNOWN_SERVICE ? > > Yes, that's likely better ... though the tokens are normally > 0, I just > had a look at the spec and it does not say anything about whether they > have to be positive or not, so negative tokens could happen, too. > I'll send a v2... in rtas_token(), token is u32, so a very big token number can appear as a negative value. But how do you return an error code if the return can be negative? Do you plan to use something like "error = rtas_token(&token, "power-off");"? Thanks, Laurent