On 03.09.21 21:48, Eduardo Habkost wrote:
On Fri, Sep 03, 2021 at 03:08:03PM +0200, Juergen Gross wrote:Today the maximum vcpu-id of a kvm guest's vcpu on x86 systems is set via a #define in a header file. In order to support higher vcpu-ids without generally increasing the memory consumption of guests on the host (some guest structures contain arrays sized by KVM_MAX_VCPU_ID) add a boot parameter for adding some bits to the vcpu-id. Additional bits are needed as the vcpu-id is constructed via bit-wise concatenation of socket-id, core-id, etc. As those ids maximum values are not always a power of 2, the vcpu-ids are sparse. The additional number of bits needed is basically the number of topology levels with a non-power-of-2 maximum value, excluding the top most level. The default value of the new parameter will be to take the correct setting from the host's topology.Having the default depend on the host topology makes the host behaviour unpredictable (which might be a problem when migrating VMs from another host with a different topology). Can't we just default to 2?
Okay, fine with me.
Calculating the maximum vcpu-id dynamically requires to allocate the arrays using KVM_MAX_VCPU_ID as the size dynamically. Signed-of-by: Juergen Gross <jgross@xxxxxxxx> --- V2: - switch to specifying additional bits (based on comment by Vitaly Kuznetsov) Signed-off-by: Juergen Gross <jgross@xxxxxxxx> ---[...]#define KVM_MAX_VCPUS 288 #define KVM_SOFT_MAX_VCPUS 240 -#define KVM_MAX_VCPU_ID 1023 +#define KVM_MAX_VCPU_ID kvm_max_vcpu_id()[...]+unsigned int kvm_max_vcpu_id(void) +{ + int n_bits = fls(KVM_MAX_VCPUS - 1); + + if (vcpu_id_add_bits < -1 || vcpu_id_add_bits > (32 - n_bits)) { + pr_err("Invalid value of vcpu_id_add_bits=%d parameter!\n", + vcpu_id_add_bits); + vcpu_id_add_bits = -1; + } + + if (vcpu_id_add_bits >= 0) { + n_bits += vcpu_id_add_bits; + } else { + n_bits++; /* One additional bit for core level. */ + if (topology_max_die_per_package() > 1) + n_bits++; /* One additional bit for die level. */ + } + + if (!n_bits) + n_bits = 1; + + return (1U << n_bits) - 1;The largest possible VCPU ID is not KVM_MAX_VCPU_ID, it's (KVM_MAX_VCPU_ID - 1). This is enforced by kvm_vm_ioctl_create_vcpu(). That would mean KVM_MAX_VCPU_ID should be (1 << n_bits) instead of ((1 << n_bits) - 1), wouldn't it?
Oh, indeed. I have been fooled by the IMO bad naming of this macro. The current value 1023 suggests it is not only me having been fooled. Shouldn't it be named "KVM_MAX_VCPU_IDS" instead? Juergen
Attachment:
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
