Skip to contentSkip to conversation

ppc: Prevent inifnite loop in decrementer auto-reload.

PATCH Applied Roman Kapl 4 participants 5 messages Jan 9, 2017 – Jan 10, 2017

lore
Share & tools
lore

Roman Kapl

author
#0 lore raw

PATCH 1/1 ppc: Prevent inifnite loop in decrementer auto-reload.

If the DECAR register is set to 0, QEMU tries to reload the decrementer with zero in an inifinite loop. According to PPC documentation, the decrementer is triggered on 1->0 transition, so avoid reloading the decrementer if if is already zero.

The problem does not manifest under Linux, but it is valid to set DECAR to zero (and may make sense as part of decrementer initialization when interrupts are disabled).

---
 hw/ppc/ppc_booke.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
hw/ppc/ppc_booke.c +6 -2
index ab8d026..f8d5c28 100644
@@ -198,8 +198,12 @@ static void booke_decr_cb(void *opaque)
     booke_update_irq(cpu);
 
     if (env->spr[SPR_BOOKE_TCR] & TCR_ARE) {
-        /* Auto Reload */
-        cpu_ppc_store_decr(env, env->spr[SPR_BOOKE_DECAR]);
+        /* Do not reload 0, it is already there. It would just trigger
+         * the timer again and lead to infinite loop */
+        if(env->spr[SPR_BOOKE_DECAR] != 0) {
+            /* Auto Reload */
+            cpu_ppc_store_decr(env, env->spr[SPR_BOOKE_DECAR]);
+        }
     }
 }
 
Signature · 1 line
2.10.1
#1 lore raw
Your series seems to have some coding style problems. See output below for more information:

Hi,

Your series seems to have some coding style problems. See output below for more information:

Message-id: [email protected]
Type: series
Subject: [Qemu-devel] [PATCH] ppc: Prevent inifnite loop in decrementer auto-reload.

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)" for c in $commits; do

    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))

done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu
* [new tag] patchew/[email protected] -> patchew/[email protected]
Switched to a new branch 'test'
571ca4b ppc: Prevent inifnite loop in decrementer auto-reload.

=== OUTPUT BEGIN ===
Checking PATCH 1/1: ppc: Prevent inifnite loop in decrementer auto-reload....
ERROR: space required before the open parenthesis '('
#30: FILE: hw/ppc/ppc_booke.c:203:
+ if(env->spr[SPR_BOOKE_DECAR] != 0) {

total: 1 errors, 0 warnings, 14 lines checked

Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1

---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to [email protected]

author
#2 lore raw

PATCH v2 1/1 ppc: Prevent inifnite loop in decrementer auto-reload.

If the DECAR register is set to 0, QEMU tries to reload the decrementer with zero in an inifinite loop. According to PPC documentation, the decrementer is triggered on 1->0 transition, so avoid reloading the decrementer if if is already zero.

The problem does not manifest under Linux, but it is valid to set DECAR to zero (and may make sense as part of decrementer initialization when interrupts are disabled).

---
 hw/ppc/ppc_booke.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
hw/ppc/ppc_booke.c +6 -2
index ab8d026..60baffa 100644
@@ -198,8 +198,12 @@ static void booke_decr_cb(void *opaque)
     booke_update_irq(cpu);
 
     if (env->spr[SPR_BOOKE_TCR] & TCR_ARE) {
-        /* Auto Reload */
-        cpu_ppc_store_decr(env, env->spr[SPR_BOOKE_DECAR]);
+        /* Do not reload 0, it is already there. It would just trigger
+         * the timer again and lead to infinite loop */
+        if (env->spr[SPR_BOOKE_DECAR] != 0) {
+            /* Auto Reload */
+            cpu_ppc_store_decr(env, env->spr[SPR_BOOKE_DECAR]);
+        }
     }
 }
 
Signature · 1 line
2.10.1
#3 lore raw

On 01/09/2017 07:07 AM, Roman Kapl wrote:

s/inifnite/infinite/ in the subject

Quoted 2 lines

If the DECAR register is set to 0, QEMU tries to reload the decrementer with zero in an inifinite loop. According to PPC documentation, the decrementer is

s/inifinite/infinite/

Quoted 6 lines

triggered on 1->0 transition, so avoid reloading the decrementer if if is already zero.

The problem does not manifest under Linux, but it is valid to set DECAR to zero (and may make sense as part of decrementer initialization when interrupts are disabled).

When sending a v2, it's best to do it as a new top-level thread rather than in-reply-to v1, as some patch tools overlook deeply nested replies.
More hints on sending patches:
http://wiki.qemu.org/Contribute/SubmitAPatch

Signature · 2 lines
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Applied
#4 lore raw
In reply to Roman Kapl
Quoted 10 lines · Roman Kapl

If the DECAR register is set to 0, QEMU tries to reload the decrementer with zero in an inifinite loop. According to PPC documentation, the decrementer is triggered on 1->0 transition, so avoid reloading the decrementer if if is already zero.

The problem does not manifest under Linux, but it is valid to set DECAR to zero (and may make sense as part of decrementer initialization when interrupts are disabled).

Applied, fixing the coding style nit (no space after if) in the process. Please remember to run checkpatch.pl in future.

Quoted 23 lines
---
 hw/ppc/ppc_booke.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
hw/ppc/ppc_booke.c +6 -2
index ab8d026..f8d5c28 100644
@@ -198,8 +198,12 @@ static void booke_decr_cb(void *opaque)
     booke_update_irq(cpu);
 
     if (env->spr[SPR_BOOKE_TCR] & TCR_ARE) {
-        /* Auto Reload */
-        cpu_ppc_store_decr(env, env->spr[SPR_BOOKE_DECAR]);
+        /* Do not reload 0, it is already there. It would just trigger
+         * the timer again and lead to infinite loop */
+        if(env->spr[SPR_BOOKE_DECAR] != 0) {
+            /* Auto Reload */
+            cpu_ppc_store_decr(env, env->spr[SPR_BOOKE_DECAR]);
+        }
     }
 }
 
Signature · 4 lines
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson