Re: [Qemu-devel] [PATCH v2 13/14] target-arm: Move reset handling to arm_cpu_reset

Re: [Qemu-devel] [PATCH v2 13/14] target-arm: Move reset handling to arm_cpu_reset

[Andreas Färber]

Am 14.04.2012 18:42, schrieb Peter Maydell:
> Now that cpu_reset_model_id() has gone we can move the
> reset code over to the class reset function and have cpu_state_reset
> simply do a reset on the CPU QOM object.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/cpu.c    |   94 +++++++++++++++++++++++++++++++++++++++++++++++--
>  target-arm/helper.c |   97 +--------------------------------------------------
>  2 files changed, 92 insertions(+), 99 deletions(-)

This forgot to update helper.c:cpu_arm_init() by calling cpu_reset().

I'll fix in my cross-target CPUState series.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v2 01/14] target-arm: Add QOM subclasses for each ARM cpu implementation

[Eduardo Habkost]

Sorry for replying to a patch 7 months later, but I just have a question
related to how we will handle CPU model classes on all targets:

On Sat, Apr 14, 2012 at 05:42:10PM +0100, Peter Maydell wrote:
> Register subclasses for each ARM CPU implementation (with the
> exception of "pxa270", which is an alias for "pxa270-a0").
> 
> Let arm_cpu_list() enumerate CPU subclasses in alphabetical order,
> except for special value "any".
> 
> Replace cpu_arm_find_by_name()'s string -> CPUID lookup by storing the
> CPUID (aka MIDR, Main ID Register) value in the class.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/cpu-qom.h |   12 +++
>  target-arm/cpu.c     |  226 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  target-arm/helper.c  |  109 ++++++++++--------------
>  3 files changed, 282 insertions(+), 65 deletions(-)
> 
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index 42d2a6b..a4bcb31 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -58,6 +58,18 @@ typedef struct ARMCPU {
[...]
> +typedef struct ARMCPUInfo {
> +    const char *name;
> +    void (*initfn)(Object *obj);
> +} ARMCPUInfo;
> +
> +static const ARMCPUInfo arm_cpus[] = {
[...]
> +    { .name = "any",         .initfn = arm_any_initfn },
> +};
> +

Do we really want to use "any" as the class name? Maybe we should use
"cpu-<model>" as the namespace for the CPU model class names? Or maybe
try "cpu-<model>" first, and then "<model>" as a fallback (making sure
that the class we found is a subclass of TYPE_<arch>_CPU).

I guess we will want address this before qdevifying the CPU class, as
the qdevification will make the CPU class names visible through the
monitor.


>  static void arm_cpu_class_init(ObjectClass *oc, void *data)
>  {
>      ARMCPUClass *acc = ARM_CPU_CLASS(oc);
> @@ -43,18 +248,37 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
>      cc->reset = arm_cpu_reset;
>  }
>  
> +static void cpu_register(const ARMCPUInfo *info)
> +{
> +    TypeInfo type_info = {
> +        .name = info->name,
> +        .parent = TYPE_ARM_CPU,
> +        .instance_size = sizeof(ARMCPU),
> +        .instance_init = info->initfn,
> +        .class_size = sizeof(ARMCPUClass),
> +    };
> +
> +    type_register_static(&type_info);
> +}
> +
>  static const TypeInfo arm_cpu_type_info = {
>      .name = TYPE_ARM_CPU,
>      .parent = TYPE_CPU,
>      .instance_size = sizeof(ARMCPU),
> -    .abstract = false,
> +    .instance_init = arm_cpu_initfn,
> +    .abstract = true,
>      .class_size = sizeof(ARMCPUClass),
>      .class_init = arm_cpu_class_init,
>  };
>  
>  static void arm_cpu_register_types(void)
>  {
> +    int i;
> +
>      type_register_static(&arm_cpu_type_info);
> +    for (i = 0; i < ARRAY_SIZE(arm_cpus); i++) {
> +        cpu_register(&arm_cpus[i]);
> +    }
>  }
>  
>  type_init(arm_cpu_register_types)

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 01/14] target-arm: Add QOM subclasses for each ARM cpu implementation

[Peter Maydell]

