mbox series

[v3,00/23] target/openrisc improvements

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

Message

Richard Henderson June 28, 2018, 3:03 a.m. UTC
Changes since v2:
  * Fix missing mtspr break.
  * Reorg print_insn_or1k and interrupt logging to the start.
  * Adjust exit after mtspr; fixing smp kernel crash.
  * Fix signals patch based on Larent's review.


r~


Richard Henderson (23):
  target/openrisc: Fix mtspr shadow gprs
  target/openrisc: Add print_insn_or1k
  target/openrisc: Log interrupts
  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: Increase the TLB size
  target/openrisc: Reorg tlb lookup
  target/openrisc: Add support in scripts/qemu-binfmt-conf.sh
  linux-user: Implement signals for openrisc
  linux-user: Fix struct sigaltstack for openrisc

 linux-user/openrisc/target_signal.h  |   2 +-
 linux-user/openrisc/target_syscall.h |  28 +--
 target/openrisc/cpu.h                |  61 +++---
 target/openrisc/helper.h             |   4 +-
 linux-user/openrisc/signal.c         | 213 +++++++------------
 linux-user/signal.c                  |   2 +-
 target/openrisc/cpu.c                |  17 +-
 target/openrisc/disas.c              | 170 +++++++++++++++
 target/openrisc/interrupt.c          |  36 +++-
 target/openrisc/interrupt_helper.c   |  35 +--
 target/openrisc/machine.c            |  44 +---
 target/openrisc/mmu.c                | 275 +++++++++---------------
 target/openrisc/mmu_helper.c         |  40 ----
 target/openrisc/sys_helper.c         |  90 ++++----
 target/openrisc/translate.c          | 305 ++++++++++-----------------
 scripts/qemu-binfmt-conf.sh          |  10 +-
 target/openrisc/Makefile.objs        |   5 +-
 17 files changed, 591 insertions(+), 746 deletions(-)
 create mode 100644 target/openrisc/disas.c
 delete mode 100644 target/openrisc/mmu_helper.c

-- 
2.17.1

Comments

Stafford Horne June 28, 2018, 9:31 p.m. UTC | #1
On Wed, Jun 27, 2018 at 08:03:07PM -0700, Richard Henderson wrote:
> Changes since v2:

>   * Fix missing mtspr break.

>   * Reorg print_insn_or1k and interrupt logging to the start.

>   * Adjust exit after mtspr; fixing smp kernel crash.

>   * Fix signals patch based on Larent's review.


Thanks, I confirmed this is working for me too.

As mentioned before there are a few items I am working on.  I will take the
series and send the pull request during the merge window.

I didnt progress on those items yet as I was stuck on the issues mentioned
above, but I will look into fixing issues with:

 - DSX handling differs from hardware
 - PICMR (interrupt masking) differs from hardware

-Stafford
no-reply@patchew.org July 2, 2018, 2:39 a.m. UTC | #2
Hi,

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

Type: series
Message-id: 20180628030330.15615-1-richard.henderson@linaro.org
Subject: [Qemu-devel] [PATCH v3 00/23] 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
Switched to a new branch 'test'
d2fcddb810 linux-user: Fix struct sigaltstack for openrisc
54bf3816e0 linux-user: Implement signals for openrisc
afd05358f9 target/openrisc: Add support in scripts/qemu-binfmt-conf.sh
6c3691e32d target/openrisc: Reorg tlb lookup
f53e2facc7 target/openrisc: Increase the TLB size
0d21fc5c8c target/openrisc: Stub out handle_mmu_fault for softmmu
f62561d1b7 target/openrisc: Use identical sizes for ITLB and DTLB
836489970f target/openrisc: Fix cpu_mmu_index
82ec0e63d4 target/openrisc: Fix tlb flushing in mtspr
7657ccb683 target/openrisc: Reduce tlb to a single dimension
0dedf4e4b9 target/openrisc: Merge mmu_helper.c into mmu.c
55d39fdea4 target/openrisc: Remove indirect function calls for mmu
68faceb291 target/openrisc: Merge tlb allocation into CPUOpenRISCState
bb22ed4bd0 target/openrisc: Form the spr index from tcg
aee6c05885 target/openrisc: Exit the TB after l.mtspr
2fa87705e3 target/openrisc: Split out is_user
8ffdeef89c target/openrisc: Link more translation blocks
14800ca37d target/openrisc: Fix singlestep_enabled
d40522dde7 target/openrisc: Use exit_tb instead of CPU_INTERRUPT_EXITTB
3571c024ff target/openrisc: Remove DISAS_JUMP & DISAS_TB_JUMP
7ae2154034 target/openrisc: Log interrupts
f1f81972ff target/openrisc: Add print_insn_or1k
92ac734d19 target/openrisc: Fix mtspr shadow gprs

