public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH qemu-server] Properly identify the CPU architecture of 32-bit VMs
@ 2023-12-11 14:12 Filip Schauer
  2023-12-11 14:37 ` Fiona Ebner
  0 siblings, 1 reply; 6+ messages in thread
From: Filip Schauer @ 2023-12-11 14:12 UTC (permalink / raw)
  To: pve-devel

Add an i386 CPU architecture which is used when a 32 bit CPU type is
detected.

This now prevents starting a 32-bit VM using a 64-bit OVMF BIOS. Instead
it throws the error, "no OVMF images known for architecture 'i386'",
triggered in get_ovmf_file. This behaviour is intended since we do not
support 32 bit OVMF images.

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 PVE/QemuServer.pm           |  8 +++++---
 PVE/QemuServer/CPUConfig.pm | 28 ++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 2063e66..54590df 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -52,7 +52,7 @@ use PVE::QemuConfig;
 use PVE::QemuServer::Helpers qw(config_aware_timeout min_version windows_version);
 use PVE::QemuServer::Cloudinit;
 use PVE::QemuServer::CGroup;
-use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options);
+use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options cpu_type_to_arch);
 use PVE::QemuServer::Drive qw(is_valid_drivename drive_is_cloudinit drive_is_cdrom drive_is_read_only parse_drive print_drive);
 use PVE::QemuServer::Machine;
 use PVE::QemuServer::Memory qw(get_current_memory);
@@ -649,7 +649,7 @@ EODESCR
 	description => "Virtual processor architecture. Defaults to the host.",
 	optional => 1,
 	type => 'string',
-	enum => [qw(x86_64 aarch64)],
+	enum => [qw(x86_64 i386 aarch64)],
     },
     smbios1 => {
 	description => "Specify SMBIOS type 1 fields.",
@@ -3293,11 +3293,12 @@ sub is_native($) {
 
 sub get_vm_arch {
     my ($conf) = @_;
-    return $conf->{arch} // get_host_arch();
+    return $conf->{arch} // cpu_type_to_arch($conf->{cpu}) // get_host_arch();
 }
 
 my $default_machines = {
     x86_64 => 'pc',
+    i386 => 'pc',
     aarch64 => 'virt',
 };
 
@@ -3390,6 +3391,7 @@ sub get_ovmf_files($$$) {
 
 my $Arch2Qemu = {
     aarch64 => '/usr/bin/qemu-system-aarch64',
+    i386 => '/usr/bin/qemu-system-i386',
     x86_64 => '/usr/bin/qemu-system-x86_64',
 };
 sub get_command_for_arch($) {
diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
index 750d3b6..cc71298 100644
--- a/PVE/QemuServer/CPUConfig.pm
+++ b/PVE/QemuServer/CPUConfig.pm
@@ -3,6 +3,8 @@ package PVE::QemuServer::CPUConfig;
 use strict;
 use warnings;
 
+use List::Util qw(first);
+
 use PVE::JSONSchema;
 use PVE::Cluster qw(cfs_register_file cfs_read_file);
 use PVE::QemuServer::Helpers qw(min_version);
@@ -12,6 +14,7 @@ use base qw(PVE::SectionConfig Exporter);
 our @EXPORT_OK = qw(
 print_cpu_device
 get_cpu_options
+cpu_type_to_arch
 );
 
 # under certain race-conditions, this module might be loaded before pve-cluster
@@ -57,6 +60,17 @@ my $depreacated_cpu_map = {
     'Icelake-Client-noTSX' => 'Icelake-Server-noTSX',
 };
 
+my @cpu_32bit_list = (
+    '486',
+    'pentium',
+    'pentium2',
+    'pentium3',
+    'coreduo',
+    'athlon',
+    'kvm32',
+    'qemu32',
+);
+
 my $cpu_vendor_list = {
     # Intel CPUs
     486 => 'GenuineIntel',
@@ -646,6 +660,20 @@ sub get_pve_cpu_flags {
     return $pve_flags;
 }
 
+sub cpu_type_to_arch {
+    my ($cputype) = @_;
+
+    if ($cputype) {
+	if (first {$_ eq $cputype} @cpu_32bit_list) {
+	    return 'i386';
+	} elsif ($cpu_vendor_list->{$cputype}) {
+	    return 'x86_64';
+	}
+    }
+
+    return undef;
+}
+
 sub get_hyperv_enlightenments {
     my ($winversion, $machine_version, $bios, $gpu_passthrough, $hv_vendor_id) = @_;
 
-- 
2.39.2





^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pve-devel] [PATCH qemu-server] Properly identify the CPU architecture of 32-bit VMs
  2023-12-11 14:12 [pve-devel] [PATCH qemu-server] Properly identify the CPU architecture of 32-bit VMs Filip Schauer
@ 2023-12-11 14:37 ` Fiona Ebner
  2023-12-12 10:39   ` Filip Schauer
  0 siblings, 1 reply; 6+ messages in thread
From: Fiona Ebner @ 2023-12-11 14:37 UTC (permalink / raw)
  To: Proxmox VE development discussion, Filip Schauer

Am 11.12.23 um 15:12 schrieb Filip Schauer:
> @@ -3293,11 +3293,12 @@ sub is_native($) {
>  
>  sub get_vm_arch {
>      my ($conf) = @_;
> -    return $conf->{arch} // get_host_arch();
> +    return $conf->{arch} // cpu_type_to_arch($conf->{cpu}) // get_host_arch();
>  }
>  
>  my $default_machines = {
>      x86_64 => 'pc',
> +    i386 => 'pc',
>      aarch64 => 'virt',
>  };
>  
> @@ -3390,6 +3391,7 @@ sub get_ovmf_files($$$) {
>  
>  my $Arch2Qemu = {
>      aarch64 => '/usr/bin/qemu-system-aarch64',
> +    i386 => '/usr/bin/qemu-system-i386',
>      x86_64 => '/usr/bin/qemu-system-x86_64',

Am I understanding correctly that you would automatically pick a
different binary depending on the CPU type? Does that actually work with
KVM and/or live migration, i.e. source was started with
qemu-system-x86_64 and now suddenly on the target with qemu-system-i386?

And even if it does, I'd rather not add support for running machines
with this binary. Especially if the only reason is to have this new
check. Once the new binary is there, we'd need to support it (even if
it's not prominently exposed).

If there is no easy way to do the check otherwise, I'd rather wait until
enough users complain and re-consider how to implement the check only then.




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pve-devel] [PATCH qemu-server] Properly identify the CPU architecture of 32-bit VMs
  2023-12-11 14:37 ` Fiona Ebner
