mbox series

[00/20] target/openrisc improvements

Message ID 20180527141324.11937-1-richard.henderson@linaro.org
Headers show
Series target/openrisc improvements | expand

Message

Richard Henderson May 27, 2018, 2:13 p.m. UTC 
This is almost a grab-bag of little improvements to the port.

patches 1-3:
  Fix singlestepping for gdbstub.  This has apparently never
  worked, as the first commit has the same bug of not advancing
  the pc when stepping.

patches 4-5:
  Exit the TB after l.mtspr insns.  In particular, storing to
  SR changes exception state so we want to return to the main
  loop to recognize any pending interrupts immediately.

patches 6-19:
  Reorganize TLB handling.  There is a fundamental bug that is
  fixed in patch 13.  However the bug has been hidden by extra
  TLB flushing elsewhere in the port.  I remove some unnecessary
  indirection that the port inherited from somewhere -- probably
  the MIPS port.  Finally, I present the QEMU TLB a unified view
  of the OpenRISC split I/D TLB.

patch 20:
  Split out disassembly from translation.

patch 21:
  Add qemu-or1k to qemu-binfmt-conf.sh.


r~


Richard Henderson (20):
  target/openrisc: Remove DISAS_JUMP & DISAS_TB_JUMP
  target/openrisc: Use exit_tb instead of CPU_INTERRUPT_EXITTB
  target/openrisc: Fix singlestep_enabled
  target/openrisc: Link more translation blocks
  target/openrisc: Split out is_user
  target/openrisc: Exit the TB after l.mtspr
  target/openrisc: Form the spr index from tcg
  target/openrisc: Merge tlb allocation into CPUOpenRISCState
  target/openrisc: Remove indirect function calls for mmu
  target/openrisc: Merge mmu_helper.c into mmu.c
  target/openrisc: Reduce tlb to a single dimension
  target/openrisc: Fix tlb flushing in mtspr
  target/openrisc: Fix cpu_mmu_index
  target/openrisc: Use identical sizes for ITLB and DTLB
  target/openrisc: Stub out handle_mmu_fault for softmmu
  target/openrisc: Log interrupts
  target/openrisc: Increase the TLB size
  target/openrisc: Reorg tlb lookup
  target/openrisc: Add print_insn_or1k
  target/or1k: Add support in scripts/qemu-binfmt-conf.sh

 target/openrisc/cpu.h              |  61 +++---
 target/openrisc/helper.h           |   4 +-
 target/openrisc/cpu.c              |  16 +-
 target/openrisc/disas.c            | 171 +++++++++++++++++
 target/openrisc/interrupt.c        |  36 ++--
 target/openrisc/interrupt_helper.c |  35 +---
 target/openrisc/machine.c          |  39 +---
 target/openrisc/mmu.c              | 275 ++++++++++----------------
 target/openrisc/mmu_helper.c       |  40 ----
 target/openrisc/sys_helper.c       |  85 ++++----
 target/openrisc/translate.c        | 298 ++++++++++-------------------
 scripts/qemu-binfmt-conf.sh        |  10 +-
 target/openrisc/Makefile.objs      |   5 +-
 13 files changed, 492 insertions(+), 583 deletions(-)
 create mode 100644 target/openrisc/disas.c
 delete mode 100644 target/openrisc/mmu_helper.c

-- 
2.17.0

Comments

no-reply@patchew.org May 27, 2018, 2:34 p.m. UTC | #1 
Hi,

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

Type: series
Message-id: 20180527141324.11937-1-richard.henderson@linaro.org
Subject: [Qemu-devel] [PATCH 00/20] target/openrisc improvements

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

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

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

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
 t [tag update]            patchew/20180525015042.31778-1-peterx@redhat.com -> patchew/20180525015042.31778-1-peterx@redhat.com
 t [tag update]            patchew/20180525045344.28347-1-kraxel@redhat.com -> patchew/20180525045344.28347-1-kraxel@redhat.com
 t [tag update]            patchew/20180525153609.13187-1-marcandre.lureau@redhat.com -> patchew/20180525153609.13187-1-marcandre.lureau@redhat.com
 * [new tag]               patchew/20180527141324.11937-1-richard.henderson@linaro.org -> patchew/20180527141324.11937-1-richard.henderson@linaro.org