=== OUTPUT BEGIN ===
Checking PATCH 1/23: target/openrisc: Fix mtspr shadow gprs...
Checking PATCH 2/23: target/openrisc: Add print_insn_or1k...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#69: 
new file mode 100644

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

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

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

total: 3 errors, 1 warnings, 923 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 3/23: target/openrisc: Log interrupts...
Checking PATCH 4/23: target/openrisc: Remove DISAS_JUMP & DISAS_TB_JUMP...
Checking PATCH 5/23: target/openrisc: Use exit_tb instead of CPU_INTERRUPT_EXITTB...
Checking PATCH 6/23: target/openrisc: Fix singlestep_enabled...
Checking PATCH 7/23: target/openrisc: Link more translation blocks...
Checking PATCH 8/23: target/openrisc: Split out is_user...
Checking PATCH 9/23: target/openrisc: Exit the TB after l.mtspr...
Checking PATCH 10/23: target/openrisc: Form the spr index from tcg...
Checking PATCH 11/23: target/openrisc: Merge tlb allocation into CPUOpenRISCState...
Checking PATCH 12/23: target/openrisc: Remove indirect function calls for mmu...
Checking PATCH 13/23: target/openrisc: Merge mmu_helper.c into mmu.c...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#49: 
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 14/23: target/openrisc: Reduce tlb to a single dimension...
Checking PATCH 15/23: target/openrisc: Fix tlb flushing in mtspr...
Checking PATCH 16/23: target/openrisc: Fix cpu_mmu_index...
Checking PATCH 17/23: target/openrisc: Use identical sizes for ITLB and DTLB...
Checking PATCH 18/23: target/openrisc: Stub out handle_mmu_fault for softmmu...
Checking PATCH 19/23: target/openrisc: Increase the TLB size...
Checking PATCH 20/23: target/openrisc: Reorg tlb lookup...
Checking PATCH 21/23: target/openrisc: Add support in scripts/qemu-binfmt-conf.sh...
WARNING: line over 80 characters
#32: 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
#33: 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.

Checking PATCH 22/23: linux-user: Implement signals for openrisc...
Checking PATCH 23/23: linux-user: Fix struct sigaltstack for openrisc...
=== 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 July 2, 2018, 12:01 p.m. UTC | #3
On Sun, Jul 01, 2018 at 07:39:08PM -0700, no-reply@patchew.org wrote:
> Hi,

> 

> This series seems to have some coding style problems. See output below for

> more information:

> 

> Type: series

> Message-id: 20180628030330.15615-1-richard.henderson@linaro.org

> Subject: [Qemu-devel] [PATCH v3 00/23] 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

> Switched to a new branch 'test'

> d2fcddb810 linux-user: Fix struct sigaltstack for openrisc

> 54bf3816e0 linux-user: Implement signals for openrisc

> afd05358f9 target/openrisc: Add support in scripts/qemu-binfmt-conf.sh

> 6c3691e32d target/openrisc: Reorg tlb lookup

> f53e2facc7 target/openrisc: Increase the TLB size

> 0d21fc5c8c target/openrisc: Stub out handle_mmu_fault for softmmu

> f62561d1b7 target/openrisc: Use identical sizes for ITLB and DTLB

