* [PATCH-for-9.0 v2] hw/i386/pc: Deprecate 64-bit CPUs on ISA-only PC machine
@ 2024-03-27 16:54 Philippe Mathieu-Daudé
2024-03-27 17:29 ` Marcin Juszkiewicz
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-03-27 16:54 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Igor Mammedov, Marcel Apfelbaum, Gerd Hoffmann,
Paolo Bonzini, Michael S. Tsirkin, Mark Cave-Ayland, devel,
Richard Henderson, Eduardo Habkost, Philippe Mathieu-Daudé,
Daniel P . Berrangé
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH-for-9.0 v2] hw/i386/pc: Deprecate 64-bit CPUs on ISA-only PC machine
2024-03-27 16:54 [PATCH-for-9.0 v2] hw/i386/pc: Deprecate 64-bit CPUs on ISA-only PC machine Philippe Mathieu-Daudé
@ 2024-03-27 17:29 ` Marcin Juszkiewicz
2024-03-27 18:29 ` Philippe Mathieu-Daudé
2024-03-28 15:12 ` Mark Cave-Ayland
2024-03-28 15:15 ` Daniel P. Berrangé
2 siblings, 1 reply; 7+ messages in thread
From: Marcin Juszkiewicz @ 2024-03-27 17:29 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Thomas Huth, Igor Mammedov, Marcel Apfelbaum, Gerd Hoffmann,
Paolo Bonzini, Michael S. Tsirkin, Mark Cave-Ayland, devel,
Richard Henderson, Eduardo Habkost, Daniel P . Berrangé
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH-for-9.0 v2] hw/i386/pc: Deprecate 64-bit CPUs on ISA-only PC machine
2024-03-27 17:29 ` Marcin Juszkiewicz
@ 2024-03-27 18:29 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-03-27 18:29 UTC (permalink / raw)
To: Marcin Juszkiewicz, qemu-devel
Cc: Thomas Huth, Igor Mammedov, Marcel Apfelbaum, Gerd Hoffmann,
Paolo Bonzini, Michael S. Tsirkin, Mark Cave-Ayland, devel,
Richard Henderson, Eduardo Habkost, Daniel P . Berrangé
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 ;)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH-for-9.0 v2] hw/i386/pc: Deprecate 64-bit CPUs on ISA-only PC machine
2024-03-27 16:54 [PATCH-for-9.0 v2] hw/i386/pc: Deprecate 64-bit CPUs on ISA-only PC machine Philippe Mathieu-Daudé
2024-03-27 17:29 ` Marcin Juszkiewicz
@ 2024-03-28 15:12 ` Mark Cave-Ayland
2024-03-28 15:39 ` Thomas Huth
2024-03-28 15:15 ` Daniel P. Berrangé
2 siblings, 1 reply; 7+ messages in thread
From: Mark Cave-Ayland @ 2024-03-28 15:12 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Thomas Huth, Igor Mammedov, Marcel Apfelbaum, Gerd Hoffmann,
Paolo Bonzini, Michael S. Tsirkin, devel, Richard Henderson,
Eduardo Habkost, Daniel P . Berrangé
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH-for-9.0 v2] hw/i386/pc: Deprecate 64-bit CPUs on ISA-only PC machine
2024-03-27 16:54 [PATCH-for-9.0 v2] hw/i386/pc: Deprecate 64-bit CPUs on ISA-only PC machine Philippe Mathieu-Daudé
2024-03-27 17:29 ` Marcin Juszkiewicz
2024-03-28 15:12 ` Mark Cave-Ayland
@ 2024-03-28 15:15 ` Daniel P. Berrangé
2 siblings, 0 replies; 7+ messages in thread
From: Daniel P. Berrangé @ 2024-03-28 15:15 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Thomas Huth, Igor Mammedov, Marcel Apfelbaum,
Gerd Hoffmann, Paolo Bonzini, Michael S. Tsirkin,
Mark Cave-Ayland, devel, Richard Henderson, Eduardo Habkost
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 :|
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH-for-9.0 v2] hw/i386/pc: Deprecate 64-bit CPUs on ISA-only PC machine
2024-03-28 15:12 ` Mark Cave-Ayland
@ 2024-03-28 15:39 ` Thomas Huth
2024-03-29 10:26 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 7+ messages in thread
From: Thomas Huth @ 2024-03-28 15:39 UTC (permalink / raw)
To: Mark Cave-Ayland, Philippe Mathieu-Daudé, qemu-devel
Cc: Igor Mammedov, Marcel Apfelbaum, Gerd Hoffmann, Paolo Bonzini,
Michael S. Tsirkin, devel, Richard Henderson, Eduardo Habkost,
Daniel P . Berrangé
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH-for-9.0 v2] hw/i386/pc: Deprecate 64-bit CPUs on ISA-only PC machine
2024-03-28 15:39 ` Thomas Huth
@ 2024-03-29 10:26 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-03-29 10:26 UTC (permalink / raw)
To: Thomas Huth, Mark Cave-Ayland, qemu-devel
Cc: Igor Mammedov, Marcel Apfelbaum, Gerd Hoffmann, Paolo Bonzini,
Michael S. Tsirkin, devel, Richard Henderson, Eduardo Habkost,
Daniel P . Berrangé
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 :/
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-03-29 10:27 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-27 16:54 [PATCH-for-9.0 v2] hw/i386/pc: Deprecate 64-bit CPUs on ISA-only PC machine Philippe Mathieu-Daudé
2024-03-27 17:29 ` Marcin Juszkiewicz
2024-03-27 18:29 ` Philippe Mathieu-Daudé
2024-03-28 15:12 ` Mark Cave-Ayland
2024-03-28 15:39 ` Thomas Huth
2024-03-29 10:26 ` Philippe Mathieu-Daudé
2024-03-28 15:15 ` Daniel P. Berrangé
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).