Re: [Qemu-devel] [PATCH v1 00/22] Spec conformance bug fixes and cleanups

[Emilio G. Cota]

On Wed, Mar 07, 2018 at 13:00:35 +1300, Michael Clark wrote:
> I'll use dry run next time and reword the patches to add 'Cc.

Just to be clear, it's totally OK to add the Cc lines to the commit
message -- much easier than editing the .patch files, especially
for long patch series that go through possibly many iterations.

E.

Re: [Qemu-devel] [PATCH v1 00/22] Spec conformance bug fixes and cleanups

[Michael Clark]

On Wed, Mar 7, 2018 at 12:47 PM, Emilio G. Cota  wrote:

> On Wed, Mar 07, 2018 at 12:07:18 +1300, Michael Clark wrote:
> > BTW Apologies for the duplicate emails. I'm still getting to grips with
> the
> > git-sendemail workflow and was using a sed script to Add Cc's which
> munged
> > the headers as it didn't take into account Subject lines flowing to two
> > lines. I guess I can just include Cc: in the commit message? and
> > git-format-patch will handle it for me? or I just should how to use
> > git-publish...
>
> I don't have experience with git-publish. The following two suggestions
> might help though:
>
> - Yes, add Cc's to individual patches -- those are picked up by send-email.
>   That also applies to the cover letter, although note that if
>   you use --compose then Cc's won't be picked up. Instead, just write
>   the cover letter into a -$cover.patch (with Cc's in there)
>   and do not use --compose.
>
> - Use send-email's --dry-run option to make sure everything looks good
>   without actually sending any emails.


I'll use dry run next time and reword the patches to add 'Cc. The previous
patch series all had subject lines that didn't flow to the next line and I
had script automation to carve up the port. I'm trying to get into a new
habit of making the first line of the patch (Subject) as short as
reasonably possible and instead putting a longer description in the body.
Short subjects and short first lines of commits are better.


> > I still have to work on Igor's requested change to CPU initializer
> > declarations, and i'd also like to get the patch in that fixes fmin/fmax
> > test failures using the riscv-tests testsuite by implementing IEEE-754
> > minimumNumber/maximumNumber. I'll try to get a patch onto the list before
> > soft-freeze. I dropped the patch which was in an earlier version of our
> > port patch series due to a conflict with changes in softfloat (conversion
> > of minmax from a macro to a static function iirc).
>
> Note that if you're fixing a bug (and the fmin/fmax patch qualifies),
> you can definitely get it merged post-soft-freeze.


 Okay. good.

Re: [Qemu-devel] [PATCH v1 00/22] Spec conformance bug fixes and cleanups

[Emilio G. Cota]

On Wed, Mar 07, 2018 at 12:07:18 +1300, Michael Clark wrote:
> BTW Apologies for the duplicate emails. I'm still getting to grips with the
> git-sendemail workflow and was using a sed script to Add Cc's which munged
> the headers as it didn't take into account Subject lines flowing to two
> lines. I guess I can just include Cc: in the commit message? and
> git-format-patch will handle it for me? or I just should how to use
> git-publish...

I don't have experience with git-publish. The following two suggestions
might help though:

- Yes, add Cc's to individual patches -- those are picked up by send-email.
  That also applies to the cover letter, although note that if
  you use --compose then Cc's won't be picked up. Instead, just write
  the cover letter into a -$cover.patch (with Cc's in there)
  and do not use --compose.

- Use send-email's --dry-run option to make sure everything looks good
  without actually sending any emails.

> I still have to work on Igor's requested change to CPU initializer
> declarations, and i'd also like to get the patch in that fixes fmin/fmax
> test failures using the riscv-tests testsuite by implementing IEEE-754
> minimumNumber/maximumNumber. I'll try to get a patch onto the list before
> soft-freeze. I dropped the patch which was in an earlier version of our
> port patch series due to a conflict with changes in softfloat (conversion
> of minmax from a macro to a static function iirc).

Note that if you're fixing a bug (and the fmin/fmax patch qualifies),
you can definitely get it merged post-soft-freeze.

E.

Re: [Qemu-devel] [PATCH v1 00/22] Spec conformance bug fixes and cleanups

[Michael Clark]

On Wed, Mar 7, 2018 at 9:43 AM, Michael Clark  wrote:

> This is the series of spec conformance bug fixes and code cleanups.
> We would like to get this series in after our core changes in v8.2.
>
> - 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
>   - 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.
> - Adds bounds checks when writing device-tree to ROM
>
> Michael Clark (22):
>   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 memory map
>   RISC-V: Remove redundant identity_translate from load_elf
>   RISC-V: Mark ROM read-only after copying in code and config
>   RISC-V: Remove unused class definitions from machines
>   RISC-V: Make sure the emulated rom has space for device-tree
>   RISC-V: Include hexidecimal instruction in disassembly
>   RISC-V: Hold rcu_read_lock when accessing memory directly
>   RISC-V: Improve page table walker spec compliance
>   RISC-V: Update E order and I extension order
>   RISC-V: Make spike and virt header guards more specific
>   RISC-V: Make virt header comment title consistent
>   RISC-V: Use memory_region_is_ram in atomic pte update
>   RISC-V: Remove EM_RISCV ELF_MACHINE indirection from load_elf
>   RISC-V: Ingore satp writes and return 0 for reads when no-mmu
>   RISC-V: Remove braces from satp case statement with no locals
>   RISC-V: riscv-qemu port supports sv39 and sv48
>   RISC-V: vectored traps are optional
>   RISC-V: No traps on writes to misa,minstret[h],mcycle[h]
>   RISC-V: Remove support for adhoc X_COP local-interrupt
>
>  disas/riscv.c   | 39 +++--
>  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 |  9 -
>  include/hw/riscv/sifive_u.h | 13 +++
>  include/hw/riscv/spike.h| 16 -
>  include/hw/riscv/virt.h | 21 ---
>  target/riscv/cpu.c  |  2 +-
>  target/riscv/cpu.h  |  6 ++--
>  target/riscv/cpu_bits.h |  3 --
>  target/riscv/helper.c   | 63 +++--
>  target/riscv/op_helper.c| 52 ++--
>  16 files changed, 193 insertions(+), 285 deletions(-)
>

BTW Apologies for the duplicate emails. I'm still getting to grips with the
git-sendemail workflow and was using a sed script to Add Cc's which munged
the headers as it didn't take into account Subject lines flowing to two
lines. I guess I can just include Cc: in the commit message? and
git-format-patch will handle it for me? or I just should how to use
git-publish...

I still have to work on Igor's requested change to CPU initializer
declarations, and i'd also like to get the patch in that fixes fmin/fmax
test failures using the riscv-tests testsuite by implementing IEEE-754
minimumNumber/maximumNumber. I'll try to get a patch onto the list before
soft-freeze. I dropped the patch which was in an earlier version of our
port patch series due to a conflict with changes in softfloat (conversion
of minmax from a macro to a static function iirc).

Michael.

[Qemu-devel] [PATCH v1 00/22] Spec conformance bug fixes and cleanups

[Michael Clark]

This is the series of spec conformance bug fixes and code cleanups.
We would like to get this series in after our core changes in v8.2.

- 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
  - 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.
- Adds bounds checks when writing device-tree to ROM

Michael Clark (22):
  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 memory map
  RISC-V: Remove redundant identity_translate from load_elf
  RISC-V: Mark ROM read-only after copying in code and config
  RISC-V: Remove unused class definitions from machines
  RISC-V: Make sure the emulated rom has space for device-tree
  RISC-V: Include hexidecimal instruction in disassembly
  RISC-V: Hold rcu_read_lock when accessing memory directly
  RISC-V: Improve page table walker spec compliance
  RISC-V: Update E order and I extension order
  RISC-V: Make spike and virt header guards more specific
  RISC-V: Make virt header comment title consistent
  RISC-V: Use memory_region_is_ram in atomic pte update
  RISC-V: Remove EM_RISCV ELF_MACHINE indirection from load_elf
  RISC-V: Ingore satp writes and return 0 for reads when no-mmu
  RISC-V: Remove braces from satp case statement with no locals
  RISC-V: riscv-qemu port supports sv39 and sv48
  RISC-V: vectored traps are optional
  RISC-V: No traps on writes to misa,minstret[h],mcycle[h]
  RISC-V: Remove support for adhoc X_COP local-interrupt

 disas/riscv.c   | 39 +++--
 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 |  9 -
 include/hw/riscv/sifive_u.h | 13 +++
 include/hw/riscv/spike.h| 16 -
 include/hw/riscv/virt.h | 21 ---
 target/riscv/cpu.c  |  2 +-
 target/riscv/cpu.h  |  6 ++--
 target/riscv/cpu_bits.h |  3 --
 target/riscv/helper.c   | 63 +++--
 target/riscv/op_helper.c| 52 ++--
 16 files changed, 193 insertions(+), 285 deletions(-)

-- 
2.7.0