[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 0/5] Add a valid_cpu_types property
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH v4 0/5] Add a valid_cpu_types property |
Date: |
Thu, 28 Dec 2017 12:59:04 -0200 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
On Thu, Dec 28, 2017 at 02:39:31PM +0100, Igor Mammedov wrote:
> On Fri, 22 Dec 2017 11:47:00 -0800
> Alistair Francis <address@hidden> wrote:
>
> > On Fri, Dec 22, 2017 at 10:45 AM, Alistair Francis
> > <address@hidden> wrote:
> > > On Wed, Dec 20, 2017 at 2:06 PM, Eduardo Habkost <address@hidden> wrote:
> > >> On Tue, Dec 19, 2017 at 05:03:59PM -0800, Alistair Francis wrote:
> > >>> On Tue, Dec 19, 2017 at 4:55 PM, Alistair Francis
> > >>> <address@hidden> wrote:
> > >>> > On Tue, Dec 19, 2017 at 4:43 PM, Peter Maydell <address@hidden> wrote:
> > >>> >> On 20 December 2017 at 00:27, Alistair Francis
> > >>> >> <address@hidden> wrote:
> > >>> >>> There are numorous QEMU machines that only have a single or a
> > >>> >>> handful of
> > >>> >>> valid CPU options. To simplyfy the management of specificying which
> > >>> >>> CPU
> > >>> >>> is/isn't valid let's create a property that can be set in the
> > >>> >>> machine
> > >>> >>> init. We can then check to see if the user supplied CPU is in that
> > >>> >>> list
> > >>> >>> or not.
> > >>> >>>
> > >>> >>> I have added the valid_cpu_types for some ARM machines only at the
> > >>> >>> moment.
> > >>> >>>
> > >>> >>> Here is what specifying the CPUs looks like now:
> > >>> >>>
> > >>> >>> $ aarch64-softmmu/qemu-system-aarch64 -M netduino2 -kernel
> > >>> >>> ./u-boot.elf -nographic -cpu "cortex-m3" -S
> > >>> >>> QEMU 2.10.50 monitor - type 'help' for more information
> > >>> >>> (qemu) info cpus
> > >>> >>> * CPU #0: thread_id=24175
> > >>> >>> (qemu) q
> > >>> >>>
> > >>> >>> $ aarch64-softmmu/qemu-system-aarch64 -M netduino2 -kernel
> > >>> >>> ./u-boot.elf -nographic -cpu "cortex-m4" -S
> > >>> >>> QEMU 2.10.50 monitor - type 'help' for more information
> > >>> >>> (qemu) q
> > >>> >>>
> > >>> >>> $ aarch64-softmmu/qemu-system-aarch64 -M netduino2 -kernel
> > >>> >>> ./u-boot.elf -nographic -cpu "cortex-m5" -S
> > >>> >>> qemu-system-aarch64: unable to find CPU model 'cortex-m5'
> > >>> >>>
> > >>> >>> $ aarch64-softmmu/qemu-system-aarch64 -M netduino2 -kernel
> > >>> >>> ./u-boot.elf -nographic -cpu "cortex-a9" -S
> > >>> >>> qemu-system-aarch64: Invalid CPU type: cortex-a9-arm-cpu
> > >>> >>> The valid types are: cortex-m3-arm-cpu, cortex-m4-arm-cpu
> > >>> >>
> > >>> >> Thanks for this; we really should be more strict about
> > >>> >> forbidding "won't work" combinations than we have
> > >>> >> been in the past.
> > >>> >>
> > >>> >> In the last of these cases, I think that when we
> > >>> >> list the invalid CPU type and the valid types
> > >>> >> we should use the same names we want the user to
> > >>> >> use on the command line, without the "-arm-cpu"
> > >>> >> suffixes.
> > >>> >
> > >>> > Hmm... That is a good point, it is confusing that they don't line up.
> > >>
> > >> Agreed.
> > >>
> > >>> >
> > >>> > The problem is that we are just doing a simple
> > >>> > object_class_dynamic_cast() in hw/core/machine.c which I think
> > >>> > (untested) requires us to have the full name in the valid cpu array.
> > >> [...]
> > >>>
> > >>> I think an earlier version of my previous series adding the support to
> > >>> machine.c did string comparison, but it was decided to utilise objects
> > >>> instead. One option is to make the array 2 wide and have the second
> > >>> string be user friendly?
> > >>
> > >> Making the array 2-column will duplicate information that we can
> > >> already find out using other methods, and it won't solve the
> > >> problem if an entry has a parent class with multiple subclasses
> > >> (the original reason I suggested object_class_dynamic_cast()).
> > >>
> > >> The main obstacle to fix this easily is that we do have a common
> > >> ObjectClass *cpu_class_by_name(const char *cpu_model)
> > >> function, but not a common method to get the model name from a
> > >> CPUClass. Implementing this is possible, but probably better to
> > >> do it after moving the existing arch-specific CPU model
> > >> enumeration hooks to common code (currently we duplicate lots of
> > >> CPU enumeration/lookup boilerplate code that we shouldn't have
> > >> to).
> > >>
> > >> Listing only the human-friendly names in the array like in the
> > >> original patch could be a reasonable temporary solution. It
> > >> won't allow us to use a single entry for all subclasses of a
> > >> given type by now (e.g. listing only TYPE_X86_CPU on PC), but at
> > >> least we can address this issue without waiting for a refactor of
> > >> the CPU model enumeration code.
> >
> > Ah, I just re-read this. Do you mean go back to the original RFC and
> > just use strcmp() to compare the human readable cpu_model?
> It's sort of going backwards but I won't object to this as far as you
> won't use machine->cpu_model (which is in process of being removed)
>
>
> BTW:
> how hard is it, to add cpu_type2cpu_name function?
It shouldn't be hard, but I would like to avoid adding yet
another arch-specific hook just for that. Probably we would need
to clean up the existing CPU model enumeration/lookup code if we
want to avoid increase duplication of code on arch-specific
hooks.
--
Eduardo
- [Qemu-devel] [PATCH v4 4/5] xlnx-zcu102: Specify the valid CPUs, (continued)
- [Qemu-devel] [PATCH v4 4/5] xlnx-zcu102: Specify the valid CPUs, Alistair Francis, 2017/12/19
- [Qemu-devel] [PATCH v4 5/5] xilinx_zynq: Specify the valid CPUs, Alistair Francis, 2017/12/19
- Re: [Qemu-devel] [PATCH v4 0/5] Add a valid_cpu_types property, Peter Maydell, 2017/12/19
- Re: [Qemu-devel] [PATCH v4 0/5] Add a valid_cpu_types property, Alistair Francis, 2017/12/19
- Re: [Qemu-devel] [PATCH v4 0/5] Add a valid_cpu_types property, Alistair Francis, 2017/12/19
- Re: [Qemu-devel] [PATCH v4 0/5] Add a valid_cpu_types property, Eduardo Habkost, 2017/12/20
- Re: [Qemu-devel] [PATCH v4 0/5] Add a valid_cpu_types property, Alistair Francis, 2017/12/22
- Re: [Qemu-devel] [PATCH v4 0/5] Add a valid_cpu_types property, Alistair Francis, 2017/12/22
- Re: [Qemu-devel] [PATCH v4 0/5] Add a valid_cpu_types property, Eduardo Habkost, 2017/12/22
- Re: [Qemu-devel] [PATCH v4 0/5] Add a valid_cpu_types property, Igor Mammedov, 2017/12/28
- Re: [Qemu-devel] [PATCH v4 0/5] Add a valid_cpu_types property,
Eduardo Habkost <=