Re: thread/core siblings info for guests

[Avi Kivity <avi@redhat.com>]

Nitin A Kamble wrote:
> Avi & Alex,
>   Thanks for your suggestions. Now I have 2 attached patches to export
> the host cpuid data to guests. 
>   It reuses the ioctl KVM_GET_SUPPORTED_CPUID as Avi pointed out. Also
> as per Alex's suggestions the changes have been simplified for easier
> merge with the qemu tree.
>
> the kvm_kernel_patch is for the kvm.git tree.
> And the kvm_userspace_patch is for the kvm-userspace.git tree.
>
>   

You need changelog entries that describe the patches.  Also, one patch 
per email.

> @ -1194,36 +1194,14 @@ static void do_cpuid_1_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  			 u32 index, int *nent, int maxnent)
>  {
> -	const u32 kvm_supported_word0_x86_features = bit(X86_FEATURE_FPU) |
> -		bit(X86_FEATURE_VME) | bit(X86_FEATURE_DE) |
> -		bit(X86_FEATURE_PSE) | bit(X86_FEATURE_TSC) |
> -		bit(X86_FEATURE_MSR) | bit(X86_FEATURE_PAE) |
> -		bit(X86_FEATURE_CX8) | bit(X86_FEATURE_APIC) |
> -		bit(X86_FEATURE_SEP) | bit(X86_FEATURE_PGE) |
> -		bit(X86_FEATURE_CMOV) | bit(X86_FEATURE_PSE36) |
> -		bit(X86_FEATURE_CLFLSH) | bit(X86_FEATURE_MMX) |
> -		bit(X86_FEATURE_FXSR) | bit(X86_FEATURE_XMM) |
> -		bit(X86_FEATURE_XMM2) | bit(X86_FEATURE_SELFSNOOP);
> -	const u32 kvm_supported_word1_x86_features = bit(X86_FEATURE_FPU) |
> -		bit(X86_FEATURE_VME) | bit(X86_FEATURE_DE) |
> -		bit(X86_FEATURE_PSE) | bit(X86_FEATURE_TSC) |
> -		bit(X86_FEATURE_MSR) | bit(X86_FEATURE_PAE) |
> -		bit(X86_FEATURE_CX8) | bit(X86_FEATURE_APIC) |
> -		bit(X86_FEATURE_PGE) |
> -		bit(X86_FEATURE_CMOV) | bit(X86_FEATURE_PSE36) |
> -		bit(X86_FEATURE_MMX) | bit(X86_FEATURE_FXSR) |
> -		bit(X86_FEATURE_SYSCALL) |
> -		(bit(X86_FEATURE_NX) && is_efer_nx()) |
> -#ifdef CONFIG_X86_64
> +	const u32 kvm_unsupported_word0_x86_features = bit(X86_FEATURE_ACC);
> +	const u32 kvm_unsupported_word1_x86_features = bit(X86_FEATURE_RDTSCP) |
> +#ifndef CONFIG_X86_64
>   

What's the motivation for this change?

A potential problem is that a newer cpu might define new bits, which we 
don't support, yet we report them as supported.

>  get_cpu() before
> @@ -1246,6 +1224,7 @@ static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  		int t, times = entry->eax & 0xff;
>  
>  		entry->flags |= KVM_CPUID_FLAG_STATEFUL_FUNC;
> +		entry->flags |= KVM_CPUID_FLAG_STATE_READ_NEXT;
>  		for (t = 1; t < times && *nent < maxnent; ++t) {
>  			do_cpuid_1_ent(&entry[t], function, 0);
>  			entry[t].flags |= KVM_CPUID_FLAG_STATEFUL_FUNC;
>   

Why?

> @@ -1276,7 +1255,7 @@ static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  		entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
>  		/* read more entries until level_type is zero */
>  		for (i = 1; *nent < maxnent; ++i) {
> -			level_type = entry[i - 1].ecx & 0xff;
> +			level_type = entry[i - 1].ecx & 0xff00;
>  			if (!level_type)
>  				break;
>  			do_cpuid_1_ent(&entry[i], function, i);
>   

Why?

> @@ -1302,10 +1281,14 @@ static int kvm_dev_ioctl_get_supported_cpuid(struct kvm_cpuid2 *cpuid,
>  {
>  	struct kvm_cpuid_entry2 *cpuid_entries;
>  	int limit, nent = 0, r = -E2BIG;
> +	int sizer = 0;
>  	u32 func;
>  
> -	if (cpuid->nent < 1)
> -		goto out;
> +	if (cpuid->nent == 0) {
> +		sizer = 1;
> +		cpuid->nent = KVM_MAX_CPUID_ENTRIES; 
> +        }
> +
>   

That's an ABI change.  New userspace could call an older kernel with 
nent = 0, and get an unexpected response.

> @@ -2860,6 +2845,13 @@ void kvm_emulate_cpuid(struct kvm_vcpu *vcpu)
>  		kvm_register_write(vcpu, VCPU_REGS_RCX, best->ecx);
>  		kvm_register_write(vcpu, VCPU_REGS_RDX, best->edx);
>  	}
> +	if (function == 0x1) { /* set the per-cpu apicid */
> +		u32 ebx = kvm_register_read(vcpu, VCPU_REGS_RBX);
> +		ebx &= 0x00ffffff;
> +		ebx |= (vcpu->vcpu_id << 24);
> +		kvm_register_write(vcpu, VCPU_REGS_RBX, ebx);
> +	}
> +          
>   

ABI change again.  This is userspace's job.

-- 
error compiling committee.c: too many arguments to function