On 12 November 2012 22:16, Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> Sorry for replying to a patch 7 months later, but I just have a question
> related to how we will handle CPU model classes on all targets:
>
> On Sat, Apr 14, 2012 at 05:42:10PM +0100, Peter Maydell wrote:
>> Register subclasses for each ARM CPU implementation (with the
>> exception of "pxa270", which is an alias for "pxa270-a0").
>>
>> Let arm_cpu_list() enumerate CPU subclasses in alphabetical order,
>> except for special value "any".
>>
>> Replace cpu_arm_find_by_name()'s string -> CPUID lookup by storing the
>> CPUID (aka MIDR, Main ID Register) value in the class.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  target-arm/cpu-qom.h |   12 +++
>>  target-arm/cpu.c     |  226 +++++++++++++++++++++++++++++++++++++++++++++++++-
>>  target-arm/helper.c  |  109 ++++++++++--------------
>>  3 files changed, 282 insertions(+), 65 deletions(-)
>>
>> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
>> index 42d2a6b..a4bcb31 100644
>> --- a/target-arm/cpu-qom.h
>> +++ b/target-arm/cpu-qom.h
>> @@ -58,6 +58,18 @@ typedef struct ARMCPU {
> [...]
>> +typedef struct ARMCPUInfo {
>> +    const char *name;
>> +    void (*initfn)(Object *obj);
>> +} ARMCPUInfo;
>> +
>> +static const ARMCPUInfo arm_cpus[] = {
> [...]
>> +    { .name = "any",         .initfn = arm_any_initfn },
>> +};
>> +
>
> Do we really want to use "any" as the class name?

Probably not, since it would make it tricky to (in some future
utopia) have a QEMU which supported more than one CPU architecture
in the same binary if they all wanted to use "any"...

> Maybe we should use
> "cpu-<model>" as the namespace for the CPU model class names?

Sounds reasonable.

-- PMM

Re: [Qemu-devel] [PATCH v2 01/14] target-arm: Add QOM subclasses for each ARM cpu implementation

[Eduardo Habkost]

On Mon, Nov 12, 2012 at 10:18:29PM +0000, Peter Maydell wrote:
> On 12 November 2012 22:16, Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> > Sorry for replying to a patch 7 months later, but I just have a question
> > related to how we will handle CPU model classes on all targets:
> >
> > On Sat, Apr 14, 2012 at 05:42:10PM +0100, Peter Maydell wrote:
> >> Register subclasses for each ARM CPU implementation (with the
> >> exception of "pxa270", which is an alias for "pxa270-a0").
> >>
> >> Let arm_cpu_list() enumerate CPU subclasses in alphabetical order,
> >> except for special value "any".
> >>
> >> Replace cpu_arm_find_by_name()'s string -> CPUID lookup by storing the
> >> CPUID (aka MIDR, Main ID Register) value in the class.
> >>
> >> Signed-off-by: Andreas Färber <afaerber@suse.de>
> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> >> ---
> >>  target-arm/cpu-qom.h |   12 +++
> >>  target-arm/cpu.c     |  226 +++++++++++++++++++++++++++++++++++++++++++++++++-
> >>  target-arm/helper.c  |  109 ++++++++++--------------
> >>  3 files changed, 282 insertions(+), 65 deletions(-)
> >>
> >> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> >> index 42d2a6b..a4bcb31 100644
> >> --- a/target-arm/cpu-qom.h
> >> +++ b/target-arm/cpu-qom.h
> >> @@ -58,6 +58,18 @@ typedef struct ARMCPU {
> > [...]
> >> +typedef struct ARMCPUInfo {
> >> +    const char *name;
> >> +    void (*initfn)(Object *obj);
> >> +} ARMCPUInfo;
> >> +
> >> +static const ARMCPUInfo arm_cpus[] = {
> > [...]
> >> +    { .name = "any",         .initfn = arm_any_initfn },
> >> +};
> >> +
> >
> > Do we really want to use "any" as the class name?
> 
> Probably not, since it would make it tricky to (in some future
> utopia) have a QEMU which supported more than one CPU architecture
> in the same binary if they all wanted to use "any"...

In that case, "cpu-any" wouldn't work, either. What about
"<arch>-cpu-<model>"?

> 
> > Maybe we should use
> > "cpu-<model>" as the namespace for the CPU model class names?
> 
> Sounds reasonable.
> 
> -- PMM

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 01/14] target-arm: Add QOM subclasses for each ARM cpu implementation

[Andreas Färber]

Am 12.11.2012 23:33, schrieb Eduardo Habkost:
> On Mon, Nov 12, 2012 at 10:18:29PM +0000, Peter Maydell wrote:
>> On 12 November 2012 22:16, Eduardo Habkost <ehabkost@redhat.com> wrote:
>>> On Sat, Apr 14, 2012 at 05:42:10PM +0100, Peter Maydell wrote:
>>>> +static const ARMCPUInfo arm_cpus[] = {
>>> [...]
>>>> +    { .name = "any",         .initfn = arm_any_initfn },
>>>> +};
>>>> +
>>>
>>> Do we really want to use "any" as the class name?
>>
>> Probably not, since it would make it tricky to (in some future
>> utopia) have a QEMU which supported more than one CPU architecture
>> in the same binary if they all wanted to use "any"...
> 
> In that case, "cpu-any" wouldn't work, either. What about
> "<arch>-cpu-<model>"?

Fine with me. However, keep in mind the previous approach was used for
command line compatibility: I would like to continue using -cpu
cortex-a9 rather than -cpu arm-cpu-cortex-a9. :)

If we introduce a more complex command-line-to-class mapping, can't we
drop these ominous "any" CPUs altogether? For my understanding they were
used as wildcard CPUs for *-user. We could do the same by instantiating
a real CPU like "cortex-a15" and possibly enabling some additional
features afterwards.

Andreas

