From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <f.ebner@proxmox.com>
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 <pve-devel@lists.proxmox.com>; 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 <pve-devel@lists.proxmox.com>; 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 <pve-devel@lists.proxmox.com>; 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 <pve-devel@lists.proxmox.com>; 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 <pve-devel@lists.proxmox.com>,
 Filip Schauer <f.schauer@proxmox.com>
References: <20231214110903.32416-1-f.schauer@proxmox.com>
Content-Language: en-US
From: Fiona Ebner <f.ebner@proxmox.com>
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 <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=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.