On October 7, 2019 2:47 pm, Stefan Reiter wrote:
> If a cputype is custom (check via prefix), try to load options from the
> custom CPU model config, and set values accordingly.
>
> While at it, extract currently hardcoded values into seperate sub and add
> reasonings.
>
> Since the new flag resolving outputs flags in sorted order for
> consistency, adapt the test cases to not break. Only the order is
> changed, not which flags are present.
>
> Signed-off-by: Stefan Reiter <s.rei...@proxmox.com>
> ---
>
> v3 -> v4:
> * use is_custom_model
>
> v3: Since it's just a few lines, I included the test changes into this patch
> directly. This way all patches in the series should be buildable without
> problems.
>
> v2: It was quite interesting to dig through old commit messages/mail archives
> to
> find the actual reasons some of the hardcoded flags are included. I do feel
> like
> the "reason" field is quite useful though, both for future developers and
> users.
>
> PVE/QemuServer/CPUConfig.pm | 185 +++++++++++++++------
> test/cfg2cmd/i440fx-win10-hostpci.conf.cmd | 2 +-
> test/cfg2cmd/minimal-defaults.conf.cmd | 2 +-
> test/cfg2cmd/q35-linux-hostpci.conf.cmd | 2 +-
> test/cfg2cmd/q35-win10-hostpci.conf.cmd | 2 +-
> test/cfg2cmd/simple1.conf.cmd | 2 +-
> test/cfg2cmd/spice-usb3.conf.cmd | 2 +-
> 7 files changed, 144 insertions(+), 53 deletions(-)
>
> diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
> index 405ad86..0948f67 100644
> --- a/PVE/QemuServer/CPUConfig.pm
> +++ b/PVE/QemuServer/CPUConfig.pm
> @@ -369,96 +369,187 @@ sub print_cpuflag_hash {
> sub get_cpu_options {
> my ($conf, $arch, $kvm, $machine_type, $kvm_off, $kvmver, $winversion,
> $gpu_passthrough) = @_;
>
> - my $cpuFlags = [];
> - my $ostype = $conf->{ostype};
> -
> - my $cpu = $kvm ? "kvm64" : "qemu64";
> + my $cputype = $kvm ? "kvm64" : "qemu64";
> if ($arch eq 'aarch64') {
> - $cpu = 'cortex-a57';
> + $cputype = 'cortex-a57';
> }
> +
> + my $cpuconf = {};
> + my $custom_cpuconf;
bad naming:
$cpu (further below)
$cpuconf
$custom_cpuconf
$custom_conf (a bit below)
maybe
$cpu -> $cpu_str (string that is generated to be passed via -cpu)
$cpuconf -> $cpu
$custom_cpuconf -> $custom_cpu
$custom_conf -> inline, maybe even into get_model_by_name ?
> my $hv_vendor_id;
> - if (my $cputype = $conf->{cpu}) {
> - my $cpuconf = PVE::JSONSchema::parse_property_string($cpu_fmt, $cputype)
> - or die "Cannot parse cpu description: $cputype\n";
> - $cpu = $cpuconf->{cputype};
> - $kvm_off = 1 if $cpuconf->{hidden};
> - $hv_vendor_id = $cpuconf->{'hv-vendor-id'};
> + if (my $cpu_prop_str = $conf->{cpu}) {
> + $cpuconf = verify_vm_cpu_conf($cpu_prop_str)
> + or die "Cannot parse cpu description: $cpu_prop_str\n";
s/verify_vm_cpu_conf/parse_cpu_conf_basic ?
>
> - if (defined(my $flags = $cpuconf->{flags})) {
> - push @$cpuFlags, split(";", $flags);
> - }
> - }
> + $cputype = $cpuconf->{cputype};
>
> - push @$cpuFlags , '+lahf_lm' if $cpu eq 'kvm64' && $arch eq 'x86_64';
> + if (is_custom_model($cputype)) {
> + my $custom_conf = load_custom_model_conf();
> + $custom_cpuconf = get_model_by_name($custom_conf, $cputype)
> + or die "invalid custom model definition for '$cputype'\n";
>
> - push @$cpuFlags , '-x2apic'
> - if $conf->{ostype} && $conf->{ostype} eq 'solaris';
> + $cputype = $custom_cpuconf->{'reported-model'};
> + $kvm_off = $custom_cpuconf->{hidden};
this is a bit tricky - the schema default is 0, get_model_by_name
applies this default, and now we override the passed-in explicit value
with the default value from the custom CPU schema.
maybe drop the default and add an 'if defined' here?
> + $hv_vendor_id = $custom_cpuconf->{'hv-vendor-id'};
> + }
>
> - push @$cpuFlags, '+sep' if $cpu eq 'kvm64' || $cpu eq 'kvm32';
> + # VM-specific settings override custom CPU config
> + $kvm_off = $cpuconf->{hidden}
> + if defined($cpuconf->{hidden});
> + $hv_vendor_id = $cpuconf->{'hv-vendor-id'}
> + if defined($cpuconf->{'hv-vendor-id'});
> + }
>
> - push @$cpuFlags, '-rdtscp' if $cpu =~ m/^Opteron/;
> + my $pve_flags = get_pve_cpu_flags($conf, $kvm, $cputype, $arch,
> + $machine_type, $kvmver);
>
> - if (qemu_machine_feature_enabled ($machine_type, $kvmver, 2, 3) && $arch
> eq 'x86_64') {
> + my $hv_flags = get_hyperv_enlightenments($winversion, $machine_type,
> $kvmver,
> + $conf->{bios}, $gpu_passthrough, $hv_vendor_id) if $kvm;
>
> - push @$cpuFlags , '+kvm_pv_unhalt' if $kvm;
> - push @$cpuFlags , '+kvm_pv_eoi' if $kvm;
> + my $custom_cputype_flags = {};
> + if ($custom_cpuconf && defined($custom_cpuconf->{flags})) {
> + foreach my $flag (split(";", $custom_cpuconf->{flags})) {
> + $flag =~ m/^([+-])(.*)$/;
> + $custom_cputype_flags->{$2} = {
> + op => $1,
> + reason => "set by custom CPU model",
> + }
> + }
this
> }
>
> - add_hyperv_enlightenments($cpuFlags, $winversion, $machine_type,
> $kvmver, $conf->{bios}, $gpu_passthrough, $hv_vendor_id) if $kvm;
> -
> - push @$cpuFlags, 'enforce' if $cpu ne 'host' && $kvm && $arch eq
> 'x86_64';
> -
> - push @$cpuFlags, 'kvm=off' if $kvm_off;
> + my $vm_flags = {};
> + if (defined(my $flags = $cpuconf->{flags})) {
> + foreach my $flag (split(";", $flags)) {
> + if ($flag =~ $cpu_flag_re) {
> + $vm_flags->{$2} = {
> + op => $1,
> + reason => "manually set for VM",
> + }
> + }
> + }
and this do the same thing:
sub parse_cpuflag_list {
my ($re, $reason, $flaglist) = @_;
my $res = {};
foreach my $flag (split(";", $flaglist)) {
if ($flag =~ $re) {
$res->{$2} = { op => $1, reason => $reason };
}
}
}
just with different parameters. also, the RE in the first case is not
the one used in the schema (which is not wrong per se, but is a bit
confusing at first glance).
> + }
>
> - if (my $cpu_vendor = $cpu_vendor_list->{$cpu}) {
> - push @$cpuFlags, "vendor=${cpu_vendor}"
> - if $cpu_vendor ne 'default';
> + 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';
> + $pve_forced_flags->{'kvm'} = {
> + value => "off",
> + reason => "hide KVM virtualization from guest",
> + } if $kvm_off;
> +
> + # $cputype is the "reported-model" for custom types, so we can just look
> up
> + # the vendor in the default list
> + my $cpu_vendor = $cpu_vendor_list->{$cputype};
> + if ($cpu_vendor) {
> + $pve_forced_flags->{'vendor'} = {
> + value => $cpu_vendor,
> + } if $cpu_vendor ne 'default';
> } elsif ($arch ne 'aarch64') {
> die "internal error"; # should not happen
> }
>
> - $cpu .= "," . join(',', @$cpuFlags) if scalar(@$cpuFlags);
> + my $cpu = $cputype;
> +
> + # will be resolved in parameter order
> + $cpu .= resolve_cpu_flags($pve_flags, $hv_flags, $custom_cputype_flags,
> + $vm_flags, $pve_forced_flags);
>
> return ('-cpu', $cpu);
> }
>
> -sub add_hyperv_enlightenments {
> - my ($cpuFlags, $winversion, $machine_type, $kvmver, $bios,
> $gpu_passthrough, $hv_vendor_id) = @_;
> +# Some hardcoded flags required by certain configurations
> +sub get_pve_cpu_flags {
> + my ($conf, $kvm, $cputype, $arch, $machine_type, $kvmver) = @_;
> +
> + my $pve_flags = {};
> + my $pve_msg = "set by PVE;";
> +
> + $pve_flags->{'lahf_lm'} = {
> + op => '+',
> + reason => "$pve_msg to support Windows 8.1+",
> + } if $cputype eq 'kvm64' && $arch eq 'x86_64';
> +
> + $pve_flags->{'x2apic'} = {
> + op => '-',
> + reason => "$pve_msg incompatible with Solaris",
> + } if $conf->{ostype} && $conf->{ostype} eq 'solaris';
> +
> + $pve_flags->{'sep'} = {
> + op => '+',
> + reason => "$pve_msg to support Windows 8+ and improve Windows XP+",
> + } if $cputype eq 'kvm64' || $cputype eq 'kvm32';
> +
> + $pve_flags->{'rdtscp'} = {
> + op => '-',
> + reason => "$pve_msg broken on AMD Opteron",
> + } if $cputype =~ m/^Opteron/;
> +
> + if (qemu_machine_feature_enabled ($machine_type, $kvmver, 2, 3)
> + && $arch eq 'x86_64' && $kvm) {
> + $pve_flags->{'kvm_pv_unhalt'} = {
> + op => '+',
> + reason => "$pve_msg improves Linux guest spinlock performance",
'to improve' to be consistent with 'to support'
> + };
> + $pve_flags->{'kvm_pv_eoi'} = {
> + op => '+',
> + reason => "$pve_msg improves Linux guest interrupt performance",
same
> + };
> + }
> +
> + return $pve_flags;
> +}
> +
> +sub get_hyperv_enlightenments {
> + my ($winversion, $machine_type, $kvmver, $bios, $gpu_passthrough,
> $hv_vendor_id) = @_;
>
> return if $winversion < 6;
> return if $bios && $bios eq 'ovmf' && $winversion < 8;
>
> - if ($gpu_passthrough || defined($hv_vendor_id)) {
> + my $flags = {};
> + my $flagfn = sub {
> + my ($flag, $value, $reason) = @_;
> + $flags->{$flag} = {
> + reason => $reason // "automatic Hyper-V enlightenment for Windows",
> + value => $value,
> + }
> + };
> +
> + my $hv_vendor_set = defined($hv_vendor_id);
> + if ($gpu_passthrough || $hv_vendor_set) {
> $hv_vendor_id //= 'proxmox';
> - push @$cpuFlags , "hv_vendor_id=$hv_vendor_id";
> + $flagfn->('hv_vendor_id', $hv_vendor_id, $hv_vendor_set ?
> + "custom hv_vendor_id set" : "NVIDIA workaround for GPU
> passthrough");
> }
>
> if (qemu_machine_feature_enabled ($machine_type, $kvmver, 2, 3)) {
> - push @$cpuFlags , 'hv_spinlocks=0x1fff';
> - push @$cpuFlags , 'hv_vapic';
> - push @$cpuFlags , 'hv_time';
> + $flagfn->('hv_spinlocks', '0x1fff');
> + $flagfn->('hv_vapic');
> + $flagfn->('hv_time');
> } else {
> - push @$cpuFlags , 'hv_spinlocks=0xffff';
> + $flagfn->('hv_spinlocks', '0xffff');
> }
>
> if (qemu_machine_feature_enabled ($machine_type, $kvmver, 2, 6)) {
> - push @$cpuFlags , 'hv_reset';
> - push @$cpuFlags , 'hv_vpindex';
> - push @$cpuFlags , 'hv_runtime';
> + $flagfn->('hv_reset');
> + $flagfn->('hv_vpindex');
> + $flagfn->('hv_runtime');
> }
>
> if ($winversion >= 7) {
could maybe add something about the windows version to the reason in
this branch?
> - push @$cpuFlags , 'hv_relaxed';
> + $flagfn->('hv_relaxed');
>
> if (qemu_machine_feature_enabled ($machine_type, $kvmver, 2, 12)) {
> - push @$cpuFlags , 'hv_synic';
> - push @$cpuFlags , 'hv_stimer';
> + $flagfn->('hv_synic');
> + $flagfn->('hv_stimer');
> }
>
> if (qemu_machine_feature_enabled ($machine_type, $kvmver, 3, 1)) {
> - push @$cpuFlags , 'hv_ipi';
> + $flagfn->('hv_ipi');
> }
> }
> +
> + return $flags;
> }
>
> sub qemu_machine_feature_enabled {
> diff --git a/test/cfg2cmd/i440fx-win10-hostpci.conf.cmd
> b/test/cfg2cmd/i440fx-win10-hostpci.conf.cmd
> index ff5d635..2a9174d 100644
> --- a/test/cfg2cmd/i440fx-win10-hostpci.conf.cmd
> +++ b/test/cfg2cmd/i440fx-win10-hostpci.conf.cmd
> @@ -15,7 +15,7 @@
> -boot
> 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg'
> \
> -vnc unix:/var/run/qemu-server/8006.vnc,password \
> -no-hpet \
> - -cpu
> 'kvm64,+lahf_lm,+sep,+kvm_pv_unhalt,+kvm_pv_eoi,hv_spinlocks=0x1fff,hv_vapic,hv_time,hv_reset,hv_vpindex,hv_runtime,hv_relaxed,hv_synic,hv_stimer,hv_ipi,enforce'
> \
> + -cpu
> 'kvm64,enforce,hv_ipi,hv_relaxed,hv_reset,hv_runtime,hv_spinlocks=0x1fff,hv_stimer,hv_synic,hv_time,hv_vapic,hv_vpindex,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep'
> \
> -m 512 \
> -object 'memory-backend-ram,id=ram-node0,size=256M' \
> -numa 'node,nodeid=0,cpus=0,memdev=ram-node0' \
> diff --git a/test/cfg2cmd/minimal-defaults.conf.cmd
> b/test/cfg2cmd/minimal-defaults.conf.cmd
> index 5abebe9..444050b 100644
> --- a/test/cfg2cmd/minimal-defaults.conf.cmd
> +++ b/test/cfg2cmd/minimal-defaults.conf.cmd
> @@ -12,7 +12,7 @@
> -nodefaults \
> -boot
> 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg'
> \
> -vnc unix:/var/run/qemu-server/8006.vnc,password \
> - -cpu kvm64,+lahf_lm,+sep,+kvm_pv_unhalt,+kvm_pv_eoi,enforce \
> + -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
> -m 512 \
> -device 'pci-bridge,id=pci.1,chassis_nr=1,bus=pci.0,addr=0x1e' \
> -device 'pci-bridge,id=pci.2,chassis_nr=2,bus=pci.0,addr=0x1f' \
> diff --git a/test/cfg2cmd/q35-linux-hostpci.conf.cmd
> b/test/cfg2cmd/q35-linux-hostpci.conf.cmd
> index 21fb18b..2072295 100644
> --- a/test/cfg2cmd/q35-linux-hostpci.conf.cmd
> +++ b/test/cfg2cmd/q35-linux-hostpci.conf.cmd
> @@ -14,7 +14,7 @@
> -nodefaults \
> -boot
> 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg'
> \
> -vnc unix:/var/run/qemu-server/8006.vnc,password \
> - -cpu kvm64,+lahf_lm,+sep,+kvm_pv_unhalt,+kvm_pv_eoi,enforce \
> + -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
> -m 512 \
> -object 'memory-backend-ram,id=ram-node0,size=256M' \
> -numa 'node,nodeid=0,cpus=0,memdev=ram-node0' \
> diff --git a/test/cfg2cmd/q35-win10-hostpci.conf.cmd
> b/test/cfg2cmd/q35-win10-hostpci.conf.cmd
> index f2c08ca..81e43d4 100644
> --- a/test/cfg2cmd/q35-win10-hostpci.conf.cmd
> +++ b/test/cfg2cmd/q35-win10-hostpci.conf.cmd
> @@ -15,7 +15,7 @@
> -boot
> 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg'
> \
> -vnc unix:/var/run/qemu-server/8006.vnc,password \
> -no-hpet \
> - -cpu
> 'kvm64,+lahf_lm,+sep,+kvm_pv_unhalt,+kvm_pv_eoi,hv_spinlocks=0x1fff,hv_vapic,hv_time,hv_reset,hv_vpindex,hv_runtime,hv_relaxed,hv_synic,hv_stimer,hv_ipi,enforce'
> \
> + -cpu
> 'kvm64,enforce,hv_ipi,hv_relaxed,hv_reset,hv_runtime,hv_spinlocks=0x1fff,hv_stimer,hv_synic,hv_time,hv_vapic,hv_vpindex,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep'
> \
> -m 512 \
> -object 'memory-backend-ram,id=ram-node0,size=256M' \
> -numa 'node,nodeid=0,cpus=0,memdev=ram-node0' \
> diff --git a/test/cfg2cmd/simple1.conf.cmd b/test/cfg2cmd/simple1.conf.cmd
> index b5c06cf..3485064 100644
> --- a/test/cfg2cmd/simple1.conf.cmd
> +++ b/test/cfg2cmd/simple1.conf.cmd
> @@ -12,7 +12,7 @@
> -nodefaults \
> -boot
> 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg'
> \
> -vnc unix:/var/run/qemu-server/8006.vnc,password \
> - -cpu kvm64,+lahf_lm,+sep,+kvm_pv_unhalt,+kvm_pv_eoi,enforce \
> + -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
> -m 768 \
> -device 'pci-bridge,id=pci.1,chassis_nr=1,bus=pci.0,addr=0x1e' \
> -device 'pci-bridge,id=pci.2,chassis_nr=2,bus=pci.0,addr=0x1f' \
> diff --git a/test/cfg2cmd/spice-usb3.conf.cmd
> b/test/cfg2cmd/spice-usb3.conf.cmd
> index 680fa64..66b4e8d 100644
> --- a/test/cfg2cmd/spice-usb3.conf.cmd
> +++ b/test/cfg2cmd/spice-usb3.conf.cmd
> @@ -12,7 +12,7 @@
> -nodefaults \
> -boot
> 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg'
> \
> -vnc unix:/var/run/qemu-server/8006.vnc,password \
> - -cpu kvm64,+lahf_lm,+sep,+kvm_pv_unhalt,+kvm_pv_eoi,enforce \
> + -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
> -m 768 \
> -device 'pci-bridge,id=pci.2,chassis_nr=2,bus=pci.0,addr=0x1f' \
> -device 'pci-bridge,id=pci.1,chassis_nr=1,bus=pci.0,addr=0x1e' \
> --
> 2.20.1
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
