On 08.05.2012, at 22:14, Eduardo Habkost wrote: > On Tue, May 08, 2012 at 02:58:11AM +0200, Alexander Graf wrote: >> On 07.05.2012, at 20:21, Eduardo Habkost wrote: >> >>> >>> Andre? Are you able to help to answer the question below? >>> >>> I would like to clarify what's the expected behavior of "-cpu host" to >>> be able to continue working on it. I believe the code will need to be >>> fixed on either case, but first we need to figure out what are the >>> expectations/requirements, to know _which_ changes will be needed. >>> >>> >>> On Tue, Apr 24, 2012 at 02:19:25PM -0300, Eduardo Habkost wrote: >>>> (CCing Andre Przywara, in case he can help to clarify what's the >>>> expected meaning of "-cpu host") >>>> >>> [...] >>>> I am not sure I understand what you are proposing. Let me explain the >>>> use case I am thinking about: >>>> >>>> - Feature FOO is of type (A) (e.g. just a new instruction set that >>>> doesn't require additional userspace support) >>>> - User has a Qemu vesion that doesn't know anything about feature FOO >>>> - User gets a new CPU that supports feature FOO >>>> - User gets a new kernel that supports feature FOO (i.e. has FOO in >>>> GET_SUPPORTED_CPUID) >>>> - User does _not_ upgrade Qemu. >>>> - User expects to get feature FOO enabled if using "-cpu host", without >>>> upgrading Qemu. >>>> >>>> The problem here is: to support the above use-case, userspace need a >>>> probing mechanism that can differentiate _new_ (previously unknown) >>>> features that are in group (A) (safe to blindly enable) from features >>>> that are in group (B) (that can't be enabled without an userspace >>>> upgrade). >>>> >>>> In short, it becomes a problem if we consider the following case: >>>> >>>> - Feature BAR is of type (B) (it can't be enabled without extra >>>> userspace support) >>>> - User has a Qemu version that doesn't know anything about feature BAR >>>> - User gets a new CPU that supports feature BAR >>>> - User gets a new kernel that supports feature BAR (i.e. has BAR in >>>> GET_SUPPORTED_CPUID) >>>> - User does _not_ upgrade Qemu. >>>> - User simply shouldn't get feature BAR enabled, even if using "-cpu >>>> host", otherwise Qemu would break. >>>> >>>> If userspace always limited itself to features it knows about, it would >>>> be really easy to implement the feature without any new probing >>>> mechanism from the kernel. But that's not how I think users expect "-cpu >>>> host" to work. Maybe I am wrong, I don't know. I am CCing Andre, who >>>> introduced the "-cpu host" feature, in case he can explain what's the >>>> expected semantics on the cases above. >> >> Can you think of any feature that'd go into category B? > > - TSC-deadline: can't be enabled unless userspace takes care to enable > the in-kernel irqchip.
The kernel can check if in-kernel irqchip has it enabled and otherwise mask it out, no? > - x2apic: ditto. Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities. > > I am not sure about XSAVE: an old qemu version would call kvm_put_fpu() > instead of kvm_put_xsave() on kvm_arch_put_registers(), but I don't know > if this would have unexpected side-effects or not. Then XSAVE awareness should be manually enabled by user space. That's what we have ENABLE_CAP for :). Do ENABLE_CAP(XSAVE) -> get XSAVE as a bit in GET_SUPPORTED_CPUID. > > I wouldn't be surprised if we find many other cases, as even the > GET_SUPPORTED_CPUID documentation is explicit about that: "Userspace can > use the information returned by this ioctl [GET_SUPPORTED_CPUID] to > construct cpuid information (for KVM_SET_CPUID2) that is consistent with > hardware, kernel, and userspace capabilities, [...]" > ^^^^^^^^^^^^^^^^^^^^^^ Yeah, so the intent is that kvm is aware of all the bits user space would care about. > >> >> All features I'm aware of work fine (without migration, but that one >> is moot for -cpu host anyway) as long as the host kvm implementation >> is fine with it (GET_SUPPORTED_CPUID). So they'd be category A. > > So, you would argue that GET_SUPPORTED_CPUID should include only > features of type A? That's the opposite of what we have today. > > Maybe we could change the semantics to type-A-only if we define "type A" > as: > > - Don't require any extra userspace support except for: > - Migration support. > - Enabling the in-kernel irqchip. > > If we agree on that semantics, "-cpu host" could safely enable all the > fetures returned by GET_SUPPORTED_CPUID blindly, after making sure that > migration support is disabled and the in-kernel irqchip is enabled. Then > all type-B features will require defining a KVM_CAP_* capability instead not instead. In addition. Define a KVM_CAP_ and do an ENABLE_CAP on that one to have it exposed. > of using GET_SUPPORTED_CPUID. It's the opposite of the direction I was > proposing earlier in this thread, but it is starting to look like a > better idea (otherwise "-cpu host" would never be reliable). > > If we agree on that semantics, it won't require any code change on the > current code, just a documentation update. Life is simple, eh? :) Alex
