Re: [PATCH v2 1/2] hw/i386/isapc.c: remove support for -cpu host and -cpu max

[Mark Cave-Ayland <mark.caveayland@nutanix.com>]

On 26/09/2025 16:12, Daniel P. Berrangé wrote:

> On Fri, Sep 26, 2025 at 04:01:23PM +0100, Mark Cave-Ayland wrote:
>> On 26/09/2025 14:55, Daniel P. Berrangé wrote:
>>
>>> On Fri, Sep 26, 2025 at 02:49:00PM +0100, Mark Cave-Ayland wrote:
>>>> On 26/09/2025 13:39, Daniel P. Berrangé wrote:
>>>>
>>>>> On Fri, Sep 26, 2025 at 01:36:25PM +0200, Markus Armbruster wrote:
>>>>>> Mark Cave-Ayland <mark.caveayland@nutanix.com> writes:
>>>>>>
>>>>>>> Following recent discussions on the mailing list, it has been decided
>>>>>>> that instead of mapping -cpu host and -cpu max to a suitable 32-bit x86 CPU,
>>>>>>> it is preferable to disallow them and use the existing valid_cpu_types
>>>>>>> validation logic so that an error is returned to the user instead.
>>>>>>>
>>>>>>> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
>>>>>>> ---
>>>>>>>     hw/i386/isapc.c | 27 ---------------------------
>>>>>>>     1 file changed, 27 deletions(-)
>>>>>>>
>>>>>>> diff --git a/hw/i386/isapc.c b/hw/i386/isapc.c
>>>>>>> index 44f4a44672..6c35a397df 100644
>>>>>>> --- a/hw/i386/isapc.c
>>>>>>> +++ b/hw/i386/isapc.c
>>>>>>> @@ -41,31 +41,6 @@ static void pc_init_isa(MachineState *machine)
>>>>>>>         DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
>>>>>>>         int i;
>>>>>>> -    /*
>>>>>>> -     * 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");
>>>>>>> -        warn_report("-cpu max is invalid for isapc machine, using pentium3");
>>>>>>> -    }
>>>>>>> -
>>>>>>> -    /*
>>>>>>> -     * Similarly if someone unintentionally passes "-cpu host" for the isapc
>>>>>>> -     * machine then display a warning and also switch to the "best" 32-bit
>>>>>>> -     * cpu possible which we consider to be the pentium3. This is because any
>>>>>>> -     * host CPU will already be modern than this, but it also ensures any
>>>>>>> -     * newer CPU flags/features are filtered out for older guests.
>>>>>>> -     */
>>>>>>> -    if (!strcmp(machine->cpu_type, X86_CPU_TYPE_NAME("host"))) {
>>>>>>> -        machine->cpu_type = X86_CPU_TYPE_NAME("pentium3");
>>>>>>> -        warn_report("-cpu host is invalid for isapc machine, using pentium3");
>>>>>>> -    }
>>>>>>> -
>>>>>>>         if (machine->ram_size > 3.5 * GiB) {
>>>>>>>             error_report("Too much memory for this machine: %" PRId64 " MiB, "
>>>>>>>                          "maximum 3584 MiB", machine->ram_size / MiB);
>>>>>>> @@ -162,8 +137,6 @@ static void isapc_machine_options(MachineClass *m)
>>>>>>>             X86_CPU_TYPE_NAME("pentium2"),
>>>>>>>             X86_CPU_TYPE_NAME("pentium3"),
>>>>>>>             X86_CPU_TYPE_NAME("qemu32"),
>>>>>>> -        X86_CPU_TYPE_NAME("max"),
>>>>>>> -        X86_CPU_TYPE_NAME("host"),
>>>>>>>             NULL
>>>>>>>         };
>>>>>>>         PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>>>>>>
>>>>>> This reverts the "smart" part of recent
>>>>>>
>>>>>> commit e1e2909f8e74051a34a044940f90d4650b6e784a
>>>>>> Author: Mark Cave-Ayland <mark.caveayland@nutanix.com>
>>>>>> Date:   Thu Aug 28 12:09:44 2025 +0100
>>>>>>
>>>>>>        hw/i386/pc_piix.c: restrict isapc machine to 32-bit CPUs
>>>>>>        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 either -cpu max or
>>>>>>        -cpu host then the "best" 32-bit CPU is used (in this case the pentium3).
>>>>>
>>>>> What is written here made sense from the POV of use of isapc with
>>>>> qemu-system-x86_64, but in qemu-system-i686, both 'max' and 'host'
>>>>> where already 32-bit CPUs IIUC. Both this original patch and
>>>>> the new patch block them from being used in qemu-system-i686
>>>>> which feels wrong given the justification above.
>>>>
>>>> I tried stepping through with -cpu host/-cpu max on qemu-system-i386 and
>>>> it's a bit confusing: I think we end up with some kind of custom AMD vendor
>>>> CPU but with LM disabled. I can't easily see a way to understand what
>>>> features are currently enabled?
>>>>
>>>> I must admit I'm struggling to see the usefulness of -cpu host/-cpu max for
>>>> isapc given that older OSs can be quite picky when it comes to hardware.
>>>
>>> But x86 CPU vendors go to ridiculous levels of complexity to retain
>>> historical back compat over many decades. If anything, I'd be surprised
>>> about the opposite - an OS that didn't work with -cpu max.
>>
>> It's not the CPU vendor I'd be worried about, but the OS vendor who may for
>> example execute CPUID and become confused if it returns an AMD vendor ID
>> instead of an Intel vendor ID.
> 
> IIRC/IIUC, AMD sold i486 CPUs with CPUID present in the ISA era, so I
> would have thought anything checking vendor ID should expect to see
> more than just Intel ?

Unfortunately I don't remember the specifics, but I have a vague memory 
that someone has mentioned this to me in the past.
>> What do you think is the best way forward? I'm still not convinced of the
>> utility of -cpu host/-cpu max for isapc, so what if instead of mapping them
>> to the pentium3 CPU we follow the standard deprecation path and emit a
>> warning on startup, and then remove them from valid_cpu_types in 2 releases
>> time? The advantage to this approach is that if people are actually using
>> -cpu host/-cpu max with the isapc machine then they would at least file an
>> issue in Gitlab and make us aware of it.
> 
> If we want to deprecate it the formal route, that's fine.

Works for me. Igor, any objections?


ATB,

Mark.