On 9/8/20 5:52 PM, Philippe Mathieu-Daudé wrote:
> On 9/8/20 5:02 PM, Alexander Bulekov wrote:
>> Hi Havard,
>> I fuzzed the npcm750-evb machine until I hit over 85% coverage over all
>> the new npcm.*\.c files. The only thing I found specific to the new
>> code, so far:
>>
>> cat << EOF | ./qemu-system-arm -machine npcm750-evb -m 128M -qtest stdio
>> write 0xf0009040 0x4 0xc4c4c4c4
>> write 0xf0009040 0x4 0x4
>> EOF
>
> This is an odd test because with -qtest the timer is not running,
> so this can not really happen on real hw.
>
> The fix is:
>
> - g_assert(t->remaining_ns > 0);
> + g_assert(qtest_enabled() || t->remaining_ns > 0);
Alex corrected me on IRC, qtest is irrelevant here.
The problem is he disables the timer twice.
So maybe something like:
static void npcm7xx_timer_pause(NPCM7xxTimer *t)
{
int64_t now;
+ if (!timer_pending(&t->qtimer)) {
+ return;
+ }
timer_del(&t->qtimer);
now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
t->remaining_ns = t->expires_ns - now;
g_assert(t->remaining_ns > 0);
}
>
>>
>> ERROR:../hw/timer/npcm7xx_timer.c:160:npcm7xx_timer_pause: assertion failed:
>> (t->remaining_ns > 0)
>> Bail out! ERROR:../hw/timer/npcm7xx_timer.c:160:npcm7xx_timer_pause:
>> assertion failed: (t->remaining_ns > 0)
>> Aborted
>>
>> I'm doing the same for the quanta-gsj machine, but I'm not sure whether
>> it will cover more code, so I'm happy to leave a:
>>
>> Tested-by: Alexander Bulekov <alx...@bu.edu>
>>
>> for the patches that add new virtual-device code (1-5, 7-12 ?)
>> -Alex
>
> Very nice from you for testing running the fuzzer!
>
> Regards,
>
> Phil.
>
>>
>>
>> On 200824 1716, Havard Skinnemoen via wrote:
>>> I also pushed this and the previous patchsets to my qemu fork on github.
>>> The branches are named npcm7xx-v[1-8].
>>>
>>> https://github.com/hskinnemoen/qemu
>>>
>>> This patch series models enough of the Nuvoton NPCM730 and NPCM750 SoCs to
>>> boot
>>> an OpenBMC image built for quanta-gsj. This includes device models for:
>>>
>>> - Global Configuration Registers
>>> - Clock Control
>>> - Timers
>>> - Fuses
>>> - Memory Controller
>>> - Flash Controller
>>>
>>> These modules, along with the existing Cortex A9 CPU cores and built-in
>>> peripherals, are integrated into a NPCM730 or NPCM750 SoC, which in turn
>>> form
>>> the foundation for the quanta-gsj and npcm750-evb machines, respectively.
>>> The
>>> two SoCs are very similar; the only difference is that NPCM730 is missing
>>> some
>>> peripherals that NPCM750 has, and which are not considered essential for
>>> datacenter use (e.g. graphics controllers). For more information, see
>>>
>>> https://www.nuvoton.com/products/cloud-computing/ibmc/
>>>
>>> Both quanta-gsj and npcm750-evb correspond to real boards supported by
>>> OpenBMC.
>>> At the end of the series, qemu can boot an OpenBMC image built for one of
>>> these
>>> boards with some minor modifications.
>>>
>>> The patches in this series were developed by Google and reviewed by
>>> Nuvoton. We
>>> will be maintaining the machine and peripheral support together.
>>>
>>> The data sheet for these SoCs is not generally available. Please let me
>>> know if
>>> more comments are needed to understand the device behavior.
>>>
>>> Changes since v7:
>>>
>>> - Move register enums to .c files throughout, leaving a single
>>> NPCM7XX_FOO_NR_REGS definition behind in the .h file. A
>>> QEMU_BUILD_BUG_ON
>>> should alert anyone accidentally expanding the register enum that they
>>> need
>>> to update the corresponding NR_REGS define, which in turn has a comment
>>> reminding them to update the vmstate version_id as well.
>>> - Skip loading the bootrom if a kernel filename is provided by the user.
>>> - New patch adding a board setup stub to tweak clocks before booting
>>> directly
>>> into the kernel.
>>> - Add stuff to meson files instead of Makefiles.
>>> - Try to disable the slowest drivers and services to speed up the flash
>>> boot
>>> acceptance test a bit. This is somewhat based on the following
>>> systemd-analyze blame report:
>>> https://gist.github.com/hskinnemoen/475cb0676530cd2cebaa1754cf16ca97
>>>
>>> Changes since v6:
>>>
>>> - Use size_to_str to report DRAM sizes in npcm7xx_gcr.
>>> - Simplify the interrupt logic in npcm7xx_timer.
>>> - Update global bios_name instead of temporary.
>>> - Add npcm7xx_bootrom to MAINTAINERS and pc-bios/README.
>>> - Use a predefined name for the gsj boot image in the acceptance test.
>>>
>>> Changes since v5:
>>>
>>> - Boot ROM included, as a git submodule and a binary blob, and loaded by
>>> default, so the -bios option is usually not necessary anymore.
>>> - Two acceptance tests added (openbmc image boot, and direct kernel boot).
>>> - npcm7xx_load_kernel() moved to SoC code.
>>> - NPCM7XX_TIMER_REF_HZ definition moved to CLK header.
>>> - Comments added clarifying available SPI flash chip selects.
>>> - Error handling adjustments:
>>> - Errors from CPU and GCR realization are propagated through the SoC
>>> since they may be triggered by user-configurable parameters.
>>> - Machine init uses error_fatal instead of error_abort for SoC
>>> realization flash init. This makes error messages more helpful.
>>> - Comments added to indicate whether peripherals may fail to realize.
>>> - Use ERRP_GUARD() instead of Error *err when possible.
>>> - Default CPU type is now set, and attempting to set it to anything else
>>> will fail.
>>> - Format string fixes (use HWADDR_PRIx, etc.)
>>> - Simplified memory size encoding and error checking in npcm7xx_gcr.
>>> - Encapsulate non-obvious pointer subtraction into helper functions in the
>>> FIU and TIMER modules.
>>> - Incorporate review feedback into the FIU module:
>>> - Add select/deselect trace events.
>>> - Use npcm7xx_fiu_{de,}select() consistently.
>>> - Use extract/deposit in more places for consistency.
>>> - Use -Wimplicit-fallthrough compatible fallthrough comments.
>>> - Use qdev_init_gpio_out_named instead of sysbus_init_irq for chip
>>> selects.
>>> - Incorporate review feedback into the TIMER module:
>>> - Assert that we never pause a timer that has already expired,
>>> instead of
>>> trying to handle it. This should be safe since QEMU_CLOCK_VIRTUAL is
>>> stopped while this code is running.
>>> - Simplify the switch blocks in the read and write handlers.
>>>
>>> I made a change to error out if a flash drive was not specified, but
>>> reverted
>>> it because it caused make check to fail (qom-test). When specifying a NULL
>>> block device, the m25p flash device initializes its in-memory storage with
>>> 0xff
>>> and doesn't attempt to write anything back. This seems correct to me.
>>>
>>> Changes since v4:
>>>
>>> - OTP cleanups suggested by Philippe Mathieu-Daudé.
>>> - Added fuse array definitions based on public Nuvoton bootblock code.
>>> - Moved class structure to .c file since it's only used internally.
>>> - Readability improvements.
>>> - Split the first patch and folded parts of it into three other patches so
>>> that CONFIG_NPCM7XX is only enabled after the initial NPCM7xx machine
>>> support is added.
>>> - DRAM init moved to machine init code.
>>> - Consistently use lower-case hex literals.
>>> - Switched to fine-grained unimplemented devices, based on public
>>> bootblock
>>> source code. Added a tiny SRAM that got left out previously.
>>> - Simplified error handling in npcm7xx_realize() since the board code will
>>> abort anyway, and SoCs are not hot-pluggable.
>>>
>>> Changes since v3:
>>>
>>> - License headers are now GPL v2-or-later throughout.
>>> - Added vmstate throughout (except the memory controller, which doesn't
>>> really have any state worth saving). Successfully booted a gsj image
>>> with two stop/savevm/quit/loadvm cycles along the way.
>>> - JFFS2 really doesn't like it if I let qemu keep running after
>>> savevm,
>>> and then jump back in time with loadvm. I assume this is expected.
>>> - Fixed an error API violation in npcm7xx_realize, removed pointless error
>>> check after object_property_set_link().
>>> - Switched the OTP device to use an embedded array instead of a
>>> g_malloc0'd
>>> one because I couldn't figure out how to set up vmstate for the latter.
>>>
>>> Changes since v2:
>>>
>>> - Simplified the MAINTAINERS entry.
>>> - Added link to OpenPOWER jenkins for gsj BMC images.
>>> - Reverted the smpboot change, back to byte swapping.
>>> - Adapted to upstream API changes:
>>> - sysbus_init_child_obj -> object_initialize_child
>>> - object_property_set_bool -> qdev_realize / sysbus_realize
>>> - ssi_create_slave_no_init -> qdev_new
>>> - qdev_init_nofail -> qdev_realize_and_unref
>>> - ssi_auto_connect_slaves removed
>>> - Moved Boot ROM loading from soc to machine init.
>>> - Plumbed power-on-straps property from GCR to the machine init code so it
>>> can be properly initialized. Turns out npcm750 memory init doesn't work
>>> without this. npcm730 is fine either way, though I'm not sure why.
>>> - Reworked the flash init code so it looks more like aspeed (i.e. the
>>> flash
>>> device gets added even if there's no drive).
>>>
>>> Changes since v1 (requested by reviewers):
>>>
>>> - Clarify the source of CLK reset values.
>>> - Made smpboot a constant byte array, eliinated byte swapping.
>>> - NPCM7xxState now stores an array of ARMCPUs, not pointers to ARMCPUs.
>>> - Clarify why EL3 is disabled.
>>> - Introduce NPCM7XX_NUM_IRQ constant.
>>> - Set the number of CPUs according to SoC variant, and disallow command
>>> line
>>> overrides (i.e. you can no longer override the number of CPUs with the
>>> -smp
>>> parameter). This is trying to follow the spirit of
>>> https://patchwork.kernel.org/patch/11595407/.
>>> - Switch register operations to DEVICE_LITTLE_ENDIAN throughout.
>>> - Machine documentation added (new patch).
>>>
>>> Changes since v1 to support flash booting:
>>>
>>> - GCR reset value changes to get past memory initialization when booting
>>> from flash (patches 2 and 5):
>>> - INTCR2 now indicates that the DDR controller is initialized.
>>> - INTCR3 is initialized according to DDR memory size. A realize()
>>> method was implemented to achieve this.
>>> - Refactor the machine initialization a bit to make it easier to drop in
>>> machine-specific flash initialization (patch 6).
>>> - Extend the series with additional patches to enable booting from flash:
>>> - Boot ROM (through the -bios option).
>>> - OTP (fuse) controller.
>>> - Memory Controller stub (just enough to skip memory training).
>>> - Flash controller.
>>> - Board-specific flash initialization.
>>>
>>> Thanks for reviewing,
>>>
>>> Havard
>>>
>>> Havard Skinnemoen (14):
>>> hw/misc: Add NPCM7xx System Global Control Registers device model
>>> hw/misc: Add NPCM7xx Clock Controller device model
>>> hw/timer: Add NPCM7xx Timer device model
>>> hw/arm: Add NPCM730 and NPCM750 SoC models
>>> hw/arm: Add two NPCM7xx-based machines
>>> roms: Add virtual Boot ROM for NPCM7xx SoCs
>>> hw/arm: Load -bios image as a boot ROM for npcm7xx
>>> hw/nvram: NPCM7xx OTP device model
>>> hw/mem: Stubbed out NPCM7xx Memory Controller model
>>> hw/ssi: NPCM7xx Flash Interface Unit device model
>>> hw/arm: Wire up BMC boot flash for npcm750-evb and quanta-gsj
>>> hw/arm/npcm7xx: add board setup stub for CPU and UART clocks
>>> docs/system: Add Nuvoton machine documentation
>>> tests/acceptance: console boot tests for quanta-gsj
>>>
>>> docs/system/arm/nuvoton.rst | 90 ++++
>>> docs/system/target-arm.rst | 1 +
>>> Makefile | 1 +
>>> default-configs/arm-softmmu.mak | 1 +
>>> include/hw/arm/npcm7xx.h | 112 +++++
>>> include/hw/mem/npcm7xx_mc.h | 36 ++
>>> include/hw/misc/npcm7xx_clk.h | 48 +++
>>> include/hw/misc/npcm7xx_gcr.h | 43 ++
>>> include/hw/nvram/npcm7xx_otp.h | 79 ++++
>>> include/hw/ssi/npcm7xx_fiu.h | 73 ++++
>>> include/hw/timer/npcm7xx_timer.h | 78 ++++
>>> hw/arm/npcm7xx.c | 532 +++++++++++++++++++++++
>>> hw/arm/npcm7xx_boards.c | 197 +++++++++
>>> hw/mem/npcm7xx_mc.c | 84 ++++
>>> hw/misc/npcm7xx_clk.c | 266 ++++++++++++
>>> hw/misc/npcm7xx_gcr.c | 269 ++++++++++++
>>> hw/nvram/npcm7xx_otp.c | 439 +++++++++++++++++++
>>> hw/ssi/npcm7xx_fiu.c | 572 +++++++++++++++++++++++++
>>> hw/timer/npcm7xx_timer.c | 509 ++++++++++++++++++++++
>>> .gitmodules | 3 +
>>> MAINTAINERS | 10 +
>>> hw/arm/Kconfig | 9 +
>>> hw/arm/meson.build | 1 +
>>> hw/mem/meson.build | 1 +
>>> hw/misc/meson.build | 4 +
>>> hw/misc/trace-events | 8 +
>>> hw/nvram/meson.build | 1 +
>>> hw/ssi/meson.build | 1 +
>>> hw/ssi/trace-events | 11 +
>>> hw/timer/meson.build | 1 +
>>> hw/timer/trace-events | 5 +
>>> pc-bios/README | 6 +
>>> pc-bios/npcm7xx_bootrom.bin | Bin 0 -> 768 bytes
>>> roms/Makefile | 7 +
>>> roms/vbootrom | 1 +
>>> tests/acceptance/boot_linux_console.py | 83 ++++
>>> 36 files changed, 3582 insertions(+)
>>> create mode 100644 docs/system/arm/nuvoton.rst
>>> create mode 100644 include/hw/arm/npcm7xx.h
>>> create mode 100644 include/hw/mem/npcm7xx_mc.h
>>> create mode 100644 include/hw/misc/npcm7xx_clk.h
>>> create mode 100644 include/hw/misc/npcm7xx_gcr.h
>>> create mode 100644 include/hw/nvram/npcm7xx_otp.h
>>> create mode 100644 include/hw/ssi/npcm7xx_fiu.h
>>> create mode 100644 include/hw/timer/npcm7xx_timer.h
>>> create mode 100644 hw/arm/npcm7xx.c
>>> create mode 100644 hw/arm/npcm7xx_boards.c
>>> create mode 100644 hw/mem/npcm7xx_mc.c
>>> create mode 100644 hw/misc/npcm7xx_clk.c
>>> create mode 100644 hw/misc/npcm7xx_gcr.c
>>> create mode 100644 hw/nvram/npcm7xx_otp.c
>>> create mode 100644 hw/ssi/npcm7xx_fiu.c
>>> create mode 100644 hw/timer/npcm7xx_timer.c
>>> create mode 100644 pc-bios/npcm7xx_bootrom.bin
>>> create mode 160000 roms/vbootrom
>>>
>>> --
>>> 2.28.0.297.g1956fa8f8d-goog
>>>
>>>
>>
>
