Re: [Qemu-devel] [PATCH] ppc: Prevent inifnite loop in decrementer auto-reload.

[David Gibson]

On Mon, Jan 09, 2017 at 12:23:38PM +0100, Roman Kapl wrote:
> 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).
> 
> Signed-off-by: Roman Kapl 

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

> ---
>  hw/ppc/ppc_booke.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/ppc_booke.c b/hw/ppc/ppc_booke.c
> index ab8d026..f8d5c28 100644
> --- a/hw/ppc/ppc_booke.c
> +++ b/hw/ppc/ppc_booke.c
> @@ -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]);
> +}
>  }
>  }
>  

-- 
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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] ppc: Prevent inifnite loop in decrementer auto-reload.

[no-reply]

Hi,

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

Message-id: 20170109112338.5629-1-...@sysgo.com
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/20170109112338.5629-1-...@sysgo.com -> 
patchew/20170109112338.5629-1-...@sysgo.com
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 patchew-de...@freelists.org

[Qemu-devel] [PATCH] ppc: Prevent inifnite loop in decrementer auto-reload.

[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).

Signed-off-by: Roman Kapl 
---
 hw/ppc/ppc_booke.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/ppc_booke.c b/hw/ppc/ppc_booke.c
index ab8d026..f8d5c28 100644
--- a/hw/ppc/ppc_booke.c
+++ b/hw/ppc/ppc_booke.c
@@ -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]);
+}
 }
 }
 
-- 
2.10.1