On Sun, Apr 21, 2013 at 01:30:51PM +0200, Borislav Petkov wrote: > On Sun, Apr 21, 2013 at 12:46:51PM +0300, Gleb Natapov wrote: > > There may be userspaces that set ->function to 0xffffffff (just > > because they do not init the buffer before calling into the kernel) > > and this will break them. > > You don't mean 0xffffffff here but rather something random which is not > properly initialized and by chance is a valid CPUID leaf. Then, if some > of the bits in the register variables e[abcd]x are also set, we return > with emulated bits set. > Yes. > > > For each set bit, we check whether we emulate the respective feature > > > and if so, we leave it untouched before returning it to userspace. > > > Otherwise, we clear it before OR-ing in the host bits and the > > > good-emulated bits like x2apic. > > > > > > Yeah, semantics need to be handled carefully, but it has this > > > knock-on-door aspect where kvm says that it actually emulates a feature > > > only if asked, i.e. with the -cpu ...,+<feature> syntax. > > > > > > * new ioctl KVM_GET_EMULATED_CPUID > > > > > > Might be overkill and might be used only in a limited fashion since we > > > don't want to emulate *every* feature in kvm. > > > > > > Hmmm. I kinda like the first one more while the second one is cleaner. > > > > > The first one needs explicit userspace support to work correctly. This > > should be other way around: old userspace should do the right thing, but may > > not support new features, new userspace should be able to support new > > feature. > > Crap, not even qemu is handing in cleared buffers with that g_malloc0() > thing, AFAICT. > > > We may extend KVM_GET_SUPPORTED_CPUID instead of adding new one. There > > is a padding field in kvm_cpuid2 that we could have reused as flags, > > That would've been lovely. > > > but unfortunately current implementation does not error if the field is > > not zero, so if there is a userspace that does not zero the padding it may > > break. Another options is to reuse high bits of nent as flags (not very > > nice, but will work). > > Not nice?! It is a very nasty hack - that's what it is. :-) > Agree, but not nastier than expecting ->function to have special value :) > Frankly speaking, I'd rather prefer adding a new ioctl. Since old > userspace won't support the new features, then we just as well can > simply add the new ioctl. > > In all the cases we need to touch userspace - be it to OR in the flags > into ->nent or to implement the new ioctl. So why not do it in a clean > manner from the get-go? > Agree here too. -- Gleb. -- 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
