[pve-devel] [PATCH v4 qemu-server 10/12] Rework get_cpu_options and allow custom CPU models

Fabian Grünbichler f.gruenbichler at proxmox.com
Mon Oct 14 13:22:17 CEST 2019


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.reiter at 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 at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 




More information about the pve-devel mailing list