>>> Maybe we should use
>>> "cpu-<model>" as the namespace for the CPU model class names?
>>
>> Sounds reasonable.
>>
>> -- PMM

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v2 01/14] target-arm: Add QOM subclasses for each ARM cpu implementation

[Peter Maydell]

On 13 November 2012 11:51, Andreas Färber <afaerber@suse.de> wrote:
> Am 12.11.2012 23:33, schrieb Eduardo Habkost:
>> In that case, "cpu-any" wouldn't work, either. What about
>> "<arch>-cpu-<model>"?
>
> Fine with me. However, keep in mind the previous approach was used for
> command line compatibility: I would like to continue using -cpu
> cortex-a9 rather than -cpu arm-cpu-cortex-a9. :)

Yes, we need to maintain the command line names as-is.

> If we introduce a more complex command-line-to-class mapping, can't we
> drop these ominous "any" CPUs altogether? For my understanding they were
> used as wildcard CPUs for *-user. We could do the same by instantiating
> a real CPU like "cortex-a15" and possibly enabling some additional
> features afterwards.

I don't see what that gains us. The easiest way to say "it's
a cpu with all the feature bits turned on" is to define it as
a cpu with all the feature bits turned on :-)

-- PMM

Re: [Qemu-devel] [PATCH v2 01/14] target-arm: Add QOM subclasses for each ARM cpu implementation

[Eduardo Habkost]

On Tue, Nov 13, 2012 at 12:51:43PM +0100, Andreas Färber wrote:
> Am 12.11.2012 23:33, schrieb Eduardo Habkost:
> > On Mon, Nov 12, 2012 at 10:18:29PM +0000, Peter Maydell wrote:
> >> On 12 November 2012 22:16, Eduardo Habkost <ehabkost@redhat.com> wrote:
> >>> On Sat, Apr 14, 2012 at 05:42:10PM +0100, Peter Maydell wrote:
> >>>> +static const ARMCPUInfo arm_cpus[] = {
> >>> [...]
> >>>> +    { .name = "any",         .initfn = arm_any_initfn },
> >>>> +};
> >>>> +
> >>>
> >>> Do we really want to use "any" as the class name?
> >>
> >> Probably not, since it would make it tricky to (in some future
> >> utopia) have a QEMU which supported more than one CPU architecture
> >> in the same binary if they all wanted to use "any"...
> > 
> > In that case, "cpu-any" wouldn't work, either. What about
> > "<arch>-cpu-<model>"?
> 
> Fine with me. However, keep in mind the previous approach was used for
> command line compatibility: I would like to continue using -cpu
> cortex-a9 rather than -cpu arm-cpu-cortex-a9. :)

Me too. We need keep "-cpu SandyBridge" working, instead of requiring
"-cpu x86_64-cpu-SandyBridge".

"-cpu FOO" could first try a class named "<arch>-cpu-FOO" (for
compatibility), then fallback to "FOO".

> 
> If we introduce a more complex command-line-to-class mapping, can't we
> drop these ominous "any" CPUs altogether? For my understanding they were
> used as wildcard CPUs for *-user. We could do the same by instantiating
> a real CPU like "cortex-a15" and possibly enabling some additional
> features afterwards.

I didn't look so deeply at the code, to understand what "any" does, and
why it exists. But whatever we do, I suppose we need to keep "-cpu any"
working, for compatibility.

> 
> Andreas
> 
> >>> Maybe we should use
> >>> "cpu-<model>" as the namespace for the CPU model class names?
> >>
> >> Sounds reasonable.
> >>
> >> -- PMM
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 01/14] target-arm: Add QOM subclasses for each ARM cpu implementation

[Eduardo Habkost]

On Tue, Nov 13, 2012 at 12:17:00PM +0000, Peter Maydell wrote:
> On 13 November 2012 11:51, Andreas Färber <afaerber@suse.de> wrote:
> > Am 12.11.2012 23:33, schrieb Eduardo Habkost:
> >> In that case, "cpu-any" wouldn't work, either. What about
> >> "<arch>-cpu-<model>"?
> >
> > Fine with me. However, keep in mind the previous approach was used for
> > command line compatibility: I would like to continue using -cpu
> > cortex-a9 rather than -cpu arm-cpu-cortex-a9. :)
> 
> Yes, we need to maintain the command line names as-is.

ACK.

> 
> > If we introduce a more complex command-line-to-class mapping, can't we
> > drop these ominous "any" CPUs altogether? For my understanding they were
> > used as wildcard CPUs for *-user. We could do the same by instantiating
> > a real CPU like "cortex-a15" and possibly enabling some additional
> > features afterwards.
> 
> I don't see what that gains us. The easiest way to say "it's
> a cpu with all the feature bits turned on" is to define it as
> a cpu with all the feature bits turned on :-)

This sounds similar to "-cpu host" on x86. The difference is that on x86
with KVM, we depend on host capabilities to know which features can be
enabled (so on x86 it's not "a cpu with all the feature bits turned on",
but "a cpu with all the feature bits _we can safely enable_ turned on").

> 
> -- PMM

-- 
Eduardo