Switched to a new branch 'test'
b9df0fdbbe target/or1k: Add support in scripts/qemu-binfmt-conf.sh
c17f7f48a0 target/openrisc: Add print_insn_or1k
2772ab0c8a target/openrisc: Reorg tlb lookup
3539987243 target/openrisc: Increase the TLB size
13446b9a88 target/openrisc: Log interrupts
762299f6ac target/openrisc: Stub out handle_mmu_fault for softmmu
e0b5725c60 target/openrisc: Use identical sizes for ITLB and DTLB
a363ba96f1 target/openrisc: Fix cpu_mmu_index
b6ec1f027c target/openrisc: Fix tlb flushing in mtspr
b0a589f797 target/openrisc: Reduce tlb to a single dimension
9e167a3b2e target/openrisc: Merge mmu_helper.c into mmu.c
15484f4429 target/openrisc: Remove indirect function calls for mmu
460253a4f3 target/openrisc: Merge tlb allocation into CPUOpenRISCState
40d1a2a12a target/openrisc: Form the spr index from tcg
5338b1f67e target/openrisc: Exit the TB after l.mtspr
c2d6b769e5 target/openrisc: Split out is_user
9bbb13a008 target/openrisc: Link more translation blocks
6714d16f2a target/openrisc: Fix singlestep_enabled
c4d2a9b70b target/openrisc: Use exit_tb instead of CPU_INTERRUPT_EXITTB
2eb8680a11 target/openrisc: Remove DISAS_JUMP & DISAS_TB_JUMP

=== OUTPUT BEGIN ===
Checking PATCH 1/20: target/openrisc: Remove DISAS_JUMP & DISAS_TB_JUMP...
Checking PATCH 2/20: target/openrisc: Use exit_tb instead of CPU_INTERRUPT_EXITTB...
Checking PATCH 3/20: target/openrisc: Fix singlestep_enabled...
Checking PATCH 4/20: target/openrisc: Link more translation blocks...
ERROR: space prohibited between function name and open parenthesis '('
#138: FILE: target/openrisc/translate.c:1453:
+                tcg_gen_lookup_and_goto_ptr ();

ERROR: space prohibited between function name and open parenthesis '('
#155: FILE: target/openrisc/translate.c:1469:
+            tcg_gen_lookup_and_goto_ptr ();

total: 2 errors, 0 warnings, 143 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.

Checking PATCH 5/20: target/openrisc: Split out is_user...
Checking PATCH 6/20: target/openrisc: Exit the TB after l.mtspr...
Checking PATCH 7/20: target/openrisc: Form the spr index from tcg...
Checking PATCH 8/20: target/openrisc: Merge tlb allocation into CPUOpenRISCState...
Checking PATCH 9/20: target/openrisc: Remove indirect function calls for mmu...
Checking PATCH 10/20: target/openrisc: Merge mmu_helper.c into mmu.c...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#45: 
deleted file mode 100644

total: 0 errors, 1 warnings, 23 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.
Checking PATCH 11/20: target/openrisc: Reduce tlb to a single dimension...
ERROR: spaces required around that '-' (ctx:VxV)
#92: FILE: target/openrisc/mmu.c:68:
+    *physical = (tr & TARGET_PAGE_MASK) | (address & (TARGET_PAGE_SIZE-1));
                                                                       ^

ERROR: spaces required around that '-' (ctx:VxV)
#136: FILE: target/openrisc/mmu.c:111:
+    *physical = (tr & TARGET_PAGE_MASK) | (address & (TARGET_PAGE_SIZE-1));
                                                                       ^

total: 2 errors, 0 warnings, 169 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.

Checking PATCH 12/20: target/openrisc: Fix tlb flushing in mtspr...
Checking PATCH 13/20: target/openrisc: Fix cpu_mmu_index...
Checking PATCH 14/20: target/openrisc: Use identical sizes for ITLB and DTLB...
Checking PATCH 15/20: target/openrisc: Stub out handle_mmu_fault for softmmu...
ERROR: space prohibited between function name and open parenthesis '('
#57: FILE: target/openrisc/mmu.c:188:
+    g_assert_not_reached ();

total: 1 errors, 0 warnings, 47 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.

Checking PATCH 16/20: target/openrisc: Log interrupts...
Checking PATCH 17/20: target/openrisc: Increase the TLB size...
Checking PATCH 18/20: target/openrisc: Reorg tlb lookup...
Checking PATCH 19/20: target/openrisc: Add print_insn_or1k...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#65: 
new file mode 100644

