[Qemu-devel] [PATCH 0/7] Some improvements in z/Arch instructions support

Pavel Zbitskiy posted 7 patches 7 years, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180810030139.25916-1-pavel.zbitskiy@gmail.com
Test checkpatch failed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test docker-quick@centos7 passed
There is a newer version of this series
target/s390x/helper.h           |  1 +
target/s390x/insn-data.def      |  7 ++++
target/s390x/int_helper.c       | 50 ++++++++++++++++++++++++++
target/s390x/mem_helper.c       | 24 ++++++++++---
target/s390x/translate.c        | 64 ++++++++++++++++++++++++++-------
tests/tcg/s390x/Makefile.target |  9 +++++
tests/tcg/s390x/csst.c          | 43 ++++++++++++++++++++++
tests/tcg/s390x/cvb.c           | 18 ++++++++++
tests/tcg/s390x/exrl-trt.c      | 48 +++++++++++++++++++++++++
tests/tcg/s390x/exrl-trtr.c     | 48 +++++++++++++++++++++++++
tests/tcg/s390x/hello-s390x.c   |  7 ++++
tests/tcg/s390x/ipm.c           | 22 ++++++++++++
tests/tcg/s390x/pack.c          | 21 +++++++++++
13 files changed, 346 insertions(+), 16 deletions(-)
create mode 100644 tests/tcg/s390x/Makefile.target
create mode 100644 tests/tcg/s390x/csst.c
create mode 100644 tests/tcg/s390x/cvb.c
create mode 100644 tests/tcg/s390x/exrl-trt.c
create mode 100644 tests/tcg/s390x/exrl-trtr.c
create mode 100644 tests/tcg/s390x/hello-s390x.c
create mode 100644 tests/tcg/s390x/ipm.c
create mode 100644 tests/tcg/s390x/pack.c
[Qemu-devel] [PATCH 0/7] Some improvements in z/Arch instructions support
Posted by Pavel Zbitskiy 7 years, 2 months ago
Found while attempting to run an old tool in qemu.

* BAL and BALR:    Added.
* CSST:            Qemu crashed after an accidental jump to garbage.
* IPM:             A tool produced an incorrect output.
* EX TRT/TRTR:     A tool ran quite slow.
* PACK:            A tool produced an incorrect output.
* CVB, CVBY, CVBG: Added.

Changes since v1:
* Tests.
* Call pc_to_link_info() instead of op_bas().
* Clarified CSST commit message.
* Rewrote IPM using extract/deposit.
* Clarified PACK commit message.
* Do not use LowCore for CONFIG_USER_ONLY.
* Reduce duplication in CVB code.

Pavel Zbitskiy (7):
  tests/tcg: add a simple s390x test
  target/s390x: add BAL and BALR instructions
  target/s390x: fix CSST decoding and runtime alignment check
  target/s390x: fix IPM polluting irrelevant bits
  target/s390x: add EX support for TRT and TRTR
  target/s390x: fix PACK reading 1 byte less and writing 1 byte more
  target/s390x: implement CVB, CVBY and CVBG

 target/s390x/helper.h           |  1 +
 target/s390x/insn-data.def      |  7 ++++
 target/s390x/int_helper.c       | 50 ++++++++++++++++++++++++++
 target/s390x/mem_helper.c       | 24 ++++++++++---
 target/s390x/translate.c        | 64 ++++++++++++++++++++++++++-------
 tests/tcg/s390x/Makefile.target |  9 +++++
 tests/tcg/s390x/csst.c          | 43 ++++++++++++++++++++++
 tests/tcg/s390x/cvb.c           | 18 ++++++++++
 tests/tcg/s390x/exrl-trt.c      | 48 +++++++++++++++++++++++++
 tests/tcg/s390x/exrl-trtr.c     | 48 +++++++++++++++++++++++++
 tests/tcg/s390x/hello-s390x.c   |  7 ++++
 tests/tcg/s390x/ipm.c           | 22 ++++++++++++
 tests/tcg/s390x/pack.c          | 21 +++++++++++
 13 files changed, 346 insertions(+), 16 deletions(-)
 create mode 100644 tests/tcg/s390x/Makefile.target
 create mode 100644 tests/tcg/s390x/csst.c
 create mode 100644 tests/tcg/s390x/cvb.c
 create mode 100644 tests/tcg/s390x/exrl-trt.c
 create mode 100644 tests/tcg/s390x/exrl-trtr.c
 create mode 100644 tests/tcg/s390x/hello-s390x.c
 create mode 100644 tests/tcg/s390x/ipm.c
 create mode 100644 tests/tcg/s390x/pack.c

-- 
2.18.0


Re: [Qemu-devel] [PATCH 0/7] Some improvements in z/Arch instructions support
Posted by Cornelia Huck 7 years, 2 months ago
On Thu,  9 Aug 2018 23:01:32 -0400
Pavel Zbitskiy <pavel.zbitskiy@gmail.com> wrote:

> Found while attempting to run an old tool in qemu.
> 
> * BAL and BALR:    Added.
> * CSST:            Qemu crashed after an accidental jump to garbage.
> * IPM:             A tool produced an incorrect output.
> * EX TRT/TRTR:     A tool ran quite slow.
> * PACK:            A tool produced an incorrect output.
> * CVB, CVBY, CVBG: Added.
> 
> Changes since v1:
> * Tests.

Nice, thanks for adding these.

> * Call pc_to_link_info() instead of op_bas().
> * Clarified CSST commit message.
> * Rewrote IPM using extract/deposit.
> * Clarified PACK commit message.
> * Do not use LowCore for CONFIG_USER_ONLY.
> * Reduce duplication in CVB code.
> 
> Pavel Zbitskiy (7):
>   tests/tcg: add a simple s390x test
>   target/s390x: add BAL and BALR instructions
>   target/s390x: fix CSST decoding and runtime alignment check
>   target/s390x: fix IPM polluting irrelevant bits
>   target/s390x: add EX support for TRT and TRTR
>   target/s390x: fix PACK reading 1 byte less and writing 1 byte more
>   target/s390x: implement CVB, CVBY and CVBG

I'll wait for some acks/reviews before applying these.

Re: [Qemu-devel] [PATCH 0/7] Some improvements in z/Arch instructions support
Posted by David Hildenbrand 7 years, 2 months ago
On 10.08.2018 05:01, Pavel Zbitskiy wrote:
> Found while attempting to run an old tool in qemu.
> 
> * BAL and BALR:    Added.
> * CSST:            Qemu crashed after an accidental jump to garbage.
> * IPM:             A tool produced an incorrect output.
> * EX TRT/TRTR:     A tool ran quite slow.
> * PACK:            A tool produced an incorrect output.
> * CVB, CVBY, CVBG: Added.
> 
> Changes since v1:
> * Tests.
> * Call pc_to_link_info() instead of op_bas().
> * Clarified CSST commit message.
> * Rewrote IPM using extract/deposit.
> * Clarified PACK commit message.
> * Do not use LowCore for CONFIG_USER_ONLY.
> * Reduce duplication in CVB code.

Thanks for looking into this and also for providing test cases!

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 0/7] Some improvements in z/Arch instructions support
Posted by no-reply@patchew.org 7 years, 2 months ago
Hi,

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

Type: series
Message-id: 20180810030139.25916-1-pavel.zbitskiy@gmail.com
Subject: [Qemu-devel] [PATCH 0/7] Some improvements in z/Arch instructions support

=== 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'
bd3e0382ce target/s390x: implement CVB, CVBY and CVBG
f4fba58f1c target/s390x: fix PACK reading 1 byte less and writing 1 byte more
164afdcf5f target/s390x: add EX support for TRT and TRTR
56d0f419b7 target/s390x: fix IPM polluting irrelevant bits
f48adc25a1 target/s390x: fix CSST decoding and runtime alignment check
63fd9ce84b target/s390x: add BAL and BALR instructions
d7d028e76f tests/tcg: add a simple s390x test

=== OUTPUT BEGIN ===
Checking PATCH 1/7: tests/tcg: add a simple s390x test...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#14: 
new file mode 100644

total: 0 errors, 1 warnings, 10 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 2/7: target/s390x: add BAL and BALR instructions...
Checking PATCH 3/7: target/s390x: fix CSST decoding and runtime alignment check...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#72: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#98: FILE: tests/tcg/s390x/csst.c:22:
+        : [op1] "+m" (op1),

ERROR: space prohibited before open square bracket '['
#102: FILE: tests/tcg/s390x/csst.c:26:
+        : [flags] "K" (0x0301),

total: 2 errors, 1 warnings, 66 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 4/7: target/s390x: fix IPM polluting irrelevant bits...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#57: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#74: FILE: tests/tcg/s390x/ipm.c:13:
+        : [cc] "+r" (cc)

ERROR: space prohibited before open square bracket '['
#75: FILE: tests/tcg/s390x/ipm.c:14:
+        : [op1] "r" (&op1),

total: 2 errors, 1 warnings, 53 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/7: target/s390x: add EX support for TRT and TRTR...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#70: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#101: FILE: tests/tcg/s390x/exrl-trt.c:27:
+        : [r1] "+r" (r1),

ERROR: space prohibited before open square bracket '['
#104: FILE: tests/tcg/s390x/exrl-trt.c:30:
+        : [op1] "r" (&op1),

ERROR: space prohibited before open square bracket '['
#155: FILE: tests/tcg/s390x/exrl-trtr.c:27:
+        : [r1] "+r" (r1),

ERROR: space prohibited before open square bracket '['
#158: FILE: tests/tcg/s390x/exrl-trtr.c:30:
+        : [op1] "r" (&op1),

total: 4 errors, 1 warnings, 141 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 6/7: target/s390x: fix PACK reading 1 byte less and writing 1 byte more...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#56: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#72: FILE: tests/tcg/s390x/pack.c:12:
+        : [data] "r" (&data[0])

