[Qemu-devel] [PATCH v3 00/24] RISC-V Post-merge spec conformance and cleanup

Michael Clark posted 24 patches 7 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1521229281-73637-1-git-send-email-mjc@sifive.com
Test checkpatch failed
Test docker-build@min-glib passed
Test docker-mingw@fedora passed
Test docker-quick@centos6 passed
Test s390x passed
There is a newer version of this series
disas/riscv.c                   |  39 +++++++------
hw/riscv/riscv_hart.c           |   6 --
hw/riscv/sifive_clint.c         |   9 +--
hw/riscv/sifive_e.c             |  34 +----------
hw/riscv/sifive_u.c             |  65 +++++++--------------
hw/riscv/spike.c                |  65 ++++++++-------------
hw/riscv/virt.c                 |  77 +++++++++----------------
include/hw/riscv/sifive_clint.h |   4 ++
include/hw/riscv/sifive_e.h     |   5 --
include/hw/riscv/sifive_u.h     |   9 ++-
include/hw/riscv/spike.h        |  15 ++---
include/hw/riscv/virt.h         |  17 +++---
target/riscv/cpu.c              | 125 ++++++++++++++++++++++------------------
target/riscv/cpu.h              |   6 +-
target/riscv/cpu_bits.h         |   3 -
target/riscv/helper.c           |  83 +++++++++++++++++++-------
target/riscv/op_helper.c        |  52 ++++++++---------
17 files changed, 279 insertions(+), 335 deletions(-)
[Qemu-devel] [PATCH v3 00/24] RISC-V Post-merge spec conformance and cleanup
Posted by Michael Clark 7 years, 7 months ago
This is a series of spec conformance bug fixes and code cleanups
that we would like to get in before the QEMU 2.12 release. This
series does not contain the fix to riscv_isa_string. Previous
versions of this series have been included in the riscv.org QEMU
repository and these changes have had extensive testing running
Fedora for RISC-V, including building QEMU inside of RISC-V QEMU
running SMP Linux.

* Implements WARL behavior for CSRs that don't support writes
* Improves specification conformance of the page table walker
  * Change access checks from ternary operator to if statements
  * Checks for misaligned PPNs
  * Disallow M-mode or S-mode from fetching from User pages
  * Adds reserved PTE flag check: W or W|X
  * Set READ flag for PTE X flag if mstatus.mxr is in effect
  * Improves page walker comments and general readability 
* Several trivial code cleanups to hw/riscv
  * Replacing hard coded constants with reference to enums
    or the machine memory maps.
  * Remove unnecessary class initialization boilerplate
* Adds bounds checks when writing device-tree to ROM
* Updates the cpu model to use a more modern interface
* Sets mtval/stval to zero on exceptions without addresses

v2

- remove unused class boilerplate retains qom parent_obj
- convert cpu definition towards future model
- honor mstatus.mxr flag in page table walker

v3

- refactor rcu_read_lock in PTE update to use single unlock
- mstatus.mxr is in effect regardless of privilege mode
- remove unnecessary class init from riscv_hart
- set mtval/stval to zero on exceptions without addresses

