| Message ID | 20240817102606.3996242-1-tavip@google.com |
|---|---|
| Headers | show |
| Series | NXP i.MX RT595, ARM SVD and device model unit tests | expand |
On Sat, 17 Aug 2024 at 11:26, Octavian Purdila <tavip@google.com> wrote: > > This patch set adds support for NXP's RT500 MCU [1] and the RT595 > EVK[2]. More RT500 device models will be submitted in future patch sets. > > The goal of this first patch set is to provide a minimal set that > allows running the NXP MCU SDK hello world example[4]. > > The patch set introduces a (python) tool that generates C header files > from ARM SVD files[3]. This significantly reduces the effort to write > a new device model by automatically generating: register definitions > and layout (including bit fields), register names for easier debugging > and tracing, reset register values, register write masks, etc. > > The generated files are commited and not generated at compile > time. Build targets are created so that they can be easily regenerated > if needed. > > It also introduces unit tests for device models. To allow accessing > registers from unit tests a system bus mock is created. > > This can potentially introduce maintainance issues, due to mocks or > unit tests getting outdated when code is refactored. However, I think > this is not an issue in this case because the APIs we mocked (system > bus MMIO access) or directly used (irq APIs, chardev APIs, clock tree > APIs) to interact with device models are stable at this > point. Anecdotally, our experience seems to confirm this: we only run > into one (trivially fixed) breaking upstream change (gpio getting > removed from hwcore) in the last three years. My main issue with the mocking is that it introduces a completely different way of testing devices that is not the same as what we use for any existing device. QEMU already has too many places where there are multiple different ways or styles of doing something, so adding a new one should be a high bar (e.g. "this lets us test XYZ that would be impossible in the old way") and preferably also have a transition plan for how we would be deprecating and dropping the old way of doing things. So my inclination here is to say "you said that you could do the testing of this device with qtest, so use qtest". If we were designing a "test devices" framework from scratch, using mocks would probably be a strong candidate for that design, but we aren't starting from scratch. thanks -- PMM
On Thu, Aug 22, 2024 at 6:28 AM Peter Maydell <peter.maydell@linaro.org> wrote: > > On Sat, 17 Aug 2024 at 11:26, Octavian Purdila <tavip@google.com> wrote: > > > > This patch set adds support for NXP's RT500 MCU [1] and the RT595 > > EVK[2]. More RT500 device models will be submitted in future patch sets. > > > > The goal of this first patch set is to provide a minimal set that > > allows running the NXP MCU SDK hello world example[4]. > > > > The patch set introduces a (python) tool that generates C header files > > from ARM SVD files[3]. This significantly reduces the effort to write > > a new device model by automatically generating: register definitions > > and layout (including bit fields), register names for easier debugging > > and tracing, reset register values, register write masks, etc. > > > > The generated files are commited and not generated at compile > > time. Build targets are created so that they can be easily regenerated > > if needed. > > > > It also introduces unit tests for device models. To allow accessing > > registers from unit tests a system bus mock is created. > > > > This can potentially introduce maintainance issues, due to mocks or > > unit tests getting outdated when code is refactored. However, I think > > this is not an issue in this case because the APIs we mocked (system > > bus MMIO access) or directly used (irq APIs, chardev APIs, clock tree > > APIs) to interact with device models are stable at this > > point. Anecdotally, our experience seems to confirm this: we only run > > into one (trivially fixed) breaking upstream change (gpio getting > > removed from hwcore) in the last three years. > > My main issue with the mocking is that it introduces a > completely different way of testing devices that is > not the same as what we use for any existing device. > QEMU already has too many places where there are multiple > different ways or styles of doing something, so adding a > new one should be a high bar (e.g. "this lets us test XYZ > that would be impossible in the old way") and preferably > also have a transition plan for how we would be > deprecating and dropping the old way of doing things. > > So my inclination here is to say "you said that you could > do the testing of this device with qtest, so use qtest". Looks like I missed some things when I looked at it, probably minor or a matter of preference in the great scheme of things. It might still be worth mentioning them here. In patch 19 we are testing exposed clocks and AFAICS there are no qtest APIs for that. We can probably add a qest API for that though. In patch 9 we are using internal APIs exposed by the generic flexcom device to check that device selection works as expected. I don't see how we can reimplement that with qtest, this is a classic example of what unit tests can enable vs functional testing. There are also a few things that are a bit cumbersome to do with qtest. In patch 13 and 16 we introduce i2c and spi echo test devices to test i2c/spi transactions. I've noticed that device qtests use an existing i2c peripheral for tests. We could add the i2c and spi test devices to qemu and have them enabled by default. Or continue using existing spi/i2c peripherals for testing spi/i2c controllers for qtests which I personally don't like because it requires knowledge of specific peripherals. That being said, I'll go ahead and switch to qtests, if only to better understand advantages / disadvantages and separate this patch from the bigger device unit tests discussion. > If we were designing a "test devices" framework from > scratch, using mocks would probably be a strong candidate > for that design, but we aren't starting from scratch. I still think that writing device tests as unit tests is better: - devices can be tested independently - we can test more, including internal behaviour and APIs, error paths, etc. - enable sanitizers by default for device tests* I wonder what people think and what would be a path to enable device unit tests. What would we need to prove, besides converting existing device qtests to unit tests, which I can certainly help with. *I was surprised to see that sanitizers are not enabled by default on unit tests when host dependencies are available, what is preventing that?
This patch set adds support for NXP's RT500 MCU [1] and the RT595 EVK[2]. More RT500 device models will be submitted in future patch sets. The goal of this first patch set is to provide a minimal set that allows running the NXP MCU SDK hello world example[4]. The patch set introduces a (python) tool that generates C header files from ARM SVD files[3]. This significantly reduces the effort to write a new device model by automatically generating: register definitions and layout (including bit fields), register names for easier debugging and tracing, reset register values, register write masks, etc. The generated files are commited and not generated at compile time. Build targets are created so that they can be easily regenerated if needed. It also introduces unit tests for device models. To allow accessing registers from unit tests a system bus mock is created. This can potentially introduce maintainance issues, due to mocks or unit tests getting outdated when code is refactored. However, I think this is not an issue in this case because the APIs we mocked (system bus MMIO access) or directly used (irq APIs, chardev APIs, clock tree APIs) to interact with device models are stable at this point. Anecdotally, our experience seems to confirm this: we only run into one (trivially fixed) breaking upstream change (gpio getting removed from hwcore) in the last three years. Changes since v1: * setup scripts/git.orderfile * svd_gen_header: add support for multi line comments * svd_gen_header: use qemu register fields instead of bitfields * svd_gen_header: generate register write mask array * add mcux-soc-svd as a subproject instead of commiting the rt595 SVD file directly * svd generated headers: don't generate them at compile time, instead commit the generated headers and create compile targets so that it is easy to regenerate them * remove pysvd dependency; developers who need to (re)generate svd headers need to have it installed on the host machine * use DEFINE_TYPES where possible * use g_strdup_printf where possible * reduce iterator variables scopes * hard code rt500 to use the cortex-m33 * use static for local arrays initializers * remove rt500 properties as they are not yet used * drop private/public comments * remove unused headers * free rt500 mem regions in rt500_unrealize() instead of rt500_finalize * use memops access constraints where possible instead of reg32_aligned_access * flexspi: use 64bit properties for mmap_size property * rstctl: use a generic abstract type and remove the "num" property * flexspi: don't call get_system_memory() directly and remove mmap_base property [1] https://www.nxp.com/docs/en/data-sheet/IMXRT500EC.pdf [2] https://www.nxp.com/webapp/Download?colCode=MIMXRT595EVKHUG [3] https://arm-software.github.io/CMSIS_5/SVD/html/index.html [4] Building and running the NXP MCU SDK hello world example Clone the following git repos: https://github.com/nxp-mcuxpresso/cmsis.git, https://github.com/nxp-mcuxpresso/mcux-sdk.git https://github.com/nxp-mcuxpresso/mcux-sdk-examples.git in the following directories: CMSIS, core, examples. cd examples/evkmimxrt595/demo_apps/hello_world/armgcc ARMGCC_DIR=/usr CFLAGS=-I../../../../../CMSIS/CMSIS/Core/Include \ sh build_flash_debug.sh qemu-system-arm --machine rt595-evk -kernel flash_debug/hello_world.elf \ -global armv7m.init-nsvtor=0x08001000 -global armv7m.init-svtor=0x08001000 \ -chardev stdio,id=flexcomm0 Octavian Purdila (19): fifo32: add peek function tests/unit: add fifo test Add mcux-soc-svd subproject hw: add register access utility functions hw/misc: add basic flexcomm device model test/unit: add register access macros and functions test/unit: add flexcomm unit test hw/char: add support for flexcomm usart test/unit: add flexcomm usart unit test hw/i2c: add support for flexcomm i2c test/unit: add i2c-tester test/unit: add unit tests for flexcomm i2c test/unit: add spi-tester hw/misc: add support for RT500's clock controller test/unit: add unit tests for RT500's clock controller hw/ssi: add support for flexspi hw/misc: add support for RT500's reset controller hw/arm: add basic support for the RT500 SoC hw/arm: add RT595-EVK board Sebastian Ene (2): hw/ssi: add support for flexcomm spi test/unit: add unit tests for flexcomm spi Stefan Stanacar (1): scripts: add script to generate C header files from SVD XML files Valentin Ghita (1): tests/unit: add system bus mock meson.build | 4 + include/hw/arm/rt500.h | 44 + include/hw/arm/svd/flexcomm.h | 109 + include/hw/arm/svd/flexcomm_i2c.h | 1134 ++++++ include/hw/arm/svd/flexcomm_spi.h | 980 ++++++ include/hw/arm/svd/flexcomm_usart.h | 959 +++++ include/hw/arm/svd/flexspi.h | 2100 +++++++++++ include/hw/arm/svd/rt500.h | 65 + include/hw/arm/svd/rt500_clkctl0.h | 2287 ++++++++++++ include/hw/arm/svd/rt500_clkctl1.h | 3109 +++++++++++++++++ include/hw/arm/svd/rt500_rstctl0.h | 832 +++++ include/hw/arm/svd/rt500_rstctl1.h | 1344 +++++++ include/hw/char/flexcomm_usart.h | 20 + include/hw/i2c/flexcomm_i2c.h | 27 + include/hw/misc/flexcomm.h | 85 + include/hw/misc/rt500_clk_freqs.h | 18 + include/hw/misc/rt500_clkctl0.h | 37 + include/hw/misc/rt500_clkctl1.h | 36 + include/hw/misc/rt500_rstctl.h | 32 + include/hw/regs.h | 34 + include/hw/ssi/flexcomm_spi.h | 20 + include/hw/ssi/flexspi.h | 32 + include/qemu/fifo32.h | 28 + tests/unit/i2c_tester.h | 30 + tests/unit/reg-utils.h | 97 + tests/unit/spi_tester.h | 32 + tests/unit/sysbus-mock.h | 82 + hw/arm/rt500.c | 335 ++ hw/arm/rt595-evk.c | 64 + hw/char/flexcomm_usart.c | 292 ++ hw/i2c/flexcomm_i2c.c | 207 ++ hw/misc/flexcomm.c | 304 ++ hw/misc/rt500_clkctl0.c | 226 ++ hw/misc/rt500_clkctl1.c | 207 ++ hw/misc/rt500_rstctl.c | 225 ++ hw/ssi/flexcomm_spi.c | 439 +++ hw/ssi/flexspi.c | 157 + tests/unit/i2c_tester.c | 108 + tests/unit/spi_tester.c | 57 + tests/unit/sysbus-mock.c | 312 ++ tests/unit/test-fifo.c | 97 + tests/unit/test-flexcomm-i2c.c | 210 ++ tests/unit/test-flexcomm-spi.c | 201 ++ tests/unit/test-flexcomm-usart.c | 320 ++ tests/unit/test-flexcomm.c | 212 ++ tests/unit/test-rt500-clkctl.c | 263 ++ hw/arm/Kconfig | 13 + hw/arm/meson.build | 4 + hw/arm/svd/meson.build | 35 + hw/char/meson.build | 1 + hw/char/trace-events | 9 + hw/i2c/meson.build | 1 + hw/i2c/trace-events | 10 + hw/misc/Kconfig | 12 + hw/misc/meson.build | 5 + hw/misc/trace-events | 18 + hw/ssi/Kconfig | 4 + hw/ssi/meson.build | 2 + hw/ssi/trace-events | 12 + meson_options.txt | 3 + scripts/meson-buildoptions.sh | 4 + scripts/svd-gen-header.py | 343 ++ subprojects/.gitignore | 1 + subprojects/mcux-soc-svd.wrap | 5 + .../packagefiles/mcux-soc-svd/meson.build | 5 + tests/unit/meson.build | 56 +- 66 files changed, 18355 insertions(+), 1 deletion(-) create mode 100644 include/hw/arm/rt500.h create mode 100644 include/hw/arm/svd/flexcomm.h create mode 100644 include/hw/arm/svd/flexcomm_i2c.h create mode 100644 include/hw/arm/svd/flexcomm_spi.h create mode 100644 include/hw/arm/svd/flexcomm_usart.h create mode 100644 include/hw/arm/svd/flexspi.h create mode 100644 include/hw/arm/svd/rt500.h create mode 100644 include/hw/arm/svd/rt500_clkctl0.h create mode 100644 include/hw/arm/svd/rt500_clkctl1.h create mode 100644 include/hw/arm/svd/rt500_rstctl0.h create mode 100644 include/hw/arm/svd/rt500_rstctl1.h create mode 100644 include/hw/char/flexcomm_usart.h create mode 100644 include/hw/i2c/flexcomm_i2c.h create mode 100644 include/hw/misc/flexcomm.h create mode 100644 include/hw/misc/rt500_clk_freqs.h create mode 100644 include/hw/misc/rt500_clkctl0.h create mode 100644 include/hw/misc/rt500_clkctl1.h create mode 100644 include/hw/misc/rt500_rstctl.h create mode 100644 include/hw/regs.h create mode 100644 include/hw/ssi/flexcomm_spi.h create mode 100644 include/hw/ssi/flexspi.h create mode 100644 tests/unit/i2c_tester.h create mode 100644 tests/unit/reg-utils.h create mode 100644 tests/unit/spi_tester.h create mode 100644 tests/unit/sysbus-mock.h create mode 100644 hw/arm/rt500.c create mode 100644 hw/arm/rt595-evk.c create mode 100644 hw/char/flexcomm_usart.c create mode 100644 hw/i2c/flexcomm_i2c.c create mode 100644 hw/misc/flexcomm.c create mode 100644 hw/misc/rt500_clkctl0.c create mode 100644 hw/misc/rt500_clkctl1.c create mode 100644 hw/misc/rt500_rstctl.c create mode 100644 hw/ssi/flexcomm_spi.c create mode 100644 hw/ssi/flexspi.c create mode 100644 tests/unit/i2c_tester.c create mode 100644 tests/unit/spi_tester.c create mode 100644 tests/unit/sysbus-mock.c create mode 100644 tests/unit/test-fifo.c create mode 100644 tests/unit/test-flexcomm-i2c.c create mode 100644 tests/unit/test-flexcomm-spi.c create mode 100644 tests/unit/test-flexcomm-usart.c create mode 100644 tests/unit/test-flexcomm.c create mode 100644 tests/unit/test-rt500-clkctl.c create mode 100644 hw/arm/svd/meson.build create mode 100755 scripts/svd-gen-header.py create mode 100644 subprojects/mcux-soc-svd.wrap create mode 100644 subprojects/packagefiles/mcux-soc-svd/meson.build