The logic, to check if the specified CPU type is supported in
machine_run_board_init(), is independent enough. Factor it out into
helper is_cpu_type_supported(). machine_run_board_init() looks a bit
clean with this. Since we're here, @machine_class is renamed to @mc to
avoid multiple line spanning of code. The comments are tweaked a bit
either.
No functional change intended.
Signed-off-by: Gavin Shan <gshan@redhat.com>
---
v8: Move the precise message hint to PATCH[v8 3/9] (Gavin)
---
hw/core/machine.c | 83 +++++++++++++++++++++++++----------------------
1 file changed, 45 insertions(+), 38 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index bde7f4af6d..1797e002f9 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1387,13 +1387,53 @@ out:
return r;
}
+static bool is_cpu_type_supported(const MachineState *machine, Error **errp)
+{
+ MachineClass *mc = MACHINE_GET_CLASS(machine);
+ ObjectClass *oc = object_class_by_name(machine->cpu_type);
+ CPUClass *cc;
+ int i;
+
+ /*
+ * Check if the user specified CPU type is supported when the valid
+ * CPU types have been determined. Note that the user specified CPU
+ * type is provided through '-cpu' option.
+ */
+ if (mc->valid_cpu_types && machine->cpu_type) {
+ for (i = 0; mc->valid_cpu_types[i]; i++) {
+ if (object_class_dynamic_cast(oc, mc->valid_cpu_types[i])) {
+ break;
+ }
+ }
+
+ /* The user specified CPU type isn't valid */
+ if (!mc->valid_cpu_types[i]) {
+ error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
+ error_append_hint(errp, "The valid types are: %s",
+ mc->valid_cpu_types[0]);
+ for (i = 1; mc->valid_cpu_types[i]; i++) {
+ error_append_hint(errp, ", %s", mc->valid_cpu_types[i]);
+ }
+
+ error_append_hint(errp, "\n");
+ return false;
+ }
+ }
+
+ /* Check if CPU type is deprecated and warn if so */
+ cc = CPU_CLASS(oc);
+ if (cc && cc->deprecation_note) {
+ warn_report("CPU model %s is deprecated -- %s",
+ machine->cpu_type, cc->deprecation_note);
+ }
+
+ return true;
+}
void machine_run_board_init(MachineState *machine, const char *mem_path, Error **errp)
{
ERRP_GUARD();
MachineClass *machine_class = MACHINE_GET_CLASS(machine);
- ObjectClass *oc = object_class_by_name(machine->cpu_type);
- CPUClass *cc;
/* This checkpoint is required by replay to separate prior clock
reading from the other reads, because timer polling functions query
@@ -1448,42 +1488,9 @@ void machine_run_board_init(MachineState *machine, const char *mem_path, Error *
machine->ram = machine_consume_memdev(machine, machine->memdev);
}
- /* 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) {
- int i;
-
- for (i = 0; machine_class->valid_cpu_types[i]; i++) {
- if (object_class_dynamic_cast(oc,
- machine_class->valid_cpu_types[i])) {
- /* The user specified CPU is in the valid field, we are
- * good to go.
- */
- break;
- }
- }
-
- if (!machine_class->valid_cpu_types[i]) {
- /* The user specified CPU is not valid */
- error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
- error_append_hint(errp, "The valid types are: %s",
- machine_class->valid_cpu_types[0]);
- for (i = 1; machine_class->valid_cpu_types[i]; i++) {
- error_append_hint(errp, ", %s",
- machine_class->valid_cpu_types[i]);
- }
-
- error_append_hint(&errp, "\n");
- return;
- }
- }
-
- /* Check if CPU type is deprecated and warn if so */
- cc = CPU_CLASS(oc);
- if (cc && cc->deprecation_note) {
- warn_report("CPU model %s is deprecated -- %s", machine->cpu_type,
- cc->deprecation_note);
+ /* Check if the CPU type is supported */
+ if (!is_cpu_type_supported(machine, errp)) {
+ return;
}
if (machine->cgs) {
--
2.42.0Hi Gavin,
On 29/11/23 05:20, Gavin Shan wrote:
> The logic, to check if the specified CPU type is supported in
> machine_run_board_init(), is independent enough. Factor it out into
> helper is_cpu_type_supported(). machine_run_board_init() looks a bit
> clean with this. Since we're here, @machine_class is renamed to @mc to
> avoid multiple line spanning of code. The comments are tweaked a bit
> either.
>
> No functional change intended.
>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
> v8: Move the precise message hint to PATCH[v8 3/9] (Gavin)
> ---
> hw/core/machine.c | 83 +++++++++++++++++++++++++----------------------
> 1 file changed, 45 insertions(+), 38 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index bde7f4af6d..1797e002f9 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -1387,13 +1387,53 @@ out:
> return r;
> }
>
> +static bool is_cpu_type_supported(const MachineState *machine, Error **errp)
> +{
> + MachineClass *mc = MACHINE_GET_CLASS(machine);
> + ObjectClass *oc = object_class_by_name(machine->cpu_type);
> + CPUClass *cc;
> + int i;
> +
> + /*
> + * Check if the user specified CPU type is supported when the valid
> + * CPU types have been determined. Note that the user specified CPU
> + * type is provided through '-cpu' option.
> + */
> + if (mc->valid_cpu_types && machine->cpu_type) {
> + for (i = 0; mc->valid_cpu_types[i]; i++) {
> + if (object_class_dynamic_cast(oc, mc->valid_cpu_types[i])) {
> + break;
> + }
> + }
> +
> + /* The user specified CPU type isn't valid */
> + if (!mc->valid_cpu_types[i]) {
> + error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
> + error_append_hint(errp, "The valid types are: %s",
> + mc->valid_cpu_types[0]);
> + for (i = 1; mc->valid_cpu_types[i]; i++) {
> + error_append_hint(errp, ", %s", mc->valid_cpu_types[i]);
> + }
> +
> + error_append_hint(errp, "\n");
> + return false;
> + }
> + }
> +
> + /* Check if CPU type is deprecated and warn if so */
> + cc = CPU_CLASS(oc);
> + if (cc && cc->deprecation_note) {
cc can't be NULL, right? Otherwise,
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> + warn_report("CPU model %s is deprecated -- %s",
> + machine->cpu_type, cc->deprecation_note);
> + }
> +
> + return true;
> +}
Hi Phil,
On 12/1/23 20:53, Philippe Mathieu-Daudé wrote:
> On 29/11/23 05:20, Gavin Shan wrote:
>> The logic, to check if the specified CPU type is supported in
>> machine_run_board_init(), is independent enough. Factor it out into
>> helper is_cpu_type_supported(). machine_run_board_init() looks a bit
>> clean with this. Since we're here, @machine_class is renamed to @mc to
>> avoid multiple line spanning of code. The comments are tweaked a bit
>> either.
>>
>> No functional change intended.
>>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>> v8: Move the precise message hint to PATCH[v8 3/9] (Gavin)
>> ---
>> hw/core/machine.c | 83 +++++++++++++++++++++++++----------------------
>> 1 file changed, 45 insertions(+), 38 deletions(-)
>>
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index bde7f4af6d..1797e002f9 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -1387,13 +1387,53 @@ out:
>> return r;
>> }
>> +static bool is_cpu_type_supported(const MachineState *machine, Error **errp)
>> +{
>> + MachineClass *mc = MACHINE_GET_CLASS(machine);
>> + ObjectClass *oc = object_class_by_name(machine->cpu_type);
>> + CPUClass *cc;
>> + int i;
>> +
>> + /*
>> + * Check if the user specified CPU type is supported when the valid
>> + * CPU types have been determined. Note that the user specified CPU
>> + * type is provided through '-cpu' option.
>> + */
>> + if (mc->valid_cpu_types && machine->cpu_type) {
>> + for (i = 0; mc->valid_cpu_types[i]; i++) {
>> + if (object_class_dynamic_cast(oc, mc->valid_cpu_types[i])) {
>> + break;
>> + }
>> + }
>> +
>> + /* The user specified CPU type isn't valid */
>> + if (!mc->valid_cpu_types[i]) {
>> + error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
>> + error_append_hint(errp, "The valid types are: %s",
>> + mc->valid_cpu_types[0]);
>> + for (i = 1; mc->valid_cpu_types[i]; i++) {
>> + error_append_hint(errp, ", %s", mc->valid_cpu_types[i]);
>> + }
>> +
>> + error_append_hint(errp, "\n");
>> + return false;
>> + }
>> + }
>> +
>> + /* Check if CPU type is deprecated and warn if so */
>> + cc = CPU_CLASS(oc);
>> + if (cc && cc->deprecation_note) {
>
> cc can't be NULL, right? Otherwise,
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
machine->cpu_type is either mc->default_cpu_type or returned from parse_cpu_option().
It can be NULL if mc->default_cpu_type is invalid, which is a program error. So
assert(cc != NULL) should be used instead. I will fold the change to PATCH[v9 3/9]
>> + warn_report("CPU model %s is deprecated -- %s",
>> + machine->cpu_type, cc->deprecation_note);
>> + }
>> +
>> + return true;
>> +}
>
Thanks,
Gavin
On 4/12/23 00:13, Gavin Shan wrote:
> Hi Phil,
>
> On 12/1/23 20:53, Philippe Mathieu-Daudé wrote:
>> On 29/11/23 05:20, Gavin Shan wrote:
>>> The logic, to check if the specified CPU type is supported in
>>> machine_run_board_init(), is independent enough. Factor it out into
>>> helper is_cpu_type_supported(). machine_run_board_init() looks a bit
>>> clean with this. Since we're here, @machine_class is renamed to @mc to
>>> avoid multiple line spanning of code. The comments are tweaked a bit
>>> either.
>>>
>>> No functional change intended.
>>>
>>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>>> ---
>>> v8: Move the precise message hint to PATCH[v8 3/9] (Gavin)
>>> ---
>>> hw/core/machine.c | 83 +++++++++++++++++++++++++----------------------
>>> 1 file changed, 45 insertions(+), 38 deletions(-)
>>>
>>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>>> index bde7f4af6d..1797e002f9 100644
>>> --- a/hw/core/machine.c
>>> +++ b/hw/core/machine.c
>>> @@ -1387,13 +1387,53 @@ out:
>>> return r;
>>> }
>>> +static bool is_cpu_type_supported(const MachineState *machine, Error
>>> **errp)
>>> +{
>>> + MachineClass *mc = MACHINE_GET_CLASS(machine);
>>> + ObjectClass *oc = object_class_by_name(machine->cpu_type);
>>> + CPUClass *cc;
>>> + int i;
>>> +
>>> + /*
>>> + * Check if the user specified CPU type is supported when the valid
>>> + * CPU types have been determined. Note that the user specified CPU
>>> + * type is provided through '-cpu' option.
>>> + */
>>> + if (mc->valid_cpu_types && machine->cpu_type) {
>>> + for (i = 0; mc->valid_cpu_types[i]; i++) {
>>> + if (object_class_dynamic_cast(oc,
>>> mc->valid_cpu_types[i])) {
>>> + break;
>>> + }
>>> + }
>>> +
>>> + /* The user specified CPU type isn't valid */
>>> + if (!mc->valid_cpu_types[i]) {
>>> + error_setg(errp, "Invalid CPU type: %s",
>>> machine->cpu_type);
>>> + error_append_hint(errp, "The valid types are: %s",
>>> + mc->valid_cpu_types[0]);
>>> + for (i = 1; mc->valid_cpu_types[i]; i++) {
>>> + error_append_hint(errp, ", %s",
>>> mc->valid_cpu_types[i]);
>>> + }
>>> +
>>> + error_append_hint(errp, "\n");
>>> + return false;
>>> + }
>>> + }
>>> +
>>> + /* Check if CPU type is deprecated and warn if so */
>>> + cc = CPU_CLASS(oc);
>>> + if (cc && cc->deprecation_note) {
>>
>> cc can't be NULL, right? Otherwise,
>>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>
> machine->cpu_type is either mc->default_cpu_type or returned from
> parse_cpu_option().
> It can be NULL if mc->default_cpu_type is invalid, which is a program
> error. So
> assert(cc != NULL) should be used instead. I will fold the change to
> PATCH[v9 3/9]
cpu_type and cc an be NULL with the 'none' machine.
>
>>> + warn_report("CPU model %s is deprecated -- %s",
>>> + machine->cpu_type, cc->deprecation_note);
>>> + }
>>> +
>>> + return true;
>>> +}
>>
>
> Thanks,
> Gavin
>
© 2016 - 2024 Red Hat, Inc.