On 2012-04-23 22:02, Eduardo Habkost wrote: > 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? First of all, I bet this was already broken with the introduction of x2apic. So TSC deadline won't make it worse. I guess we need to address this in userspace, first by masking those features out, later by actually emulating them. > > 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." That suggests such features are always emulated - which is not true. They are either emulated, or nothing _prevents_ their emulation by user space. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux -- 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
