[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v6 06/18] hw/arm/allwinner: add CPU Configuration module

From: Niek Linnenbank
Subject: Re: [PATCH v6 06/18] hw/arm/allwinner: add CPU Configuration module
Date: Tue, 3 Mar 2020 21:15:16 +0100

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 <address@hidden> wrote:

Niek Linnenbank <address@hidden> 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 <address@hidden>
> +
> +/* CPUCFG constants */
> +enum {
> +};
> +
> +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
                      "[ARM]%s: Starting AArch64 CPU %" PRId64
                      " in AArch32 mode is not supported yet\n",
                      __func__, cpuid);

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.



Alex Bennée

Niek Linnenbank

reply via email to

[Prev in Thread] Current Thread [Next in Thread]