total: 1 errors, 1 warnings, 43 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 7/7: target/s390x: implement CVB, CVBY and CVBG...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#139: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#154: FILE: tests/tcg/s390x/cvb.c:11:
+        : [result] "+r" (result)

ERROR: space prohibited before open square bracket '['
#155: FILE: tests/tcg/s390x/cvb.c:12:
+        : [data] "m" (data));

total: 2 errors, 1 warnings, 117 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
Re: [Qemu-devel] [PATCH 0/7] Some improvements in z/Arch instructions support
Posted by no-reply@patchew.org 7 years, 2 months ago
Hi,

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

Type: series
Message-id: 20180810030139.25916-1-pavel.zbitskiy@gmail.com
Subject: [Qemu-devel] [PATCH 0/7] Some improvements in z/Arch instructions support

=== 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'
fe6ceb115b target/s390x: implement CVB, CVBY and CVBG
1252e06f7b target/s390x: fix PACK reading 1 byte less and writing 1 byte more
051847fbce target/s390x: add EX support for TRT and TRTR
c8fb5a8ab8 target/s390x: fix IPM polluting irrelevant bits
beeec6e5c4 target/s390x: fix CSST decoding and runtime alignment check
0d91395d44 target/s390x: add BAL and BALR instructions
06e504d408 tests/tcg: add a simple s390x test

=== OUTPUT BEGIN ===
Checking PATCH 1/7: tests/tcg: add a simple s390x test...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#13: 
new file mode 100644

total: 0 errors, 1 warnings, 10 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 2/7: target/s390x: add BAL and BALR instructions...
Checking PATCH 3/7: target/s390x: fix CSST decoding and runtime alignment check...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#72: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#98: FILE: tests/tcg/s390x/csst.c:22:
+        : [op1] "+m" (op1),

ERROR: space prohibited before open square bracket '['
#102: FILE: tests/tcg/s390x/csst.c:26:
+        : [flags] "K" (0x0301),

total: 2 errors, 1 warnings, 66 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 4/7: target/s390x: fix IPM polluting irrelevant bits...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#57: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#74: FILE: tests/tcg/s390x/ipm.c:13:
+        : [cc] "+r" (cc)

ERROR: space prohibited before open square bracket '['
#75: FILE: tests/tcg/s390x/ipm.c:14:
+        : [op1] "r" (&op1),

total: 2 errors, 1 warnings, 53 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/7: target/s390x: add EX support for TRT and TRTR...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#70: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#101: FILE: tests/tcg/s390x/exrl-trt.c:27:
+        : [r1] "+r" (r1),

ERROR: space prohibited before open square bracket '['
#104: FILE: tests/tcg/s390x/exrl-trt.c:30:
+        : [op1] "r" (&op1),

ERROR: space prohibited before open square bracket '['
#155: FILE: tests/tcg/s390x/exrl-trtr.c:27:
+        : [r1] "+r" (r1),

ERROR: space prohibited before open square bracket '['
#158: FILE: tests/tcg/s390x/exrl-trtr.c:30:
+        : [op1] "r" (&op1),

total: 4 errors, 1 warnings, 141 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 6/7: target/s390x: fix PACK reading 1 byte less and writing 1 byte more...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#56: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#72: FILE: tests/tcg/s390x/pack.c:12:
+        : [data] "r" (&data[0])

total: 1 errors, 1 warnings, 43 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 7/7: target/s390x: implement CVB, CVBY and CVBG...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#139: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#154: FILE: tests/tcg/s390x/cvb.c:11:
+        : [result] "+r" (result)

ERROR: space prohibited before open square bracket '['
#155: FILE: tests/tcg/s390x/cvb.c:12:
+        : [data] "m" (data));

total: 2 errors, 1 warnings, 117 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
Re: [Qemu-devel] [PATCH 0/7] Some improvements in z/Arch instructions support
Posted by Cornelia Huck 7 years, 2 months ago
On Wed, 15 Aug 2018 11:25:26 -0700 (PDT)
no-reply@patchew.org wrote:

> === OUTPUT BEGIN ===
> Checking PATCH 1/7: tests/tcg: add a simple s390x test...
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #13: 
> new file mode 100644

tests/tcg/s390x/ does not seem to have a MAINTAINERS entry yet (it did
not contain anything interesting before, anyway.) Probably should be
added to the s390x/tcg section, as for the other architectures.

> 
> total: 0 errors, 1 warnings, 10 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 2/7: target/s390x: add BAL and BALR instructions...
> Checking PATCH 3/7: target/s390x: fix CSST decoding and runtime alignment check...
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #72: 
> new file mode 100644
> 
> ERROR: space prohibited before open square bracket '['
> #98: FILE: tests/tcg/s390x/csst.c:22:
> +        : [op1] "+m" (op1),
> 
> ERROR: space prohibited before open square bracket '['
> #102: FILE: tests/tcg/s390x/csst.c:26:
> +        : [flags] "K" (0x0301),

Checkpatch often seems to have a hard time dealing with inline
assemblies. Probably best to just ignore the errors.