From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48665) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eCUid-0001U6-Sc for qemu-devel@nongnu.org; Wed, 08 Nov 2017 13:05:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eCUhZ-00074b-Ih for qemu-devel@nongnu.org; Wed, 08 Nov 2017 13:04:55 -0500 Received: from mail-qt0-x243.google.com ([2607:f8b0:400d:c0d::243]:50477) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1eCUhY-00073U-RP for qemu-devel@nongnu.org; Wed, 08 Nov 2017 13:03:49 -0500 Received: by mail-qt0-x243.google.com with SMTP id d9so4446259qtd.7 for ; Wed, 08 Nov 2017 10:03:48 -0800 (PST) Sender: =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= References: <20171030040056.11780-1-f4bug@amsat.org> <20171030082029.GY2993@localhost.localdomain> <20171108132857.GT12670@redhat.com> <1ecb6a4b-465f-0949-e039-441038f1adf8@amsat.org> <20171108165905.GN3111@localhost.localdomain> <20171108170629.GD12670@redhat.com> <20171108172547.GP3111@localhost.localdomain> From: =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= Message-ID: Date: Wed, 8 Nov 2017 15:03:34 -0300 MIME-Version: 1.0 In-Reply-To: <20171108172547.GP3111@localhost.localdomain> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [RFC PATCH] vl: only display available accelerators List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost , "Daniel P. Berrange" Cc: Thomas Huth , Stefano Stabellini , Markus Armbruster , Peter Xu , qemu-devel@nongnu.org, "Emilio G . Cota" , Anthony Perard , Paolo Bonzini , Richard Henderson , Christoffer Dall , Vincent Palatin On 11/08/2017 02:25 PM, Eduardo Habkost wrote: > On Wed, Nov 08, 2017 at 05:06:29PM +0000, Daniel P. Berrange wrote: >> On Wed, Nov 08, 2017 at 02:59:05PM -0200, Eduardo Habkost wrote: >>> On Wed, Nov 08, 2017 at 01:21:33PM -0300, Philippe Mathieu-Daudé wrote: >>>> On 11/08/2017 10:28 AM, Daniel P. Berrange wrote: >>>>> On Mon, Oct 30, 2017 at 09:20:29AM +0100, Eduardo Habkost wrote: >>>>>> On Mon, Oct 30, 2017 at 01:00:56AM -0300, Philippe Mathieu-Daudé wrote: >>>>>>> examples configuring with '--enable-kvm --disable-tcg' >>>>>>> >>>>>>> - before >>>>>>> >>>>>>> $ qemu-system-x86_64 -accel help >>>>>>> Possible accelerators: kvm, xen, hax, tcg >>>>>>> >>>>>>> $ qemu-system-x86_64 -accel tcg >>>>>>> qemu-system-x86_64: -machine accel=tcg: No accelerator found >>>>>>> >>>>>>> # qemu-system-x86_64 -accel hax >>>>>>> qemu-system-x86_64: -machine accel=hax: No accelerator found >>>>>>> >>>>>>> # qemu-system-x86_64 -accel xen >>>>>>> xencall: error: Could not obtain handle on privileged command interface: No such file or directory >>>>>>> xen be core: xen be core: can't open xen interface >>>>>>> can't open xen interface >>>>>>> qemu-system-x86_64: failed to initialize Xen: Operation not permitted >>>>>>> >>>>>>> - after >>>>>>> >>>>>>> $ qemu-system-x86_64 -accel help >>>>>>> Possible accelerators: kvm >>>>>>> >>>>>>> Signed-off-by: Philippe Mathieu-Daudé >>>>>>> --- >>>>>>> RFC because: >>>>>>> - I don't think this is the nicest way, too much #ifdef'fery in main() >>>>>> >>>>>> I suggest using object_class_get_list(TYPE_ACCEL, false). >>>>> >>>>> And check the result of the available() method on the returned classes >>>>> too, to filter the results. >>>> >>>> Good idea! I'll use that. >>> >>> It looks like QTest is the only accelerator that implements >>> ->available(), and its return value is a build-time constant that >>> depends only on CONFIG_POSIX. >>> >>> I wonder why we don't simply avoid compiling the qtest class if >>> CONFIG_POSIX is unset, making the ->available() method >>> unnecessary. >> >> Yeah that does seem simpler, though I'm surprised that Xen does not >> implement the available method. Xen is an accel I'd expect to see >> compiled into an x86 build, but is only available if the host is >> actually booted under a Xen hypervisor. Likewise shouldn't kvm >> only report itself as available if the /dev/kvm actually exists. >> But maybe that's not the kind of semantics code using available() >> expects ? > > Currently the only caller of ->available() calls ->init_machine() > immediately afterwards, so for the current code it doesn't matter > if the check is inside ->available() or ->init_machine(). > > That said, I'm not sure we should look for /dev/kvm or check for > the Xen hypervisor when handling "-accel help". I expect help > text to tell the user what the QEMU binary supports, not what the > current host supports. Yes I prefer that too, I'll write something up such: static bool kvm_available(void) { return access("/dev/kvm", R_OK|W_OK) == 0; } However I wonder, if an user is not in the kvm group but is in sudoers and run "qemu-system-aarch64 -accel help" he won't see KVM as available... (I tend to not use sudo when looking for -help output). same with: static bool hax_available(void) { return access("/dev/HAX", R_OK|W_OK) == 0; } ok for: static bool xen_available(void) { return access("/sys/hypervisor/properties/features", F_OK) == 0; } On POWER7 we have: /* On ppc64, the test only works with kvm-hv, but not with kvm-pr */ accel = access("/sys/module/kvm_hv", F_OK) ? "tcg" : "kvm:tcg"; Isn't it cleaner to register 2 accelerators, KVM-PR and KVM-HV and have KVM being an alias?