Re: [PATCH v2 3/4] x86/kvm/hyper-v: direct mode for synthetic timers

From: Roman Kagan
Date: Tue Nov 27 2018 - 14:05:30 EST


On Tue, Nov 27, 2018 at 02:54:21PM +0100, Paolo Bonzini wrote:
> On 27/11/18 09:37, Roman Kagan wrote:
> > On Mon, Nov 26, 2018 at 05:44:24PM +0100, Paolo Bonzini wrote:
> >> On 26/11/18 16:47, Vitaly Kuznetsov wrote:
> >>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >>> index 5cd5647120f2..b21b5ceb8d26 100644
> >>> --- a/arch/x86/kvm/x86.c
> >>> +++ b/arch/x86/kvm/x86.c
> >>> @@ -2997,6 +2997,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> >>> case KVM_CAP_HYPERV_TLBFLUSH:
> >>> case KVM_CAP_HYPERV_SEND_IPI:
> >>> case KVM_CAP_HYPERV_ENLIGHTENED_VMCS:
> >>> + case KVM_CAP_HYPERV_STIMER_DIRECT:
> >>> case KVM_CAP_PCI_SEGMENT:
> >>> case KVM_CAP_DEBUGREGS:
> >>> case KVM_CAP_X86_ROBUST_SINGLESTEP:
> >>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> >>> index 2b7a652c9fa4..b8da14cee8e5 100644
> >>> --- a/include/uapi/linux/kvm.h
> >>> +++ b/include/uapi/linux/kvm.h
> >>> @@ -975,6 +975,7 @@ struct kvm_ppc_resize_hpt {
> >>> #define KVM_CAP_HYPERV_ENLIGHTENED_VMCS 163
> >>> #define KVM_CAP_EXCEPTION_PAYLOAD 164
> >>> #define KVM_CAP_ARM_VM_IPA_SIZE 165
> >>> +#define KVM_CAP_HYPERV_STIMER_DIRECT 166
> >>
> >> I wonder if all these capabilities shouldn't be replaced by a single
> >> KVM_GET_HYPERV_SUPPORTED_CPUID ioctl, or something like that.
> >
> > Hmm, why? Are we running short of cap numbers?
> >
> > Capabilities are a well-established and unambiguous negotiation
> > mechanism, why invent another one? Besides, not all features map
> > conveniently onto cpuid bits, e.g. currently we have two versions of
> > SynIC support, which differ in the way the userspace deals with it, but
> > not in the cpuid bits we expose in the guest. IMO such an ioctl would
> > bring more complexity rather than less.
>
> Yes, in that case (for bugfixes basically) you'd have to use
> capabilities. But if the feature is completely hidden to userspace
> except for the CPUID bit, it seems like a ioctl would be more consistent
> with the rest of the KVM API.

So there will be some features that are more equal than others?

I think that it's the current scheme which is more consistent: there are
features that are implemented in KVM, and there are caps for negotiating
them with userspace, and then -- separately -- there's a mix of bits to
show to the guest in response to CPUID, which only the userspace has to
bother with.

Thanks,
Roman.