hw/arm/bcm2836.c | 2 +- hw/arm/netduino2.c | 10 +++++++++- hw/arm/raspi.c | 7 +++++++ hw/arm/xilinx_zynq.c | 6 ++++++ hw/arm/xlnx-zcu102.c | 17 +++++++++++++++++ 5 files changed, 40 insertions(+), 2 deletions(-)
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 V4: - Rebase - Remove spaces V3: - Make the varialbes static V2: - Rebase - Reorder patches - Add a Raspberry Pi 2 CPU fix V1: - Small fixes to prepare a series instead of RFC - Add commit messages for the commits - Expand the machine support to ARM machines RFC v2: - Rebase on Igor's work - Use more QEMUisms inside the code - List the supported machines in a NULL terminated array Alistair Francis (5): netduino2: Specify the valid CPUs bcm2836: Use the Cortex-A7 instead of Cortex-A15 raspi: Specify the valid CPUs xlnx-zcu102: Specify the valid CPUs xilinx_zynq: Specify the valid CPUs hw/arm/bcm2836.c | 2 +- hw/arm/netduino2.c | 10 +++++++++- hw/arm/raspi.c | 7 +++++++ hw/arm/xilinx_zynq.c | 6 ++++++ hw/arm/xlnx-zcu102.c | 17 +++++++++++++++++ 5 files changed, 40 insertions(+), 2 deletions(-) -- 2.14.1
On 20 December 2017 at 00:27, Alistair Francis <alistair.francis@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. thanks -- PMM
On Tue, Dec 19, 2017 at 4:43 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 20 December 2017 at 00:27, Alistair Francis > <alistair.francis@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. 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. Alistair > > thanks > -- PMM
On Tue, Dec 19, 2017 at 4:55 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> On Tue, Dec 19, 2017 at 4:43 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 20 December 2017 at 00:27, Alistair Francis
>> <alistair.francis@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.
>
> 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.
Yeah, so I tested this diff on top of this series:
diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
index 111a1d0aba..702e5e8fcf 100644
--- a/hw/arm/netduino2.c
+++ b/hw/arm/netduino2.c
@@ -42,8 +42,8 @@ static void netduino2_init(MachineState *machine)
}
static const char *netduino_valid_cpus[] = {
- ARM_CPU_TYPE_NAME("cortex-m3"),
- ARM_CPU_TYPE_NAME("cortex-m4"),
+ "cortex-m3",
+ "cortex-m4",
NULL
};
diff --git a/hw/core/machine.c b/hw/core/machine.c
index c857f3f934..4b817ccc58 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -761,8 +761,8 @@ void machine_run_board_init(MachineState *machine)
/* If the machine supports the valid_cpu_types check and the user
* specified a CPU with -cpu check here that the user CPU is supported.
*/
- if (machine_class->valid_cpu_types && machine->cpu_type) {
- ObjectClass *class = object_class_by_name(machine->cpu_type);
+ if (machine_class->valid_cpu_types && machine->cpu_model) {
+ ObjectClass *class = object_class_by_name(machine->cpu_model);
int i;
for (i = 0; machine_class->valid_cpu_types[i]; i++) {
@@ -777,7 +777,7 @@ void machine_run_board_init(MachineState *machine)
if (!machine_class->valid_cpu_types[i]) {
/* The user specified CPU is not valid */
- error_report("Invalid CPU type: %s", machine->cpu_type);
+ error_report("Invalid CPU type: %s", machine->cpu_model);
error_printf("The valid types are: %s",
machine_class->valid_cpu_types[0]);
for (i = 1; machine_class->valid_cpu_types[i]; i++) {
and I get this:
$ aarch64-softmmu/qemu-system-aarch64 -M netduino2 -kernel
./u-boot.elf -nographic -cpu "cortex-m4" -S
qemu-system-aarch64: Invalid CPU type: cortex-m4
The valid types are: cortex-m3, cortex-m4
So we loose the object_class_dynamic_cast() ability.
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?
Alistair
>
> Alistair
>
>>
>> thanks
>> -- PMM
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.francis@xilinx.com> wrote: > > On Tue, Dec 19, 2017 at 4:43 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > >> On 20 December 2017 at 00:27, Alistair Francis > >> <alistair.francis@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. -- Eduardo
On Wed, Dec 20, 2017 at 2:06 PM, Eduardo Habkost <ehabkost@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.francis@xilinx.com> wrote: >> > On Tue, Dec 19, 2017 at 4:43 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >> >> On 20 December 2017 at 00:27, Alistair Francis >> >> <alistair.francis@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. 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 >
On Fri, Dec 22, 2017 at 10:45 AM, Alistair Francis <alistair.francis@xilinx.com> wrote: > On Wed, Dec 20, 2017 at 2:06 PM, Eduardo Habkost <ehabkost@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.francis@xilinx.com> wrote: >>> > On Tue, Dec 19, 2017 at 4:43 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >>> >> On 20 December 2017 at 00:27, Alistair Francis >>> >> <alistair.francis@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 >>
On Fri, Dec 22, 2017 at 11:47:00AM -0800, Alistair Francis wrote: > On Fri, Dec 22, 2017 at 10:45 AM, Alistair Francis > <alistair.francis@xilinx.com> wrote: > > On Wed, Dec 20, 2017 at 2:06 PM, Eduardo Habkost <ehabkost@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.francis@xilinx.com> wrote: > >>> > On Tue, Dec 19, 2017 at 4:43 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > >>> >> On 20 December 2017 at 00:27, Alistair Francis > >>> >> <alistair.francis@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? Yes, I think this would be simpler, considering that the current users don't need the object_class_dynamic_cast() stuff. Then whoever decide to make PC simply list TYPE_X86_CPU later (possibly me) will need to clean up the CPU model enumeration/lookup code and implement a cpu_model_from_type_name() helper. -- Eduardo
On Fri, 22 Dec 2017 11:47:00 -0800 Alistair Francis <alistair.francis@xilinx.com> wrote: > On Fri, Dec 22, 2017 at 10:45 AM, Alistair Francis > <alistair.francis@xilinx.com> wrote: > > On Wed, Dec 20, 2017 at 2:06 PM, Eduardo Habkost <ehabkost@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.francis@xilinx.com> wrote: > >>> > On Tue, Dec 19, 2017 at 4:43 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > >>> >> On 20 December 2017 at 00:27, Alistair Francis > >>> >> <alistair.francis@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? 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? > 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 > >>
On Thu, Dec 28, 2017 at 02:39:31PM +0100, Igor Mammedov wrote: > On Fri, 22 Dec 2017 11:47:00 -0800 > Alistair Francis <alistair.francis@xilinx.com> wrote: > > > On Fri, Dec 22, 2017 at 10:45 AM, Alistair Francis > > <alistair.francis@xilinx.com> wrote: > > > On Wed, Dec 20, 2017 at 2:06 PM, Eduardo Habkost <ehabkost@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.francis@xilinx.com> wrote: > > >>> > On Tue, Dec 19, 2017 at 4:43 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > > >>> >> On 20 December 2017 at 00:27, Alistair Francis > > >>> >> <alistair.francis@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? > 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
On Thu, Dec 28, 2017 at 6:59 AM, Eduardo Habkost <ehabkost@redhat.com> wrote: > On Thu, Dec 28, 2017 at 02:39:31PM +0100, Igor Mammedov wrote: >> On Fri, 22 Dec 2017 11:47:00 -0800 >> Alistair Francis <alistair.francis@xilinx.com> wrote: >> >> > On Fri, Dec 22, 2017 at 10:45 AM, Alistair Francis >> > <alistair.francis@xilinx.com> wrote: >> > > On Wed, Dec 20, 2017 at 2:06 PM, Eduardo Habkost <ehabkost@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.francis@xilinx.com> wrote: >> > >>> > On Tue, Dec 19, 2017 at 4:43 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >> > >>> >> On 20 December 2017 at 00:27, Alistair Francis >> > >>> >> <alistair.francis@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? >> 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) Wait, machine->cpu_model is the human readable name. Without using that we can't use just human readable strings for the valid cpu types. Alistair >> >> >> 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 >
On Wed, Jan 10, 2018 at 01:30:29PM -0800, Alistair Francis wrote: > On Thu, Dec 28, 2017 at 6:59 AM, Eduardo Habkost <ehabkost@redhat.com> wrote: > > On Thu, Dec 28, 2017 at 02:39:31PM +0100, Igor Mammedov wrote: > >> On Fri, 22 Dec 2017 11:47:00 -0800 > >> Alistair Francis <alistair.francis@xilinx.com> wrote: > >> > >> > On Fri, Dec 22, 2017 at 10:45 AM, Alistair Francis > >> > <alistair.francis@xilinx.com> wrote: > >> > > On Wed, Dec 20, 2017 at 2:06 PM, Eduardo Habkost <ehabkost@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.francis@xilinx.com> wrote: > >> > >>> > On Tue, Dec 19, 2017 at 4:43 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > >> > >>> >> On 20 December 2017 at 00:27, Alistair Francis > >> > >>> >> <alistair.francis@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? > >> 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) > > Wait, machine->cpu_model is the human readable name. Without using > that we can't use just human readable strings for the valid cpu types. Well, if we want to deprecate machine->cpu_model we need to offer an alternative first, otherwise we can't prevent people from using it. Igor, do you see an (existing) alternative to machine->cpu_model that would allow us to avoid using it in machine_run_board_init()? -- Eduardo
On Wed, 10 Jan 2018 19:48:00 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Wed, Jan 10, 2018 at 01:30:29PM -0800, Alistair Francis wrote:
> > On Thu, Dec 28, 2017 at 6:59 AM, Eduardo Habkost <ehabkost@redhat.com> wrote:
> > > On Thu, Dec 28, 2017 at 02:39:31PM +0100, Igor Mammedov wrote:
> > >> On Fri, 22 Dec 2017 11:47:00 -0800
> > >> Alistair Francis <alistair.francis@xilinx.com> wrote:
> > >>
> > >> > On Fri, Dec 22, 2017 at 10:45 AM, Alistair Francis
> > >> > <alistair.francis@xilinx.com> wrote:
> > >> > > On Wed, Dec 20, 2017 at 2:06 PM, Eduardo Habkost <ehabkost@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.francis@xilinx.com> wrote:
> > >> > >>> > On Tue, Dec 19, 2017 at 4:43 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> > >> > >>> >> On 20 December 2017 at 00:27, Alistair Francis
> > >> > >>> >> <alistair.francis@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?
> > >> 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)
> >
> > Wait, machine->cpu_model is the human readable name. Without using
> > that we can't use just human readable strings for the valid cpu types.
>
> Well, if we want to deprecate machine->cpu_model we need to offer
> an alternative first, otherwise we can't prevent people from
> using it.
>
> Igor, do you see an (existing) alternative to machine->cpu_model
> that would allow us to avoid using it in
> machine_run_board_init()?
In recently merged refactoring machine->cpu_model is being replaced
by machine->cpu_type. So currently we don't need machine->cpu_model
anywhere except machine('none'), and once I refactor that it could
be dropped completely and after some work on *-user targets we can
practically get rid of cpu_model notion completely
(excluding of -cpu option parser).
My dislike of idea is that it's adding back cpumodel strings
in boards code again (which I've just got rid of).
I hate to say that but it looks like we need more refactoring
for this series to print cpumodels back to user.
We already have FOO_cpu_list()/FOO_query_cpu_definitions()
which already do cpu type => cpumodel conversion (and even
have some code duplication within a target), I'd suggest
generalizing that across targets and then using generic
helper for printing back to user converted cpu types from
mc->valid_cpu_types which this series introduces.
On Thu, Jan 11, 2018 at 11:25:08AM +0100, Igor Mammedov wrote:
> On Wed, 10 Jan 2018 19:48:00 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > On Wed, Jan 10, 2018 at 01:30:29PM -0800, Alistair Francis wrote:
> > > On Thu, Dec 28, 2017 at 6:59 AM, Eduardo Habkost <ehabkost@redhat.com> wrote:
> > > > On Thu, Dec 28, 2017 at 02:39:31PM +0100, Igor Mammedov wrote:
> > > >> On Fri, 22 Dec 2017 11:47:00 -0800
> > > >> Alistair Francis <alistair.francis@xilinx.com> wrote:
> > > >>
> > > >> > On Fri, Dec 22, 2017 at 10:45 AM, Alistair Francis
> > > >> > <alistair.francis@xilinx.com> wrote:
> > > >> > > On Wed, Dec 20, 2017 at 2:06 PM, Eduardo Habkost <ehabkost@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.francis@xilinx.com> wrote:
> > > >> > >>> > On Tue, Dec 19, 2017 at 4:43 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> > > >> > >>> >> On 20 December 2017 at 00:27, Alistair Francis
> > > >> > >>> >> <alistair.francis@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?
> > > >> 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)
> > >
> > > Wait, machine->cpu_model is the human readable name. Without using
> > > that we can't use just human readable strings for the valid cpu types.
> >
> > Well, if we want to deprecate machine->cpu_model we need to offer
> > an alternative first, otherwise we can't prevent people from
> > using it.
> >
> > Igor, do you see an (existing) alternative to machine->cpu_model
> > that would allow us to avoid using it in
> > machine_run_board_init()?
> In recently merged refactoring machine->cpu_model is being replaced
> by machine->cpu_type. So currently we don't need machine->cpu_model
> anywhere except machine('none'), and once I refactor that it could
> be dropped completely and after some work on *-user targets we can
> practically get rid of cpu_model notion completely
> (excluding of -cpu option parser).
>
> My dislike of idea is that it's adding back cpumodel strings
> in boards code again (which I've just got rid of).
>
> I hate to say that but it looks like we need more refactoring
> for this series to print cpumodels back to user.
>
> We already have FOO_cpu_list()/FOO_query_cpu_definitions()
> which already do cpu type => cpumodel conversion (and even
> have some code duplication within a target), I'd suggest
> generalizing that across targets and then using generic
> helper for printing back to user converted cpu types from
> mc->valid_cpu_types which this series introduces.
I agree with the long term goal of making cpu type => cpu model
conversion generic. But until we refactor the arch-specific code
and implement that, we have 2 options:
1) Keep printing a confusing error message until we implement cpu
type => cpu model conversion;
2) Keep the MachineState::cpu_model field until we implement cpu
type => cpu model conversion.
I don't see any reason to pick (1) instead of (2).
--
Eduardo
On Thu, Jan 11, 2018 at 4:58 AM, Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Thu, Jan 11, 2018 at 11:25:08AM +0100, Igor Mammedov wrote:
>> On Wed, 10 Jan 2018 19:48:00 -0200
>> Eduardo Habkost <ehabkost@redhat.com> wrote:
>>
>> > On Wed, Jan 10, 2018 at 01:30:29PM -0800, Alistair Francis wrote:
>> > > On Thu, Dec 28, 2017 at 6:59 AM, Eduardo Habkost <ehabkost@redhat.com> wrote:
>> > > > On Thu, Dec 28, 2017 at 02:39:31PM +0100, Igor Mammedov wrote:
>> > > >> On Fri, 22 Dec 2017 11:47:00 -0800
>> > > >> Alistair Francis <alistair.francis@xilinx.com> wrote:
>> > > >>
>> > > >> > On Fri, Dec 22, 2017 at 10:45 AM, Alistair Francis
>> > > >> > <alistair.francis@xilinx.com> wrote:
>> > > >> > > On Wed, Dec 20, 2017 at 2:06 PM, Eduardo Habkost <ehabkost@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.francis@xilinx.com> wrote:
>> > > >> > >>> > On Tue, Dec 19, 2017 at 4:43 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> > > >> > >>> >> On 20 December 2017 at 00:27, Alistair Francis
>> > > >> > >>> >> <alistair.francis@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?
>> > > >> 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)
>> > >
>> > > Wait, machine->cpu_model is the human readable name. Without using
>> > > that we can't use just human readable strings for the valid cpu types.
>> >
>> > Well, if we want to deprecate machine->cpu_model we need to offer
>> > an alternative first, otherwise we can't prevent people from
>> > using it.
>> >
>> > Igor, do you see an (existing) alternative to machine->cpu_model
>> > that would allow us to avoid using it in
>> > machine_run_board_init()?
>> In recently merged refactoring machine->cpu_model is being replaced
>> by machine->cpu_type. So currently we don't need machine->cpu_model
>> anywhere except machine('none'), and once I refactor that it could
>> be dropped completely and after some work on *-user targets we can
>> practically get rid of cpu_model notion completely
>> (excluding of -cpu option parser).
>>
>> My dislike of idea is that it's adding back cpumodel strings
>> in boards code again (which I've just got rid of).
>>
>> I hate to say that but it looks like we need more refactoring
>> for this series to print cpumodels back to user.
>>
>> We already have FOO_cpu_list()/FOO_query_cpu_definitions()
>> which already do cpu type => cpumodel conversion (and even
>> have some code duplication within a target), I'd suggest
>> generalizing that across targets and then using generic
>> helper for printing back to user converted cpu types from
>> mc->valid_cpu_types which this series introduces.
>
> I agree with the long term goal of making cpu type => cpu model
> conversion generic. But until we refactor the arch-specific code
> and implement that, we have 2 options:
>
> 1) Keep printing a confusing error message until we implement cpu
> type => cpu model conversion;
> 2) Keep the MachineState::cpu_model field until we implement cpu
> type => cpu model conversion.
>
> I don't see any reason to pick (1) instead of (2).
Ok, I'm going over this again (sorry for the delay).
I'm going to respin using machine->cpu_model. It shouldn't be hard to
refactor in the future to machine->cpu_type, once that is or can be
translated to a user friendly string.
Alistair
>
> --
> Eduardo
>
© 2016 - 2024 Red Hat, Inc.