[Qemu-devel] [PATCH v3 0/4] target-openrisc hw/openrisc: Some OpenRISC fix.

[Qemu-devel] [PATCH v3 0/4] target-openrisc hw/openrisc: Some OpenRISC fix.

[Jia Liu]

Fix OpenRISC CPU and sim broad, we should free typename and check cpu models
by name in cpu.c, and we should use stderr output in openrisc_sim.c.

Updates from V3:
* Replace a typo fix using a stderr fix.

Updates from V2:
* Combine typo patchset and typename free.
* Add cpu_model by name fix, make any and or1200 works both OK.

Jia Liu (4):
  hw/openrisc: Indent typo
  hw/openrisc: Use stderr output instead of qemu_log
  target-openrisc: Free typename
  target-openrisc: Fix cpu_model by name

 hw/openrisc/openrisc_sim.c |  6 +++---
 target-openrisc/cpu.c      | 17 +++++++++++++----
 2 files changed, 16 insertions(+), 7 deletions(-)

-- 
1.7.12.4 (Apple Git-37)

[Qemu-devel] [PATCH v3 1/4] hw/openrisc: Indent typo

[Jia Liu]

Indent typo.

Signed-off-by: Jia Liu <proljc@gmail.com>
---
 hw/openrisc/openrisc_sim.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index 924438b..250f5b5 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -96,7 +96,7 @@ static void openrisc_sim_init(QEMUMachineInitArgs *args)
     ram_addr_t ram_size = args->ram_size;
     const char *cpu_model = args->cpu_model;
     const char *kernel_filename = args->kernel_filename;
-   OpenRISCCPU *cpu = NULL;
+    OpenRISCCPU *cpu = NULL;
     MemoryRegion *ram;
     int n;
 
-- 
1.7.12.4 (Apple Git-37)

Re: [Qemu-devel] [PATCH v3 1/4] hw/openrisc: Indent typo

[Peter Maydell]

On 22 July 2013 09:56, Jia Liu <proljc@gmail.com> wrote:
> Indent typo.
>
> Signed-off-by: Jia Liu <proljc@gmail.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

[Qemu-devel] [PATCH v3 2/4] hw/openrisc: Use stderr output instead of qemu_log

[Jia Liu]

We should use stderr output instead of qemu_log in order to output ErrMsg
onto the screen.

