.travis.yml | 2 +- MAINTAINERS | 31 + arch_init.c | 2 + configure | 7 + default-configs/avr-softmmu.mak | 5 + gdb-xml/avr-cpu.xml | 49 + hw/avr/Kconfig | 9 + hw/avr/Makefile.objs | 3 + hw/avr/arduino.c | 151 ++ hw/avr/atmega.c | 470 ++++++ hw/avr/atmega.h | 48 + hw/avr/boot.c | 74 + hw/avr/boot.h | 33 + hw/char/Kconfig | 3 + hw/char/Makefile.objs | 1 + hw/char/avr_usart.c | 320 ++++ hw/misc/Kconfig | 3 + hw/misc/Makefile.objs | 2 + hw/misc/avr_power.c | 112 ++ hw/timer/Kconfig | 3 + hw/timer/Makefile.objs | 2 + hw/timer/avr_timer16.c | 604 ++++++++ include/disas/dis-asm.h | 19 + include/elf.h | 2 + include/hw/char/avr_usart.h | 93 ++ include/hw/misc/avr_power.h | 46 + include/hw/timer/avr_timer16.h | 94 ++ include/sysemu/arch_init.h | 1 + qapi/machine.json | 3 +- qemu-doc.texi | 51 + target/avr/Makefile.objs | 34 + target/avr/cpu-param.h | 37 + target/avr/cpu-qom.h | 54 + target/avr/cpu.c | 818 +++++++++++ target/avr/cpu.h | 259 ++++ target/avr/disas.c | 246 ++++ target/avr/gdbstub.c | 84 ++ target/avr/helper.c | 342 +++++ target/avr/helper.h | 29 + target/avr/insn.decode | 182 +++ target/avr/machine.c | 121 ++ target/avr/translate.c | 2997 ++++++++++++++++++++++++++++++++++++++ tests/acceptance/machine_avr6.py | 50 + tests/qtest/Makefile.include | 2 + tests/qtest/boot-serial-test.c | 11 + tests/qtest/machine-none-test.c | 1 + 46 files changed, 7508 insertions(+), 2 deletions(-) create mode 100644 default-configs/avr-softmmu.mak create mode 100644 gdb-xml/avr-cpu.xml create mode 100644 hw/avr/Kconfig create mode 100644 hw/avr/Makefile.objs create mode 100644 hw/avr/arduino.c create mode 100644 hw/avr/atmega.c create mode 100644 hw/avr/atmega.h create mode 100644 hw/avr/boot.c create mode 100644 hw/avr/boot.h create mode 100644 hw/char/avr_usart.c create mode 100644 hw/misc/avr_power.c create mode 100644 hw/timer/avr_timer16.c create mode 100644 include/hw/char/avr_usart.h create mode 100644 include/hw/misc/avr_power.h create mode 100644 include/hw/timer/avr_timer16.h create mode 100644 target/avr/Makefile.objs create mode 100644 target/avr/cpu-param.h create mode 100644 target/avr/cpu-qom.h create mode 100644 target/avr/cpu.c create mode 100644 target/avr/cpu.h create mode 100644 target/avr/disas.c create mode 100644 target/avr/gdbstub.c create mode 100644 target/avr/helper.c create mode 100644 target/avr/helper.h create mode 100644 target/avr/insn.decode create mode 100644 target/avr/machine.c create mode 100644 target/avr/translate.c create mode 100644 tests/acceptance/machine_avr6.py
From: Aleksandar Markovic <amarkovic@wavecomp.com>
This is the AVR port from Michael, release (merge) candidate 4.
The series can be found also in this repository:
https://github.com/AMarkovic/qemu-avr-merger-rc4
History:
Since v3:
- Removed a patch on load_elf() modification, since it has been merged
- Removed references to CONFIG_USER_ONLY and provided a guard against
building lunux user mode for AVR
- Removed all references to 'Atmel' (including file renames)
- Rebased the code (there was common interface change regarding 'props')
- Various corrections of commit messages
- A bit field for AVRFeatures is nor 64 bit long
- Other minor fixes
Since v2:
- First patch is split into six smaller logical units (net result
remains the same)
- Patch "hw/core/loader: Let load_elf populate the processor-specific
flags" was redone to reflect the original intent that was lost in
transalation between multiple autors
- Patch "hw/avr: Add helper to load raw/ELF firmware binaries" was
corrected only in one line to rectify type of "e_flags"
- Patch "target/avr: Add section about AVR into QEMU documentation"
- Spurious <message-Id:> elements were removed
- The series was rebased to the latest code
Since v1:
- Addressed Thomas comments
- Fixed a non-critical bug in ATmega (incorrect SRAM base address)
- Added ELF parsing requested by Aleksandar
- Dropped default machine (as with the ARM port)
Michael Rolnik (25):
target/avr: Add basic parameters for new AVR platform
target/avr: Introduce AVR CPU class object
target/avr: Add migration support
target/avr: Add GDB support
target/avr: Introduce enumeration AVRFeature
target/avr: Add defintions of AVR core types
target/avr: Add instruction helpers
target/avr: Add instruction translation - Register definitions
target/avr: Add instruction translation - Arithmetic and Logic
Instructions
target/avr: Add instruction translation - Branch Instructions
target/avr: Add instruction translation - Data Transfer Instructions
target/avr: Add instruction translation - Bit and Bit-test
Instructions
target/avr: Add instruction translation - MCU Control Instructions
target/avr: Add instruction translation - CPU main translation
function
target/avr: Add instruction disassembly function
hw/char: Add limited support for AVR USART peripheral
hw/timer: Add limited support for AVR 16-bit timer peripheral
hw/misc: Add limited support for AVR power device
target/avr: Add section about AVR into QEMU documentation
target/avr: Register AVR support with the rest of QEMU
target/avr: Add machine none test
target/avr: Update MAINTAINERS file
target/avr: Update build system
tests/boot-serial-test: Test some Arduino boards (AVR based)
tests/acceptance: Test the Arduino MEGA2560 board
Philippe Mathieu-Daudé (4):
hw/avr: Add helper to load raw/ELF firmware binaries
hw/avr: Add some ATmega microcontrollers
hw/avr: Add some Arduino boards
.travis.yml: Run the AVR acceptance tests
.travis.yml | 2 +-
MAINTAINERS | 31 +
arch_init.c | 2 +
configure | 7 +
default-configs/avr-softmmu.mak | 5 +
gdb-xml/avr-cpu.xml | 49 +
hw/avr/Kconfig | 9 +
hw/avr/Makefile.objs | 3 +
hw/avr/arduino.c | 151 ++
hw/avr/atmega.c | 470 ++++++
hw/avr/atmega.h | 48 +
hw/avr/boot.c | 74 +
hw/avr/boot.h | 33 +
hw/char/Kconfig | 3 +
hw/char/Makefile.objs | 1 +
hw/char/avr_usart.c | 320 ++++
hw/misc/Kconfig | 3 +
hw/misc/Makefile.objs | 2 +
hw/misc/avr_power.c | 112 ++
hw/timer/Kconfig | 3 +
hw/timer/Makefile.objs | 2 +
hw/timer/avr_timer16.c | 604 ++++++++
include/disas/dis-asm.h | 19 +
include/elf.h | 2 +
include/hw/char/avr_usart.h | 93 ++
include/hw/misc/avr_power.h | 46 +
include/hw/timer/avr_timer16.h | 94 ++
include/sysemu/arch_init.h | 1 +
qapi/machine.json | 3 +-
qemu-doc.texi | 51 +
target/avr/Makefile.objs | 34 +
target/avr/cpu-param.h | 37 +
target/avr/cpu-qom.h | 54 +
target/avr/cpu.c | 818 +++++++++++
target/avr/cpu.h | 259 ++++
target/avr/disas.c | 246 ++++
target/avr/gdbstub.c | 84 ++
target/avr/helper.c | 342 +++++
target/avr/helper.h | 29 +
target/avr/insn.decode | 182 +++
target/avr/machine.c | 121 ++
target/avr/translate.c | 2997 ++++++++++++++++++++++++++++++++++++++
tests/acceptance/machine_avr6.py | 50 +
tests/qtest/Makefile.include | 2 +
tests/qtest/boot-serial-test.c | 11 +
tests/qtest/machine-none-test.c | 1 +
46 files changed, 7508 insertions(+), 2 deletions(-)
create mode 100644 default-configs/avr-softmmu.mak
create mode 100644 gdb-xml/avr-cpu.xml
create mode 100644 hw/avr/Kconfig
create mode 100644 hw/avr/Makefile.objs
create mode 100644 hw/avr/arduino.c
create mode 100644 hw/avr/atmega.c
create mode 100644 hw/avr/atmega.h
create mode 100644 hw/avr/boot.c
create mode 100644 hw/avr/boot.h
create mode 100644 hw/char/avr_usart.c
create mode 100644 hw/misc/avr_power.c
create mode 100644 hw/timer/avr_timer16.c
create mode 100644 include/hw/char/avr_usart.h
create mode 100644 include/hw/misc/avr_power.h
create mode 100644 include/hw/timer/avr_timer16.h
create mode 100644 target/avr/Makefile.objs
create mode 100644 target/avr/cpu-param.h
create mode 100644 target/avr/cpu-qom.h
create mode 100644 target/avr/cpu.c
create mode 100644 target/avr/cpu.h
create mode 100644 target/avr/disas.c
create mode 100644 target/avr/gdbstub.c
create mode 100644 target/avr/helper.c
create mode 100644 target/avr/helper.h
create mode 100644 target/avr/insn.decode
create mode 100644 target/avr/machine.c
create mode 100644 target/avr/translate.c
create mode 100644 tests/acceptance/machine_avr6.py
--
2.7.4
Michael, Philippe, Can you guys do a quick checkup of this rc4? rc4, rc3,and rc2 should be functionally 100% equivalent. Thank you, Aleksandar On Fri, Jan 31, 2020 at 1:06 AM Aleksandar Markovic <aleksandar.markovic@rt-rk.com> wrote: > > From: Aleksandar Markovic <amarkovic@wavecomp.com> > > This is the AVR port from Michael, release (merge) candidate 4. > > The series can be found also in this repository: > > https://github.com/AMarkovic/qemu-avr-merger-rc4 > > History: > > Since v3: > > - Removed a patch on load_elf() modification, since it has been merged > - Removed references to CONFIG_USER_ONLY and provided a guard against > building lunux user mode for AVR > - Removed all references to 'Atmel' (including file renames) > - Rebased the code (there was common interface change regarding 'props') > - Various corrections of commit messages > - A bit field for AVRFeatures is nor 64 bit long > - Other minor fixes > > Since v2: > > - First patch is split into six smaller logical units (net result > remains the same) > - Patch "hw/core/loader: Let load_elf populate the processor-specific > flags" was redone to reflect the original intent that was lost in > transalation between multiple autors > - Patch "hw/avr: Add helper to load raw/ELF firmware binaries" was > corrected only in one line to rectify type of "e_flags" > - Patch "target/avr: Add section about AVR into QEMU documentation" > - Spurious <message-Id:> elements were removed > - The series was rebased to the latest code > > Since v1: > > - Addressed Thomas comments > - Fixed a non-critical bug in ATmega (incorrect SRAM base address) > - Added ELF parsing requested by Aleksandar > - Dropped default machine (as with the ARM port) > > Michael Rolnik (25): > target/avr: Add basic parameters for new AVR platform > target/avr: Introduce AVR CPU class object > target/avr: Add migration support > target/avr: Add GDB support > target/avr: Introduce enumeration AVRFeature > target/avr: Add defintions of AVR core types > target/avr: Add instruction helpers > target/avr: Add instruction translation - Register definitions > target/avr: Add instruction translation - Arithmetic and Logic > Instructions > target/avr: Add instruction translation - Branch Instructions > target/avr: Add instruction translation - Data Transfer Instructions > target/avr: Add instruction translation - Bit and Bit-test > Instructions > target/avr: Add instruction translation - MCU Control Instructions > target/avr: Add instruction translation - CPU main translation > function > target/avr: Add instruction disassembly function > hw/char: Add limited support for AVR USART peripheral > hw/timer: Add limited support for AVR 16-bit timer peripheral > hw/misc: Add limited support for AVR power device > target/avr: Add section about AVR into QEMU documentation > target/avr: Register AVR support with the rest of QEMU > target/avr: Add machine none test > target/avr: Update MAINTAINERS file > target/avr: Update build system > tests/boot-serial-test: Test some Arduino boards (AVR based) > tests/acceptance: Test the Arduino MEGA2560 board > > Philippe Mathieu-Daudé (4): > hw/avr: Add helper to load raw/ELF firmware binaries > hw/avr: Add some ATmega microcontrollers > hw/avr: Add some Arduino boards > .travis.yml: Run the AVR acceptance tests > > .travis.yml | 2 +- > MAINTAINERS | 31 + > arch_init.c | 2 + > configure | 7 + > default-configs/avr-softmmu.mak | 5 + > gdb-xml/avr-cpu.xml | 49 + > hw/avr/Kconfig | 9 + > hw/avr/Makefile.objs | 3 + > hw/avr/arduino.c | 151 ++ > hw/avr/atmega.c | 470 ++++++ > hw/avr/atmega.h | 48 + > hw/avr/boot.c | 74 + > hw/avr/boot.h | 33 + > hw/char/Kconfig | 3 + > hw/char/Makefile.objs | 1 + > hw/char/avr_usart.c | 320 ++++ > hw/misc/Kconfig | 3 + > hw/misc/Makefile.objs | 2 + > hw/misc/avr_power.c | 112 ++ > hw/timer/Kconfig | 3 + > hw/timer/Makefile.objs | 2 + > hw/timer/avr_timer16.c | 604 ++++++++ > include/disas/dis-asm.h | 19 + > include/elf.h | 2 + > include/hw/char/avr_usart.h | 93 ++ > include/hw/misc/avr_power.h | 46 + > include/hw/timer/avr_timer16.h | 94 ++ > include/sysemu/arch_init.h | 1 + > qapi/machine.json | 3 +- > qemu-doc.texi | 51 + > target/avr/Makefile.objs | 34 + > target/avr/cpu-param.h | 37 + > target/avr/cpu-qom.h | 54 + > target/avr/cpu.c | 818 +++++++++++ > target/avr/cpu.h | 259 ++++ > target/avr/disas.c | 246 ++++ > target/avr/gdbstub.c | 84 ++ > target/avr/helper.c | 342 +++++ > target/avr/helper.h | 29 + > target/avr/insn.decode | 182 +++ > target/avr/machine.c | 121 ++ > target/avr/translate.c | 2997 ++++++++++++++++++++++++++++++++++++++ > tests/acceptance/machine_avr6.py | 50 + > tests/qtest/Makefile.include | 2 + > tests/qtest/boot-serial-test.c | 11 + > tests/qtest/machine-none-test.c | 1 + > 46 files changed, 7508 insertions(+), 2 deletions(-) > create mode 100644 default-configs/avr-softmmu.mak > create mode 100644 gdb-xml/avr-cpu.xml > create mode 100644 hw/avr/Kconfig > create mode 100644 hw/avr/Makefile.objs > create mode 100644 hw/avr/arduino.c > create mode 100644 hw/avr/atmega.c > create mode 100644 hw/avr/atmega.h > create mode 100644 hw/avr/boot.c > create mode 100644 hw/avr/boot.h > create mode 100644 hw/char/avr_usart.c > create mode 100644 hw/misc/avr_power.c > create mode 100644 hw/timer/avr_timer16.c > create mode 100644 include/hw/char/avr_usart.h > create mode 100644 include/hw/misc/avr_power.h > create mode 100644 include/hw/timer/avr_timer16.h > create mode 100644 target/avr/Makefile.objs > create mode 100644 target/avr/cpu-param.h > create mode 100644 target/avr/cpu-qom.h > create mode 100644 target/avr/cpu.c > create mode 100644 target/avr/cpu.h > create mode 100644 target/avr/disas.c > create mode 100644 target/avr/gdbstub.c > create mode 100644 target/avr/helper.c > create mode 100644 target/avr/helper.h > create mode 100644 target/avr/insn.decode > create mode 100644 target/avr/machine.c > create mode 100644 target/avr/translate.c > create mode 100644 tests/acceptance/machine_avr6.py > > -- > 2.7.4 > >
On 1/31/20 1:12 AM, Aleksandar Markovic wrote: > Michael, Philippe, > > Can you guys do a quick checkup of this rc4? rc4, rc3,and rc2 should > be functionally 100% equivalent. Tested OK. git-backport-diff with rc2: Key: [----] : patches are identical [####] : number of functional differences between upstream/downstream patch [down] : patch is downstream-only The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively 001/31:[down] 'target/avr: Add basic parameters for new AVR platform' 002/31:[down] 'target/avr: Introduce AVR CPU class object' 003/31:[down] 'target/avr: Add migration support' 004/31:[down] 'target/avr: Add GDB support' 005/31:[down] 'target/avr: Introduce enumeration AVRFeature' 006/31:[down] 'target/avr: Add defintions of AVR core types' 007/31:[0148] [FC] 'target/avr: Add instruction helpers' 008/31:[down] 'target/avr: Add instruction translation - Register definitions' 009/31:[----] [--] 'target/avr: Add instruction translation - Arithmetic and Logic Instructions' 010/31:[----] [--] 'target/avr: Add instruction translation - Branch Instructions' 011/31:[----] [--] 'target/avr: Add instruction translation - Data Transfer Instructions' 012/31:[----] [--] 'target/avr: Add instruction translation - Bit and Bit-test Instructions' 013/31:[----] [--] 'target/avr: Add instruction translation - MCU Control Instructions' 014/31:[----] [--] 'target/avr: Add instruction translation - CPU main translation function' 015/31:[----] [--] 'target/avr: Add instruction disassembly function' 016/31:[down] 'hw/char: Add limited support for AVR USART peripheral' 017/31:[down] 'hw/timer: Add limited support for AVR 16-bit timer peripheral' 018/31:[down] 'hw/misc: Add limited support for AVR power device' 019/31:[0012] [FC] 'target/avr: Add section about AVR into QEMU documentation' 020/31:[0002] [FC] 'target/avr: Register AVR support with the rest of QEMU' 021/31:[----] [--] 'target/avr: Add machine none test' 022/31:[0014] [FC] 'target/avr: Update MAINTAINERS file' 023/31:[0002] [FC] 'hw/avr: Add helper to load raw/ELF firmware binaries' 024/31:[0026] [FC] 'hw/avr: Add some ATmega microcontrollers' 025/31:[0009] [FC] 'hw/avr: Add some Arduino boards' 026/31:[----] [--] 'target/avr: Update build system' 027/31:[----] [--] 'tests/boot-serial-test: Test some Arduino boards (AVR based)' 028/31:[----] [--] 'tests/acceptance: Test the Arduino MEGA2560 board' 029/31:[----] [--] '.travis.yml: Run the AVR acceptance tests' 030/31:[down] '!fixup "hw/misc: Add limited support for AVR power device"' 031/31:[down] '!fixup "hw/timer: Add limited support for AVR 16-bit timer peripheral"' One thing that annoys me is we ignored the review comments from Joaquin, but I think it might now be easier to address them as new patches, once this series is merged. I made 2 comments (definitions in incorrect patch, and definition misplaced in elf.h), and sent 2 patches converting the PRINTF() to trace-events. Thanks for preparing the rc4, hopefully we are done! > > Thank you, > Aleksandar > > On Fri, Jan 31, 2020 at 1:06 AM Aleksandar Markovic > <aleksandar.markovic@rt-rk.com> wrote: >> >> From: Aleksandar Markovic <amarkovic@wavecomp.com> >> >> This is the AVR port from Michael, release (merge) candidate 4. >> >> The series can be found also in this repository: >> >> https://github.com/AMarkovic/qemu-avr-merger-rc4 >> >> History: >> >> Since v3: >> >> - Removed a patch on load_elf() modification, since it has been merged >> - Removed references to CONFIG_USER_ONLY and provided a guard against >> building lunux user mode for AVR >> - Removed all references to 'Atmel' (including file renames) >> - Rebased the code (there was common interface change regarding 'props') >> - Various corrections of commit messages >> - A bit field for AVRFeatures is nor 64 bit long >> - Other minor fixes >> >> Since v2: >> >> - First patch is split into six smaller logical units (net result >> remains the same) >> - Patch "hw/core/loader: Let load_elf populate the processor-specific >> flags" was redone to reflect the original intent that was lost in >> transalation between multiple autors >> - Patch "hw/avr: Add helper to load raw/ELF firmware binaries" was >> corrected only in one line to rectify type of "e_flags" >> - Patch "target/avr: Add section about AVR into QEMU documentation" >> - Spurious <message-Id:> elements were removed >> - The series was rebased to the latest code >> >> Since v1: >> >> - Addressed Thomas comments >> - Fixed a non-critical bug in ATmega (incorrect SRAM base address) >> - Added ELF parsing requested by Aleksandar >> - Dropped default machine (as with the ARM port) >> >> Michael Rolnik (25): >> target/avr: Add basic parameters for new AVR platform >> target/avr: Introduce AVR CPU class object >> target/avr: Add migration support >> target/avr: Add GDB support >> target/avr: Introduce enumeration AVRFeature >> target/avr: Add defintions of AVR core types >> target/avr: Add instruction helpers >> target/avr: Add instruction translation - Register definitions >> target/avr: Add instruction translation - Arithmetic and Logic >> Instructions >> target/avr: Add instruction translation - Branch Instructions >> target/avr: Add instruction translation - Data Transfer Instructions >> target/avr: Add instruction translation - Bit and Bit-test >> Instructions >> target/avr: Add instruction translation - MCU Control Instructions >> target/avr: Add instruction translation - CPU main translation >> function >> target/avr: Add instruction disassembly function >> hw/char: Add limited support for AVR USART peripheral >> hw/timer: Add limited support for AVR 16-bit timer peripheral >> hw/misc: Add limited support for AVR power device >> target/avr: Add section about AVR into QEMU documentation >> target/avr: Register AVR support with the rest of QEMU >> target/avr: Add machine none test >> target/avr: Update MAINTAINERS file >> target/avr: Update build system >> tests/boot-serial-test: Test some Arduino boards (AVR based) >> tests/acceptance: Test the Arduino MEGA2560 board >> >> Philippe Mathieu-Daudé (4): >> hw/avr: Add helper to load raw/ELF firmware binaries >> hw/avr: Add some ATmega microcontrollers >> hw/avr: Add some Arduino boards >> .travis.yml: Run the AVR acceptance tests >> >> .travis.yml | 2 +- >> MAINTAINERS | 31 + >> arch_init.c | 2 + >> configure | 7 + >> default-configs/avr-softmmu.mak | 5 + >> gdb-xml/avr-cpu.xml | 49 + >> hw/avr/Kconfig | 9 + >> hw/avr/Makefile.objs | 3 + >> hw/avr/arduino.c | 151 ++ >> hw/avr/atmega.c | 470 ++++++ >> hw/avr/atmega.h | 48 + >> hw/avr/boot.c | 74 + >> hw/avr/boot.h | 33 + >> hw/char/Kconfig | 3 + >> hw/char/Makefile.objs | 1 + >> hw/char/avr_usart.c | 320 ++++ >> hw/misc/Kconfig | 3 + >> hw/misc/Makefile.objs | 2 + >> hw/misc/avr_power.c | 112 ++ >> hw/timer/Kconfig | 3 + >> hw/timer/Makefile.objs | 2 + >> hw/timer/avr_timer16.c | 604 ++++++++ >> include/disas/dis-asm.h | 19 + >> include/elf.h | 2 + >> include/hw/char/avr_usart.h | 93 ++ >> include/hw/misc/avr_power.h | 46 + >> include/hw/timer/avr_timer16.h | 94 ++ >> include/sysemu/arch_init.h | 1 + >> qapi/machine.json | 3 +- >> qemu-doc.texi | 51 + >> target/avr/Makefile.objs | 34 + >> target/avr/cpu-param.h | 37 + >> target/avr/cpu-qom.h | 54 + >> target/avr/cpu.c | 818 +++++++++++ >> target/avr/cpu.h | 259 ++++ >> target/avr/disas.c | 246 ++++ >> target/avr/gdbstub.c | 84 ++ >> target/avr/helper.c | 342 +++++ >> target/avr/helper.h | 29 + >> target/avr/insn.decode | 182 +++ >> target/avr/machine.c | 121 ++ >> target/avr/translate.c | 2997 ++++++++++++++++++++++++++++++++++++++ >> tests/acceptance/machine_avr6.py | 50 + >> tests/qtest/Makefile.include | 2 + >> tests/qtest/boot-serial-test.c | 11 + >> tests/qtest/machine-none-test.c | 1 + >> 46 files changed, 7508 insertions(+), 2 deletions(-) >> create mode 100644 default-configs/avr-softmmu.mak >> create mode 100644 gdb-xml/avr-cpu.xml >> create mode 100644 hw/avr/Kconfig >> create mode 100644 hw/avr/Makefile.objs >> create mode 100644 hw/avr/arduino.c >> create mode 100644 hw/avr/atmega.c >> create mode 100644 hw/avr/atmega.h >> create mode 100644 hw/avr/boot.c >> create mode 100644 hw/avr/boot.h >> create mode 100644 hw/char/avr_usart.c >> create mode 100644 hw/misc/avr_power.c >> create mode 100644 hw/timer/avr_timer16.c >> create mode 100644 include/hw/char/avr_usart.h >> create mode 100644 include/hw/misc/avr_power.h >> create mode 100644 include/hw/timer/avr_timer16.h >> create mode 100644 target/avr/Makefile.objs >> create mode 100644 target/avr/cpu-param.h >> create mode 100644 target/avr/cpu-qom.h >> create mode 100644 target/avr/cpu.c >> create mode 100644 target/avr/cpu.h >> create mode 100644 target/avr/disas.c >> create mode 100644 target/avr/gdbstub.c >> create mode 100644 target/avr/helper.c >> create mode 100644 target/avr/helper.h >> create mode 100644 target/avr/insn.decode >> create mode 100644 target/avr/machine.c >> create mode 100644 target/avr/translate.c >> create mode 100644 tests/acceptance/machine_avr6.py >> >> -- >> 2.7.4 >> >> >
Looks good. Thanks On Fri, Jan 31, 2020 at 3:24 AM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > On 1/31/20 1:12 AM, Aleksandar Markovic wrote: > > Michael, Philippe, > > > > Can you guys do a quick checkup of this rc4? rc4, rc3,and rc2 should > > be functionally 100% equivalent. > > Tested OK. > > git-backport-diff with rc2: > > Key: > [----] : patches are identical > [####] : number of functional differences between upstream/downstream patch > [down] : patch is downstream-only > The flags [FC] indicate (F)unctional and (C)ontextual differences, > respectively > > 001/31:[down] 'target/avr: Add basic parameters for new AVR platform' > 002/31:[down] 'target/avr: Introduce AVR CPU class object' > 003/31:[down] 'target/avr: Add migration support' > 004/31:[down] 'target/avr: Add GDB support' > 005/31:[down] 'target/avr: Introduce enumeration AVRFeature' > 006/31:[down] 'target/avr: Add defintions of AVR core types' > 007/31:[0148] [FC] 'target/avr: Add instruction helpers' > 008/31:[down] 'target/avr: Add instruction translation - Register > definitions' > 009/31:[----] [--] 'target/avr: Add instruction translation - Arithmetic > and Logic Instructions' > 010/31:[----] [--] 'target/avr: Add instruction translation - Branch > Instructions' > 011/31:[----] [--] 'target/avr: Add instruction translation - Data > Transfer Instructions' > 012/31:[----] [--] 'target/avr: Add instruction translation - Bit and > Bit-test Instructions' > 013/31:[----] [--] 'target/avr: Add instruction translation - MCU > Control Instructions' > 014/31:[----] [--] 'target/avr: Add instruction translation - CPU main > translation function' > 015/31:[----] [--] 'target/avr: Add instruction disassembly function' > 016/31:[down] 'hw/char: Add limited support for AVR USART peripheral' > 017/31:[down] 'hw/timer: Add limited support for AVR 16-bit timer > peripheral' > 018/31:[down] 'hw/misc: Add limited support for AVR power device' > 019/31:[0012] [FC] 'target/avr: Add section about AVR into QEMU > documentation' > 020/31:[0002] [FC] 'target/avr: Register AVR support with the rest of QEMU' > 021/31:[----] [--] 'target/avr: Add machine none test' > 022/31:[0014] [FC] 'target/avr: Update MAINTAINERS file' > 023/31:[0002] [FC] 'hw/avr: Add helper to load raw/ELF firmware binaries' > 024/31:[0026] [FC] 'hw/avr: Add some ATmega microcontrollers' > 025/31:[0009] [FC] 'hw/avr: Add some Arduino boards' > 026/31:[----] [--] 'target/avr: Update build system' > 027/31:[----] [--] 'tests/boot-serial-test: Test some Arduino boards > (AVR based)' > 028/31:[----] [--] 'tests/acceptance: Test the Arduino MEGA2560 board' > 029/31:[----] [--] '.travis.yml: Run the AVR acceptance tests' > 030/31:[down] '!fixup "hw/misc: Add limited support for AVR power device"' > 031/31:[down] '!fixup "hw/timer: Add limited support for AVR 16-bit > timer peripheral"' > > One thing that annoys me is we ignored the review comments from Joaquin, > but I think it might now be easier to address them as new patches, once > this series is merged. > > I made 2 comments (definitions in incorrect patch, and definition > misplaced in elf.h), and sent 2 patches converting the PRINTF() to > trace-events. Thanks for preparing the rc4, hopefully we are done! > > > > > Thank you, > > Aleksandar > > > > On Fri, Jan 31, 2020 at 1:06 AM Aleksandar Markovic > > <aleksandar.markovic@rt-rk.com> wrote: > >> > >> From: Aleksandar Markovic <amarkovic@wavecomp.com> > >> > >> This is the AVR port from Michael, release (merge) candidate 4. > >> > >> The series can be found also in this repository: > >> > >> https://github.com/AMarkovic/qemu-avr-merger-rc4 > >> > >> History: > >> > >> Since v3: > >> > >> - Removed a patch on load_elf() modification, since it has been merged > >> - Removed references to CONFIG_USER_ONLY and provided a guard against > >> building lunux user mode for AVR > >> - Removed all references to 'Atmel' (including file renames) > >> - Rebased the code (there was common interface change regarding 'props') > >> - Various corrections of commit messages > >> - A bit field for AVRFeatures is nor 64 bit long > >> - Other minor fixes > >> > >> Since v2: > >> > >> - First patch is split into six smaller logical units (net result > >> remains the same) > >> - Patch "hw/core/loader: Let load_elf populate the processor-specific > >> flags" was redone to reflect the original intent that was lost in > >> transalation between multiple autors > >> - Patch "hw/avr: Add helper to load raw/ELF firmware binaries" was > >> corrected only in one line to rectify type of "e_flags" > >> - Patch "target/avr: Add section about AVR into QEMU documentation" > >> - Spurious <message-Id:> elements were removed > >> - The series was rebased to the latest code > >> > >> Since v1: > >> > >> - Addressed Thomas comments > >> - Fixed a non-critical bug in ATmega (incorrect SRAM base address) > >> - Added ELF parsing requested by Aleksandar > >> - Dropped default machine (as with the ARM port) > >> > >> Michael Rolnik (25): > >> target/avr: Add basic parameters for new AVR platform > >> target/avr: Introduce AVR CPU class object > >> target/avr: Add migration support > >> target/avr: Add GDB support > >> target/avr: Introduce enumeration AVRFeature > >> target/avr: Add defintions of AVR core types > >> target/avr: Add instruction helpers > >> target/avr: Add instruction translation - Register definitions > >> target/avr: Add instruction translation - Arithmetic and Logic > >> Instructions > >> target/avr: Add instruction translation - Branch Instructions > >> target/avr: Add instruction translation - Data Transfer Instructions > >> target/avr: Add instruction translation - Bit and Bit-test > >> Instructions > >> target/avr: Add instruction translation - MCU Control Instructions > >> target/avr: Add instruction translation - CPU main translation > >> function > >> target/avr: Add instruction disassembly function > >> hw/char: Add limited support for AVR USART peripheral > >> hw/timer: Add limited support for AVR 16-bit timer peripheral > >> hw/misc: Add limited support for AVR power device > >> target/avr: Add section about AVR into QEMU documentation > >> target/avr: Register AVR support with the rest of QEMU > >> target/avr: Add machine none test > >> target/avr: Update MAINTAINERS file > >> target/avr: Update build system > >> tests/boot-serial-test: Test some Arduino boards (AVR based) > >> tests/acceptance: Test the Arduino MEGA2560 board > >> > >> Philippe Mathieu-Daudé (4): > >> hw/avr: Add helper to load raw/ELF firmware binaries > >> hw/avr: Add some ATmega microcontrollers > >> hw/avr: Add some Arduino boards > >> .travis.yml: Run the AVR acceptance tests > >> > >> .travis.yml | 2 +- > >> MAINTAINERS | 31 + > >> arch_init.c | 2 + > >> configure | 7 + > >> default-configs/avr-softmmu.mak | 5 + > >> gdb-xml/avr-cpu.xml | 49 + > >> hw/avr/Kconfig | 9 + > >> hw/avr/Makefile.objs | 3 + > >> hw/avr/arduino.c | 151 ++ > >> hw/avr/atmega.c | 470 ++++++ > >> hw/avr/atmega.h | 48 + > >> hw/avr/boot.c | 74 + > >> hw/avr/boot.h | 33 + > >> hw/char/Kconfig | 3 + > >> hw/char/Makefile.objs | 1 + > >> hw/char/avr_usart.c | 320 ++++ > >> hw/misc/Kconfig | 3 + > >> hw/misc/Makefile.objs | 2 + > >> hw/misc/avr_power.c | 112 ++ > >> hw/timer/Kconfig | 3 + > >> hw/timer/Makefile.objs | 2 + > >> hw/timer/avr_timer16.c | 604 ++++++++ > >> include/disas/dis-asm.h | 19 + > >> include/elf.h | 2 + > >> include/hw/char/avr_usart.h | 93 ++ > >> include/hw/misc/avr_power.h | 46 + > >> include/hw/timer/avr_timer16.h | 94 ++ > >> include/sysemu/arch_init.h | 1 + > >> qapi/machine.json | 3 +- > >> qemu-doc.texi | 51 + > >> target/avr/Makefile.objs | 34 + > >> target/avr/cpu-param.h | 37 + > >> target/avr/cpu-qom.h | 54 + > >> target/avr/cpu.c | 818 +++++++++++ > >> target/avr/cpu.h | 259 ++++ > >> target/avr/disas.c | 246 ++++ > >> target/avr/gdbstub.c | 84 ++ > >> target/avr/helper.c | 342 +++++ > >> target/avr/helper.h | 29 + > >> target/avr/insn.decode | 182 +++ > >> target/avr/machine.c | 121 ++ > >> target/avr/translate.c | 2997 > ++++++++++++++++++++++++++++++++++++++ > >> tests/acceptance/machine_avr6.py | 50 + > >> tests/qtest/Makefile.include | 2 + > >> tests/qtest/boot-serial-test.c | 11 + > >> tests/qtest/machine-none-test.c | 1 + > >> 46 files changed, 7508 insertions(+), 2 deletions(-) > >> create mode 100644 default-configs/avr-softmmu.mak > >> create mode 100644 gdb-xml/avr-cpu.xml > >> create mode 100644 hw/avr/Kconfig > >> create mode 100644 hw/avr/Makefile.objs > >> create mode 100644 hw/avr/arduino.c > >> create mode 100644 hw/avr/atmega.c > >> create mode 100644 hw/avr/atmega.h > >> create mode 100644 hw/avr/boot.c > >> create mode 100644 hw/avr/boot.h > >> create mode 100644 hw/char/avr_usart.c > >> create mode 100644 hw/misc/avr_power.c > >> create mode 100644 hw/timer/avr_timer16.c > >> create mode 100644 include/hw/char/avr_usart.h > >> create mode 100644 include/hw/misc/avr_power.h > >> create mode 100644 include/hw/timer/avr_timer16.h > >> create mode 100644 target/avr/Makefile.objs > >> create mode 100644 target/avr/cpu-param.h > >> create mode 100644 target/avr/cpu-qom.h > >> create mode 100644 target/avr/cpu.c > >> create mode 100644 target/avr/cpu.h > >> create mode 100644 target/avr/disas.c > >> create mode 100644 target/avr/gdbstub.c > >> create mode 100644 target/avr/helper.c > >> create mode 100644 target/avr/helper.h > >> create mode 100644 target/avr/insn.decode > >> create mode 100644 target/avr/machine.c > >> create mode 100644 target/avr/translate.c > >> create mode 100644 tests/acceptance/machine_avr6.py > >> > >> -- > >> 2.7.4 > >> > >> > > > > -- Best Regards, Michael Rolnik
Aleksandar, I addressed Alex Bennée comment as fixup, so you can squash directly. See: https://www.mail-archive.com/qemu-devel@nongnu.org/msg673846.html - convert DB_PRINT() to trace-events - fix style/indentation Based-on: <1580428993-4767-1-git-send-email-aleksandar.markovic@rt-rk.com> Philippe Mathieu-Daudé (2): !fixup "hw/misc: Add limited support for AVR power device" !fixup "hw/timer: Add limited support for AVR 16-bit timer peripheral" hw/misc/avr_power.c | 17 +++++++++-------- hw/timer/avr_timer16.c | 25 +++++++++++++++---------- hw/misc/trace-events | 4 ++++ hw/timer/trace-events | 12 ++++++++++++ 4 files changed, 40 insertions(+), 18 deletions(-) -- 2.21.1
On Fri, Jan 31, 2020 at 2:09 AM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > Aleksandar, I addressed Alex Bennée comment as fixup, so you > can squash directly. See: > https://www.mail-archive.com/qemu-devel@nongnu.org/msg673846.html > > - convert DB_PRINT() to trace-events > - fix style/indentation > > Based-on: <1580428993-4767-1-git-send-email-aleksandar.markovic@rt-rk.com> > That is great! I am going to squash it into corresponding patches, and mention that in commit messages. Then I guess some r-b (by Alex) can be added. Cool! > Philippe Mathieu-Daudé (2): > !fixup "hw/misc: Add limited support for AVR power device" > !fixup "hw/timer: Add limited support for AVR 16-bit timer peripheral" > > hw/misc/avr_power.c | 17 +++++++++-------- > hw/timer/avr_timer16.c | 25 +++++++++++++++---------- > hw/misc/trace-events | 4 ++++ > hw/timer/trace-events | 12 ++++++++++++ > 4 files changed, 40 insertions(+), 18 deletions(-) > > -- > 2.21.1 >
- convert DB_PRINT() to trace-events
- fix style/indentation
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/misc/avr_power.c | 17 +++++++++--------
hw/misc/trace-events | 4 ++++
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/hw/misc/avr_power.c b/hw/misc/avr_power.c
index 598bc7279c..65ff7c4405 100644
--- a/hw/misc/avr_power.c
+++ b/hw/misc/avr_power.c
@@ -27,9 +27,7 @@
#include "qemu/log.h"
#include "hw/qdev-properties.h"
#include "hw/irq.h"
-
-#define DB_PRINT(fmt, args...) /* Nothing */
-/*#define DB_PRINT(fmt, args...) printf("%s: " fmt "\n", __func__, ## args)*/
+#include "trace.h"
static void avr_mask_reset(DeviceState *dev)
{
@@ -48,19 +46,20 @@ static uint64_t avr_mask_read(void *opaque, hwaddr offset, unsigned size)
assert(offset == 0);
AVRMaskState *s = opaque;
+ trace_avr_power_read(s->val);
+
return (uint64_t)s->val;
}
static void avr_mask_write(void *opaque, hwaddr offset,
- uint64_t val64, unsigned size)
+ uint64_t val64, unsigned size)
{
assert(size == 1);
assert(offset == 0);
AVRMaskState *s = opaque;
uint8_t val8 = val64;
- DB_PRINT("write %d to offset %d", val8, (uint8_t)offset);
-
+ trace_avr_power_write(val8);
s->val = val8;
for (int i = 0; i < 8; i++) {
qemu_set_irq(s->irq[i], (val8 & (1 << i)) != 0);
@@ -71,7 +70,9 @@ static const MemoryRegionOps avr_mask_ops = {
.read = avr_mask_read,
.write = avr_mask_write,
.endianness = DEVICE_NATIVE_ENDIAN,
- .impl = {.max_access_size = 1}
+ .impl = {
+ .max_access_size = 1,
+ },
};
static void avr_mask_init(Object *dev)
@@ -80,7 +81,7 @@ static void avr_mask_init(Object *dev)
SysBusDevice *busdev = SYS_BUS_DEVICE(dev);
memory_region_init_io(&s->iomem, dev, &avr_mask_ops, s, TYPE_AVR_MASK,
- 0x01);
+ 0x01);
sysbus_init_mmio(busdev, &s->iomem);
for (int i = 0; i < 8; i++) {
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 7f0f5dff3a..f716881bb1 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -179,3 +179,7 @@ via1_rtc_cmd_pram_read(int addr, int value) "addr=%u value=0x%02x"
via1_rtc_cmd_pram_write(int addr, int value) "addr=%u value=0x%02x"
via1_rtc_cmd_pram_sect_read(int sector, int offset, int addr, int value) "sector=%u offset=%u addr=%d value=0x%02x"
via1_rtc_cmd_pram_sect_write(int sector, int offset, int addr, int value) "sector=%u offset=%u addr=%d value=0x%02x"
+
+# avr_power.c
+avr_power_read(uint8_t value) "power_reduc read value:%u"
+avr_power_write(uint8_t value) "power_reduc write value:%u"
--
2.21.1
Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> - convert DB_PRINT() to trace-events
> - fix style/indentation
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/misc/avr_power.c | 17 +++++++++--------
> hw/misc/trace-events | 4 ++++
> 2 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/hw/misc/avr_power.c b/hw/misc/avr_power.c
> index 598bc7279c..65ff7c4405 100644
> --- a/hw/misc/avr_power.c
> +++ b/hw/misc/avr_power.c
> @@ -27,9 +27,7 @@
> #include "qemu/log.h"
> #include "hw/qdev-properties.h"
> #include "hw/irq.h"
> -
> -#define DB_PRINT(fmt, args...) /* Nothing */
> -/*#define DB_PRINT(fmt, args...) printf("%s: " fmt "\n", __func__, ## args)*/
> +#include "trace.h"
>
> static void avr_mask_reset(DeviceState *dev)
> {
> @@ -48,19 +46,20 @@ static uint64_t avr_mask_read(void *opaque, hwaddr offset, unsigned size)
> assert(offset == 0);
> AVRMaskState *s = opaque;
>
> + trace_avr_power_read(s->val);
> +
> return (uint64_t)s->val;
> }
>
> static void avr_mask_write(void *opaque, hwaddr offset,
> - uint64_t val64, unsigned size)
> + uint64_t val64, unsigned size)
> {
> assert(size == 1);
> assert(offset == 0);
> AVRMaskState *s = opaque;
> uint8_t val8 = val64;
>
> - DB_PRINT("write %d to offset %d", val8, (uint8_t)offset);
> -
> + trace_avr_power_write(val8);
You've dropped offset in this trace point which is probably worth
keeping so you track where is being written to. Same with the read.
> s->val = val8;
> for (int i = 0; i < 8; i++) {
> qemu_set_irq(s->irq[i], (val8 & (1 << i)) != 0);
> @@ -71,7 +70,9 @@ static const MemoryRegionOps avr_mask_ops = {
> .read = avr_mask_read,
> .write = avr_mask_write,
> .endianness = DEVICE_NATIVE_ENDIAN,
> - .impl = {.max_access_size = 1}
> + .impl = {
> + .max_access_size = 1,
> + },
> };
>
> static void avr_mask_init(Object *dev)
> @@ -80,7 +81,7 @@ static void avr_mask_init(Object *dev)
> SysBusDevice *busdev = SYS_BUS_DEVICE(dev);
>
> memory_region_init_io(&s->iomem, dev, &avr_mask_ops, s, TYPE_AVR_MASK,
> - 0x01);
> + 0x01);
> sysbus_init_mmio(busdev, &s->iomem);
>
> for (int i = 0; i < 8; i++) {
> diff --git a/hw/misc/trace-events b/hw/misc/trace-events
> index 7f0f5dff3a..f716881bb1 100644
> --- a/hw/misc/trace-events
> +++ b/hw/misc/trace-events
> @@ -179,3 +179,7 @@ via1_rtc_cmd_pram_read(int addr, int value) "addr=%u value=0x%02x"
> via1_rtc_cmd_pram_write(int addr, int value) "addr=%u value=0x%02x"
> via1_rtc_cmd_pram_sect_read(int sector, int offset, int addr, int value) "sector=%u offset=%u addr=%d value=0x%02x"
> via1_rtc_cmd_pram_sect_write(int sector, int offset, int addr, int value) "sector=%u offset=%u addr=%d value=0x%02x"
> +
> +# avr_power.c
> +avr_power_read(uint8_t value) "power_reduc read value:%u"
> +avr_power_write(uint8_t value) "power_reduc write value:%u"
--
Alex Bennée
On Fri, Jan 31, 2020 at 12:27 PM Alex Bennée <alex.bennee@linaro.org> wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>
> > - convert DB_PRINT() to trace-events
> > - fix style/indentation
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > ---
> > hw/misc/avr_power.c | 17 +++++++++--------
> > hw/misc/trace-events | 4 ++++
> > 2 files changed, 13 insertions(+), 8 deletions(-)
> >
> > diff --git a/hw/misc/avr_power.c b/hw/misc/avr_power.c
> > index 598bc7279c..65ff7c4405 100644
> > --- a/hw/misc/avr_power.c
> > +++ b/hw/misc/avr_power.c
> > @@ -27,9 +27,7 @@
> > #include "qemu/log.h"
> > #include "hw/qdev-properties.h"
> > #include "hw/irq.h"
> > -
> > -#define DB_PRINT(fmt, args...) /* Nothing */
> > -/*#define DB_PRINT(fmt, args...) printf("%s: " fmt "\n", __func__, ## args)*/
> > +#include "trace.h"
> >
> > static void avr_mask_reset(DeviceState *dev)
> > {
> > @@ -48,19 +46,20 @@ static uint64_t avr_mask_read(void *opaque, hwaddr offset, unsigned size)
> > assert(offset == 0);
> > AVRMaskState *s = opaque;
> >
> > + trace_avr_power_read(s->val);
> > +
> > return (uint64_t)s->val;
> > }
> >
> > static void avr_mask_write(void *opaque, hwaddr offset,
> > - uint64_t val64, unsigned size)
> > + uint64_t val64, unsigned size)
> > {
> > assert(size == 1);
> > assert(offset == 0);
> > AVRMaskState *s = opaque;
> > uint8_t val8 = val64;
> >
> > - DB_PRINT("write %d to offset %d", val8, (uint8_t)offset);
> > -
> > + trace_avr_power_write(val8);
>
> You've dropped offset in this trace point which is probably worth
> keeping so you track where is being written to. Same with the read.
I dropped it because it is always 0x00, the register is 8bit wide. See
below, memory_region_init_io(...,1).
I thought about adding a "name" property so each instance can display
the device it belongs to, but this was too invasive, so I decided to
keep this change for later.
> > s->val = val8;
> > for (int i = 0; i < 8; i++) {
> > qemu_set_irq(s->irq[i], (val8 & (1 << i)) != 0);
> > @@ -71,7 +70,9 @@ static const MemoryRegionOps avr_mask_ops = {
> > .read = avr_mask_read,
> > .write = avr_mask_write,
> > .endianness = DEVICE_NATIVE_ENDIAN,
> > - .impl = {.max_access_size = 1}
> > + .impl = {
> > + .max_access_size = 1,
> > + },
> > };
> >
> > static void avr_mask_init(Object *dev)
> > @@ -80,7 +81,7 @@ static void avr_mask_init(Object *dev)
> > SysBusDevice *busdev = SYS_BUS_DEVICE(dev);
> >
> > memory_region_init_io(&s->iomem, dev, &avr_mask_ops, s, TYPE_AVR_MASK,
> > - 0x01);
> > + 0x01);
^ Region has only 1 address: 0x00.
> > sysbus_init_mmio(busdev, &s->iomem);
> >
> > for (int i = 0; i < 8; i++) {
> > diff --git a/hw/misc/trace-events b/hw/misc/trace-events
> > index 7f0f5dff3a..f716881bb1 100644
> > --- a/hw/misc/trace-events
> > +++ b/hw/misc/trace-events
> > @@ -179,3 +179,7 @@ via1_rtc_cmd_pram_read(int addr, int value) "addr=%u value=0x%02x"
> > via1_rtc_cmd_pram_write(int addr, int value) "addr=%u value=0x%02x"
> > via1_rtc_cmd_pram_sect_read(int sector, int offset, int addr, int value) "sector=%u offset=%u addr=%d value=0x%02x"
> > via1_rtc_cmd_pram_sect_write(int sector, int offset, int addr, int value) "sector=%u offset=%u addr=%d value=0x%02x"
> > +
> > +# avr_power.c
> > +avr_power_read(uint8_t value) "power_reduc read value:%u"
> > +avr_power_write(uint8_t value) "power_reduc write value:%u"
>
>
> --
> Alex Bennée
>
Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> On Fri, Jan 31, 2020 at 12:27 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>>
>> > - convert DB_PRINT() to trace-events
>> > - fix style/indentation
>> >
>> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> > ---
>> > hw/misc/avr_power.c | 17 +++++++++--------
>> > hw/misc/trace-events | 4 ++++
>> > 2 files changed, 13 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/hw/misc/avr_power.c b/hw/misc/avr_power.c
>> > index 598bc7279c..65ff7c4405 100644
>> > --- a/hw/misc/avr_power.c
>> > +++ b/hw/misc/avr_power.c
>> > @@ -27,9 +27,7 @@
>> > #include "qemu/log.h"
>> > #include "hw/qdev-properties.h"
>> > #include "hw/irq.h"
>> > -
>> > -#define DB_PRINT(fmt, args...) /* Nothing */
>> > -/*#define DB_PRINT(fmt, args...) printf("%s: " fmt "\n", __func__, ## args)*/
>> > +#include "trace.h"
>> >
>> > static void avr_mask_reset(DeviceState *dev)
>> > {
>> > @@ -48,19 +46,20 @@ static uint64_t avr_mask_read(void *opaque, hwaddr offset, unsigned size)
>> > assert(offset == 0);
>> > AVRMaskState *s = opaque;
>> >
>> > + trace_avr_power_read(s->val);
>> > +
>> > return (uint64_t)s->val;
>> > }
>> >
>> > static void avr_mask_write(void *opaque, hwaddr offset,
>> > - uint64_t val64, unsigned size)
>> > + uint64_t val64, unsigned size)
>> > {
>> > assert(size == 1);
>> > assert(offset == 0);
>> > AVRMaskState *s = opaque;
>> > uint8_t val8 = val64;
>> >
>> > - DB_PRINT("write %d to offset %d", val8, (uint8_t)offset);
>> > -
>> > + trace_avr_power_write(val8);
>>
>> You've dropped offset in this trace point which is probably worth
>> keeping so you track where is being written to. Same with the read.
>
> I dropped it because it is always 0x00, the register is 8bit wide. See
> below, memory_region_init_io(...,1).
> I thought about adding a "name" property so each instance can display
> the device it belongs to, but this was too invasive, so I decided to
> keep this change for later.
Ahh I did wonder (I was reviewing without applying). Might be worth
mentioning in the commit then.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>
>> > s->val = val8;
>> > for (int i = 0; i < 8; i++) {
>> > qemu_set_irq(s->irq[i], (val8 & (1 << i)) != 0);
>> > @@ -71,7 +70,9 @@ static const MemoryRegionOps avr_mask_ops = {
>> > .read = avr_mask_read,
>> > .write = avr_mask_write,
>> > .endianness = DEVICE_NATIVE_ENDIAN,
>> > - .impl = {.max_access_size = 1}
>> > + .impl = {
>> > + .max_access_size = 1,
>> > + },
>> > };
>> >
>> > static void avr_mask_init(Object *dev)
>> > @@ -80,7 +81,7 @@ static void avr_mask_init(Object *dev)
>> > SysBusDevice *busdev = SYS_BUS_DEVICE(dev);
>> >
>> > memory_region_init_io(&s->iomem, dev, &avr_mask_ops, s, TYPE_AVR_MASK,
>> > - 0x01);
>> > + 0x01);
>
> ^ Region has only 1 address: 0x00.
>
>> > sysbus_init_mmio(busdev, &s->iomem);
>> >
>> > for (int i = 0; i < 8; i++) {
>> > diff --git a/hw/misc/trace-events b/hw/misc/trace-events
>> > index 7f0f5dff3a..f716881bb1 100644
>> > --- a/hw/misc/trace-events
>> > +++ b/hw/misc/trace-events
>> > @@ -179,3 +179,7 @@ via1_rtc_cmd_pram_read(int addr, int value) "addr=%u value=0x%02x"
>> > via1_rtc_cmd_pram_write(int addr, int value) "addr=%u value=0x%02x"
>> > via1_rtc_cmd_pram_sect_read(int sector, int offset, int addr, int value) "sector=%u offset=%u addr=%d value=0x%02x"
>> > via1_rtc_cmd_pram_sect_write(int sector, int offset, int addr, int value) "sector=%u offset=%u addr=%d value=0x%02x"
>> > +
>> > +# avr_power.c
>> > +avr_power_read(uint8_t value) "power_reduc read value:%u"
>> > +avr_power_write(uint8_t value) "power_reduc write value:%u"
>>
>>
>> --
>> Alex Bennée
>>
--
Alex Bennée
Convert DB_PRINT() to trace events.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/timer/avr_timer16.c | 25 +++++++++++++++----------
hw/timer/trace-events | 12 ++++++++++++
2 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/hw/timer/avr_timer16.c b/hw/timer/avr_timer16.c
index 4e16afc61c..073f36aa76 100644
--- a/hw/timer/avr_timer16.c
+++ b/hw/timer/avr_timer16.c
@@ -36,6 +36,7 @@
#include "qemu/log.h"
#include "hw/irq.h"
#include "hw/qdev-properties.h"
+#include "trace.h"
/* Register offsets */
#define T16_CRA 0x0
@@ -104,7 +105,6 @@
/* Helper macros */
#define VAL16(l, h) ((h << 8) | l)
#define DB_PRINT(fmt, args...) /* Nothing */
-/*#define DB_PRINT(fmt, args...) printf("%s: " fmt "\n", __func__, ## args)*/
static inline int64_t avr_timer16_ns_to_ticks(AVRTimer16State *t16, int64_t t)
{
@@ -168,8 +168,8 @@ static void avr_timer16_clksrc_update(AVRTimer16State *t16)
if (divider) {
t16->freq_hz = t16->cpu_freq_hz / divider;
t16->period_ns = NANOSECONDS_PER_SECOND / t16->freq_hz;
- DB_PRINT("Timer frequency %" PRIu64 " hz, period %" PRIu64 " ns (%f s)",
- t16->freq_hz, t16->period_ns, 1 / (double)t16->freq_hz);
+ trace_avr_timer16_clksrc_update(t16->freq_hz, t16->period_ns,
+ (uint64_t)(1e6 / t16->freq_hz));
}
}
@@ -235,8 +235,7 @@ static void avr_timer16_set_alarm(AVRTimer16State *t16)
t16->reset_time_ns + ((CNT(t16) + alarm_offset) * t16->period_ns);
timer_mod(t16->timer, alarm_ns);
- DB_PRINT("next alarm %" PRIu64 " ns from now",
- alarm_offset * t16->period_ns);
+ trace_avr_timer16_next_alarm(alarm_offset * t16->period_ns);
}
static void avr_timer16_interrupt(void *opaque)
@@ -253,11 +252,11 @@ static void avr_timer16_interrupt(void *opaque)
return;
}
- DB_PRINT("interrupt, cnt = %d", CNT(t16));
+ trace_avr_timer16_interrupt_count(CNT(t16));
/* Counter overflow */
if (t16->next_interrupt == OVERFLOW) {
- DB_PRINT("0xffff overflow");
+ trace_avr_timer16_interrupt_overflow("counter 0xffff");
avr_timer16_clock_reset(t16);
if (t16->imsk & T16_INT_TOV) {
t16->ifr |= T16_INT_TOV;
@@ -266,12 +265,12 @@ static void avr_timer16_interrupt(void *opaque)
}
/* Check for ocra overflow in CTC mode */
if (mode == T16_MODE_CTC_OCRA && t16->next_interrupt == COMPA) {
- DB_PRINT("CTC OCRA overflow");
+ trace_avr_timer16_interrupt_overflow("CTC OCRA");
avr_timer16_clock_reset(t16);
}
/* Check for icr overflow in CTC mode */
if (mode == T16_MODE_CTC_ICR && t16->next_interrupt == CAPT) {
- DB_PRINT("CTC ICR overflow");
+ trace_avr_timer16_interrupt_overflow("CTC ICR");
avr_timer16_clock_reset(t16);
if (t16->imsk & T16_INT_IC) {
t16->ifr |= T16_INT_IC;
@@ -367,6 +366,8 @@ static uint64_t avr_timer16_read(void *opaque, hwaddr offset, unsigned size)
default:
break;
}
+ trace_avr_timer16_read(offset, retval);
+
return (uint64_t)retval;
}
@@ -378,7 +379,7 @@ static void avr_timer16_write(void *opaque, hwaddr offset,
uint8_t val8 = (uint8_t)val64;
uint8_t prev_clk_src = CLKSRC(t16);
- DB_PRINT("write %d to offset %d", val8, (uint8_t)offset);
+ trace_avr_timer16_write(offset, val8);
switch (offset) {
case T16_CRA:
@@ -475,6 +476,7 @@ static uint64_t avr_timer16_imsk_read(void *opaque,
{
assert(size == 1);
AVRTimer16State *t16 = opaque;
+ trace_avr_timer16_read_imsk(offset ? 0 : t16->imsk);
if (offset != 0) {
return 0;
}
@@ -486,6 +488,7 @@ static void avr_timer16_imsk_write(void *opaque, hwaddr offset,
{
assert(size == 1);
AVRTimer16State *t16 = opaque;
+ trace_avr_timer16_write_imsk(val64);
if (offset != 0) {
return;
}
@@ -498,6 +501,7 @@ static uint64_t avr_timer16_ifr_read(void *opaque,
{
assert(size == 1);
AVRTimer16State *t16 = opaque;
+ trace_avr_timer16_read_ifr(offset ? 0 : t16->ifr);
if (offset != 0) {
return 0;
}
@@ -509,6 +513,7 @@ static void avr_timer16_ifr_write(void *opaque, hwaddr offset,
{
assert(size == 1);
AVRTimer16State *t16 = opaque;
+ trace_avr_timer16_write_imsk(val64);
if (offset != 0) {
return;
}
diff --git a/hw/timer/trace-events b/hw/timer/trace-events
index 29fda7870e..5d9f4c93fb 100644
--- a/hw/timer/trace-events
+++ b/hw/timer/trace-events
@@ -74,3 +74,15 @@ nrf51_timer_write(uint64_t addr, uint32_t value, unsigned size) "write addr 0x%"
bcm2835_systmr_irq(bool enable) "timer irq state %u"
bcm2835_systmr_read(uint64_t offset, uint64_t data) "timer read: offset 0x%" PRIx64 " data 0x%" PRIx64
bcm2835_systmr_write(uint64_t offset, uint64_t data) "timer write: offset 0x%" PRIx64 " data 0x%" PRIx64
+
+# avr_timer16.c
+avr_timer16_read(uint8_t addr, uint8_t value) "timer16 read addr:%u value:%u"
+avr_timer16_read_ifr(uint8_t value) "timer16 read addr:ifr value:%u"
+avr_timer16_read_imsk(uint8_t value) "timer16 read addr:imsk value:%u"
+avr_timer16_write(uint8_t addr, uint8_t value) "timer16 write addr:%u value:%u"
+avr_timer16_write_ifr(uint8_t value) "timer16 write addr:ifr value:%u"
+avr_timer16_write_imsk(uint8_t value) "timer16 write addr:imsk value:%u"
+avr_timer16_interrupt_count(uint8_t cnt) "count: %u"
+avr_timer16_interrupt_overflow(const char *reason) "overflow: %s"
+avr_timer16_next_alarm(uint64_t delay_ns) "next alarm: %" PRIu64 " ns from now"
+avr_timer16_clksrc_update(uint64_t freq_hz, uint64_t period_ns, uint64_t delay_s) "timer frequency: %" PRIu64 " Hz, period: %" PRIu64 " ns (%" PRId64 " us)"
--
2.21.1
Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> Convert DB_PRINT() to trace events.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/timer/avr_timer16.c | 25 +++++++++++++++----------
> hw/timer/trace-events | 12 ++++++++++++
> 2 files changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/hw/timer/avr_timer16.c b/hw/timer/avr_timer16.c
> index 4e16afc61c..073f36aa76 100644
> --- a/hw/timer/avr_timer16.c
> +++ b/hw/timer/avr_timer16.c
> @@ -36,6 +36,7 @@
> #include "qemu/log.h"
> #include "hw/irq.h"
> #include "hw/qdev-properties.h"
> +#include "trace.h"
>
> /* Register offsets */
> #define T16_CRA 0x0
> @@ -104,7 +105,6 @@
> /* Helper macros */
> #define VAL16(l, h) ((h << 8) | l)
> #define DB_PRINT(fmt, args...) /* Nothing */
> -/*#define DB_PRINT(fmt, args...) printf("%s: " fmt "\n", __func__, ## args)*/
>
> static inline int64_t avr_timer16_ns_to_ticks(AVRTimer16State *t16, int64_t t)
> {
> @@ -168,8 +168,8 @@ static void avr_timer16_clksrc_update(AVRTimer16State *t16)
> if (divider) {
> t16->freq_hz = t16->cpu_freq_hz / divider;
> t16->period_ns = NANOSECONDS_PER_SECOND / t16->freq_hz;
> - DB_PRINT("Timer frequency %" PRIu64 " hz, period %" PRIu64 " ns (%f s)",
> - t16->freq_hz, t16->period_ns, 1 / (double)t16->freq_hz);
> + trace_avr_timer16_clksrc_update(t16->freq_hz, t16->period_ns,
> + (uint64_t)(1e6 /
> t16->freq_hz));
Is it worth including a computed value here?
> }
> }
>
> @@ -235,8 +235,7 @@ static void avr_timer16_set_alarm(AVRTimer16State *t16)
> t16->reset_time_ns + ((CNT(t16) + alarm_offset) * t16->period_ns);
> timer_mod(t16->timer, alarm_ns);
>
> - DB_PRINT("next alarm %" PRIu64 " ns from now",
> - alarm_offset * t16->period_ns);
> + trace_avr_timer16_next_alarm(alarm_offset * t16->period_ns);
> }
>
> static void avr_timer16_interrupt(void *opaque)
> @@ -253,11 +252,11 @@ static void avr_timer16_interrupt(void *opaque)
> return;
> }
>
> - DB_PRINT("interrupt, cnt = %d", CNT(t16));
> + trace_avr_timer16_interrupt_count(CNT(t16));
>
> /* Counter overflow */
> if (t16->next_interrupt == OVERFLOW) {
> - DB_PRINT("0xffff overflow");
> + trace_avr_timer16_interrupt_overflow("counter 0xffff");
> avr_timer16_clock_reset(t16);
> if (t16->imsk & T16_INT_TOV) {
> t16->ifr |= T16_INT_TOV;
> @@ -266,12 +265,12 @@ static void avr_timer16_interrupt(void *opaque)
> }
> /* Check for ocra overflow in CTC mode */
> if (mode == T16_MODE_CTC_OCRA && t16->next_interrupt == COMPA) {
> - DB_PRINT("CTC OCRA overflow");
> + trace_avr_timer16_interrupt_overflow("CTC OCRA");
> avr_timer16_clock_reset(t16);
> }
> /* Check for icr overflow in CTC mode */
> if (mode == T16_MODE_CTC_ICR && t16->next_interrupt == CAPT) {
> - DB_PRINT("CTC ICR overflow");
> + trace_avr_timer16_interrupt_overflow("CTC ICR");
Rather than spamming strings to the trace log I'd suggest expanding
trace_avr_timer16_interrupt_count to include the mode and next_interrupt
state as hex values.
> avr_timer16_clock_reset(t16);
> if (t16->imsk & T16_INT_IC) {
> t16->ifr |= T16_INT_IC;
> @@ -367,6 +366,8 @@ static uint64_t avr_timer16_read(void *opaque, hwaddr offset, unsigned size)
> default:
> break;
> }
> + trace_avr_timer16_read(offset, retval);
> +
> return (uint64_t)retval;
> }
>
> @@ -378,7 +379,7 @@ static void avr_timer16_write(void *opaque, hwaddr offset,
> uint8_t val8 = (uint8_t)val64;
> uint8_t prev_clk_src = CLKSRC(t16);
>
> - DB_PRINT("write %d to offset %d", val8, (uint8_t)offset);
> + trace_avr_timer16_write(offset, val8);
>
> switch (offset) {
> case T16_CRA:
> @@ -475,6 +476,7 @@ static uint64_t avr_timer16_imsk_read(void *opaque,
> {
> assert(size == 1);
> AVRTimer16State *t16 = opaque;
> + trace_avr_timer16_read_imsk(offset ? 0 : t16->imsk);
> if (offset != 0) {
> return 0;
> }
> @@ -486,6 +488,7 @@ static void avr_timer16_imsk_write(void *opaque, hwaddr offset,
> {
> assert(size == 1);
> AVRTimer16State *t16 = opaque;
> + trace_avr_timer16_write_imsk(val64);
> if (offset != 0) {
> return;
> }
> @@ -498,6 +501,7 @@ static uint64_t avr_timer16_ifr_read(void *opaque,
> {
> assert(size == 1);
> AVRTimer16State *t16 = opaque;
> + trace_avr_timer16_read_ifr(offset ? 0 : t16->ifr);
> if (offset != 0) {
> return 0;
> }
> @@ -509,6 +513,7 @@ static void avr_timer16_ifr_write(void *opaque, hwaddr offset,
> {
> assert(size == 1);
> AVRTimer16State *t16 = opaque;
> + trace_avr_timer16_write_imsk(val64);
> if (offset != 0) {
> return;
> }
> diff --git a/hw/timer/trace-events b/hw/timer/trace-events
> index 29fda7870e..5d9f4c93fb 100644
> --- a/hw/timer/trace-events
> +++ b/hw/timer/trace-events
> @@ -74,3 +74,15 @@ nrf51_timer_write(uint64_t addr, uint32_t value, unsigned size) "write addr 0x%"
> bcm2835_systmr_irq(bool enable) "timer irq state %u"
> bcm2835_systmr_read(uint64_t offset, uint64_t data) "timer read: offset 0x%" PRIx64 " data 0x%" PRIx64
> bcm2835_systmr_write(uint64_t offset, uint64_t data) "timer write: offset 0x%" PRIx64 " data 0x%" PRIx64
> +
> +# avr_timer16.c
> +avr_timer16_read(uint8_t addr, uint8_t value) "timer16 read addr:%u value:%u"
> +avr_timer16_read_ifr(uint8_t value) "timer16 read addr:ifr value:%u"
> +avr_timer16_read_imsk(uint8_t value) "timer16 read addr:imsk value:%u"
> +avr_timer16_write(uint8_t addr, uint8_t value) "timer16 write addr:%u value:%u"
> +avr_timer16_write_ifr(uint8_t value) "timer16 write addr:ifr value:%u"
> +avr_timer16_write_imsk(uint8_t value) "timer16 write addr:imsk value:%u"
> +avr_timer16_interrupt_count(uint8_t cnt) "count: %u"
> +avr_timer16_interrupt_overflow(const char *reason) "overflow: %s"
> +avr_timer16_next_alarm(uint64_t delay_ns) "next alarm: %" PRIu64 " ns from now"
> +avr_timer16_clksrc_update(uint64_t freq_hz, uint64_t period_ns, uint64_t delay_s) "timer frequency: %" PRIu64 " Hz, period: %" PRIu64 " ns (%" PRId64 " us)"
--
Alex Bennée
© 2016 - 2025 Red Hat, Inc.