> 836489970f target/openrisc: Fix cpu_mmu_index

> 82ec0e63d4 target/openrisc: Fix tlb flushing in mtspr

> 7657ccb683 target/openrisc: Reduce tlb to a single dimension

> 0dedf4e4b9 target/openrisc: Merge mmu_helper.c into mmu.c

> 55d39fdea4 target/openrisc: Remove indirect function calls for mmu

> 68faceb291 target/openrisc: Merge tlb allocation into CPUOpenRISCState

> bb22ed4bd0 target/openrisc: Form the spr index from tcg

> aee6c05885 target/openrisc: Exit the TB after l.mtspr

> 2fa87705e3 target/openrisc: Split out is_user

> 8ffdeef89c target/openrisc: Link more translation blocks

> 14800ca37d target/openrisc: Fix singlestep_enabled

> d40522dde7 target/openrisc: Use exit_tb instead of CPU_INTERRUPT_EXITTB

> 3571c024ff target/openrisc: Remove DISAS_JUMP & DISAS_TB_JUMP

> 7ae2154034 target/openrisc: Log interrupts

> f1f81972ff target/openrisc: Add print_insn_or1k

> 92ac734d19 target/openrisc: Fix mtspr shadow gprs

> 

> === OUTPUT BEGIN ===

> Checking PATCH 1/23: target/openrisc: Fix mtspr shadow gprs...

> Checking PATCH 2/23: target/openrisc: Add print_insn_or1k...

> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?

> #69: 

> new file mode 100644

> 

> ERROR: Macros with complex values should be enclosed in parenthesis

> #104: FILE: target/openrisc/disas.c:31:

> +#define output(mnemonic, format, ...) \