Signed-off-by: Jia Liu <proljc@gmail.com>
---
 hw/openrisc/openrisc_sim.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index 250f5b5..a08f27c 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -82,7 +82,7 @@ static void cpu_openrisc_load_kernel(ram_addr_t ram_size,
         }
 
         if (kernel_size < 0) {
-            qemu_log("QEMU: couldn't load the kernel '%s'\n",
+            fprintf(stderr, "QEMU: couldn't load the kernel '%s'\n",
                     kernel_filename);
             exit(1);
         }
@@ -107,7 +107,7 @@ static void openrisc_sim_init(QEMUMachineInitArgs *args)
     for (n = 0; n < smp_cpus; n++) {
         cpu = cpu_openrisc_init(cpu_model);
         if (cpu == NULL) {
-            qemu_log("Unable to find CPU definition!\n");
+            fprintf(stderr, "Unable to find CPU definition!\n");
             exit(1);
         }
         qemu_register_reset(main_cpu_reset, cpu);
-- 
1.7.12.4 (Apple Git-37)

Re: [Qemu-devel] [PATCH v3 2/4] hw/openrisc: Use stderr output instead of qemu_log

[Peter Maydell]

On 22 July 2013 09:56, Jia Liu <proljc@gmail.com> wrote:
> We should use stderr output instead of qemu_log in order to output ErrMsg
> onto the screen.
>
> Signed-off-by: Jia Liu <proljc@gmail.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

[Qemu-devel] [PATCH v3 3/4] target-openrisc: Free typename

[Jia Liu]

We should free typename here.

Signed-off-by: Jia Liu <proljc@gmail.com>
---
 target-openrisc/cpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
index 6d40f1b..e348df0 100644
--- a/target-openrisc/cpu.c
+++ b/target-openrisc/cpu.c
@@ -99,6 +99,7 @@ static ObjectClass *openrisc_cpu_class_by_name(const char *cpu_model)
 
     typename = g_strdup_printf("%s-" TYPE_OPENRISC_CPU, cpu_model);
     oc = object_class_by_name(typename);
+    g_free(typename);
     if (oc != NULL && (!object_class_dynamic_cast(oc, TYPE_OPENRISC_CPU) ||
                        object_class_is_abstract(oc))) {
         return NULL;
-- 
1.7.12.4 (Apple Git-37)

Re: [Qemu-devel] [PATCH v3 3/4] target-openrisc: Free typename

[Jia Liu]

Hi Andreas,

On Mon, Jul 22, 2013 at 4:56 PM, Jia Liu <proljc@gmail.com> wrote:
> We should free typename here.
>
> Signed-off-by: Jia Liu <proljc@gmail.com>

Sorry I didn't know I need add Signed-off-by: Andreas Färber <your@mail> here.


> ---
>  target-openrisc/cpu.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
> index 6d40f1b..e348df0 100644
> --- a/target-openrisc/cpu.c
> +++ b/target-openrisc/cpu.c
> @@ -99,6 +99,7 @@ static ObjectClass *openrisc_cpu_class_by_name(const char *cpu_model)
>
>      typename = g_strdup_printf("%s-" TYPE_OPENRISC_CPU, cpu_model);
>      oc = object_class_by_name(typename);
> +    g_free(typename);
>      if (oc != NULL && (!object_class_dynamic_cast(oc, TYPE_OPENRISC_CPU) ||
>                         object_class_is_abstract(oc))) {
>          return NULL;
> --
> 1.7.12.4 (Apple Git-37)
>

Regards,
Jia.

Re: [Qemu-devel] [PATCH v3 3/4] target-openrisc: Free typename

[Peter Maydell]

On 22 July 2013 10:37, Jia Liu <proljc@gmail.com> wrote:
> Hi Andreas,
>
> On Mon, Jul 22, 2013 at 4:56 PM, Jia Liu <proljc@gmail.com> wrote:
>> We should free typename here.
>>
>> Signed-off-by: Jia Liu <proljc@gmail.com>
>
> Sorry I didn't know I need add Signed-off-by: Andreas Färber <your@mail> here.

Not Signed-off-by, Reviewed-by. The distinction is
important.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 3/4] target-openrisc: Free typename

[Jia Liu]

On Mon, Jul 22, 2013 at 5:41 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 22 July 2013 10:37, Jia Liu <proljc@gmail.com> wrote:
>> Hi Andreas,
>>
>> On Mon, Jul 22, 2013 at 4:56 PM, Jia Liu <proljc@gmail.com> wrote:
>>> We should free typename here.
>>>
>>> Signed-off-by: Jia Liu <proljc@gmail.com>
>>
>> Sorry I didn't know I need add Signed-off-by: Andreas Färber <your@mail> here.
>
> Not Signed-off-by, Reviewed-by. The distinction is
> important.

Thanks Peter, I got.

>
> thanks
> -- PMM

Regards,
Jia

[Qemu-devel] [PATCH v3 4/4] target-openrisc: Fix cpu_model by name

[Jia Liu]

Fix cpu_model by name, make any and or1200 works both OK.

Signed-off-by: Jia Liu <proljc@gmail.com>
---
 target-openrisc/cpu.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
index e348df0..6637166 100644
--- a/target-openrisc/cpu.c
+++ b/target-openrisc/cpu.c
@@ -100,11 +100,19 @@ static ObjectClass *openrisc_cpu_class_by_name(const char *cpu_model)
     typename = g_strdup_printf("%s-" TYPE_OPENRISC_CPU, cpu_model);
     oc = object_class_by_name(typename);
     g_free(typename);
-    if (oc != NULL && (!object_class_dynamic_cast(oc, TYPE_OPENRISC_CPU) ||
-                       object_class_is_abstract(oc))) {
-        return NULL;
+
+    if (oc != NULL && object_class_dynamic_cast(oc, TYPE_OPENRISC_CPU) != NULL
+        && !object_class_is_abstract(oc)) {
+        return oc;
     }
-    return oc;
+
+    oc = object_class_by_name(cpu_model);
+    if (oc != NULL && object_class_dynamic_cast(oc, TYPE_OPENRISC_CPU) != NULL
+        && !object_class_is_abstract(oc)) {
+        return oc;
+    }
+
+    return NULL;
 }
 
 static void or1200_initfn(Object *obj)
-- 
1.7.12.4 (Apple Git-37)

Re: [Qemu-devel] [PATCH v3 4/4] target-openrisc: Fix cpu_model by name

[Peter Maydell]

On 22 July 2013 09:56, Jia Liu <proljc@gmail.com> wrote:
> Fix cpu_model by name, make any and or1200 works both OK.
>
> Signed-off-by: Jia Liu <proljc@gmail.com>
> ---
>  target-openrisc/cpu.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
> index e348df0..6637166 100644
> --- a/target-openrisc/cpu.c
> +++ b/target-openrisc/cpu.c
> @@ -100,11 +100,19 @@ static ObjectClass *openrisc_cpu_class_by_name(const char *cpu_model)
>      typename = g_strdup_printf("%s-" TYPE_OPENRISC_CPU, cpu_model);
>      oc = object_class_by_name(typename);
>      g_free(typename);
> -    if (oc != NULL && (!object_class_dynamic_cast(oc, TYPE_OPENRISC_CPU) ||
> -                       object_class_is_abstract(oc))) {
> -        return NULL;
> +
> +    if (oc != NULL && object_class_dynamic_cast(oc, TYPE_OPENRISC_CPU) != NULL
> +        && !object_class_is_abstract(oc)) {
> +        return oc;
>      }
> -    return oc;
> +
> +    oc = object_class_by_name(cpu_model);
> +    if (oc != NULL && object_class_dynamic_cast(oc, TYPE_OPENRISC_CPU) != NULL
> +        && !object_class_is_abstract(oc)) {
> +        return oc;
> +    }
> +
> +    return NULL;
>  }

This looks a bit odd. The commit message suggests it's an
attempt to fix the bug that was fixed by commit 071b3364e --
is it really needed? Andreas?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 4/4] target-openrisc: Fix cpu_model by name

[Jia Liu]

Hi Andreas and Peter,

On Mon, Jul 22, 2013 at 5:29 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 22 July 2013 09:56, Jia Liu <proljc@gmail.com> wrote:
>> Fix cpu_model by name, make any and or1200 works both OK.
>>
>> Signed-off-by: Jia Liu <proljc@gmail.com>
>> ---
>>  target-openrisc/cpu.c | 16 ++++++++++++----
>>  1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
>> index e348df0..6637166 100644
>> --- a/target-openrisc/cpu.c
>> +++ b/target-openrisc/cpu.c
>> @@ -100,11 +100,19 @@ static ObjectClass *openrisc_cpu_class_by_name(const char *cpu_model)
>>      typename = g_strdup_printf("%s-" TYPE_OPENRISC_CPU, cpu_model);
>>      oc = object_class_by_name(typename);
>>      g_free(typename);
>> -    if (oc != NULL && (!object_class_dynamic_cast(oc, TYPE_OPENRISC_CPU) ||
>> -                       object_class_is_abstract(oc))) {
>> -        return NULL;
>> +
>> +    if (oc != NULL && object_class_dynamic_cast(oc, TYPE_OPENRISC_CPU) != NULL
>> +        && !object_class_is_abstract(oc)) {
>> +        return oc;
>>      }
>> -    return oc;
>> +
>> +    oc = object_class_by_name(cpu_model);
>> +    if (oc != NULL && object_class_dynamic_cast(oc, TYPE_OPENRISC_CPU) != NULL
>> +        && !object_class_is_abstract(oc)) {
>> +        return oc;
>> +    }
>> +
>> +    return NULL;
>>  }
>
> This looks a bit odd. The commit message suggests it's an
> attempt to fix the bug that was fixed by commit 071b3364e --
> is it really needed? Andreas?

commit 071b3364e fix the typename only, but this one make "-cpu any"
"-cpu or1200" "-cpu any-or32-cpu" "-cpu or1200-or32-cpu" working fine.

>
> thanks
> -- PMM

Regards,
Jia

Re: [Qemu-devel] [PATCH v3 4/4] target-openrisc: Fix cpu_model by name

[Peter Maydell]

On 22 July 2013 10:42, Jia Liu <proljc@gmail.com> wrote:
> Hi Andreas and Peter,
>
> On Mon, Jul 22, 2013 at 5:29 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> This looks a bit odd. The commit message suggests it's an
>> attempt to fix the bug that was fixed by commit 071b3364e --
>> is it really needed? Andreas?
>
> commit 071b3364e fix the typename only, but this one make "-cpu any"
> "-cpu or1200" "-cpu any-or32-cpu" "-cpu or1200-or32-cpu" working fine.

The last two of these would be wrong: -cpu should only
accept CPU names, ie "any" or "or1200".

-- PMM

Re: [Qemu-devel] [PATCH v3 4/4] target-openrisc: Fix cpu_model by name

[Jia Liu]

On Mon, Jul 22, 2013 at 5:43 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 22 July 2013 10:42, Jia Liu <proljc@gmail.com> wrote:
>> Hi Andreas and Peter,
>>
>> On Mon, Jul 22, 2013 at 5:29 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> This looks a bit odd. The commit message suggests it's an
>>> attempt to fix the bug that was fixed by commit 071b3364e --
>>> is it really needed? Andreas?
>>
>> commit 071b3364e fix the typename only, but this one make "-cpu any"
>> "-cpu or1200" "-cpu any-or32-cpu" "-cpu or1200-or32-cpu" working fine.
>
> The last two of these would be wrong: -cpu should only
> accept CPU names, ie "any" or "or1200".

So, I need make only "-cpu any" and "-cpu or1200" accepted, thanks.

>
> -- PMM

Regards,
Jia

Re: [Qemu-devel] [PATCH v3 4/4] target-openrisc: Fix cpu_model by name

[Jia Liu]

On Mon, Jul 22, 2013 at 5:43 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 22 July 2013 10:42, Jia Liu <proljc@gmail.com> wrote:
>> Hi Andreas and Peter,
>>
>> On Mon, Jul 22, 2013 at 5:29 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> This looks a bit odd. The commit message suggests it's an
>>> attempt to fix the bug that was fixed by commit 071b3364e --
>>> is it really needed? Andreas?

Yes! you are right, it is not needed. It was my mistake, I didn't
understand Andreas.
Thanks!

It seems I need resend a v4 with Reviewed-by Andreas&Peter.

>>
>> commit 071b3364e fix the typename only, but this one make "-cpu any"
>> "-cpu or1200" "-cpu any-or32-cpu" "-cpu or1200-or32-cpu" working fine.
>
> The last two of these would be wrong: -cpu should only
> accept CPU names, ie "any" or "or1200".
>
> -- PMM

Regards,
Jia.

Re: [Qemu-devel] [PATCH v3 4/4] target-openrisc: Fix cpu_model by name

[Andreas Färber]

Am 22.07.2013 11:43, schrieb Peter Maydell:
> On 22 July 2013 10:42, Jia Liu <proljc@gmail.com> wrote:
>> Hi Andreas and Peter,
>>
>> On Mon, Jul 22, 2013 at 5:29 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> This looks a bit odd. The commit message suggests it's an
>>> attempt to fix the bug that was fixed by commit 071b3364e --
>>> is it really needed? Andreas?
>>
>> commit 071b3364e fix the typename only, but this one make "-cpu any"
>> "-cpu or1200" "-cpu any-or32-cpu" "-cpu or1200-or32-cpu" working fine.
> 
> The last two of these would be wrong: -cpu should only
> accept CPU names, ie "any" or "or1200".

Peter, the story is that due to an oversight on my part, it was possible
to use -cpu or1200-or32-cpu only for some weeks or months. This has been
fixed so that only -cpu or1200 works.

It is up to Jia whether he sees a need to have both work or whether -cpu
or1200 would be sufficient. In any case, I was not CC'ed on this series,
so I only came across one patch by accident during a search for
something else. ;)

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 v3 4/4] target-openrisc: Fix cpu_model by name