Michael Clark (24):
  RISC-V: Make virt create_fdt interface consistent
  RISC-V: Replace hardcoded constants with enum values
  RISC-V: Make virt board description match spike
  RISC-V: Use ROM base address and size from memmap
  RISC-V: Remove identity_translate from load_elf
  RISC-V: Mark ROM read-only after copying in code
  RISC-V: Remove unused class definitions
  RISC-V: Make sure rom has space for fdt
  RISC-V: Include intruction hex in disassembly
  RISC-V: Hold rcu_read_lock when accessing memory
  RISC-V: Improve page table walker spec compliance
  RISC-V: Update E order and I extension order
  RISC-V: Make some header guards more specific
  RISC-V: Make virt header comment title consistent
  RISC-V: Use memory_region_is_ram in pte update
  RISC-V: Remove EM_RISCV ELF_MACHINE indirection
  RISC-V: Hardwire satp to 0 for no-mmu case
  RISC-V: Remove braces from satp case statement
  RISC-V: riscv-qemu port supports sv39 and sv48
  RISC-V: vectored traps are optional
  RISC-V: No traps on writes to misa,minstret,mcycle
  RISC-V: Remove support for adhoc X_COP interrupt
  RISC-V: Convert cpu definition towards future model
  RISC-V: Clear mtval/stval on exceptions without info

 disas/riscv.c                   |  39 +++++++------
 hw/riscv/riscv_hart.c           |   6 --
 hw/riscv/sifive_clint.c         |   9 +--
 hw/riscv/sifive_e.c             |  34 +----------
 hw/riscv/sifive_u.c             |  65 +++++++--------------
 hw/riscv/spike.c                |  65 ++++++++-------------
 hw/riscv/virt.c                 |  77 +++++++++----------------
 include/hw/riscv/sifive_clint.h |   4 ++
 include/hw/riscv/sifive_e.h     |   5 --
 include/hw/riscv/sifive_u.h     |   9 ++-
 include/hw/riscv/spike.h        |  15 ++---
 include/hw/riscv/virt.h         |  17 +++---
 target/riscv/cpu.c              | 125 ++++++++++++++++++++++------------------
 target/riscv/cpu.h              |   6 +-
 target/riscv/cpu_bits.h         |   3 -
 target/riscv/helper.c           |  83 +++++++++++++++++++-------
 target/riscv/op_helper.c        |  52 ++++++++---------
 17 files changed, 279 insertions(+), 335 deletions(-)

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>

-- 
2.7.0


Re: [Qemu-devel] [PATCH v3 00/24] RISC-V Post-merge spec conformance and cleanup
Posted by no-reply@patchew.org 7 years, 7 months ago
Hi,

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

Type: series
Message-id: 1521229281-73637-1-git-send-email-mjc@sifive.com
Subject: [Qemu-devel] [PATCH v3 00/24] RISC-V Post-merge spec conformance and cleanup

=== 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
 * [new tag]               patchew/1521229281-73637-1-git-send-email-mjc@sifive.com -> patchew/1521229281-73637-1-git-send-email-mjc@sifive.com
Switched to a new branch 'test'
cf9ad8471c RISC-V: Clear mtval/stval on exceptions without info
e0eb5ad03f RISC-V: Convert cpu definition towards future model
cb986bd661 RISC-V: Remove support for adhoc X_COP interrupt
c1bea999fe RISC-V: No traps on writes to misa, minstret, mcycle
c45005f71d RISC-V: vectored traps are optional
23dccf73aa RISC-V: riscv-qemu port supports sv39 and sv48
9beecddea0 RISC-V: Remove braces from satp case statement
db362a77b9 RISC-V: Hardwire satp to 0 for no-mmu case
c696060959 RISC-V: Remove EM_RISCV ELF_MACHINE indirection
e48c09d25a RISC-V: Use memory_region_is_ram in pte update
eaa01d4e61 RISC-V: Make virt header comment title consistent
9435045fd8 RISC-V: Make some header guards more specific
23973a84f7 RISC-V: Update E order and I extension order
6efbb781e4 RISC-V: Improve page table walker spec compliance
924e46b0b8 RISC-V: Hold rcu_read_lock when accessing memory
eef95b067f RISC-V: Include intruction hex in disassembly
3884a14086 RISC-V: Make sure rom has space for fdt
49ad774612 RISC-V: Remove unused class definitions
4b771876d8 RISC-V: Mark ROM read-only after copying in code
646a8d2508 RISC-V: Remove identity_translate from load_elf
c582b7bbf0 RISC-V: Use ROM base address and size from memmap
8a3ef54b0d RISC-V: Make virt board description match spike
ba0cbace2d RISC-V: Replace hardcoded constants with enum values
ec0c0eea8c RISC-V: Make virt create_fdt interface consistent

