[v4] hw/i386: separate isapc out from pc_piix

[PATCH v4 01/18] hw/i386/pc_piix.c: restrict isapc machine to 32-bit CPUs

Posted by Mark Cave-Ayland 4 months, 1 week ago

The isapc machine represents a legacy ISA PC with a 486 CPU. Whilst it is
possible to specify any CPU via -cpu on the command line, it makes no
sense to allow modern 64-bit CPUs to be used.

Restrict the isapc machine to the available 32-bit CPUs, taking care to
handle the case where if a user inadvertently uses -cpu max then the "best"
32-bit CPU is used (in this case the pentium3).

Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
---
 hw/i386/pc_piix.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index ea7572e783..67c52d79b2 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -418,6 +418,18 @@ static void pc_set_south_bridge(Object *obj, int value, Error **errp)
 #ifdef CONFIG_ISAPC
 static void pc_init_isa(MachineState *machine)
 {
+    /*
+     * There is a small chance that someone unintentionally passes "-cpu max"
+     * for the isapc machine, which will provide a much more modern 32-bit
+     * CPU than would be expected for an ISA-era PC. If the "max" cpu type has
+     * been specified, choose the "best" 32-bit cpu possible which we consider
+     * be the pentium3 (deliberately choosing an Intel CPU given that the
+     * default 486 CPU for the isapc machine is also an Intel CPU).
+     */
+    if (!strcmp(machine->cpu_type, X86_CPU_TYPE_NAME("max"))) {
+        machine->cpu_type = X86_CPU_TYPE_NAME("pentium3");
+    }
+
     pc_init1(machine, NULL);
 }
 #endif
@@ -786,7 +798,19 @@ DEFINE_I440FX_MACHINE(2, 6);
 #ifdef CONFIG_ISAPC
 static void isapc_machine_options(MachineClass *m)
 {
+    static const char * const valid_cpu_types[] = {
+        X86_CPU_TYPE_NAME("486"),
+        X86_CPU_TYPE_NAME("athlon"),
+        X86_CPU_TYPE_NAME("kvm32"),
+        X86_CPU_TYPE_NAME("pentium"),
+        X86_CPU_TYPE_NAME("pentium2"),
+        X86_CPU_TYPE_NAME("pentium3"),
+        X86_CPU_TYPE_NAME("qemu32"),
+        X86_CPU_TYPE_NAME("max"),
+        NULL
+    };
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+
     m->desc = "ISA-only PC";
     m->max_cpus = 1;
     m->option_rom_has_mr = true;
@@ -799,6 +823,7 @@ static void isapc_machine_options(MachineClass *m)
     pcmc->has_reserved_memory = false;
     m->default_nic = "ne2k_isa";
     m->default_cpu_type = X86_CPU_TYPE_NAME("486");
+    m->valid_cpu_types = valid_cpu_types;
     m->no_floppy = !module_object_class_by_name(TYPE_ISA_FDC);
     m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
 }
-- 
2.43.0

Re: [PATCH v4 01/18] hw/i386/pc_piix.c: restrict isapc machine to 32-bit CPUs

Posted by Philippe Mathieu-Daudé 4 months, 1 week ago

On 10/7/25 10:52, Mark Cave-Ayland wrote:
> The isapc machine represents a legacy ISA PC with a 486 CPU. Whilst it is
> possible to specify any CPU via -cpu on the command line, it makes no
> sense to allow modern 64-bit CPUs to be used.
> 
> Restrict the isapc machine to the available 32-bit CPUs, taking care to
> handle the case where if a user inadvertently uses -cpu max then the "best"
> 32-bit CPU is used (in this case the pentium3).
> 
> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
> ---
>   hw/i386/pc_piix.c | 25 +++++++++++++++++++++++++
>   1 file changed, 25 insertions(+)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index ea7572e783..67c52d79b2 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -418,6 +418,18 @@ static void pc_set_south_bridge(Object *obj, int value, Error **errp)
>   #ifdef CONFIG_ISAPC
>   static void pc_init_isa(MachineState *machine)
>   {
> +    /*
> +     * There is a small chance that someone unintentionally passes "-cpu max"
> +     * for the isapc machine, which will provide a much more modern 32-bit
> +     * CPU than would be expected for an ISA-era PC. If the "max" cpu type has
> +     * been specified, choose the "best" 32-bit cpu possible which we consider
> +     * be the pentium3 (deliberately choosing an Intel CPU given that the
> +     * default 486 CPU for the isapc machine is also an Intel CPU).
> +     */
> +    if (!strcmp(machine->cpu_type, X86_CPU_TYPE_NAME("max"))) {
> +        machine->cpu_type = X86_CPU_TYPE_NAME("pentium3");

Please warn() the user, otherwise:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +    }
> +
>       pc_init1(machine, NULL);
>   }

Re: [PATCH v4 01/18] hw/i386/pc_piix.c: restrict isapc machine to 32-bit CPUs

Posted by Mark Cave-Ayland 4 months, 1 week ago

On 10/07/2025 11:46, Philippe Mathieu-Daudé wrote:

> On 10/7/25 10:52, Mark Cave-Ayland wrote:
>> The isapc machine represents a legacy ISA PC with a 486 CPU. Whilst it is
>> possible to specify any CPU via -cpu on the command line, it makes no
>> sense to allow modern 64-bit CPUs to be used.
>>
>> Restrict the isapc machine to the available 32-bit CPUs, taking care to
>> handle the case where if a user inadvertently uses -cpu max then the 
>> "best"
>> 32-bit CPU is used (in this case the pentium3).
>>
>> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
>> ---
>>   hw/i386/pc_piix.c | 25 +++++++++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>>
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index ea7572e783..67c52d79b2 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -418,6 +418,18 @@ static void pc_set_south_bridge(Object *obj, int 
>> value, Error **errp)
>>   #ifdef CONFIG_ISAPC
>>   static void pc_init_isa(MachineState *machine)
>>   {
>> +    /*
>> +     * There is a small chance that someone unintentionally passes "- 
>> cpu max"
>> +     * for the isapc machine, which will provide a much more modern 
>> 32-bit
>> +     * CPU than would be expected for an ISA-era PC. If the "max" cpu 
>> type has
>> +     * been specified, choose the "best" 32-bit cpu possible which we 
>> consider
>> +     * be the pentium3 (deliberately choosing an Intel CPU given that 
>> the
>> +     * default 486 CPU for the isapc machine is also an Intel CPU).
>> +     */
>> +    if (!strcmp(machine->cpu_type, X86_CPU_TYPE_NAME("max"))) {
>> +        machine->cpu_type = X86_CPU_TYPE_NAME("pentium3");
> 
> Please warn() the user, otherwise:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
>> +    }
>> +
>>       pc_init1(machine, NULL);
>>   }

Thanks! That sounds reasonable - I can add that in v5.


ATB,

Mark.