2022-03-22 13:34:30

by Paolo Bonzini

[permalink] [raw]
Subject: [PATCH 3/3] Documentation: KVM: add API issues section

Add a section to document all the different ways in which the KVM API sucks.

I am sure there are way more, give people a place to vent so that userspace
authors are aware.

Signed-off-by: Paolo Bonzini <[email protected]>
---
Documentation/virt/kvm/api.rst | 46 ++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 9f3172376ec3..8787fcd3b23f 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -7575,3 +7575,49 @@ The argument to KVM_ENABLE_CAP is also a bitmask, and must be a subset
of the result of KVM_CHECK_EXTENSION. KVM will forward to userspace
the hypercalls whose corresponding bit is in the argument, and return
ENOSYS for the others.
+
+9. Known KVM API problems
+=========================
+
+In some cases, KVM's API has some inconsistencies or common pitfalls
+that userspace need to be aware of. This section details some of
+these issues.
+
+Most of them are architecture specific, so the section is split by
+architecture.
+
+9.1. x86
+--------
+
+``KVM_GET_SUPPORTED_CPUID`` issues
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+In general, ``KVM_GET_SUPPORTED_CPUID`` is designed so that it is possible
+to take its result and pass it directly to ``KVM_SET_CPUID2``. This section
+documents some cases in which that requires some care.
+
+Local APIC features
+~~~~~~~~~~~~~~~~~~~
+
+CPU[EAX=1]:ECX[21] (X2APIC) is reported by ``KVM_GET_SUPPORTED_CPUID``,
+but it can only be enabled if ``KVM_CREATE_IRQCHIP`` or
+``KVM_ENABLE_CAP(KVM_CAP_IRQCHIP_SPLIT)`` are used to enable in-kernel emulation of
+the local APIC.
+
+The same is true for the ``KVM_FEATURE_PV_UNHALT`` paravirtualized feature.
+
+CPU[EAX=1]:ECX[24] (TSC_DEADLINE) is not reported by ``KVM_GET_SUPPORTED_CPUID``.
+It can be enabled if ``KVM_CAP_TSC_DEADLINE_TIMER`` is present and the kernel
+has enabled in-kernel emulation of the local APIC.
+
+Obsolete ioctls and capabilities
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+KVM_CAP_DISABLE_QUIRKS does not let userspace know which quirks are actually
+available. Use ``KVM_CHECK_EXTENSION(KVM_CAP_DISABLE_QUIRKS2)`` instead if
+available.
+
+Ordering of KVM_GET_*/KVM_SET_* ioctls
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+TBD
--
2.35.1


2022-03-24 06:04:33

by Oliver Upton

[permalink] [raw]
Subject: Re: [PATCH 3/3] Documentation: KVM: add API issues section

Hi Paolo,

On Tue, Mar 22, 2022 at 12:07:12PM +0100, Paolo Bonzini wrote:
> Add a section to document all the different ways in which the KVM API sucks.
>
> I am sure there are way more, give people a place to vent so that userspace
> authors are aware.
>
> Signed-off-by: Paolo Bonzini <[email protected]>

Do you think we should vent about our mistakes inline with the
descriptions of the corresponding UAPI? One example that comes to mind
is ARM's CNTV_CVAL_EL0/CNTVCT_EL0 mixup, which is mentioned in 4.68
'KVM_SET_ONE_REG'. That, of course, doesn't cover the
previously-undocumented bits of UAPI that are problematic :)

If we go that route we likely should have a good format for documenting
the ugliness.

--
Thanks,
Oliver

2022-03-24 23:33:58

by Oliver Upton

[permalink] [raw]
Subject: Re: [PATCH 3/3] Documentation: KVM: add API issues section

On Wed, Mar 23, 2022 at 10:32:50PM +0100, Paolo Bonzini wrote:
> On 3/23/22 19:16, Oliver Upton wrote:
> > Do you think we should vent about our mistakes inline with the
> > descriptions of the corresponding UAPI? One example that comes to mind
> > is ARM's CNTV_CVAL_EL0/CNTVCT_EL0 mixup, which is mentioned in 4.68
> > 'KVM_SET_ONE_REG'. That, of course, doesn't cover the
> > previously-undocumented bits of UAPI that are problematic:)
>
> It depends. My intention was to use this document more for hidden
> interdependencies, in this case between KVM_GET_SUPPORTED_CPUID and
> KVM_CREATE_IRQCHIP, KVM_ENABLE_CAP(KVM_CAP_IRQCHIP_SPLIT),
> KVM_CAP_TSC_DEADLINE_TIMER.

Ah good point. I agree bad cross interactions need to be called out
separately.

Thanks!

--
Oliver

2022-03-25 03:58:17

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH 3/3] Documentation: KVM: add API issues section

On 3/23/22 19:16, Oliver Upton wrote:
> Do you think we should vent about our mistakes inline with the
> descriptions of the corresponding UAPI? One example that comes to mind
> is ARM's CNTV_CVAL_EL0/CNTVCT_EL0 mixup, which is mentioned in 4.68
> 'KVM_SET_ONE_REG'. That, of course, doesn't cover the
> previously-undocumented bits of UAPI that are problematic:)

It depends. My intention was to use this document more for hidden
interdependencies, in this case between KVM_GET_SUPPORTED_CPUID and
KVM_CREATE_IRQCHIP, KVM_ENABLE_CAP(KVM_CAP_IRQCHIP_SPLIT),
KVM_CAP_TSC_DEADLINE_TIMER.

Paolo