@ 2023-12-12 10:39   ` Filip Schauer
  2023-12-12 11:48     ` Fiona Ebner
  0 siblings, 1 reply; 6+ messages in thread
From: Filip Schauer @ 2023-12-12 10:39 UTC (permalink / raw)
  To: Fiona Ebner, Proxmox VE development discussion

It's actually not a different binary. qemu-system-i386 is a symlink that
points to qemu-system-x86_64. But still this does indeed break migration
between a node that has this patch applied and another node without the
patch.

A patch v2 that only adds the check is available here:
https://lists.proxmox.com/pipermail/pve-devel/2023-December/061034.html

On 11/12/2023 15:37, Fiona Ebner wrote:
> Am 11.12.23 um 15:12 schrieb Filip Schauer:
>> @@ -3293,11 +3293,12 @@ sub is_native($) {
>>   
>>   sub get_vm_arch {
>>       my ($conf) = @_;
>> -    return $conf->{arch} // get_host_arch();
>> +    return $conf->{arch} // cpu_type_to_arch($conf->{cpu}) // get_host_arch();
>>   }
>>   
>>   my $default_machines = {
>>       x86_64 => 'pc',
>> +    i386 => 'pc',
>>       aarch64 => 'virt',
>>   };
>>   
>> @@ -3390,6 +3391,7 @@ sub get_ovmf_files($$$) {
>>   
>>   my $Arch2Qemu = {
>>       aarch64 => '/usr/bin/qemu-system-aarch64',
>> +    i386 => '/usr/bin/qemu-system-i386',
>>       x86_64 => '/usr/bin/qemu-system-x86_64',
> Am I understanding correctly that you would automatically pick a
> different binary depending on the CPU type? Does that actually work with
> KVM and/or live migration, i.e. source was started with
> qemu-system-x86_64 and now suddenly on the target with qemu-system-i386?
>
> And even if it does, I'd rather not add support for running machines
> with this binary. Especially if the only reason is to have this new
> check. Once the new binary is there, we'd need to support it (even if
> it's not prominently exposed).
>
> If there is no easy way to do the check otherwise, I'd rather wait until
> enough users complain and re-consider how to implement the check only then.




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pve-devel] [PATCH qemu-server] Properly identify the CPU architecture of 32-bit VMs
  2023-12-12 10:39   ` Filip Schauer
@ 2023-12-12 11:48     ` Fiona Ebner
  2023-12-13 17:31       ` Filip Schauer
  0 siblings, 1 reply; 6+ messages in thread
From: Fiona Ebner @ 2023-12-12 11:48 UTC (permalink / raw)
  To: Filip Schauer, Proxmox VE development discussion

Am 12.12.23 um 11:39 schrieb Filip Schauer:
> It's actually not a different binary. qemu-system-i386 is a symlink that
> points to qemu-system-x86_64. But still this does indeed break migration
> between a node that has this patch applied and another node without the
> patch.
> 

Oh, okay. But then that's a bit surprising. From a quick glance, we do
have some logic matching arch 'x86_64' specifically in CPUConfig.pm, so
that might be it. E.g.:

>     my $pve_forced_flags = {};
>     $pve_forced_flags->{'enforce'} = {
>         reason => "error if requested CPU settings not available",
>     } if $cputype ne 'host' && $kvm && $arch eq 'x86_64';




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pve-devel] [PATCH qemu-server] Properly identify the CPU architecture of 32-bit VMs
  2023-12-12 11:48     ` Fiona Ebner
@ 2023-12-13 17:31       ` Filip Schauer
  2023-12-14  9:13         ` Fiona Ebner
  0 siblings, 1 reply; 6+ messages in thread
From: Filip Schauer @ 2023-12-13 17:31 UTC (permalink / raw)
  To: Fiona Ebner, Proxmox VE development discussion

On 12/12/2023 12:48, Fiona Ebner wrote:
> Am 12.12.23 um 11:39 schrieb Filip Schauer:
>> It's actually not a different binary. qemu-system-i386 is a symlink that
>> points to qemu-system-x86_64. But still this does indeed break migration
>> between a node that has this patch applied and another node without the
>> patch.
>>
> Oh, okay. But then that's a bit surprising. From a quick glance, we do
> have some logic matching arch 'x86_64' specifically in CPUConfig.pm, so
> that might be it. E.g.:
>
>>      my $pve_forced_flags = {};
>>      $pve_forced_flags->{'enforce'} = {
>>          reason => "error if requested CPU settings not available",
>>      } if $cputype ne 'host' && $kvm && $arch eq 'x86_64';


This check does not make any difference in my case since $kvm is not set
when using a qemu32 CPU.

Not sure what causes this to break migration, not that it matters but
here are my results anyway.

Journal of an unpatched node when migrating a running VM with CPU type
qemu32 from a node with this patch v1 applied:

Dec 13 18:09:28 pve2 QEMU[124616]: KVM: entry failed, hardware error 
0x80000021
Dec 13 18:09:28 pve2 QEMU[124616]: If you're running a guest on an Intel 
machine without unrestricted mode
Dec 13 18:09:28 pve2 QEMU[124616]: support, the failure can be most 
likely due to the guest entering an invalid
Dec 13 18:09:28 pve2 QEMU[124616]: state for Intel VT. For example, the 
guest maybe running in big real mode
Dec 13 18:09:28 pve2 QEMU[124616]: which is not supported on less recent 
Intel processors.
Dec 13 18:09:28 pve2 QEMU[124616]: EAX=00003433 EBX=0006880c 
ECX=00002fa5 EDX=89817860
Dec 13 18:09:28 pve2 QEMU[124616]: ESI=898098c0 EDI=8980f758 
EBP=00183aec ESP=00183ad0
Dec 13 18:09:28 pve2 QEMU[124616]: EIP=00292afa EFL=00200006 [-----P-] 
CPL=0 II=0 A20=1 SMM=0 HLT=0
Dec 13 18:09:28 pve2 QEMU[124616]: ES =0030 00000000 ffffffff 00c09300 
DPL=0 DS   [-WA]
Dec 13 18:09:28 pve2 QEMU[124616]: CS =0020 00000000 ffffffff 00c09b00 
DPL=0 CS32 [-RA]
Dec 13 18:09:28 pve2 QEMU[124616]: SS =0030 00000000 ffffffff 00c09300 
DPL=0 DS   [-WA]
Dec 13 18:09:28 pve2 QEMU[124616]: DS =0030 00000000 ffffffff 00c09300 
DPL=0 DS   [-WA]
Dec 13 18:09:28 pve2 QEMU[124616]: FS =0060 00023de0 0000ffff 00009300 
DPL=0 DS16 [-WA]
Dec 13 18:09:28 pve2 QEMU[124616]: GS =0060 00023de0 0000ffff 00009300 
DPL=0 DS16 [-WA]
Dec 13 18:09:28 pve2 QEMU[124616]: LDT=0000 00000000 00000000 00008200 
DPL=0 LDT
Dec 13 18:09:28 pve2 QEMU[124616]: TR =0040 00025260 00000077 00008900 
DPL=0 TSS32-avl
Dec 13 18:09:28 pve2 QEMU[124616]: GDT=     00184000 0000007f
Dec 13 18:09:28 pve2 QEMU[124616]: IDT=     00184080 000007ff
Dec 13 18:09:28 pve2 QEMU[124616]: CR0=80000011 CR2=00000000 
CR3=00185000 CR4=00000000
Dec 13 18:09:28 pve2 kernel: set kvm_intel.dump_invalid_vmcs=1 to dump 
internal KVM state.
Dec 13 18:09:28 pve2 QEMU[124616]: DR0=0000000000000000 
DR1=0000000000000000 DR2=0000000000000000 DR3=0000000000000000
Dec 13 18:09:28 pve2 QEMU[124616]: DR6=00000000ffff0ff0 DR7=0000000000000400
Dec 13 18:09:28 pve2 QEMU[124616]: EFER=0000000000000000
Dec 13 18:09:28 pve2 QEMU[124616]: Code=0c 89 08 89 7a 0c 8b 45 0c 83 45 
10 02 8b c8 2b cf 23 4a 08 <8b> 3a 8a 1c 0f 88 1c 07 40 41 ff 4d 10 83 
7d 10 00 7f ed 3b 45 f4 8b 7d fc 89 45 0c 0f 8c


Journal of the patched node when migrating the same VM from an unpatched
node:

Dec 13 18:12:17 pve1 QEMU[125150]: qemu-system-i386: Unknown savevm 
section or instance 'kvmclock' 0. Make sure that your current VM setup 
matches your saved VM setup, including any hotplugged devices
Dec 13 18:12:17 pve1 QEMU[125150]: qemu-system-i386: load of migration 
failed: Invalid argument





^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pve-devel] [PATCH qemu-server] Properly identify the CPU architecture of 32-bit VMs
  2023-12-13 17:31       ` Filip Schauer
@ 2023-12-14  9:13         ` Fiona Ebner
  0 siblings, 0 replies; 6+ messages in thread
From: Fiona Ebner @ 2023-12-14  9:13 UTC (permalink / raw)
  To: Filip Schauer, Proxmox VE development discussion

Am 13.12.23 um 18:31 schrieb Filip Schauer:
> On 12/12/2023 12:48, Fiona Ebner wrote:
>> Am 12.12.23 um 11:39 schrieb Filip Schauer:
>>> It's actually not a different binary. qemu-system-i386 is a symlink that
>>> points to qemu-system-x86_64. But still this does indeed break migration
>>> between a node that has this patch applied and another node without the
>>> patch.
>>>
>> Oh, okay. But then that's a bit surprising. From a quick glance, we do
>> have some logic matching arch 'x86_64' specifically in CPUConfig.pm, so
>> that might be it. E.g.:
>>
>>>      my $pve_forced_flags = {};
>>>      $pve_forced_flags->{'enforce'} = {
>>>          reason => "error if requested CPU settings not available",
>>>      } if $cputype ne 'host' && $kvm && $arch eq 'x86_64';
> 
> 
> This check does not make any difference in my case since $kvm is not set
> when using a qemu32 CPU.
> 

Well, that is just one example for such a flag, there are others ;)

From a quick look, if kvm is not explicitly turned off in the config and
if you are not using arch i386/aarch64 in the config, the $kvm option
will get set. It does not seem to depend on the CPU type:

> sub is_native($) {
>     my ($arch) = @_;
>     return get_host_arch() eq $arch;
> }   
>                         
> sub get_vm_arch {
>     my ($conf) = @_;
>     return $conf->{arch} // get_host_arch();
> }   

> sub config_to_command {

...

>     my $kvm = $conf->{kvm};
>     my $nodename = nodename();
>     
>     my $arch = get_vm_arch($conf);

...

>     $kvm //= 1 if is_native($arch);

If you really want to know what causes the issue, you can compare the
QEMU commandline on source and target of the migration.




^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-12-14  9:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-11 14:12 [pve-devel] [PATCH qemu-server] Properly identify the CPU architecture of 32-bit VMs Filip Schauer
2023-12-11 14:37 ` Fiona Ebner
2023-12-12 10:39   ` Filip Schauer
2023-12-12 11:48     ` Fiona Ebner
2023-12-13 17:31       ` Filip Schauer
2023-12-14  9:13         ` Fiona Ebner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal