[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v6 00/13] Add Nuvoton NPCM730/NPCM750 SoCs and two BMC machin
From: |
Cédric Le Goater |
Subject: |
Re: [PATCH v6 00/13] Add Nuvoton NPCM730/NPCM750 SoCs and two BMC machines |
Date: |
Fri, 17 Jul 2020 22:32:17 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 |
On 7/17/20 8:02 AM, Havard Skinnemoen wrote:
> I also pushed this and the previous two patchsets to my qemu fork on github.
> The branches are named npcm7xx-v[1-6].
>
> 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.
I think this series is ready to go.
Patch 6 "roms: Add virtual Boot ROM for NPCM7xx SoCs" needs a few
add-ons which can come later.
Patch 13 "tests/acceptance: console boot tests for quanta-gsj" needs
an Acked-by by our test gurus. Aspeed needs the same kind of tests
if possible. It depends on the pressure that the QEMU CI will put on
the web servers hosting the images.
I think that the rest of the comments are minor and can be fixed
as follow-ups when 5.2 is being assembled.
Thanks,
C.
> 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 (13):
> 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
> 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 | 111 +++++
> include/hw/mem/npcm7xx_mc.h | 36 ++
> include/hw/misc/npcm7xx_clk.h | 72 ++++
> include/hw/misc/npcm7xx_gcr.h | 76 ++++
> include/hw/nvram/npcm7xx_otp.h | 88 ++++
> include/hw/ssi/npcm7xx_fiu.h | 100 +++++
> include/hw/timer/npcm7xx_timer.h | 96 +++++
> hw/arm/npcm7xx.c | 500 +++++++++++++++++++++++
> hw/arm/npcm7xx_boards.c | 195 +++++++++
> hw/mem/npcm7xx_mc.c | 84 ++++
> hw/misc/npcm7xx_clk.c | 234 +++++++++++
> hw/misc/npcm7xx_gcr.c | 227 +++++++++++
> hw/nvram/npcm7xx_otp.c | 424 +++++++++++++++++++
> hw/ssi/npcm7xx_fiu.c | 539 +++++++++++++++++++++++++
> hw/timer/npcm7xx_timer.c | 489 ++++++++++++++++++++++
> .gitmodules | 3 +
> MAINTAINERS | 8 +
> hw/arm/Kconfig | 9 +
> hw/arm/Makefile.objs | 1 +
> hw/mem/Makefile.objs | 1 +
> hw/misc/Makefile.objs | 2 +
> hw/misc/trace-events | 8 +
> hw/nvram/Makefile.objs | 1 +
> hw/ssi/Makefile.objs | 1 +
> hw/ssi/trace-events | 11 +
> hw/timer/Makefile.objs | 1 +
> hw/timer/trace-events | 5 +
> pc-bios/npcm7xx_bootrom.bin | Bin 0 -> 768 bytes
> roms/Makefile | 7 +
> roms/vbootrom | 1 +
> tests/acceptance/boot_linux_console.py | 65 +++
> 35 files changed, 3488 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
>
- Re: [PATCH v6 07/13] hw/arm: Load -bios image as a boot ROM for npcm7xx, (continued)
- [PATCH v6 08/13] hw/nvram: NPCM7xx OTP device model, Havard Skinnemoen, 2020/07/17
- [PATCH v6 10/13] hw/ssi: NPCM7xx Flash Interface Unit device model, Havard Skinnemoen, 2020/07/17
- [PATCH v6 11/13] hw/arm: Wire up BMC boot flash for npcm750-evb and quanta-gsj, Havard Skinnemoen, 2020/07/17
- [PATCH v6 09/13] hw/mem: Stubbed out NPCM7xx Memory Controller model, Havard Skinnemoen, 2020/07/17
- [PATCH v6 12/13] docs/system: Add Nuvoton machine documentation, Havard Skinnemoen, 2020/07/17
- [PATCH v6 13/13] tests/acceptance: console boot tests for quanta-gsj, Havard Skinnemoen, 2020/07/17
- Re: [PATCH v6 00/13] Add Nuvoton NPCM730/NPCM750 SoCs and two BMC machines,
Cédric Le Goater <=