=== OUTPUT BEGIN ===
Checking PATCH 1/24: RISC-V: Make virt create_fdt interface consistent...
Checking PATCH 2/24: RISC-V: Replace hardcoded constants with enum values...
Checking PATCH 3/24: RISC-V: Make virt board description match spike...
Checking PATCH 4/24: RISC-V: Use ROM base address and size from memmap...
Checking PATCH 5/24: RISC-V: Remove identity_translate from load_elf...
Checking PATCH 6/24: RISC-V: Mark ROM read-only after copying in code...
Checking PATCH 7/24: RISC-V: Remove unused class definitions...
Checking PATCH 8/24: RISC-V: Make sure rom has space for fdt...
Checking PATCH 9/24: RISC-V: Include intruction hex in disassembly...
Checking PATCH 10/24: RISC-V: Hold rcu_read_lock when accessing memory...
ERROR: switch and case should be at the same indent
#50: FILE: target/riscv/helper.c:240:
+                switch (action) {
+                    case success: break;
+                    case translate_fail: return TRANSLATE_FAIL;
+                    case restart_walk: goto restart;

ERROR: trailing statements should be on next line
#51: FILE: target/riscv/helper.c:241:
+                    case success: break;

ERROR: trailing statements should be on next line
#53: FILE: target/riscv/helper.c:243:
+                    case restart_walk: goto restart;

total: 3 errors, 0 warnings, 32 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/24: RISC-V: Improve page table walker spec compliance...
Checking PATCH 12/24: RISC-V: Update E order and I extension order...
Checking PATCH 13/24: RISC-V: Make some header guards more specific...
Checking PATCH 14/24: RISC-V: Make virt header comment title consistent...
Checking PATCH 15/24: RISC-V: Use memory_region_is_ram in pte update...
Checking PATCH 16/24: RISC-V: Remove EM_RISCV ELF_MACHINE indirection...
Checking PATCH 17/24: RISC-V: Hardwire satp to 0 for no-mmu case...
Checking PATCH 18/24: RISC-V: Remove braces from satp case statement...
Checking PATCH 19/24: RISC-V: riscv-qemu port supports sv39 and sv48...
Checking PATCH 20/24: RISC-V: vectored traps are optional...
ERROR: trailing whitespace
#27: FILE: target/riscv/op_helper.c:265:
+        /* we do not support vectored traps for asynchrounous interrupts */ $

ERROR: trailing whitespace
#42: FILE: target/riscv/op_helper.c:286:
+        /* we do not support vectored traps for asynchrounous interrupts */ $

total: 2 errors, 0 warnings, 28 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 21/24: RISC-V: No traps on writes to misa, minstret, mcycle...
Checking PATCH 22/24: RISC-V: Remove support for adhoc X_COP interrupt...
Checking PATCH 23/24: RISC-V: Convert cpu definition towards future model...
Checking PATCH 24/24: RISC-V: Clear mtval/stval on exceptions without info...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
Re: [Qemu-devel] [patches] Re: [PATCH v3 00/24] RISC-V Post-merge spec conformance and cleanup
Posted by Michael Clark 7 years, 7 months ago
On Fri, Mar 16, 2018 at 1:06 PM, <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: 1521229281-73637-1-git-send-email-mjc@sifive.com
> Subject: [Qemu-devel] [PATCH v3 00/24] RISC-V Post-merge spec conformance
> and cleanup
>
> === 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
>  * [new tag]               patchew/1521229281-73637-1-
> git-send-email-mjc@sifive.com -> patchew/1521229281-73637-1-
> git-send-email-mjc@sifive.com
> Switched to a new branch 'test'
> cf9ad8471c RISC-V: Clear mtval/stval on exceptions without info
> e0eb5ad03f RISC-V: Convert cpu definition towards future model
> cb986bd661 RISC-V: Remove support for adhoc X_COP interrupt
> c1bea999fe RISC-V: No traps on writes to misa, minstret, mcycle
> c45005f71d RISC-V: vectored traps are optional
> 23dccf73aa RISC-V: riscv-qemu port supports sv39 and sv48
> 9beecddea0 RISC-V: Remove braces from satp case statement
> db362a77b9 RISC-V: Hardwire satp to 0 for no-mmu case
> c696060959 RISC-V: Remove EM_RISCV ELF_MACHINE indirection
> e48c09d25a RISC-V: Use memory_region_is_ram in pte update
> eaa01d4e61 RISC-V: Make virt header comment title consistent
> 9435045fd8 RISC-V: Make some header guards more specific
> 23973a84f7 RISC-V: Update E order and I extension order
> 6efbb781e4 RISC-V: Improve page table walker spec compliance
> 924e46b0b8 RISC-V: Hold rcu_read_lock when accessing memory
> eef95b067f RISC-V: Include intruction hex in disassembly
> 3884a14086 RISC-V: Make sure rom has space for fdt
> 49ad774612 RISC-V: Remove unused class definitions
> 4b771876d8 RISC-V: Mark ROM read-only after copying in code
> 646a8d2508 RISC-V: Remove identity_translate from load_elf
> c582b7bbf0 RISC-V: Use ROM base address and size from memmap
> 8a3ef54b0d RISC-V: Make virt board description match spike
> ba0cbace2d RISC-V: Replace hardcoded constants with enum values
> ec0c0eea8c RISC-V: Make virt create_fdt interface consistent
>
> === OUTPUT BEGIN ===
> Checking PATCH 1/24: RISC-V: Make virt create_fdt interface consistent...
> Checking PATCH 2/24: RISC-V: Replace hardcoded constants with enum
> values...
> Checking PATCH 3/24: RISC-V: Make virt board description match spike...
> Checking PATCH 4/24: RISC-V: Use ROM base address and size from memmap...
> Checking PATCH 5/24: RISC-V: Remove identity_translate from load_elf...
> Checking PATCH 6/24: RISC-V: Mark ROM read-only after copying in code...
> Checking PATCH 7/24: RISC-V: Remove unused class definitions...
> Checking PATCH 8/24: RISC-V: Make sure rom has space for fdt...
> Checking PATCH 9/24: RISC-V: Include intruction hex in disassembly...
> Checking PATCH 10/24: RISC-V: Hold rcu_read_lock when accessing memory...
> ERROR: switch and case should be at the same indent
> #50: FILE: target/riscv/helper.c:240:
> +                switch (action) {
> +                    case success: break;
> +                    case translate_fail: return TRANSLATE_FAIL;
> +                    case restart_walk: goto restart;
>
> ERROR: trailing statements should be on next line
> #51: FILE: target/riscv/helper.c:241:
> +                    case success: break;
>
> ERROR: trailing statements should be on next line
> #53: FILE: target/riscv/helper.c:243:
> +                    case restart_walk: goto restart;
>
> total: 3 errors, 0 warnings, 32 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/24: RISC-V: Improve page table walker spec compliance...
> Checking PATCH 12/24: RISC-V: Update E order and I extension order...
> Checking PATCH 13/24: RISC-V: Make some header guards more specific...
> Checking PATCH 14/24: RISC-V: Make virt header comment title consistent...
> Checking PATCH 15/24: RISC-V: Use memory_region_is_ram in pte update...
> Checking PATCH 16/24: RISC-V: Remove EM_RISCV ELF_MACHINE indirection...
> Checking PATCH 17/24: RISC-V: Hardwire satp to 0 for no-mmu case...
> Checking PATCH 18/24: RISC-V: Remove braces from satp case statement...
> Checking PATCH 19/24: RISC-V: riscv-qemu port supports sv39 and sv48...
> Checking PATCH 20/24: RISC-V: vectored traps are optional...
> ERROR: trailing whitespace
> #27: FILE: target/riscv/op_helper.c:265:
> +        /* we do not support vectored traps for asynchrounous interrupts
> */ $
>
> ERROR: trailing whitespace
> #42: FILE: target/riscv/op_helper.c:286:
> +        /* we do not support vectored traps for asynchrounous interrupts
> */ $
>
> total: 2 errors, 0 warnings, 28 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 21/24: RISC-V: No traps on writes to misa, minstret,
> mcycle...
> Checking PATCH 22/24: RISC-V: Remove support for adhoc X_COP interrupt...
> Checking PATCH 23/24: RISC-V: Convert cpu definition towards future
> model...
> Checking PATCH 24/24: RISC-V: Clear mtval/stval on exceptions without
> info...
> === OUTPUT END ===
>
> Test command exited with code: 1



BTW: I've just fixed the checkpatch warnings and they are in my tree here:

- https://github.com/michaeljclark/riscv-qemu/tree/qemu-devel