On 31.05.2017 11:00, Laurent Vivier wrote:
> 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 <thuth@xxxxxxxxxx>
>>>> ---
>>>> 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");"?
Hmmm, though it is very unlikely that we ever encounter an RTAS
implementation that uses 0xffffffff as token, it still could
theoretically happen.
So I guess I have to bite the bullet, implement something like you
suggested and change all spots that currently use rtas_token()...
Thomas
