[PATCH-for-9.0 v2] hw/i386/pc: Deprecate 64-bit CPUs on ISA-only PC machine
Per Daniel suggestion [*]:
isapc could arguably be restricted to just 32-bit CPU models, because we should not need it to support any feature that didn't exist prior to circa 1995. eg refuse to start with isapc, if 'lm' is present in the CPU model for example.
Display a warning when such CPU is used: $ qemu-system-x86_64 -monitor stdio -S -M isapc -cpu Westmere qemu-system-x86_64: warning: Use of 64-bit CPU 'Westmere' is deprecated on the ISA-only PC machine QEMU 8.2.91 monitor - type 'help' for more information (qemu) q $ qemu-system-x86_64 -monitor stdio -S -M isapc -cpu athlon QEMU 8.2.91 monitor - type 'help' for more information (qemu) q [*] https://lore.kernel.org/qemu-devel/ZgQkS4RPmSt5Xa08@redhat.com/ Suggested-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- docs/about/deprecated.rst | 7 +++++++ include/hw/i386/pc.h | 1 + hw/i386/pc_piix.c | 14 ++++++++++++++ 3 files changed, 22 insertions(+) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 7b548519b5..345c35507f 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -208,6 +208,13 @@ is no longer packaged in any distro making it harder to run the ``check-tcg`` tests. Unless we can improve the testing situation there is a chance the code will bitrot without anyone noticing. +64-bit (x86_64) CPUs on the ``isapc`` machine (since 9.0) +''''''''''''''''''''''''''''''''''''''''''''''''''''''''' + +The ``isapc`` machine aims to emulate old PC machine without PCI was +generalized, so hardware available around 1995, before 64-bit intel +CPUs were produced. + System emulator machines ------------------------ diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 27a68071d7..2d202b9549 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -96,6 +96,7 @@ struct PCMachineClass { const char *default_south_bridge; /* Compat options: */ + bool deprecate_64bit_cpu; /* Specific to the 'isapc' machine */ /* Default CPU model version. See x86_cpu_set_default_version(). */ int default_cpu_version; diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 18ba076609..2e5b2efc33 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -182,7 +182,20 @@ static void pc_init1(MachineState *machine, const char *pci_type) } pc_machine_init_sgx_epc(pcms); + x86_cpus_init(x86ms, pcmc->default_cpu_version); + if (pcmc->deprecate_64bit_cpu) { + X86CPU *cpu = X86_CPU(first_cpu); + + if (cpu->env.features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) { + const char *cpu_type = object_get_typename(OBJECT(first_cpu)); + int cpu_len = strlen(cpu_type) - strlen(X86_CPU_TYPE_SUFFIX); + + warn_report("Use of 64-bit CPU '%.*s' is deprecated" + " on the ISA-only PC machine", + cpu_len, cpu_type); + } + } if (kvm_enabled()) { kvmclock_create(pcmc->kvmclock_create_always); @@ -918,6 +931,7 @@ static void isapc_machine_options(MachineClass *m) pcmc->gigabyte_align = false; pcmc->smbios_legacy_mode = true; pcmc->has_reserved_memory = false; + pcmc->deprecate_64bit_cpu = true; m->default_nic = "ne2k_isa"; m->default_cpu_type = X86_CPU_TYPE_NAME("486"); m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL); -- 2.41.0
W dniu 27.03.2024 o 17:54, Philippe Mathieu-Daudé pisze:
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 7b548519b5..345c35507f 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -208,6 +208,13 @@ is no longer packaged in any distro making it harder to run the ``check-tcg`` tests. Unless we can improve the testing situation there is a chance the code will bitrot without anyone noticing.
+64-bit (x86_64) CPUs on the ``isapc`` machine (since 9.0) +''''''''''''''''''''''''''''''''''''''''''''''''''''''''' + +The ``isapc`` machine aims to emulate old PC machine without PCI was +generalized, so hardware available around 1995, before 64-bit intel +CPUs were produced.
Can you s/Intel/x86-64/ here? Intel was not first with x86-64 (AMD invented it). Also "64-bit Intel" smells of Itanium too much.
On 27/3/24 18:29, Marcin Juszkiewicz wrote:
W dniu 27.03.2024 o 17:54, Philippe Mathieu-Daudé pisze:
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 7b548519b5..345c35507f 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -208,6 +208,13 @@ is no longer packaged in any distro making it harder to run the ``check-tcg`` tests. Unless we can improve the testing situation there is a chance the code will bitrot without anyone noticing. +64-bit (x86_64) CPUs on the ``isapc`` machine (since 9.0) +''''''''''''''''''''''''''''''''''''''''''''''''''''''''' + +The ``isapc`` machine aims to emulate old PC machine without PCI was +generalized, so hardware available around 1995, before 64-bit intel +CPUs were produced.
Can you s/Intel/x86-64/ here? Intel was not first with x86-64 (AMD invented it). Also "64-bit Intel" smells of Itanium too much.
OK ;)
On 27/03/2024 16:54, Philippe Mathieu-Daudé wrote:
Per Daniel suggestion [*]:
isapc could arguably be restricted to just 32-bit CPU models, because we should not need it to support any feature that didn't exist prior to circa 1995. eg refuse to start with isapc, if 'lm' is present in the CPU model for example.
Display a warning when such CPU is used:
$ qemu-system-x86_64 -monitor stdio -S -M isapc -cpu Westmere qemu-system-x86_64: warning: Use of 64-bit CPU 'Westmere' is deprecated on the ISA-only PC machine QEMU 8.2.91 monitor - type 'help' for more information (qemu) q
$ qemu-system-x86_64 -monitor stdio -S -M isapc -cpu athlon QEMU 8.2.91 monitor - type 'help' for more information (qemu) q
[*] https://lore.kernel.org/qemu-devel/ZgQkS4RPmSt5Xa08@redhat.com/
Suggested-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- docs/about/deprecated.rst | 7 +++++++ include/hw/i386/pc.h | 1 + hw/i386/pc_piix.c | 14 ++++++++++++++ 3 files changed, 22 insertions(+)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 7b548519b5..345c35507f 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -208,6 +208,13 @@ is no longer packaged in any distro making it harder to run the ``check-tcg`` tests. Unless we can improve the testing situation there is a chance the code will bitrot without anyone noticing.
+64-bit (x86_64) CPUs on the ``isapc`` machine (since 9.0) +''''''''''''''''''''''''''''''''''''''''''''''''''''''''' + +The ``isapc`` machine aims to emulate old PC machine without PCI was +generalized, so hardware available around 1995, before 64-bit intel +CPUs were produced. + System emulator machines ------------------------
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 27a68071d7..2d202b9549 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -96,6 +96,7 @@ struct PCMachineClass { const char *default_south_bridge;
/* Compat options: */ + bool deprecate_64bit_cpu; /* Specific to the 'isapc' machine */
/* Default CPU model version. See x86_cpu_set_default_version(). */ int default_cpu_version; diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 18ba076609..2e5b2efc33 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -182,7 +182,20 @@ static void pc_init1(MachineState *machine, const char *pci_type) }
pc_machine_init_sgx_epc(pcms); + x86_cpus_init(x86ms, pcmc->default_cpu_version); + if (pcmc->deprecate_64bit_cpu) { + X86CPU *cpu = X86_CPU(first_cpu); + + if (cpu->env.features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) { + const char *cpu_type = object_get_typename(OBJECT(first_cpu)); + int cpu_len = strlen(cpu_type) - strlen(X86_CPU_TYPE_SUFFIX); + + warn_report("Use of 64-bit CPU '%.*s' is deprecated" + " on the ISA-only PC machine", + cpu_len, cpu_type); + } + }
if (kvm_enabled()) { kvmclock_create(pcmc->kvmclock_create_always); @@ -918,6 +931,7 @@ static void isapc_machine_options(MachineClass *m) pcmc->gigabyte_align = false; pcmc->smbios_legacy_mode = true; pcmc->has_reserved_memory = false; + pcmc->deprecate_64bit_cpu = true; m->default_nic = "ne2k_isa"; m->default_cpu_type = X86_CPU_TYPE_NAME("486"); m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
The logic around checking CPUID_EXT2_LM looks good to me. Slightly curious as to whether people feel updating PCMachineClass is necessary, or you can simply do qdev_get_machine() and use object_dynamic_cast() to see if the machine matches MACHINE_NAME("isapc") and warn that way? FWIW I'd be amazed if anyone were actually overriding the default and trying to do this, but I guess that's what the warn_report() is for.... anyhow: Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> ATB, Mark.
On 28/03/2024 16.12, Mark Cave-Ayland wrote:
On 27/03/2024 16:54, Philippe Mathieu-Daudé wrote:
Per Daniel suggestion [*]:
> isapc could arguably be restricted to just 32-bit CPU models, > because we should not need it to support any feature that didn't > exist prior to circa 1995. eg refuse to start with isapc, if 'lm' > is present in the CPU model for example.
Display a warning when such CPU is used:
$ qemu-system-x86_64 -monitor stdio -S -M isapc -cpu Westmere qemu-system-x86_64: warning: Use of 64-bit CPU 'Westmere' is deprecated on the ISA-only PC machine QEMU 8.2.91 monitor - type 'help' for more information (qemu) q
$ qemu-system-x86_64 -monitor stdio -S -M isapc -cpu athlon QEMU 8.2.91 monitor - type 'help' for more information (qemu) q
[*] https://lore.kernel.org/qemu-devel/ZgQkS4RPmSt5Xa08@redhat.com/
Suggested-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- docs/about/deprecated.rst | 7 +++++++ include/hw/i386/pc.h | 1 + hw/i386/pc_piix.c | 14 ++++++++++++++ 3 files changed, 22 insertions(+)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 7b548519b5..345c35507f 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -208,6 +208,13 @@ is no longer packaged in any distro making it harder to run the ``check-tcg`` tests. Unless we can improve the testing situation there is a chance the code will bitrot without anyone noticing. +64-bit (x86_64) CPUs on the ``isapc`` machine (since 9.0) +''''''''''''''''''''''''''''''''''''''''''''''''''''''''' + +The ``isapc`` machine aims to emulate old PC machine without PCI was +generalized, so hardware available around 1995, before 64-bit intel +CPUs were produced. + System emulator machines ------------------------ diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 27a68071d7..2d202b9549 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -96,6 +96,7 @@ struct PCMachineClass { const char *default_south_bridge; /* Compat options: */ + bool deprecate_64bit_cpu; /* Specific to the 'isapc' machine */ /* Default CPU model version. See x86_cpu_set_default_version(). */ int default_cpu_version; diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 18ba076609..2e5b2efc33 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -182,7 +182,20 @@ static void pc_init1(MachineState *machine, const char *pci_type) } pc_machine_init_sgx_epc(pcms); + x86_cpus_init(x86ms, pcmc->default_cpu_version); + if (pcmc->deprecate_64bit_cpu) { + X86CPU *cpu = X86_CPU(first_cpu); + + if (cpu->env.features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) { + const char *cpu_type = object_get_typename(OBJECT(first_cpu)); + int cpu_len = strlen(cpu_type) - strlen(X86_CPU_TYPE_SUFFIX); + + warn_report("Use of 64-bit CPU '%.*s' is deprecated" + " on the ISA-only PC machine", + cpu_len, cpu_type); + } + } if (kvm_enabled()) { kvmclock_create(pcmc->kvmclock_create_always); @@ -918,6 +931,7 @@ static void isapc_machine_options(MachineClass *m) pcmc->gigabyte_align = false; pcmc->smbios_legacy_mode = true; pcmc->has_reserved_memory = false; + pcmc->deprecate_64bit_cpu = true; m->default_nic = "ne2k_isa"; m->default_cpu_type = X86_CPU_TYPE_NAME("486"); m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
The logic around checking CPUID_EXT2_LM looks good to me. Slightly curious as to whether people feel updating PCMachineClass is necessary, or you can simply do qdev_get_machine() and use object_dynamic_cast() to see if the machine matches MACHINE_NAME("isapc") and warn that way?
Why don't you simply pass it as a parameter from pc_init_isa() instead? Or do the whole check in pc_init_isa() instead? Thomas
On 28/3/24 16:39, Thomas Huth wrote:
On 28/03/2024 16.12, Mark Cave-Ayland wrote:
On 27/03/2024 16:54, Philippe Mathieu-Daudé wrote:
Per Daniel suggestion [*]:
> isapc could arguably be restricted to just 32-bit CPU models, > because we should not need it to support any feature that didn't > exist prior to circa 1995. eg refuse to start with isapc, if 'lm' > is present in the CPU model for example.
Display a warning when such CPU is used:
$ qemu-system-x86_64 -monitor stdio -S -M isapc -cpu Westmere qemu-system-x86_64: warning: Use of 64-bit CPU 'Westmere' is deprecated on the ISA-only PC machine QEMU 8.2.91 monitor - type 'help' for more information (qemu) q
$ qemu-system-x86_64 -monitor stdio -S -M isapc -cpu athlon QEMU 8.2.91 monitor - type 'help' for more information (qemu) q
[*] https://lore.kernel.org/qemu-devel/ZgQkS4RPmSt5Xa08@redhat.com/
Suggested-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- docs/about/deprecated.rst | 7 +++++++ include/hw/i386/pc.h | 1 + hw/i386/pc_piix.c | 14 ++++++++++++++ 3 files changed, 22 insertions(+)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 7b548519b5..345c35507f 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -208,6 +208,13 @@ is no longer packaged in any distro making it harder to run the ``check-tcg`` tests. Unless we can improve the testing situation there is a chance the code will bitrot without anyone noticing. +64-bit (x86_64) CPUs on the ``isapc`` machine (since 9.0) +''''''''''''''''''''''''''''''''''''''''''''''''''''''''' + +The ``isapc`` machine aims to emulate old PC machine without PCI was +generalized, so hardware available around 1995, before 64-bit intel +CPUs were produced. + System emulator machines ------------------------ diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 27a68071d7..2d202b9549 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -96,6 +96,7 @@ struct PCMachineClass { const char *default_south_bridge; /* Compat options: */ + bool deprecate_64bit_cpu; /* Specific to the 'isapc' machine */ /* Default CPU model version. See x86_cpu_set_default_version(). */ int default_cpu_version; diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 18ba076609..2e5b2efc33 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -182,7 +182,20 @@ static void pc_init1(MachineState *machine, const char *pci_type) } pc_machine_init_sgx_epc(pcms); + x86_cpus_init(x86ms, pcmc->default_cpu_version); + if (pcmc->deprecate_64bit_cpu) { + X86CPU *cpu = X86_CPU(first_cpu); + + if (cpu->env.features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) { + const char *cpu_type = object_get_typename(OBJECT(first_cpu)); + int cpu_len = strlen(cpu_type) - strlen(X86_CPU_TYPE_SUFFIX); + + warn_report("Use of 64-bit CPU '%.*s' is deprecated" + " on the ISA-only PC machine", + cpu_len, cpu_type); + } + } if (kvm_enabled()) { kvmclock_create(pcmc->kvmclock_create_always); @@ -918,6 +931,7 @@ static void isapc_machine_options(MachineClass *m) pcmc->gigabyte_align = false; pcmc->smbios_legacy_mode = true; pcmc->has_reserved_memory = false; + pcmc->deprecate_64bit_cpu = true; m->default_nic = "ne2k_isa"; m->default_cpu_type = X86_CPU_TYPE_NAME("486"); m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
The logic around checking CPUID_EXT2_LM looks good to me. Slightly curious as to whether people feel updating PCMachineClass is necessary, or you can simply do qdev_get_machine() and use object_dynamic_cast() to see if the machine matches MACHINE_NAME("isapc") and warn that way?
Why don't you simply pass it as a parameter from pc_init_isa() instead? Or do the whole check in pc_init_isa() instead?
Because the CPU isn't instantiated so we can't check the CPUID_EXT2_LM feature :/
On Wed, Mar 27, 2024 at 05:54:56PM +0100, Philippe Mathieu-Daudé wrote:
Per Daniel suggestion [*]:
isapc could arguably be restricted to just 32-bit CPU models, because we should not need it to support any feature that didn't exist prior to circa 1995. eg refuse to start with isapc, if 'lm' is present in the CPU model for example.
Display a warning when such CPU is used:
$ qemu-system-x86_64 -monitor stdio -S -M isapc -cpu Westmere qemu-system-x86_64: warning: Use of 64-bit CPU 'Westmere' is deprecated on the ISA-only PC machine QEMU 8.2.91 monitor - type 'help' for more information (qemu) q
$ qemu-system-x86_64 -monitor stdio -S -M isapc -cpu athlon QEMU 8.2.91 monitor - type 'help' for more information (qemu) q
I've thought of a possible problem here.. $ qemu-system-x86_64 -monitor stdio -S -M isapc -cpu max is going to enable 'lm' (and a bazillion other new features) for 'isapc', which is a shame, as 'max' is something we want to be usable in general. I'm not sure how to square that circle. I might suggest that 'isapc' instead only makes sense in the context of qemu-system-i386, since that only has 32-bit CPUs. We wanted to kill that binary in favour of qemu-system-x86_64 for both 32 & 64 bit though, so we can't block 'isapc' from qemu-system-x86_64.
[*] https://lore.kernel.org/qemu-devel/ZgQkS4RPmSt5Xa08@redhat.com/
Suggested-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- docs/about/deprecated.rst | 7 +++++++ include/hw/i386/pc.h | 1 + hw/i386/pc_piix.c | 14 ++++++++++++++ 3 files changed, 22 insertions(+)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 7b548519b5..345c35507f 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -208,6 +208,13 @@ is no longer packaged in any distro making it harder to run the ``check-tcg`` tests. Unless we can improve the testing situation there is a chance the code will bitrot without anyone noticing.
+64-bit (x86_64) CPUs on the ``isapc`` machine (since 9.0) +''''''''''''''''''''''''''''''''''''''''''''''''''''''''' + +The ``isapc`` machine aims to emulate old PC machine without PCI was +generalized, so hardware available around 1995, before 64-bit intel +CPUs were produced. + System emulator machines ------------------------
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 27a68071d7..2d202b9549 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -96,6 +96,7 @@ struct PCMachineClass { const char *default_south_bridge;
/* Compat options: */ + bool deprecate_64bit_cpu; /* Specific to the 'isapc' machine */
/* Default CPU model version. See x86_cpu_set_default_version(). */ int default_cpu_version; diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 18ba076609..2e5b2efc33 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -182,7 +182,20 @@ static void pc_init1(MachineState *machine, const char *pci_type) }
pc_machine_init_sgx_epc(pcms); + x86_cpus_init(x86ms, pcmc->default_cpu_version); + if (pcmc->deprecate_64bit_cpu) { + X86CPU *cpu = X86_CPU(first_cpu); + + if (cpu->env.features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) { + const char *cpu_type = object_get_typename(OBJECT(first_cpu)); + int cpu_len = strlen(cpu_type) - strlen(X86_CPU_TYPE_SUFFIX); + + warn_report("Use of 64-bit CPU '%.*s' is deprecated" + " on the ISA-only PC machine", + cpu_len, cpu_type); + } + }
if (kvm_enabled()) { kvmclock_create(pcmc->kvmclock_create_always); @@ -918,6 +931,7 @@ static void isapc_machine_options(MachineClass *m) pcmc->gigabyte_align = false; pcmc->smbios_legacy_mode = true; pcmc->has_reserved_memory = false; + pcmc->deprecate_64bit_cpu = true; m->default_nic = "ne2k_isa"; m->default_cpu_type = X86_CPU_TYPE_NAME("486"); m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL); -- 2.41.0
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (5)
-
Daniel P. Berrangé -
Marcin Juszkiewicz -
Mark Cave-Ayland -
Philippe Mathieu-Daudé -
Thomas Huth