RISC-V: Fixes and cleanups for QEMU 2.12

[Qemu-devel] [PATCH v6 00/26] RISC-V: Fixes and cleanups for QEMU 2.12

Posted by Michael Clark 6 years, 1 month ago

This is a series of bug fixes and code cleanups that we would
like to get in before the QEMU 2.12 release. We are respinning
v6 of this series to include two new bug fixes. These changes 
are present in the downstream riscv.org riscv-all branch:

- https://github.com/riscv/riscv-qemu/commits/riscv-all

This series also addresses post-merge feedback such as updating
the cpu initialization model to conform with other architectures
as requested by Igor Mammedov.

The riscv_isa_string patch has been dropped as it was merged
independently. The patch to hold rcu_read_lock when accessing
physical memory has been dropped as requested by Paolo Bonzini.

* 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
* Adds hexidecimal instruction bytes to disassembly output
* Sets mtval/stval to zero on exceptions without addresses
* Critical fix for an mstatus.FS bug when MTTCG is enabled
* Fix for incorrect disassembly of addiw instructions

v6

* added workaround for critical mstatus.FS MTTCG bug
* added fix for incorrect disassembly of addiw

v5

* dropped fix for memory allocation bug in riscv_isa_string
* dropped Hold rcu_read_lock when accessing memory

v4

* added fix for memory allocation bug in riscv_isa_string
* trivial fix to remove erroneous comment from translate.c

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

v2

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

v1

* initial post merge cleanup patch series

Michael Clark (26):
  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: 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
  RISC-V: Remove erroneous comment from translate.c
  RISC-V: Fix incorrect disassembly for addiw
  RISC-V: Workaround for critical mstatus.FS MTTCG bug

 disas/riscv.c                   |  41 ++++++-------
 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           |  69 ++++++++++++++++------
 target/riscv/op_helper.c        |  71 ++++++++++++++---------
 target/riscv/translate.c        |   1 -
 18 files changed, 285 insertions(+), 337 deletions(-)

-- 
2.7.0

Re: [Qemu-devel] [PATCH v6 00/26] RISC-V: Fixes and cleanups for QEMU 2.12

Posted by Peter Maydell 6 years ago

On 24 March 2018 at 18:13, Michael Clark <mjc@sifive.com> wrote:
> This is a series of bug fixes and code cleanups that we would
> like to get in before the QEMU 2.12 release. We are respinning
> v6 of this series to include two new bug fixes. These changes
> are present in the downstream riscv.org riscv-all branch:
>
> - https://github.com/riscv/riscv-qemu/commits/riscv-all
>
> This series also addresses post-merge feedback such as updating
> the cpu initialization model to conform with other architectures
> as requested by Igor Mammedov.

Hi. It looks to me like a fair number of these patches
are already reviewed, so we don't need to wait on the
rest being reviewed to get those into master.

My suggestion is that you send a pullrequest now for the
reviewed patches, and send a patchset for review for the
new ones or the ones that still need review. (If there
are patches that are reviewed but depend on earlier ones
that need to go in set 2 then they go in set 2 as well.)

26 patches is a lot to still be carrying around much
beyond rc1, so I would like to see the size of this set
reducing rather than increasing. As the release process
moves forward the bar for "can this still go in" gradually
goes up -- by about rc3 it is at about "is this a
really critical bug or regression from the previous
release".

(Also something seems to have unhelpfully decided to eat
or delay about half of your emails in this patchset :-(
Patchew only sees 14 of the 26. Our mailing list server
does seem to do that occasionally so that would be my
first guess at the culprit, but it's possible it's
something at your end.)

thanks
-- PMM