Re: [PATCH v2 1/3] KVM: x86: Reject nested CAP enablement if nested virtualization is disabled

[Huang, Kai]

> but now KVM is over-reporting support for
> KVM_CAP_NESTED_STATE and KVM_CAP_HYPERV_ENLIGHTENED_VMCS.
> 
> 
[...]

> @@ -2346,7 +2346,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  			r &= ~KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST;
>  		break;
>  	case KVM_CAP_NESTED_STATE:
> -		r = kvm_x86_ops.nested_ops->get_state ?
> +		r = kvm_x86_ops.nested_ops->enabled ?
>  			kvm_x86_ops.nested_ops->get_state(NULL, NULL, 0) : 0;
>  		break;

[...]

>  #ifdef CONFIG_KVM_HYPERV
> @@ -2354,7 +2354,8 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  		r = kvm_x86_ops.enable_l2_tlb_flush != NULL;
>  		break;
>  	case KVM_CAP_HYPERV_ENLIGHTENED_VMCS:
> -		r = kvm_x86_ops.nested_ops->enable_evmcs != NULL;
> +		r = kvm_x86_ops.nested_ops->enabled &&
> +		    kvm_x86_ops.nested_ops->enable_evmcs != NULL;
> 

IMHO an additional 'enabled' seems a bit redundant?  And it seems there's no
rule whether the common x86 code should check it before (checking and) making
the call or not.

How about letting VMX/SVM code to explicitly clear the function pointer in
xx_hardware_setup()?

Perhaps as an ultimate goal, I am not sure whether VMX/SVM should provide a all-
NULL version of kvm_x86_nested_ops and the kvm_nested_ops_update() should treat
all as either OPTIONAL or OPTIONAL_RET0.  There will be still call sites which
needs to check the pointer is NULL or not and return -EXXX but I guess we can
live with that (kvm_x86_ops is similar too anyway) ?