[Peter Maydell]

On 22 July 2013 11:37, Andreas Färber <afaerber@suse.de> wrote:
> Am 22.07.2013 11:43, schrieb Peter Maydell:
>>> commit 071b3364e fix the typename only, but this one make "-cpu any"
>>> "-cpu or1200" "-cpu any-or32-cpu" "-cpu or1200-or32-cpu" working fine.
>>
>> The last two of these would be wrong: -cpu should only
>> accept CPU names, ie "any" or "or1200".
>
> Peter, the story is that due to an oversight on my part, it was possible
> to use -cpu or1200-or32-cpu only for some weeks or months. This has been
> fixed so that only -cpu or1200 works.

Yep; my view is that the acceptance of the -or32-cpu varieties
was a bug which we have now fixed. In any case we should be
consistent across target architectures about what we allow.
I'm happy with the approach Jia has taken of just dropping
this patch in v4 of this series.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 4/4] target-openrisc: Fix cpu_model by name

[Andreas Färber]

Am 22.07.2013 12:40, schrieb Peter Maydell:
> On 22 July 2013 11:37, Andreas Färber <afaerber@suse.de> wrote:
>> Am 22.07.2013 11:43, schrieb Peter Maydell:
>>>> commit 071b3364e fix the typename only, but this one make "-cpu any"
>>>> "-cpu or1200" "-cpu any-or32-cpu" "-cpu or1200-or32-cpu" working fine.
>>>
>>> The last two of these would be wrong: -cpu should only
>>> accept CPU names, ie "any" or "or1200".
>>
>> Peter, the story is that due to an oversight on my part, it was possible
>> to use -cpu or1200-or32-cpu only for some weeks or months. This has been
>> fixed so that only -cpu or1200 works.
> 
> Yep; my view is that the acceptance of the -or32-cpu varieties
> was a bug which we have now fixed. In any case we should be
> consistent across target architectures about what we allow.
> I'm happy with the approach Jia has taken of just dropping
> this patch in v4 of this series.

