From mboxrd@z Thu Jan 1 00:00:00 1970 From: Borislav Petkov Subject: Re: [PATCH -v2] kvm: Emulate MOVBE Date: Sun, 21 Apr 2013 13:30:51 +0200 Message-ID: <20130421113051.GB4594@pd.tnic> References: <20130411142818.GA17919@redhat.com> <20130411153733.GE27062@pd.tnic> <20130414074107.GD17919@redhat.com> <20130414173215.GD20547@pd.tnic> <20130416174236.GE5807@redhat.com> <20130417110433.GD11807@pd.tnic> <20130417133829.GH1682@redhat.com> <20130417140200.GE11807@pd.tnic> <20130418224848.GA20712@pd.tnic> <20130421094651.GA8997@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Cc: Andre Przywara , kvm@vger.kernel.org, =?utf-8?B?SsO2cmcgUsO2ZGVs?= , "H. Peter Anvin" , x86-ml To: Gleb Natapov Return-path: Received: from mail.skyhub.de ([78.46.96.112]:48330 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751573Ab3DULal (ORCPT ); Sun, 21 Apr 2013 07:30:41 -0400 Content-Disposition: inline In-Reply-To: <20130421094651.GA8997@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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. > > 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 ...,+ 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. :-) 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? Hmmm. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. --