On Mon, Mar 9, 2020 at 12:18 PM Peter Maydell <peter.mayd...@linaro.org>
wrote:
> On Tue, 3 Mar 2020 at 20:15, Niek Linnenbank <nieklinnenb...@gmail.com>
> wrote:
> > On Tue, Mar 3, 2020 at 1:09 PM Alex Bennée <alex.ben...@linaro.org>
> wrote:
> >> Niek Linnenbank <nieklinnenb...@gmail.com> writes:
> >> > +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.
> >
> > 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.
>
> > 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?
>
> If the intended behaviour is "just power on the CPU into EL3
> in the aarch32/aarch64 state it would naturally have out of reset"
> then you can get the right value to pass to target_aa64 like this:
>
> ARMCPU *target_cpu = ARM_CPU(arm_get_cpu_by_id(cpu_id));
>
> if (!target_cpu) {
> /*
> * handle the case of "guest wrote bogus value to RST_CTRL
> * field, probably by returning doing nothing
> */
> }
> target_aa64 = arm_feature(&target_cpu->env, ARM_FEATURE_AARCH64);
>
>
Thanks for explaining Peter. This solution is working fine. I'm including
it in the v7 respin.
Regards,
Niek
> Or if the required behaviour for a 64-bit CPU is more complicated
> you can just assert or something with a comment that this would need
> updating if we ever modelled a 64-bit Allwinner SoC.
>
> thanks
> -- PMM
>
--
Niek Linnenbank
