On 22/09/2025 13:35, Igor Mammedov wrote:
On Mon, 22 Sep 2025 14:05:13 +0200
Philippe Mathieu-Daudé <[email protected]> wrote:
On 27/8/25 13:46, Daniel P. Berrangé wrote:
On Wed, Aug 27, 2025 at 12:10:00PM +0100, Mark Cave-Ayland wrote:
On 26/08/2025 08:25, Xiaoyao Li wrote:
On 8/22/2025 8:11 PM, 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 <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
---
hw/i386/pc_piix.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index c03324281b..5720b6b556 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -436,6 +436,19 @@ 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");
+ warn_report("-cpu max is invalid for isapc machine, using
pentium3");
+ }
Do we need to handle the case of "-cpu host"?
I don't believe so. I wasn't originally planning to support "-cpu max" for
isapc, however Daniel mentioned that it could possibly be generated from
libvirt so it makes sense to add the above check to warn in this case and
then continue.
Libvirt will support sending any valid -cpu flag, including both
'max' (any config) and 'host' (if KVM).
If 'isapc' still expects to support KVM, then it would be odd to
reject 'host', but KVM presumably has no built-in way to limit to
32-bit without QEMU manually masking many features ?
I'm a little worried about implications of libvirt sending '-cpu max'
and QEMU secretly turning that into '-cpu pentium3', as opposed to
having '-cpu max' expand to equiv to 'pentium3', which might cauase
confusion when libvirt queries the expanded CPU ? Copying Jiri for
an opinion from libvirt side, as I might be worrying about nothing.
OK, on 2nd thought, even while warning the user, changing the type
under the hood isn't great.
I second that,
Please don't do magical mutations of CPUs, just error out.
we used to 'fix|tweak' CPUs using machine compat hack,
however with introduction of versioned cpu models we shouldn't do that anymore.
(aka: existing CPU devices should stay immutable if possible, and any visible
changes should go into new version)
The original suggestion for allowing "max"/"host" was so that it
wouldn't cause any regressions with command lines erroneously including
-cpu max or -cpu host (which I believe may be possible with libvirt).
What about simply removing "max" of valid_cpu_types[], since it is
clearly confusing "max" == "pentium3"...
it seems that specifying supported cpu models in valid_cpu_types[] is the way
to go.
That was what I did in v1 and v2 version of the series, but I can submit
a patch to change this once there is agreement on the desired behaviour.
ATB,
Mark.