ERROR: Macros with complex values should be enclosed in parenthesis
#100: FILE: target/openrisc/disas.c:31:
+#define output(mnemonic, format, ...) \
+    info->fprintf_func(info->stream, "%-9s " format, \
+                       mnemonic, ##__VA_ARGS__)

ERROR: that open brace { should be on the previous line
#111: FILE: target/openrisc/disas.c:42:
+    if (status != 0)
+      {

ERROR: spaces required around that '*' (ctx:WxV)
#126: FILE: target/openrisc/disas.c:57:
+    arg_l_##opcode *a, uint32_t insn)                   \
                    ^

ERROR: spaces required around that '*' (ctx:WxV)
#221: FILE: target/openrisc/disas.c:152:
+    arg_lf_##opcode##_##suffix *a, uint32_t insn)                \
                                ^

ERROR: space required after that ',' (ctx:VxV)
#227: FILE: target/openrisc/disas.c:158:
+FP_INSN(add,s,  "r%d, r%d, r%d", a->d, a->a, a->b)
            ^

ERROR: space required after that ',' (ctx:VxV)
#228: FILE: target/openrisc/disas.c:159:
+FP_INSN(sub,s,  "r%d, r%d, r%d", a->d, a->a, a->b)
            ^

ERROR: space required after that ',' (ctx:VxV)
#229: FILE: target/openrisc/disas.c:160:
+FP_INSN(mul,s,  "r%d, r%d, r%d", a->d, a->a, a->b)
            ^

ERROR: space required after that ',' (ctx:VxV)
#230: FILE: target/openrisc/disas.c:161:
+FP_INSN(div,s,  "r%d, r%d, r%d", a->d, a->a, a->b)
            ^

ERROR: space required after that ',' (ctx:VxV)
#231: FILE: target/openrisc/disas.c:162:
+FP_INSN(rem,s,  "r%d, r%d, r%d", a->d, a->a, a->b)
            ^

ERROR: space required after that ',' (ctx:VxV)
#232: FILE: target/openrisc/disas.c:163:
+FP_INSN(itof,s, "r%d, r%d", a->d, a->a)
             ^

ERROR: space required after that ',' (ctx:VxV)
#233: FILE: target/openrisc/disas.c:164:
+FP_INSN(ftoi,s, "r%d, r%d", a->d, a->a)
             ^

ERROR: space required after that ',' (ctx:VxV)
#234: FILE: target/openrisc/disas.c:165:
+FP_INSN(madd,s, "r%d, r%d, r%d", a->d, a->a, a->b)
             ^

ERROR: space required after that ',' (ctx:VxV)
#235: FILE: target/openrisc/disas.c:166:
+FP_INSN(sfeq,s, "r%d, r%d", a->a, a->b)
             ^

ERROR: space required after that ',' (ctx:VxV)
#236: FILE: target/openrisc/disas.c:167:
+FP_INSN(sfne,s, "r%d, r%d", a->a, a->b)
             ^

ERROR: space required after that ',' (ctx:VxV)
#237: FILE: target/openrisc/disas.c:168:
+FP_INSN(sfgt,s, "r%d, r%d", a->a, a->b)
             ^

ERROR: space required after that ',' (ctx:VxV)
#238: FILE: target/openrisc/disas.c:169:
+FP_INSN(sfge,s, "r%d, r%d", a->a, a->b)
             ^

ERROR: space required after that ',' (ctx:VxV)
#239: FILE: target/openrisc/disas.c:170:
+FP_INSN(sflt,s, "r%d, r%d", a->a, a->b)
             ^

ERROR: space required after that ',' (ctx:VxV)
#240: FILE: target/openrisc/disas.c:171:
+FP_INSN(sfle,s, "r%d, r%d", a->a, a->b)
             ^

total: 18 errors, 1 warnings, 924 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.

Checking PATCH 20/20: target/or1k: Add support in scripts/qemu-binfmt-conf.sh...
WARNING: line over 80 characters
#31: FILE: scripts/qemu-binfmt-conf.sh:127:
+or1k_magic='\x7fELF\x01\x02\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x5c'

ERROR: line over 90 characters
#32: FILE: scripts/qemu-binfmt-conf.sh:128:
+or1k_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff'

total: 1 errors, 1 warnings, 23 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-devel@redhat.com
Stafford Horne May 30, 2018, 11:12 p.m. UTC | #2 
On Sun, May 27, 2018 at 09:13:04AM -0500, Richard Henderson wrote:
> This is almost a grab-bag of little improvements to the port.

> 

> patches 1-3:

>   Fix singlestepping for gdbstub.  This has apparently never

>   worked, as the first commit has the same bug of not advancing

>   the pc when stepping.


This is a big cleanup, thanks.

> patches 4-5:

>   Exit the TB after l.mtspr insns.  In particular, storing to

>   SR changes exception state so we want to return to the main

>   loop to recognize any pending interrupts immediately.


CC'ing Davidson

This might explain part of the problem he was seeing with setting PICMR.

> patches 6-19:

>   Reorganize TLB handling.  There is a fundamental bug that is

>   fixed in patch 13.  However the bug has been hidden by extra

>   TLB flushing elsewhere in the port.  I remove some unnecessary

>   indirection that the port inherited from somewhere -- probably

>   the MIPS port.  Finally, I present the QEMU TLB a unified view

>   of the OpenRISC split I/D TLB.

> 

> patch 20:

>   Split out disassembly from translation.

> 

> patch 21:

>   Add qemu-or1k to qemu-binfmt-conf.sh.


Again, I have nothing in my queue if you can send the pull request it would be
appreciated, if not let me know.

I will reply on the individual patches too, Give me some time, I reviewed on my
phone.

Thanks,

-Stafford