On Mon, Apr 23, 2012 at 06:31:25PM +0200, Jan Kiszka wrote: > On 2012-04-23 16:48, Eduardo Habkost wrote: > > Trying to summarize the points above: > > > > Groups (A) and (B) are: > > > > A) a feature that KVM supports and emulate by itself and can be enabled > > by userspace blindly, without requiring any additional userspace > > code to work. > > B) a feature that KVM supports but need support from userspace to work. > > > > We have to differentiate those two groups somehow, otherwise "-cpu host" > > will always risk being unstable (in case we can't identify group (B) and > > end up enabling a feature that will break) or useless (if group (A) is > > considered always empty). > > > > (If you think this two-group model is not sufficient, please let me know.) > > > > Note that I am discussing two things above: > > > > - Whether GET_SUPPORTED_CPUID should expose only features from group > > (A), or group (B) too. > > - One problem here is that today GET_SUPPORTED_CPUIDS have many > > examples of (B) features inside it. Should we stop doing that? > > We have exactly two for the category that I was concerned about: TSC > deadline and X2APIC. The latter is already exposed unconditionally, even > if the kernel does not provide emulation. So, you are right, TSC > deadline is not a new scenario. Oh, that explains why you were seing it differently: if the kernel doesn't control anything about the feature exposure, there was no obvious need to add it to GET_SUPPORTED_CPUID. > > > - TSC-deadline is the first case where we are _not_ doing that. It > > is the first CPU feature in KVM that can be enabled by userspace > > (as long as userspace does the proper setup), but it's not > > included on GET_SUPPORTED_CPUIDs. > > - Even the current documentation implies that group (B) is included: > > > > "This ioctl returns x86 cpuid features which are supported by both > > the hardware and kvm. Userspace can use the information returned by > > this ioctl to construct cpuid information (for KVM_SET_CPUID2) that > > is consistent with hardware, kernel, and userspace capabilities, and > > ^^^^^^^^^^^^^^^^^^^^^^^^^^ > > with user requirements (for example, the user may wish to constrain > > cpuid to emulate older hardware, or for feature consistency across a > > cluster)." > > > > In the specific case of TSC-deadline, I consider "Qemu knowing that > > TSC-deadline can be enabled only if in-kernel irqchip is enabled" as > > an "userpace capability". > > > > - How to precisely define the groups (A) and (B)? > > - "requires additional code only if migration is required" qualifies > > as (B) or (A)? > > - "requires in-kernel irqchip to be enabled to work" qualifies as (B), > > doesn't it? > > My problem is that features like X2APIC and TSC deadline are exposed by > the kernel without "contributing" to them (if in-kernel irqchip is off). They are not simply "exposed by the kernel": they are enabled by userspace, after userspace deciding whether it's safe or not (based on multiple factors). > However, that was how I interpreted this GET_SUPPORTED_CPUID. In fact, > it is used as "kernel or hardware does not _prevent_" already. And in > that sense, it's ok to enable even features that are not in > kernel/hardware hands. We should point out this fact in the documentation. I see GET_SUPPORTED_CPUID as just a "what userspace can enable because the kernel and the hardware support it (= don't prevent it), as long as userspace has the required support" (meaning A+B). It's a bit like KVM_CHECK_EXTENSION, but with the nice feature that that the capabilities map directly to CPUID bits. So, it's not clear to me: now you are OK with adding TSC_DEADLINE to GET_SUPPORTED_CPUID? But we still have the issue of "-cpu host" not knowing what can be safely enabled (without userspace feature-specific setup code), or not. Do you have any suggestion for that? Avi, do you have any suggestion? And I still don't know the answer to: > > - How to precisely define the groups (A) and (B)? > > - "requires additional code only if migration is required" qualifies > > as (B) or (A)? Re: documentation, isn't the following paragraph (already present on api.txt) sufficient? "The entries returned are the host cpuid as returned by the cpuid instruction, with unknown or unsupported features masked out. Some features (for example, x2apic), may not be present in the host cpu, but are exposed by kvm if it can emulate them efficiently." -- Eduardo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