alpha allows both, but -cpu ? just gave me a segfault. :/
It didn't have -cpu ? before QOM, so we decided to print the type names
there. Will need to investigate what's going wrong currently.

Stripping -alpha-cpu off typenames would surely be possible.

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 v3 4/4] target-openrisc: Fix cpu_model by name

[Peter Maydell]

On 22 July 2013 12:17, Andreas Färber <afaerber@suse.de> wrote:
> Am 22.07.2013 12:40, schrieb Peter Maydell:
>> Yep; my view is that the acceptance of the -or32-cpu varieties
>> was a bug which we have now fixed. In any case we should be
>> consistent across target architectures about what we allow.
>> I'm happy with the approach Jia has taken of just dropping
>> this patch in v4 of this series.
>
> alpha allows both, but -cpu ? just gave me a segfault. :/

Odd; that works for me:
cam-vm-266:precise:qemu$ ./build/x86-all/alpha-softmmu/qemu-system-alpha -cpu ?
Available CPUs:
  ev4-alpha-cpu
  ev5-alpha-cpu
  ev56-alpha-cpu
  ev6-alpha-cpu
  ev67-alpha-cpu
  ev68-alpha-cpu
  pca56-alpha-cpu

> It didn't have -cpu ? before QOM, so we decided to print the type names
> there.

