| Message ID | 20220120152146.1912559-1-michael.opdenacker@bootlin.com |
|---|---|
| State | New |
| Headers | show |
| Series | runqemu: automatically turn on "kvm" on x86 CPUs with VT | expand |
I think this should cover aarch64 on aarch64 too :) And presence of (and ability to open) /dev/kvm is probably a better check than looking through cpuinfo? Alex On Thu, 20 Jan 2022 at 16:21, Michael Opdenacker < michael.opdenacker@bootlin.com> wrote: > This automatically turns on the "kvm" option when emulating > an x86 system on x86 CPUs with VT capability. > > On an Intel i7-5600U CPU at 2.60GHz, using the "kvm" > option is at least 4x faster, booting "core-image-minimal" for qemux86-64. > The performance difference can even be bigger for larger systems. > > Rather than changing the documentation to remind users > to use this option, that's easier to enable this option by > default on suitable systems. > > A "noautokvm" option is added to disable this default, > and keep the previous behavior. > > Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com> > --- > scripts/runqemu | 22 +++++++++++++++++----- > 1 file changed, 17 insertions(+), 5 deletions(-) > > diff --git a/scripts/runqemu b/scripts/runqemu > index 4e05c1bb15..b500ba2afa 100755 > --- a/scripts/runqemu > +++ b/scripts/runqemu > @@ -78,8 +78,9 @@ of the following environment variables (in any order): > serialstdio - enable a serial console on the console (regardless of > graphics mode) > slirp - enable user networking, no root privileges is required > snapshot - don't write changes to back to images > - kvm - enable KVM when running x86/x86_64 (VT-capable CPU required) > + kvm - enable KVM. Enabled by default when running x86/x86_64 with a > VT-capable CPU > kvm-vhost - enable KVM with vhost when running x86/x86_64 (VT-capable > CPU required) > + noautokvm - don't enable KVM automatically on x86/x86_64 with a > VT-capable CPU > publicvnc - enable a VNC server open to all hosts > audio - enable audio > [*/]ovmf* - OVMF firmware file or base name for booting with UEFI > @@ -170,6 +171,7 @@ class BaseConfig(object): > self.fstype = '' > self.kvm_enabled = False > self.vhost_enabled = False > + self.noautokvm = False > self.slirp_enabled = False > self.net_bridge = None > self.nfs_instance = 0 > @@ -506,6 +508,8 @@ class BaseConfig(object): > self.kvm_enabled = True > elif arg == 'kvm-vhost': > self.vhost_enabled = True > + elif arg == 'noautokvm': > + self.noautokvm = True > elif arg == 'slirp': > self.slirp_enabled = True > elif arg.startswith('bridge='): > @@ -550,8 +554,18 @@ class BaseConfig(object): > if s: > self.set("DEPLOY_DIR_IMAGE", s.group(1)) > > + def kvm_cap_x86(self): > + with open('/proc/cpuinfo', 'r') as f: > + return re.search('vmx|svm', "".join(f.readlines())) > + > def check_kvm(self): > - """Check kvm and kvm-host""" > + """Check kvm and kvm-vhost""" > + # Turn on KVM by default emulating x86 on x86 CPUs with VT > + # Can be disabled with the "noautokvm" option > + if self.qemu_system.endswith(('i386', 'x86_64')) and not > self.noautokvm: > + if self.kvm_cap_x86(): > + self.kvm_enabled = True > + > if not (self.kvm_enabled or self.vhost_enabled): > self.qemu_opt_script += ' %s %s %s' % > (self.get('QB_MACHINE'), self.get('QB_CPU'), self.get('QB_SMP')) > return > @@ -565,9 +579,7 @@ class BaseConfig(object): > dev_kvm = '/dev/kvm' > dev_vhost = '/dev/vhost-net' > if self.qemu_system.endswith(('i386', 'x86_64')): > - with open('/proc/cpuinfo', 'r') as f: > - kvm_cap = re.search('vmx|svm', "".join(f.readlines())) > - if not kvm_cap: > + if not self.kvm_cap_x86(): > logger.error("You are trying to enable KVM on a cpu > without VT support.") > logger.error("Remove kvm from the command-line, or > refer:") > raise RunQemuError(yocto_kvm_wiki) > -- > 2.25.1 > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#160769): > https://lists.openembedded.org/g/openembedded-core/message/160769 > Mute This Topic: https://lists.openembedded.org/mt/88560844/1686489 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [ > alex.kanavin@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- > >
Hi Alex On 1/20/22 4:28 PM, Alexander Kanavin wrote: > I think this should cover aarch64 on aarch64 too :) And presence of > (and ability to open) /dev/kvm is probably a better check than looking > through cpuinfo? Thanks for the review! However, my abilities to build and test on aarch64 are limited. At least this change would take me much more time to develop and is more likely to cause regressions. Wouldn't this version already be a worthy change for users? I'm not sure we have many users running QEMU on aarch64. Would automatically passing the "kvm" option on applicable cases have value in our automated tests? Cheers Michael.
On Thu, 20 Jan 2022 at 19:43, Michael Opdenacker < michael.opdenacker@bootlin.com> wrote: > However, my abilities to build and test on aarch64 are limited. At least > this change would take me much more time to develop and is more likely > to cause regressions. > Checking for readability of /dev/kvm doesn't need aarch64 testing, it's universal and works on x86 too. It's a good check to do because quite often /dev/kvm has wrong permissions so attempting to auto-enable kvm is not going to work. > Wouldn't this version already be a worthy change for users? I'm not sure > we have many users running QEMU on aarch64. Would automatically passing > the "kvm" option on applicable cases have value in our automated tests? > Automated tests (e.g. bitbake -c testimage) already do this; the value is in manually starting runqemu. Alex
On Thu, 2022-01-20 at 19:43 +0100, Michael Opdenacker wrote: > Hi Alex > > On 1/20/22 4:28 PM, Alexander Kanavin wrote: > > I think this should cover aarch64 on aarch64 too :) And presence of > > (and ability to open) /dev/kvm is probably a better check than looking > > through cpuinfo? > > Thanks for the review! > > However, my abilities to build and test on aarch64 are limited. At least > this change would take me much more time to develop and is more likely > to cause regressions. > > Wouldn't this version already be a worthy change for users? I'm not sure > we have many users running QEMU on aarch64. Would automatically passing > the "kvm" option on applicable cases have value in our automated tests? I think the patch as it stands will actually regress things for many users. You must be already in the right groups to be able to access kvm on your system but I don't think that is the default for many distros. On those distros you would see a "permission denied" message from qemu which users find very confusing and I think this is why the code is in the form it is in. We might be able to improve the test further with the open permissions Alexander mentions but that is probably necessary to make the patch mergable. Cheers, Richard
On Thu, 20 Jan 2022 at 21:46, Richard Purdie < richard.purdie@linuxfoundation.org> wrote: > > I think the patch as it stands will actually regress things for many > users. You > must be already in the right groups to be able to access kvm on your > system but > I don't think that is the default for many distros. On those distros you > would > see a "permission denied" message from qemu which users find very > confusing and > I think this is why the code is in the form it is in. > > We might be able to improve the test further with the open permissions > Alexander > mentions but that is probably necessary to make the patch mergable. > I guess what we can do for a start is to check if host and target arches match (e.g. x86 on x86), and then if kvm is not asked for, gently suggest to the users that they do, for a much better experience, with a link to the kvm on yocto guide. Then we can look into auto-attempting it carefully, so that such attempts don't make the experience worse. Alex
Come to think of it, what would work best is some kind of ~/.enable-runqemu-kvm file, which presence would indicate to runqemu that it is permitted to auto-enable kvm when host and target arches match. Alex On Fri, 21 Jan 2022 at 12:43, Alexander Kanavin via lists.openembedded.org <alex.kanavin=gmail.com@lists.openembedded.org> wrote: > On Thu, 20 Jan 2022 at 21:46, Richard Purdie < > richard.purdie@linuxfoundation.org> wrote: > >> >> I think the patch as it stands will actually regress things for many >> users. You >> must be already in the right groups to be able to access kvm on your >> system but >> I don't think that is the default for many distros. On those distros you >> would >> see a "permission denied" message from qemu which users find very >> confusing and >> I think this is why the code is in the form it is in. >> >> We might be able to improve the test further with the open permissions >> Alexander >> mentions but that is probably necessary to make the patch mergable. >> > > I guess what we can do for a start is to check if host and target arches > match (e.g. x86 on x86), and then if kvm is not asked for, gently suggest > to the users that they do, for a much better experience, with a link to the > kvm on yocto guide. Then we can look into auto-attempting it carefully, so > that such attempts don't make the experience worse. > > Alex > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#160819): > https://lists.openembedded.org/g/openembedded-core/message/160819 > Mute This Topic: https://lists.openembedded.org/mt/88560844/1686489 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [ > alex.kanavin@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- > >
On 1/21/22 12:43 PM, Alexander Kanavin wrote: > On Thu, 20 Jan 2022 at 21:46, Richard Purdie > <richard.purdie@linuxfoundation.org > <mailto:richard.purdie@linuxfoundation.org>> wrote: > > > I think the patch as it stands will actually regress things for > many users. You > must be already in the right groups to be able to access kvm on > your system but > I don't think that is the default for many distros. On those > distros you would > see a "permission denied" message from qemu which users find very > confusing and > I think this is why the code is in the form it is in. > > We might be able to improve the test further with the open > permissions Alexander > mentions but that is probably necessary to make the patch mergable. > > > I guess what we can do for a start is to check if host and target > arches match (e.g. x86 on x86), and then if kvm is not asked for, > gently suggest to the users that they do, for a much better > experience, with a link to the kvm on yocto guide. Then we can look > into auto-attempting it carefully, so that such attempts don't make > the experience worse. Thanks for your ideas. If you think this idea of automatically enabling KVM is risky, it sounds like a good idea indeed to make this suggestion when applicable. I'll also update the documentation to remind people. I guess that mainly beginner users are impacted, and those are the ones who will read the our docs. Thanks again Michael.
diff --git a/scripts/runqemu b/scripts/runqemu index 4e05c1bb15..b500ba2afa 100755 --- a/scripts/runqemu +++ b/scripts/runqemu @@ -78,8 +78,9 @@ of the following environment variables (in any order): serialstdio - enable a serial console on the console (regardless of graphics mode) slirp - enable user networking, no root privileges is required snapshot - don't write changes to back to images - kvm - enable KVM when running x86/x86_64 (VT-capable CPU required) + kvm - enable KVM. Enabled by default when running x86/x86_64 with a VT-capable CPU kvm-vhost - enable KVM with vhost when running x86/x86_64 (VT-capable CPU required) + noautokvm - don't enable KVM automatically on x86/x86_64 with a VT-capable CPU publicvnc - enable a VNC server open to all hosts audio - enable audio [*/]ovmf* - OVMF firmware file or base name for booting with UEFI @@ -170,6 +171,7 @@ class BaseConfig(object): self.fstype = '' self.kvm_enabled = False self.vhost_enabled = False + self.noautokvm = False self.slirp_enabled = False self.net_bridge = None self.nfs_instance = 0 @@ -506,6 +508,8 @@ class BaseConfig(object): self.kvm_enabled = True elif arg == 'kvm-vhost': self.vhost_enabled = True + elif arg == 'noautokvm': + self.noautokvm = True elif arg == 'slirp': self.slirp_enabled = True elif arg.startswith('bridge='): @@ -550,8 +554,18 @@ class BaseConfig(object): if s: self.set("DEPLOY_DIR_IMAGE", s.group(1)) + def kvm_cap_x86(self): + with open('/proc/cpuinfo', 'r') as f: + return re.search('vmx|svm', "".join(f.readlines())) + def check_kvm(self): - """Check kvm and kvm-host""" + """Check kvm and kvm-vhost""" + # Turn on KVM by default emulating x86 on x86 CPUs with VT + # Can be disabled with the "noautokvm" option + if self.qemu_system.endswith(('i386', 'x86_64')) and not self.noautokvm: + if self.kvm_cap_x86(): + self.kvm_enabled = True + if not (self.kvm_enabled or self.vhost_enabled): self.qemu_opt_script += ' %s %s %s' % (self.get('QB_MACHINE'), self.get('QB_CPU'), self.get('QB_SMP')) return @@ -565,9 +579,7 @@ class BaseConfig(object): dev_kvm = '/dev/kvm' dev_vhost = '/dev/vhost-net' if self.qemu_system.endswith(('i386', 'x86_64')): - with open('/proc/cpuinfo', 'r') as f: - kvm_cap = re.search('vmx|svm', "".join(f.readlines())) - if not kvm_cap: + if not self.kvm_cap_x86(): logger.error("You are trying to enable KVM on a cpu without VT support.") logger.error("Remove kvm from the command-line, or refer:") raise RunQemuError(yocto_kvm_wiki)
This automatically turns on the "kvm" option when emulating an x86 system on x86 CPUs with VT capability. On an Intel i7-5600U CPU at 2.60GHz, using the "kvm" option is at least 4x faster, booting "core-image-minimal" for qemux86-64. The performance difference can even be bigger for larger systems. Rather than changing the documentation to remind users to use this option, that's easier to enable this option by default on suitable systems. A "noautokvm" option is added to disable this default, and keep the previous behavior. Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com> --- scripts/runqemu | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)