From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 90D91BAB3B for ; Fri, 15 Dec 2023 11:08:39 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 539F33B4 for ; Fri, 15 Dec 2023 11:08:34 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Fri, 15 Dec 2023 11:08:33 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 7D8D14797D for ; Fri, 15 Dec 2023 11:08:33 +0100 (CET) Message-ID: <21e5a2e9-b646-4352-85ff-ae17e71b9eac@proxmox.com> Date: Fri, 15 Dec 2023 11:08:29 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Proxmox VE development discussion , Filip Schauer References: <20231214110903.32416-1-f.schauer@proxmox.com> Content-Language: en-US From: Fiona Ebner In-Reply-To: <20231214110903.32416-1-f.schauer@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.076 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pve-devel] [PATCH v6 qemu-server] Prevent starting a 32-bit VM using a 64-bit OVMF BIOS X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 15 Dec 2023 10:08:39 -0000 Am 14.12.23 um 12:09 schrieb Filip Schauer:> @@ -3689,6 +3689,18 @@ sub config_to_command { > } > > if ($conf->{bios} && $conf->{bios} eq 'ovmf') { > + my $cputype; > + > + if ($conf->{cpu}) { > + my $cpu = PVE::JSONSchema::parse_property_string('pve-vm-cpu-conf', $conf->{cpu}) > + or die "Cannot parse cpu description: $conf->{cpu}\n"; > + > + $cputype = $cpu->{cputype}; > + } > + > + die "OVMF (UEFI) BIOS is not supported on 32-bit CPU types\n" > + if get_cpu_bitness($cputype, $arch) == 32; > + If $forcecpu is set, we should probably just skip the check. > @@ -719,6 +731,18 @@ sub get_cpu_from_running_vm { > return $1; > } > > +sub get_cpu_bitness { > + my ($cputype, $arch) = @_; > + Sorry, I only noticed this now, but the cputype can also be a custom-xyz one. And then you need to look at the reported model for the check (compare with what print_cpu_device() and get_cpu_options() do). So I think it's best to pass along the property string and do all the parsing and figuring out what the actual cpu type is here. > + die "missing 'arch'\n" if !$arch; > + $cputype = $cpu_fmt->{'cputype'}->{'default'} if !$cputype; Turns out, the default is actually not this :/ Again, comparing with get_cpu_options(): my $cputype = $kvm ? "kvm64" : "qemu64"; if ($arch eq 'aarch64') { $cputype = 'cortex-a57'; } Doesn't matter for your function, because all are 64 bit, but maybe we want to be explicit about it. Seems like the above should become a helper for getting the default CPU type. There's two small pre-existing bugs: 1. The schema claims that the default CPU type is 'kvm64' which we've already seen is not always true. And print_cpu_device() doesn't use the same default CPU type for aarch64 as get_cpu_options() does. Now, starting qemu-system-aarch64 with kvm64 or qemu64 CPU type doesn't work anyways so I think we can just make print_cpu_device() compatible with get_cpu_options() by using the new helper. Would be great if you could incorporate that into a patch series. The following is independent and deserves more discussion: 2. The schema claims that the default for 'kvm' is 1, but config_to_command() uses: $kvm //= 1 if is_native($arch); While print_cpu_device() uses: my $kvm = $conf->{kvm} // 1; Hot-plugging with aarch64 doesn't work at the moment anyways: # FIXME: hot plugging other architectures like our unofficial arch64 support? so we only need to care about the case when having a different host arch and emulating an x86_64 VM. But there it does make a difference and would be a breaking change because the default for hotplug would switch from 'kvm64' to 'qemu64'. Question is if we care enough. It's not officially supported to run Proxmox VE on different architectures at the moment and IMHO it'd be better to break it now (or with Proxmox VE 9) and be consistent for the future. And if we don't want to break it, add a comment for it.