Looking at all of the '-cpu help' output, alpha seems to be
the odd one out here: none of the others list valid CPUs
with "-$arch-cpu" suffixes.

> Stripping -alpha-cpu off typenames would surely be possible.

I think that that would be better in the name of consistency.

Also regarding consistency, not all targets react very well
to being asked for a nonexistent cpu via "-cpu xyzzy":
 alpha and s390x just plough on without an error
 lm32 and unicore32 segfault

(some of this may be default board model bugs rather than
target-* bugs).

-- PMM

Re: [Qemu-devel] -cpu behavior (was: [PATCH v3 4/4] target-openrisc: Fix cpu_model by name)

[Andreas Färber]

Am 22.07.2013 13:34, schrieb Peter Maydell:
> On 22 July 2013 12:17, Andreas Färber <afaerber@suse.de> wrote:
>> Am 22.07.2013 12:40, schrieb Peter Maydell:
>>> In any case we should be
>>> consistent across target architectures about what we allow.
>>
>> alpha allows both, but -cpu ? just gave me a segfault. :/
> 
> Odd; that works for me:
> cam-vm-266:precise:qemu$ ./build/x86-all/alpha-softmmu/qemu-system-alpha -cpu ?
> Available CPUs:
>   ev4-alpha-cpu
>   ev5-alpha-cpu
>   ev56-alpha-cpu
>   ev6-alpha-cpu
>   ev67-alpha-cpu
>   ev68-alpha-cpu
>   pca56-alpha-cpu

For -cpu ? I got

Program received signal SIGSEGV, Segmentation fault.
0x00005555557323fa in object_class_dynamic_cast (class=0x55555606df20,
    typename=typename@entry=0x55555582b82e "virtio-scsi-common")
    at /home/andreas/QEMU/qemu/qom/object.c:496
496	    if (type->class->interfaces &&

but that might be related to local QOM patches... :/ confirmed, qom-next
is working fine.

For -cpu ev67-alpha-cpu I got:

0x00005555557af890 in access_with_adjusted_size (addr=addr@entry=1021,
    value=value@entry=0x7fffe73bb5e0, size=size@entry=1, access_size_min=1,
    access_size_max=4, access=access@entry=
    0x5555557afb80 <memory_region_read_accessor>, opaque=opaque@entry=
    0x5555560bd3b0) at /home/andreas/QEMU/qemu/memory.c:429
429	    access_size = MAX(MIN(size, access_size_max), access_size_min);

Seems unrelated and is no longer reproducible either. Hmm.

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 v3 4/4] target-openrisc: Fix cpu_model by name

[Andreas Färber]

Am 22.07.2013 13:34, schrieb Peter Maydell:
> On 22 July 2013 12:17, Andreas Färber <afaerber@suse.de> wrote:
>> Am 22.07.2013 12:40, schrieb Peter Maydell:
>>> In any case we should be
>>> consistent across target architectures about what we allow.
>>
>> alpha allows both
[...]
>> It didn't have -cpu ? before QOM, so we decided to print the type names
>> there.
> 
> Looking at all of the '-cpu help' output, alpha seems to be
> the odd one out here: none of the others list valid CPUs
> with "-$arch-cpu" suffixes.

Right, because all others had implemented -cpu ? before we introduced
that naming scheme and I tried to keep output compatibility for them.
Focus for alpha was therefore on -cpu foo compatibility only.

Anthony had clearly stated on a KVM call that using full type names for
future CPU hot-add was the right thing to do and possibly even composite
convenience types like 4core-xeonblabla-x86_64-cpu; how that relates to
-cpu and new targets was never clearly defined though. ;)

For VMSD we decided to deviate for new migratable targets from legacy
CPUs in favor of consistency with devices, for instance.

>> Stripping -alpha-cpu off typenames would surely be possible.
> 
> I think that that would be better in the name of consistency.

> Also regarding consistency, not all targets react very well
> to being asked for a nonexistent cpu via "-cpu xyzzy":
>  alpha and s390x just plough on without an error

s390x does not have models yet. This will get fixed once we have agreed
on model names and their implementation.

>  lm32 and unicore32 segfault
> 
> (some of this may be default board model bugs rather than
> target-* bugs).

Yeah, for one sh4 board where I noticed it while refactoring I already
applied an error check. I guess cpu_init() / cpu_*_init() is not
NULL-checked in more machines. Not sure if trying invalid arguments
would be applicable for a qtest?

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 v3 4/4] target-openrisc: Fix cpu_model by name

[Anthony Liguori]

Andreas Färber <afaerber@suse.de> writes:

> Am 22.07.2013 13:34, schrieb Peter Maydell:
>> On 22 July 2013 12:17, Andreas Färber <afaerber@suse.de> wrote:
>>> Am 22.07.2013 12:40, schrieb Peter Maydell:
>>>> In any case we should be
>>>> consistent across target architectures about what we allow.
>>>
>>> alpha allows both
> [...]
>>> It didn't have -cpu ? before QOM, so we decided to print the type names
>>> there.
>> 
>> Looking at all of the '-cpu help' output, alpha seems to be
>> the odd one out here: none of the others list valid CPUs
>> with "-$arch-cpu" suffixes.
>
> Right, because all others had implemented -cpu ? before we introduced
> that naming scheme and I tried to keep output compatibility for them.
> Focus for alpha was therefore on -cpu foo compatibility only.
>
> Anthony had clearly stated on a KVM call that using full type names for
> future CPU hot-add was the right thing to do and possibly even composite
> convenience types like 4core-xeonblabla-x86_64-cpu; how that relates to
> -cpu and new targets was never clearly defined though. ;)

That's pretty gross, but yes, we should have:

qemu -device Xeon-E5-4610,id=sock0 -device Xeon-E5-4610,id=sock1

Which effectively does:

qemu -cpu SandyBridge -smp cores=6,threads=2,sockets=2

By today's standards.

I think this applies equally well to other architecture.  Model hardware
more closely.

Regards,

Anthony Liguori

>
> For VMSD we decided to deviate for new migratable targets from legacy
> CPUs in favor of consistency with devices, for instance.
>
>>> Stripping -alpha-cpu off typenames would surely be possible.
>> 
>> I think that that would be better in the name of consistency.
>
>> Also regarding consistency, not all targets react very well
>> to being asked for a nonexistent cpu via "-cpu xyzzy":
>>  alpha and s390x just plough on without an error
>
> s390x does not have models yet. This will get fixed once we have agreed
> on model names and their implementation.
>
>>  lm32 and unicore32 segfault
>> 
>> (some of this may be default board model bugs rather than
>> target-* bugs).
>
> Yeah, for one sh4 board where I noticed it while refactoring I already
> applied an error check. I guess cpu_init() / cpu_*_init() is not
> NULL-checked in more machines. Not sure if trying invalid arguments
> would be applicable for a qtest?
>
> 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 v3 4/4] target-openrisc: Fix cpu_model by name

[Peter Maydell]

On 22 July 2013 16:25, Anthony Liguori <anthony@codemonkey.ws> wrote:
> Andreas Färber <afaerber@suse.de> writes:
>> Am 22.07.2013 13:34, schrieb Peter Maydell:
>>> Looking at all of the '-cpu help' output, alpha seems to be
>>> the odd one out here: none of the others list valid CPUs
>>> with "-$arch-cpu" suffixes.
>>
>> Right, because all others had implemented -cpu ? before we introduced
>> that naming scheme and I tried to keep output compatibility for them.
>> Focus for alpha was therefore on -cpu foo compatibility only.
>>
>> Anthony had clearly stated on a KVM call that using full type names for
>> future CPU hot-add was the right thing to do and possibly even composite
>> convenience types like 4core-xeonblabla-x86_64-cpu; how that relates to
>> -cpu and new targets was never clearly defined though. ;)
>
> That's pretty gross, but yes, we should have:
>
> qemu -device Xeon-E5-4610,id=sock0 -device Xeon-E5-4610,id=sock1
>
> Which effectively does:
>
> qemu -cpu SandyBridge -smp cores=6,threads=2,sockets=2
>
> By today's standards.

