Re: [Qemu-devel] [RFC 0/3] generalize parsing of cpu_model

[Peter Maydell]

On 20 February 2017 at 18:55, Igor Mammedov <address@hidden> wrote:
> Peter Maydell <address@hidden> wrote:
>> This API seems a little awkward for the SoC case, where
>> the board model doesn't actually know what the default
>> CPU model or the valid CPU models are, because it just
>> wants to say "create me a BCM2836 SoC" and let the SoC
>> object deal with determining whether it's always a cortex-a15
>> or if it might allow some user configurability either in
>> cpu choices or in optional flags.
>> Any thoughts about that use case?
>
> I've probably been too aggressive trying to force
> conversion of all boards, assuming that all boards support
> "-cpu CLI" option, currently option could be specified for
> any board but it is ignored if board doesn't care
> about it explicitly.

I think the conversion in this patchset is OK for
the boards you've touched. It's the ones that it
doesn't touch that I need to think a little more about.

>  "-cpu cpu_name,feat_str" handling is always based on
>  availability of base CPU type supported by target as
>  it's needed both for
>    - translating cpu_name to QOM CPU type
>        cpu_class_by_name(typename, name)
>    - converting feat_str into set of global properties
>      for matching QOM CPU type
>        cc->parse_features(object_class_get_name(oc), featurestr, &err);

> Boards that don't care/need '-cpu' support won't need
> to do anything as we can skip cpu_name,feat_str parsing
> if MachineClass::base_cpu_type isn't set (NULL by default),
> that way we won't break ignored CLI if we care.
> And currently we don't have convenient way to disable
> feat_str parsing, but machine could be extended
> with flag to [en|dis]able it explicitly.
>
> The goal of this series is generalizing/consolidating
> parsing of '-cpu' for boards that use it, removing
> code duplication scattered around codebase and trying
> normalize default cpu_model initialization.

Yeah, I definitely like the cleanup to avoid all the
duplicate parse calls.

> For SoC with fixed CPU type it would be better to use
> directly CPU type names directly but that hits legacy
> cpu_init()/cpu_foo_init() wall, which is currently
> cpu_model based and would be a lot of re-factoring.
> I would convert cpu_init(cpu_model) to cpu_init(cpu_type)
> and call '-cpu' handling logic from generic
> machine/user_[bsd|linux] code (3 places in total).
>
>> (The stm32f205 SoC object has a "cpu-model" QOM property
>> that the board sets, but I think that's as much because
>> we somewhat awkwardly need to pass it into armv7m_init()
>> as a deliberate design choice.)

> its sole user netduino2 board has cpu_model hardcodded
> at board level which could be left alone or converted
> to this API.

I think this may be for the benefit of the not-yet-in-tree
netduinoplus2 board, which is a very similar SoC but
with a cortex-m4. There are likely other ways we could
model that, though.

> Using API would make code more consistent
> at the cost of more code for default/valid callbacks.

Mmm, we could have the board code just forward the
default/valid callbacks through to the SoC (which
would then either implement them or forward them again).
But I think we should definitely see if we can make
the code for handling "the CPU is actually inside some
other object" as clean as we can, because I think
that's by far the most common case. Machines like
the x86 pc and the ARM virt board where the CPU is
instantiated directly in the board code are the
non-standard cases I think. We have a lot of boards
where we do instantiate the CPU in the board code
just for legacy reasons where we don't properly
model the SoC the CPU is inside. But hardware that
really has the CPU as a top level pluggable object
is rare.

thanks
-- PMM