On Fri, Dec 22, 2017 at 10:45 AM, Alistair Francis <alistair.fran...@xilinx.com> wrote: > On Wed, Dec 20, 2017 at 2:06 PM, Eduardo Habkost <ehabk...@redhat.com> 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 >>> <alistair.fran...@xilinx.com> wrote: >>> > On Tue, Dec 19, 2017 at 4:43 PM, Peter Maydell <peter.mayd...@linaro.org> >>> > wrote: >>> >> On 20 December 2017 at 00:27, Alistair Francis >>> >> <alistair.fran...@xilinx.com> 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? Alistair > > Ok, so it sounds like I'll respin this series with an extra column in > the array for human readable names. Then in the future we can work on > removing that. > > Alistair > >> >> -- >> Eduardo >>
