Hi Alex,
First thanks for all your reviews, I'll add the tags in the next version of
this series.
On Tue, Mar 3, 2020 at 1:09 PM Alex Bennée <alex.ben...@linaro.org> wrote:
>
> Niek Linnenbank <nieklinnenb...@gmail.com> writes:
>
> > Various Allwinner System on Chip designs contain multiple processors
> > that can be configured and reset using the generic CPU Configuration
> > module interface. This commit adds support for the Allwinner CPU
> > configuration interface which emulates the following features:
> >
> > * CPU reset
> > * CPU status
> >
> > Signed-off-by: Niek Linnenbank <nieklinnenb...@gmail.com>
> <snip>
> > +
> > +/* CPUCFG constants */
> > +enum {
> > + CPU_EXCEPTION_LEVEL_ON_RESET = 3, /* EL3 */
> > +};
> > +
> > +static void allwinner_cpucfg_cpu_reset(AwCpuCfgState *s, uint8_t cpu_id)
> > +{
> > + int ret;
> > +
> > + trace_allwinner_cpucfg_cpu_reset(cpu_id, s->entry_addr);
> > +
> > + ret = arm_set_cpu_on(cpu_id, s->entry_addr, 0,
> > + CPU_EXCEPTION_LEVEL_ON_RESET, false);
>
> According to the arm_set_cpu_on code:
>
> if (!target_aa64 && arm_feature(&target_cpu->env,
> ARM_FEATURE_AARCH64)) {
> /*
> * For now we don't support booting an AArch64 CPU in AArch32 mode
> * TODO: We should add this support later
> */
> qemu_log_mask(LOG_UNIMP,
> "[ARM]%s: Starting AArch64 CPU %" PRId64
> " in AArch32 mode is not supported yet\n",
> __func__, cpuid);
> return QEMU_ARM_POWERCTL_INVALID_PARAM;
> }
>
> Do you really want to reboot in aarch32 mode on a reset? If so we should
> fix the TODO.
>
Very good remark, I'll try to clarify what I did here. In earlier version
of this series, I made this CPU Configuration Module specific
to the Allwinner H3 SoC, as I thought it was a SoC specific component.
Later I discovered that the CPU Configuration Module is
actually a generic Allwinner component and used in multiple SoCs. Taking
that into account, I renamed it to allwinner-cpucfg.c and
updated the comments/docs. That way it should be re-usable in future SoC
modules.
The Allwinner H3 has four ARM Cortex-A7 cores and are 32-bit, so in the
early version I filled the @target_aa64 parameter with false to
keep it in 32-bit mode. And for usage in the current Allwinner H3 SoC that
is sufficient, but as soon as a potential new SoC implementation
with a 64-bit CPU starts to use this module, the hardcoded @target_aa64
parameter will become a problem.
And the TODO inside the arm_set_cpu_on() function I was not yet aware of as
well, so that will also need to be resolved
indeed once this module is going to be used for a new SoC with a 64-bit CPU.
Maybe I should just use some function to check if the current emulated CPU
its 32-bit or 64-bit and use that for @target_aa64?
That way the CPU Configuration Module itself should be ready for 64-bit and
the TODO for arm_set_cpu_on() can be
resolved in a later series for newer SoCs.
Regards,
Niek
>
> --
> Alex Bennée
>
--
Niek Linnenbank
