Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Eduardo Habkost <ehabkost@redhat.com>]

On Fri, Apr 20, 2012 at 05:19:17PM +0200, Jan Kiszka wrote:
> On 2012-04-20 17:00, Eduardo Habkost wrote:
> > On Fri, Apr 20, 2012 at 12:12:38PM +0200, Jan Kiszka wrote:
> >> On 2012-04-19 22:03, Eduardo Habkost wrote:
> >>> Jan/Avi: ping?
> >>>
> >>> I would like to get this ABI detail clarified so it can be implemented
> >>> the right way on Qemu and KVM.
> >>>
> >>> My proposal is to simply add tsc-deadline to the data returned by
> >>> GET_SUPPORTED_CPUID, making KVM_CAP_TSC_DEADLINE_TIMER unnecessary.
> >>>
> >>
> >> IIRC, there were problems with this model to exclude that the feature
> >> gets reported this way without ensuring that in-kernel irqchip is
> >> actually activated. Please browse the discussions, it should be
> >> documented there.
> > 
> > The flag was never added to GET_SUPPORTED_CPUID on any of the git
> > repositories I have checked, and I don't see that being done anywhere on
> > my KVM mailing list archives, either. So I don't see how we could have
> > had issues with GET_SUPPORTED_CPUID, if it was never present on the
> > code.
> > 
> > What was present on the code before the fix, was coded that
> > unconditinally enabled the flag even if Qemu never asked for it, and
> > that really was wrong.
> > 
> > GET_SUPPORTED_CPUID doesn't enable any flag: it just tells userspace
> > that the hardware and KVM supports the feature (and, surprise, this is
> > exactly what KVM_CAP_TSC_DEADLINE_TIMER means too, isn't it?). It's up
> > to userspace to enable the CPUID bits according to user requirements and
> > userspace capabilities (in other words: only when userspace knows it is
> > safe because the in-kernel irqchip is enabled).
> > 
> > If the above is not what GET_SUPPORTED_CPUID means, I would like to get
> > that clarified, so I can submit an updated to KVM API documentation.
> 
> Does old userspace filter out flags from GET_SUPPORTED_CPUID that it
> does not understand?

It's even more strict than that: it only enables what was explicitly
enabled on the CPU model asked by the user.

But:

The only exception is "-cpu host", that tries to enable everything, even
flags Qemu never heard of, and that is something that may require some
changes on the API design (or at least documentation clarifications).

Today "-cpu host" can't differentiate (A) "a feature that KVM supports
and emulate by itself and can be enabled without any support from
userspace" and (B) "a feature that KVM supports but need support from
userspace to be enabled". I am sure we will be able to find multiple
examples of (B) inside the flags returned by GET_SUPPORTED_CPUID today.

We could decide to never add any new flag to GET_SUPPORTED_CPUID if it
requires additional userspace support to work, from now on, and create
new KVM_CAP_* flags for them. But, we really want to do that for almost
every new CPUID feature bit in the future?

We also have the problem of defining what "requires support from
userspace to work" means: I am not sure this has the same meaning for
everybody. Many new features require userspace support only for
migration, and nothing else, but some users don't need migration to
work. Do those features qualify as (A), or as (B)?

-- 
Eduardo