> +    info->fprintf_func(info->stream, "%-9s " format, \

> +                       mnemonic, ##__VA_ARGS__)

> 

> ERROR: spaces required around that '*' (ctx:WxV)

> #129: FILE: target/openrisc/disas.c:56:

> +    arg_l_##opcode *a, uint32_t insn)                   \

>                     ^

> 

> ERROR: spaces required around that '*' (ctx:WxV)

> #224: FILE: target/openrisc/disas.c:151:

> +    arg_lf_##opcode##_##suffix *a, uint32_t insn)                \

>                                 ^


I have fixed these on my branch.

  https://github.com/stffrdhrn/qemu/tree/or1k-fixes-212-1

> total: 3 errors, 1 warnings, 923 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 3/23: target/openrisc: Log interrupts...

> Checking PATCH 4/23: target/openrisc: Remove DISAS_JUMP & DISAS_TB_JUMP...

> Checking PATCH 5/23: target/openrisc: Use exit_tb instead of CPU_INTERRUPT_EXITTB...

> Checking PATCH 6/23: target/openrisc: Fix singlestep_enabled...

> Checking PATCH 7/23: target/openrisc: Link more translation blocks...

> Checking PATCH 8/23: target/openrisc: Split out is_user...

> Checking PATCH 9/23: target/openrisc: Exit the TB after l.mtspr...

> Checking PATCH 10/23: target/openrisc: Form the spr index from tcg...

> Checking PATCH 11/23: target/openrisc: Merge tlb allocation into CPUOpenRISCState...

> Checking PATCH 12/23: target/openrisc: Remove indirect function calls for mmu...

> Checking PATCH 13/23: target/openrisc: Merge mmu_helper.c into mmu.c...

> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?

> #49: 

> 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 14/23: target/openrisc: Reduce tlb to a single dimension...

> Checking PATCH 15/23: target/openrisc: Fix tlb flushing in mtspr...

> Checking PATCH 16/23: target/openrisc: Fix cpu_mmu_index...

> Checking PATCH 17/23: target/openrisc: Use identical sizes for ITLB and DTLB...

> Checking PATCH 18/23: target/openrisc: Stub out handle_mmu_fault for softmmu...

> Checking PATCH 19/23: target/openrisc: Increase the TLB size...

> Checking PATCH 20/23: target/openrisc: Reorg tlb lookup...

> Checking PATCH 21/23: target/openrisc: Add support in scripts/qemu-binfmt-conf.sh...

> WARNING: line over 80 characters

> #32: 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

> #33: 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'


I will not fix this one.  All other lines are just like this.

I just realized the merge window is nearing an end.  I will send a pull request
soon.

-Stafford

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

> 

> Checking PATCH 22/23: linux-user: Implement signals for openrisc...

> Checking PATCH 23/23: linux-user: Fix struct sigaltstack for openrisc...

> === 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
Peter Maydell July 2, 2018, 12:27 p.m. UTC | #4
On 28 June 2018 at 22:31, Stafford Horne <shorne@gmail.com> wrote:
> On Wed, Jun 27, 2018 at 08:03:07PM -0700, Richard Henderson wrote:

>> Changes since v2:

>>   * Fix missing mtspr break.

>>   * Reorg print_insn_or1k and interrupt logging to the start.

>>   * Adjust exit after mtspr; fixing smp kernel crash.

>>   * Fix signals patch based on Larent's review.

>

> Thanks, I confirmed this is working for me too.

>

> As mentioned before there are a few items I am working on.  I will take the

> series and send the pull request during the merge window.


QEMU doesn't have "merge windows". We just have a softfreeze
date. During the "general development" part of the release
cycle, send pullrequests whenever you like, for new features,
bugfixes, etc. Prefer "little and often" rather than saving
up lots of patches and sending huge pull requests infrequently.

The last pullrequests before softfreeze need to be on the
mailing list by the softfreeze date. After softfreeze, only
bug fixes should be submitted. We get gradually stricter
about how bad the bug has to be to justify allowing in a
fix as we go along; so in rc1 we're fairly relaxed about
larger changes that fix bugs, and by the time we get to
rc3 or rc4 we really only want safe small changes that
fix bugs that are regressions from the previous release.

Pull request cover letters during the freeze part of the
release cycle should include enough detail about the bugs
being fixed to let me judge whether they match the criteria
for going into the next rc, especially at the end of the cycle.

thanks
-- PMM
Richard Henderson July 2, 2018, 2:26 p.m. UTC | #5
On 07/02/2018 05:01 AM, Stafford Horne wrote:
>> ERROR: spaces required around that '*' (ctx:WxV)

>> #129: FILE: target/openrisc/disas.c:56:

>> +    arg_l_##opcode *a, uint32_t insn)                   \

>>                     ^

>>

>> ERROR: spaces required around that '*' (ctx:WxV)

>> #224: FILE: target/openrisc/disas.c:151:

>> +    arg_lf_##opcode##_##suffix *a, uint32_t insn)                \

>>                                 ^

> 

> I have fixed these on my branch.


These should not have been "fixed".

This is checkpatch being stupid and not realizing that
the thing on the left is a type and this is a pointer
declaration, not a multiplication.


r~
Stafford Horne July 2, 2018, 2:39 p.m. UTC | #6
On Mon, Jul 02, 2018 at 07:26:26AM -0700, Richard Henderson wrote:
> On 07/02/2018 05:01 AM, Stafford Horne wrote:

> >> ERROR: spaces required around that '*' (ctx:WxV)

> >> #129: FILE: target/openrisc/disas.c:56:

> >> +    arg_l_##opcode *a, uint32_t insn)                   \

> >>                     ^

> >>

> >> ERROR: spaces required around that '*' (ctx:WxV)

> >> #224: FILE: target/openrisc/disas.c:151:

> >> +    arg_lf_##opcode##_##suffix *a, uint32_t insn)                \

> >>                                 ^

> > 

> > I have fixed these on my branch.

> 

> These should not have been "fixed".

> 

> This is checkpatch being stupid and not realizing that

> the thing on the left is a type and this is a pointer

> declaration, not a multiplication.


Ah, right, and this is me being stupid just following it.

Let me get it fixed again, wait a bit then send a v2 pull.

-Stafford