That doesn't really answer the question of "should the argument
to -cpu be a QOM typename or a human friendly name?" though
(though I note none of your -cpu or -device argument examples
are QOM type names, since they're missing the -$arch-cpu suffix).

> I think this applies equally well to other architecture.
>  Model hardware more closely.

For ARM this would mean "don't support -cpu at all, it
is always hardwired by the board model" :-)

-- PMM

Re: [Qemu-devel] -cpu behavior (was: [PATCH v3 4/4] target-openrisc: Fix cpu_model by name)

[Andreas Färber]

Am 22.07.2013 17:38, schrieb Peter Maydell:
> On 22 July 2013 16:25, Anthony Liguori <anthony@codemonkey.ws> wrote:
>> Andreas Färber <afaerber@suse.de> writes:
>>> Am 22.07.2013 13:34, schrieb Peter Maydell:
>>>> Looking at all of the '-cpu help' output, alpha seems to be
>>>> the odd one out here: none of the others list valid CPUs
>>>> with "-$arch-cpu" suffixes.
>>>
>>> Right, because all others had implemented -cpu ? before we introduced
>>> that naming scheme and I tried to keep output compatibility for them.
>>> Focus for alpha was therefore on -cpu foo compatibility only.
>>>
>>> Anthony had clearly stated on a KVM call that using full type names for
>>> future CPU hot-add was the right thing to do and possibly even composite
>>> convenience types like 4core-xeonblabla-x86_64-cpu; how that relates to
>>> -cpu and new targets was never clearly defined though. ;)
>>
>> That's pretty gross, but yes, we should have:
>>
>> qemu -device Xeon-E5-4610,id=sock0 -device Xeon-E5-4610,id=sock1
>>
>> Which effectively does:
>>
>> qemu -cpu SandyBridge -smp cores=6,threads=2,sockets=2
>>
>> By today's standards.
> 
> That doesn't really answer the question of "should the argument
> to -cpu be a QOM typename or a human friendly name?" though
> (though I note none of your -cpu or -device argument examples
> are QOM type names, since they're missing the -$arch-cpu suffix).

Depending on how we register those types, they don't necessarily need an
-$arch-cpu suffix iff they are deemed globally unique. In this case
there would be a 32-bit and 64-bit type though, I guess.

But there's no qemu executable either, so we shouldn't take the example
literally. :P

>> I think this applies equally well to other architecture.
>>  Model hardware more closely.
> 
> For ARM this would mean "don't support -cpu at all, it
> is always hardwired by the board model" :-)

No, it means the board creates the equivalent of -device tegra2-soc. :)
Or exynos4210, highbank-soc or whatever. Less code in machine init, more
code in QOM devices reusable with -M none -readconfig foo.

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 v3 4/4] target-openrisc: Fix cpu_model by name

[Anthony Liguori]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 22 July 2013 16:25, Anthony Liguori <anthony@codemonkey.ws> wrote:
>> Andreas Färber <afaerber@suse.de> writes:
>>> Am 22.07.2013 13:34, schrieb Peter Maydell:
>>>> Looking at all of the '-cpu help' output, alpha seems to be
>>>> the odd one out here: none of the others list valid CPUs
>>>> with "-$arch-cpu" suffixes.
>>>
>>> Right, because all others had implemented -cpu ? before we introduced
>>> that naming scheme and I tried to keep output compatibility for them.
>>> Focus for alpha was therefore on -cpu foo compatibility only.
>>>
>>> Anthony had clearly stated on a KVM call that using full type names for
>>> future CPU hot-add was the right thing to do and possibly even composite
>>> convenience types like 4core-xeonblabla-x86_64-cpu; how that relates to
>>> -cpu and new targets was never clearly defined though. ;)
>>
>> That's pretty gross, but yes, we should have:
>>
>> qemu -device Xeon-E5-4610,id=sock0 -device Xeon-E5-4610,id=sock1
>>
>> Which effectively does:
>>
>> qemu -cpu SandyBridge -smp cores=6,threads=2,sockets=2
>>
>> By today's standards.
>
> That doesn't really answer the question of "should the argument
> to -cpu be a QOM typename or a human friendly name?"

They shouldn't be different things IMHO.

> though
> (though I note none of your -cpu or -device argument examples
> are QOM type names, since they're missing the -$arch-cpu suffix).

I'm not sure the rationale of $arch-cpu but I think having a forced
suffix is a bad idea.

>> I think this applies equally well to other architecture.
>>  Model hardware more closely.
>
> For ARM this would mean "don't support -cpu at all, it
> is always hardwired by the board model" :-)

Is that a bad thing?

I really hate the -cpu option.  I hope it dies a horrible bitrotten
death over time once -device can be used to replace it.

Regards,

Anthony Liguori

>
> -- PMM