| Message ID | 5069A9DF.4040606@siemens.com |
|---|---|
| State | New |
| Headers | show |
Jan Kiszka <jan.kiszka@siemens.com> writes: > If we built a target for a host that supports KVM in principle, set the > default accelerator to KVM as well. This also means the start of QEMU > will fail to start if KVM support turns out to be unavailable at > runtime. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > kvm-all.c | 1 + > kvm-stub.c | 1 + > kvm.h | 1 + > vl.c | 4 ++-- > 4 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/kvm-all.c b/kvm-all.c > index 92a7137..4d5f86c 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -103,6 +103,7 @@ struct KVMState > #endif > }; > > +bool kvm_configured = true; > KVMState *kvm_state; > bool kvm_kernel_irqchip; > bool kvm_async_interrupts_allowed; > diff --git a/kvm-stub.c b/kvm-stub.c > index 3c52eb5..86a6451 100644 > --- a/kvm-stub.c > +++ b/kvm-stub.c > @@ -17,6 +17,7 @@ > #include "gdbstub.h" > #include "kvm.h" > > +bool kvm_configured; > KVMState *kvm_state; > bool kvm_kernel_irqchip; > bool kvm_async_interrupts_allowed; > diff --git a/kvm.h b/kvm.h > index dea2998..9936e5f 100644 > --- a/kvm.h > +++ b/kvm.h > @@ -22,6 +22,7 @@ > #include <linux/kvm.h> > #endif > > +extern bool kvm_configured; > extern int kvm_allowed; > extern bool kvm_kernel_irqchip; > extern bool kvm_async_interrupts_allowed; > diff --git a/vl.c b/vl.c > index 8d305ca..f557bd1 100644 > --- a/vl.c > +++ b/vl.c > @@ -2215,8 +2215,8 @@ static int configure_accelerator(void) > } > > if (p == NULL) { > - /* Use the default "accelerator", tcg */ > - p = "tcg"; > + /* The default accelerator depends on the availability of KVM. */ > + p = kvm_configured ? "kvm" : "tcg"; > } How about making this an arch_init() function call and then using a #if defined(KVM_CONFIG) in arch_init.c? I hate to introduce another global variable if we can avoid it... Otherwise: Acked-by: Anthony Liguori <aliguori@us.ibm.com> Blue/Aurelien, any objections? Regards, Anthony Liguori > > while (!accel_initialised && *p != '\0') { > -- > 1.7.3.4
Hello Jan, Am 01.10.2012 16:34, schrieb Jan Kiszka: > If we built a target for a host that supports KVM in principle, set the > default accelerator to KVM as well. This also means the start of QEMU > will fail to start if KVM support turns out to be unavailable at > runtime. From a distro point of view this of course means that we will build against KVM and that the new KVM default will start to fail for users on very old hardware. Can't we do a runtime check to select the default? Would be nice to at least amend the commit message with how they are expected to remedy that via command line. -machine accel=tcg? Regards, Andreas > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > kvm-all.c | 1 + > kvm-stub.c | 1 + > kvm.h | 1 + > vl.c | 4 ++-- > 4 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/kvm-all.c b/kvm-all.c > index 92a7137..4d5f86c 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -103,6 +103,7 @@ struct KVMState > #endif > }; > > +bool kvm_configured = true; > KVMState *kvm_state; > bool kvm_kernel_irqchip; > bool kvm_async_interrupts_allowed; > diff --git a/kvm-stub.c b/kvm-stub.c > index 3c52eb5..86a6451 100644 > --- a/kvm-stub.c > +++ b/kvm-stub.c > @@ -17,6 +17,7 @@ > #include "gdbstub.h" > #include "kvm.h" > > +bool kvm_configured; > KVMState *kvm_state; > bool kvm_kernel_irqchip; > bool kvm_async_interrupts_allowed; > diff --git a/kvm.h b/kvm.h > index dea2998..9936e5f 100644 > --- a/kvm.h > +++ b/kvm.h > @@ -22,6 +22,7 @@ > #include <linux/kvm.h> > #endif > > +extern bool kvm_configured; > extern int kvm_allowed; > extern bool kvm_kernel_irqchip; > extern bool kvm_async_interrupts_allowed; > diff --git a/vl.c b/vl.c > index 8d305ca..f557bd1 100644 > --- a/vl.c > +++ b/vl.c > @@ -2215,8 +2215,8 @@ static int configure_accelerator(void) > } > > if (p == NULL) { > - /* Use the default "accelerator", tcg */ > - p = "tcg"; > + /* The default accelerator depends on the availability of KVM. */ > + p = kvm_configured ? "kvm" : "tcg"; > } > > while (!accel_initialised && *p != '\0') { >
On Mon, Oct 01, 2012 at 06:43:00PM +0200, Andreas Färber wrote: > Hello Jan, > > Am 01.10.2012 16:34, schrieb Jan Kiszka: > > If we built a target for a host that supports KVM in principle, set the > > default accelerator to KVM as well. This also means the start of QEMU > > will fail to start if KVM support turns out to be unavailable at > > runtime. > > From a distro point of view this of course means that we will build > against KVM and that the new KVM default will start to fail for users on > very old hardware. Can't we do a runtime check to select the default? NB, this is *not* only about old hardware. There are plenty of users who use QEMU inside VMs. One very common usage I know of is image building tools which are run inside Amazon VMs, using libguestfs & QEMU. IMHO, default to KVM, fallback to TCG is the most friendly default behaviour. Daniel
On Mon, Oct 01, 2012 at 11:20:41AM -0500, Anthony Liguori wrote: > Jan Kiszka <jan.kiszka@siemens.com> writes: > > > If we built a target for a host that supports KVM in principle, set the > > default accelerator to KVM as well. This also means the start of QEMU > > will fail to start if KVM support turns out to be unavailable at > > runtime. > > > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > --- > > kvm-all.c | 1 + > > kvm-stub.c | 1 + > > kvm.h | 1 + > > vl.c | 4 ++-- > > 4 files changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/kvm-all.c b/kvm-all.c > > index 92a7137..4d5f86c 100644 > > --- a/kvm-all.c > > +++ b/kvm-all.c > > @@ -103,6 +103,7 @@ struct KVMState > > #endif > > }; > > > > +bool kvm_configured = true; > > KVMState *kvm_state; > > bool kvm_kernel_irqchip; > > bool kvm_async_interrupts_allowed; > > diff --git a/kvm-stub.c b/kvm-stub.c > > index 3c52eb5..86a6451 100644 > > --- a/kvm-stub.c > > +++ b/kvm-stub.c > > @@ -17,6 +17,7 @@ > > #include "gdbstub.h" > > #include "kvm.h" > > > > +bool kvm_configured; > > KVMState *kvm_state; > > bool kvm_kernel_irqchip; > > bool kvm_async_interrupts_allowed; > > diff --git a/kvm.h b/kvm.h > > index dea2998..9936e5f 100644 > > --- a/kvm.h > > +++ b/kvm.h > > @@ -22,6 +22,7 @@ > > #include <linux/kvm.h> > > #endif > > > > +extern bool kvm_configured; > > extern int kvm_allowed; > > extern bool kvm_kernel_irqchip; > > extern bool kvm_async_interrupts_allowed; > > diff --git a/vl.c b/vl.c > > index 8d305ca..f557bd1 100644 > > --- a/vl.c > > +++ b/vl.c > > @@ -2215,8 +2215,8 @@ static int configure_accelerator(void) > > } > > > > if (p == NULL) { > > - /* Use the default "accelerator", tcg */ > > - p = "tcg"; > > + /* The default accelerator depends on the availability of KVM. */ > > + p = kvm_configured ? "kvm" : "tcg"; > > } > > How about making this an arch_init() function call and then using a #if > defined(KVM_CONFIG) in arch_init.c? > > I hate to introduce another global variable if we can avoid it... > > Otherwise: > > Acked-by: Anthony Liguori <aliguori@us.ibm.com> > > Blue/Aurelien, any objections? > I am not sure this way of doing is really distribution friendly, where the QEMU package is built for a large variety of hosts, some of them maybe without KVM support. I am all for enabling KVM by default, but I think it should fall back to TCG if it is not enabled (probably with a warning so that the user is aware of that). On the other hand, if the user explicitly enables KVM support with -enable-kvm or with accel=kvm, it should fail to start. In other words, a bit like the configure script options, by default we use auto-detection, but if an option is explicitly enabled, it fails if it can't be enabled.
"Daniel P. Berrange" <berrange@redhat.com> writes: > On Mon, Oct 01, 2012 at 06:43:00PM +0200, Andreas Färber wrote: >> Hello Jan, >> >> Am 01.10.2012 16:34, schrieb Jan Kiszka: >> > If we built a target for a host that supports KVM in principle, set the >> > default accelerator to KVM as well. This also means the start of QEMU >> > will fail to start if KVM support turns out to be unavailable at >> > runtime. >> >> From a distro point of view this of course means that we will build >> against KVM and that the new KVM default will start to fail for users on >> very old hardware. Can't we do a runtime check to select the default? > > NB, this is *not* only about old hardware. There are plenty of users who > use QEMU inside VMs. One very common usage I know of is image building > tools which are run inside Amazon VMs, using libguestfs & QEMU. But libguest can set it's accelerator option to whatever it wants. If your running QEMU under a VM, it's pretty reasonable to have to use a special option IMHO. > IMHO, default to KVM, fallback to TCG is the most friendly default > behaviour. Except if a user expects good network performance and can't understand why they're getting 100kb/s. Regards, Anthony Liguori > > Daniel > -- > |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| > |: http://libvirt.org -o- http://virt-manager.org :| > |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| > |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
> But libguest can set it's accelerator option to whatever it wants. > > If your running QEMU under a VM, it's pretty reasonable to have to > use a special option IMHO. It's also reasonable to have consecutive releases change defaults in a more "friendly" way (i.e. from tcg to kvm:tcg), especially since we'll get users that formerly used qemu-kvm and never had to specify neither -machine accel nor --enable-kvm. > > IMHO, default to KVM, fallback to TCG is the most friendly default > > behaviour. > > Except if a user expects good network performance and can't > understand why they're getting 100kb/s. libguestfs doesn't need network at all (though I wonder if they could use lxc instead...). Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: >> But libguest can set it's accelerator option to whatever it wants. >> >> If your running QEMU under a VM, it's pretty reasonable to have to >> use a special option IMHO. > > It's also reasonable to have consecutive releases change defaults in > a more "friendly" way (i.e. from tcg to kvm:tcg), especially since > we'll get users that formerly used qemu-kvm and never had to specify > neither -machine accel nor --enable-kvm. I agree with you except for the 'kvm:tcg' part. > >> > IMHO, default to KVM, fallback to TCG is the most friendly default >> > behaviour. >> >> Except if a user expects good network performance and can't >> understand why they're getting 100kb/s. > > libguestfs doesn't need network at all (though I wonder if they could > use lxc instead...). FWIW, libguestfs already uses -machine accel=kvm:tcg so we can completely the libguestfs use-case for this discussion. Regards, Anthony Liguori > > Paolo
"Daniel P. Berrange" <berrange@redhat.com> writes: > On Mon, Oct 01, 2012 at 06:43:00PM +0200, Andreas Färber wrote: >> Hello Jan, >> >> Am 01.10.2012 16:34, schrieb Jan Kiszka: >> > If we built a target for a host that supports KVM in principle, set the >> > default accelerator to KVM as well. This also means the start of QEMU >> > will fail to start if KVM support turns out to be unavailable at >> > runtime. >> >> From a distro point of view this of course means that we will build >> against KVM and that the new KVM default will start to fail for users on >> very old hardware. Can't we do a runtime check to select the default? > > NB, this is *not* only about old hardware. There are plenty of users who > use QEMU inside VMs. One very common usage I know of is image building > tools which are run inside Amazon VMs, using libguestfs & QEMU. > > IMHO, default to KVM, fallback to TCG is the most friendly default > behaviour. Friendly perhaps, generating an infinite series of questions "why is my guest slow as molasses?" certainly. And for each instance of the question, there's an unknown number of users who give QEMU a quick try, screw up KVM unknowingly, observe the glacial speed, and conclude it's crap.
On Tue, Oct 02, 2012 at 09:46:12AM +0200, Markus Armbruster wrote: > "Daniel P. Berrange" <berrange@redhat.com> writes: > > > On Mon, Oct 01, 2012 at 06:43:00PM +0200, Andreas Färber wrote: > >> Hello Jan, > >> > >> Am 01.10.2012 16:34, schrieb Jan Kiszka: > >> > If we built a target for a host that supports KVM in principle, set the > >> > default accelerator to KVM as well. This also means the start of QEMU > >> > will fail to start if KVM support turns out to be unavailable at > >> > runtime. > >> > >> From a distro point of view this of course means that we will build > >> against KVM and that the new KVM default will start to fail for users on > >> very old hardware. Can't we do a runtime check to select the default? > > > > NB, this is *not* only about old hardware. There are plenty of users who > > use QEMU inside VMs. One very common usage I know of is image building > > tools which are run inside Amazon VMs, using libguestfs & QEMU. > > > > IMHO, default to KVM, fallback to TCG is the most friendly default > > behaviour. > > Friendly perhaps, generating an infinite series of questions "why is my > guest slow as molasses?" certainly. > > And for each instance of the question, there's an unknown number of > users who give QEMU a quick try, screw up KVM unknowingly, observe the > glacial speed, and conclude it's crap. > That's why it should not fallback silently to TCG, but warn the user about that. On the other hand, on a machine without KVM support (which might just be because of local policy admin policy which doesn't provide access the /dev/kvm, nested virtualization, etc.), if QEMU fails to start (while previous versions were working), the user can conclude that QEMU is crap.
On 02.10.2012 11:46, Markus Armbruster wrote: > "Daniel P. Berrange" <berrange@redhat.com> writes: >> IMHO, default to KVM, fallback to TCG is the most friendly default >> behaviour. > > Friendly perhaps, generating an infinite series of questions "why is my > guest slow as molasses?" certainly. With a warning about "switching to slow emulation mode because .." printed at startup that becomes a non-issue, because there's no reason to ask more questions about why it is slow - it already said why. Yes some may try to ask what to do, which is different. Every howto nowadays mentions kvm modules and /dev/kvm device permissions. > And for each instance of the question, there's an unknown number of > users who give QEMU a quick try, screw up KVM unknowingly, observe the > glacial speed, and conclude it's crap. This is, again, I think, unfair. With the warning message it becomes more or less obvious. If you're talking about users who run it with -daemonize argument - this is a) stupid to do when TRYING it out, so it's not a big deal to lose another stupid user, and b) qemu should init everything first and throw all warnings and fatal errors before daemonizing, if this is not the case it should be fixed in the code. And if you're talking about management software (libvirt and others), it controls all the required privileges already and explicitly requests acceleration and other stuff. So the best thing to do is what Daniel, Aurelien, Paolo and others are suggested: accel=kvm:tcg with a warning. Thanks, /mjt
On 2012-10-01 18:20, Anthony Liguori wrote: > Jan Kiszka <jan.kiszka@siemens.com> writes: > >> If we built a target for a host that supports KVM in principle, set the >> default accelerator to KVM as well. This also means the start of QEMU >> will fail to start if KVM support turns out to be unavailable at >> runtime. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> kvm-all.c | 1 + >> kvm-stub.c | 1 + >> kvm.h | 1 + >> vl.c | 4 ++-- >> 4 files changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/kvm-all.c b/kvm-all.c >> index 92a7137..4d5f86c 100644 >> --- a/kvm-all.c >> +++ b/kvm-all.c >> @@ -103,6 +103,7 @@ struct KVMState >> #endif >> }; >> >> +bool kvm_configured = true; >> KVMState *kvm_state; >> bool kvm_kernel_irqchip; >> bool kvm_async_interrupts_allowed; >> diff --git a/kvm-stub.c b/kvm-stub.c >> index 3c52eb5..86a6451 100644 >> --- a/kvm-stub.c >> +++ b/kvm-stub.c >> @@ -17,6 +17,7 @@ >> #include "gdbstub.h" >> #include "kvm.h" >> >> +bool kvm_configured; >> KVMState *kvm_state; >> bool kvm_kernel_irqchip; >> bool kvm_async_interrupts_allowed; >> diff --git a/kvm.h b/kvm.h >> index dea2998..9936e5f 100644 >> --- a/kvm.h >> +++ b/kvm.h >> @@ -22,6 +22,7 @@ >> #include <linux/kvm.h> >> #endif >> >> +extern bool kvm_configured; >> extern int kvm_allowed; >> extern bool kvm_kernel_irqchip; >> extern bool kvm_async_interrupts_allowed; >> diff --git a/vl.c b/vl.c >> index 8d305ca..f557bd1 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -2215,8 +2215,8 @@ static int configure_accelerator(void) >> } >> >> if (p == NULL) { >> - /* Use the default "accelerator", tcg */ >> - p = "tcg"; >> + /* The default accelerator depends on the availability of KVM. */ >> + p = kvm_configured ? "kvm" : "tcg"; >> } > > How about making this an arch_init() function call and then using a #if > defined(KVM_CONFIG) in arch_init.c? > > I hate to introduce another global variable if we can avoid it... Hacked too quickly. In fact, kvm_configured is simply kvm_available(). However, resistance appear to be too high here. Jan > > Otherwise: > > Acked-by: Anthony Liguori <aliguori@us.ibm.com> > > Blue/Aurelien, any objections? > > Regards, > > Anthony Liguori > >> >> while (!accel_initialised && *p != '\0') { >> -- >> 1.7.3.4 > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On 2012-10-03 08:58, Michael Tokarev wrote: > On 02.10.2012 11:46, Markus Armbruster wrote: >> "Daniel P. Berrange" <berrange@redhat.com> writes: > >>> IMHO, default to KVM, fallback to TCG is the most friendly default >>> behaviour. >> >> Friendly perhaps, generating an infinite series of questions "why is my >> guest slow as molasses?" certainly. > > With a warning about "switching to slow emulation mode because .." > printed at startup that becomes a non-issue, because there's no > reason to ask more questions about why it is slow - it already > said why. Yes some may try to ask what to do, which is different. > > Every howto nowadays mentions kvm modules and /dev/kvm device > permissions. > >> And for each instance of the question, there's an unknown number of >> users who give QEMU a quick try, screw up KVM unknowingly, observe the >> glacial speed, and conclude it's crap. > > This is, again, I think, unfair. With the warning message it becomes > more or less obvious. > > If you're talking about users who run it with -daemonize argument - > this is a) stupid to do when TRYING it out, so it's not a big deal > to lose another stupid user, and b) qemu should init everything > first and throw all warnings and fatal errors before daemonizing, > if this is not the case it should be fixed in the code. > > And if you're talking about management software (libvirt and others), > it controls all the required privileges already and explicitly > requests acceleration and other stuff. > > So the best thing to do is what Daniel, Aurelien, Paolo and others > are suggested: accel=kvm:tcg with a warning. Well, we had a lot of problems with such a fallback in the past, but I think we had no proper warnings back then. I'm not fully believing in users will always realize the console message. I would therefore suggest to change the window title of QEMU as well if we fail to initialize some accelerator. Something like "QEMU without KVM" (could be "QEMU without $FAILED_ACCEL" in the end, i.e. not just for KVM). If that makes sense for everyone, I'll hack the required patches. Jan
On Mon, Oct 1, 2012 at 4:20 PM, Anthony Liguori <anthony@codemonkey.ws> wrote: > Jan Kiszka <jan.kiszka@siemens.com> writes: > >> If we built a target for a host that supports KVM in principle, set the >> default accelerator to KVM as well. This also means the start of QEMU >> will fail to start if KVM support turns out to be unavailable at >> runtime. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> kvm-all.c | 1 + >> kvm-stub.c | 1 + >> kvm.h | 1 + >> vl.c | 4 ++-- >> 4 files changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/kvm-all.c b/kvm-all.c >> index 92a7137..4d5f86c 100644 >> --- a/kvm-all.c >> +++ b/kvm-all.c >> @@ -103,6 +103,7 @@ struct KVMState >> #endif >> }; >> >> +bool kvm_configured = true; >> KVMState *kvm_state; >> bool kvm_kernel_irqchip; >> bool kvm_async_interrupts_allowed; >> diff --git a/kvm-stub.c b/kvm-stub.c >> index 3c52eb5..86a6451 100644 >> --- a/kvm-stub.c >> +++ b/kvm-stub.c >> @@ -17,6 +17,7 @@ >> #include "gdbstub.h" >> #include "kvm.h" >> >> +bool kvm_configured; >> KVMState *kvm_state; >> bool kvm_kernel_irqchip; >> bool kvm_async_interrupts_allowed; >> diff --git a/kvm.h b/kvm.h >> index dea2998..9936e5f 100644 >> --- a/kvm.h >> +++ b/kvm.h >> @@ -22,6 +22,7 @@ >> #include <linux/kvm.h> >> #endif >> >> +extern bool kvm_configured; >> extern int kvm_allowed; >> extern bool kvm_kernel_irqchip; >> extern bool kvm_async_interrupts_allowed; >> diff --git a/vl.c b/vl.c >> index 8d305ca..f557bd1 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -2215,8 +2215,8 @@ static int configure_accelerator(void) >> } >> >> if (p == NULL) { >> - /* Use the default "accelerator", tcg */ >> - p = "tcg"; >> + /* The default accelerator depends on the availability of KVM. */ >> + p = kvm_configured ? "kvm" : "tcg"; >> } > > How about making this an arch_init() function call and then using a #if > defined(KVM_CONFIG) in arch_init.c? > > I hate to introduce another global variable if we can avoid it... > > Otherwise: > > Acked-by: Anthony Liguori <aliguori@us.ibm.com> > > Blue/Aurelien, any objections? No, maybe a message could be printed that says that the default has changed, for a few releases. > > Regards, > > Anthony Liguori > >> >> while (!accel_initialised && *p != '\0') { >> -- >> 1.7.3.4
On 3 October 2012 21:01, Blue Swirl <blauwirbel@gmail.com> wrote: > On Mon, Oct 1, 2012 at 4:20 PM, Anthony Liguori <anthony@codemonkey.ws> wrote: >> Jan Kiszka <jan.kiszka@siemens.com> writes: >>> + /* The default accelerator depends on the availability of KVM. */ >>> + p = kvm_configured ? "kvm" : "tcg"; >>> } >> Blue/Aurelien, any objections? > > No, maybe a message could be printed that says that the default has > changed, for a few releases. I've lost track of the conversation, are we currently proposing the accelerator default to be "kvm" (as per the original patch you quote here) or "kvm:tcg" ? I'm not entirely sure which I prefer from an ARM perspective For some time to come and for a lot of targets (ie any target CPU except A15), having a default of "kvm" is going to cause existing working commandlines to stop working. [I expect that ARM-host qemu binaries will be built with CONFIG_KVM once ARM KVM support lands, but the same binary will be run on hosts without virtualization extensions.] On the other hand, perhaps there just aren't really very many people who run QEMU on ARM hosts, and so we can ignore them :-) -- PMM
On 03.10.2012, at 22:26, Peter Maydell wrote: > On 3 October 2012 21:01, Blue Swirl <blauwirbel@gmail.com> wrote: >> On Mon, Oct 1, 2012 at 4:20 PM, Anthony Liguori <anthony@codemonkey.ws> wrote: >>> Jan Kiszka <jan.kiszka@siemens.com> writes: >>>> + /* The default accelerator depends on the availability of KVM. */ >>>> + p = kvm_configured ? "kvm" : "tcg"; >>>> } > >>> Blue/Aurelien, any objections? >> >> No, maybe a message could be printed that says that the default has >> changed, for a few releases. > > I've lost track of the conversation, are we currently proposing > the accelerator default to be "kvm" (as per the original patch > you quote here) or "kvm:tcg" ? > > I'm not entirely sure which I prefer from an ARM perspective > For some time to come and for a lot of targets (ie any target > CPU except A15), having a default of "kvm" is going to cause > existing working commandlines to stop working. [I expect that > ARM-host qemu binaries will be built with CONFIG_KVM once ARM > KVM support lands, but the same binary will be run on hosts > without virtualization extensions.] On the other hand, perhaps > there just aren't really very many people who run QEMU on > ARM hosts, and so we can ignore them :-) We get similar problems on PPC. Take the following example: $ qemu-system-ppc -M mpc8544ds -kernel uImage -nographic That command line would work just fine if you run it on a G5 today. However, if we switch to accel=kvm:tcg, it will stop working, because kvm on the G5 can not virtualize an e500 CPU. I don't know any good way around that one either the way the code is layered today. We only know the type of CPU we want to create after we made the decision whether we do KVM or not. Alex
Alexander Graf <agraf@suse.de> writes: > On 03.10.2012, at 22:26, Peter Maydell wrote: > >> On 3 October 2012 21:01, Blue Swirl <blauwirbel@gmail.com> wrote: >>> On Mon, Oct 1, 2012 at 4:20 PM, Anthony Liguori <anthony@codemonkey.ws> wrote: >>>> Jan Kiszka <jan.kiszka@siemens.com> writes: >>>>> + /* The default accelerator depends on the availability of KVM. */ >>>>> + p = kvm_configured ? "kvm" : "tcg"; >>>>> } >> >>>> Blue/Aurelien, any objections? >>> >>> No, maybe a message could be printed that says that the default has >>> changed, for a few releases. >> >> I've lost track of the conversation, are we currently proposing >> the accelerator default to be "kvm" (as per the original patch >> you quote here) or "kvm:tcg" ? >> >> I'm not entirely sure which I prefer from an ARM perspective >> For some time to come and for a lot of targets (ie any target >> CPU except A15), having a default of "kvm" is going to cause >> existing working commandlines to stop working. [I expect that >> ARM-host qemu binaries will be built with CONFIG_KVM once ARM >> KVM support lands, but the same binary will be run on hosts >> without virtualization extensions.] On the other hand, perhaps >> there just aren't really very many people who run QEMU on >> ARM hosts, and so we can ignore them :-) > > We get similar problems on PPC. Take the following example: > > $ qemu-system-ppc -M mpc8544ds -kernel uImage -nographic But do you really expect people to do this? I have to believe that people running on PPC hardware and running qemu-system-ppc most likely want to do KVM... Kernel development seems unlikely... The folks that I know doing PPC kernel development with QEMU still qemu-system-ppc on x86. Regards, Anthony Liguori > > That command line would work just fine if you run it on a G5 today. However, if we switch to accel=kvm:tcg, it will stop working, because kvm on the G5 can not virtualize an e500 CPU. > > I don't know any good way around that one either the way the code is layered today. We only know the type of CPU we want to create after we made the decision whether we do KVM or not. > > > Alex
On 05.10.2012, at 04:17, Anthony Liguori wrote: > Alexander Graf <agraf@suse.de> writes: > >> On 03.10.2012, at 22:26, Peter Maydell wrote: >> >>> On 3 October 2012 21:01, Blue Swirl <blauwirbel@gmail.com> wrote: >>>> On Mon, Oct 1, 2012 at 4:20 PM, Anthony Liguori <anthony@codemonkey.ws> wrote: >>>>> Jan Kiszka <jan.kiszka@siemens.com> writes: >>>>>> + /* The default accelerator depends on the availability of KVM. */ >>>>>> + p = kvm_configured ? "kvm" : "tcg"; >>>>>> } >>> >>>>> Blue/Aurelien, any objections? >>>> >>>> No, maybe a message could be printed that says that the default has >>>> changed, for a few releases. >>> >>> I've lost track of the conversation, are we currently proposing >>> the accelerator default to be "kvm" (as per the original patch >>> you quote here) or "kvm:tcg" ? >>> >>> I'm not entirely sure which I prefer from an ARM perspective >>> For some time to come and for a lot of targets (ie any target >>> CPU except A15), having a default of "kvm" is going to cause >>> existing working commandlines to stop working. [I expect that >>> ARM-host qemu binaries will be built with CONFIG_KVM once ARM >>> KVM support lands, but the same binary will be run on hosts >>> without virtualization extensions.] On the other hand, perhaps >>> there just aren't really very many people who run QEMU on >>> ARM hosts, and so we can ignore them :-) >> >> We get similar problems on PPC. Take the following example: >> >> $ qemu-system-ppc -M mpc8544ds -kernel uImage -nographic > > But do you really expect people to do this? I have to believe that > people running on PPC hardware and running qemu-system-ppc most likely > want to do KVM... Sure. But we wouldn't be able to even tell them what went wrong, as we don't have a negotiation mechanism right now that could tell user space "hey, the CPU you selected is unknown to me". However, if during cpu init we could add such a check and then fall back to tcg mode if accel=kvm:tcg with a warning, that'd be nice user experience. We could do the same for ARM. If you do -M beagle on an A15 KVM enabled machine, you would still be able to do so, but KVM tells you it can't emulate an A8 right now. And if in the future KVM learns how to expose an A8 on A15, we could just not bail out and things would magically work. Apart from that, I like the idea of kvm:tcg with a warning as the default for qemu. We should still have a qemu-kvm binary in the distro that does accel=kvm so people don't accidentally fall back to tcg mode. Alex
On 5 October 2012 03:24, Alexander Graf <agraf@suse.de> wrote: > On 05.10.2012, at 04:17, Anthony Liguori wrote: >> Alexander Graf <agraf@suse.de> writes: >>> We get similar problems on PPC. Take the following example: >>> >>> $ qemu-system-ppc -M mpc8544ds -kernel uImage -nographic >> >> But do you really expect people to do this? I have to believe that >> people running on PPC hardware and running qemu-system-ppc most likely >> want to do KVM... > > Sure. But we wouldn't be able to even tell them what went wrong, > as we don't have a negotiation mechanism right now that could tell > user space "hey, the CPU you selected is unknown to me". On ARM we will at least be able to say what happened, because our kvm_arch_init_vcpu() will fail (when we try the ioctl to tell the kernel "be this kind of CPU") and we can print a message there. However QEMU as a whole doesn't have any way of falling back to TCG at that point so it just bails out. -- PMM
Am 05.10.2012 04:24, schrieb Alexander Graf: > > On 05.10.2012, at 04:17, Anthony Liguori wrote: > >> Alexander Graf <agraf@suse.de> writes: >> >>> On 03.10.2012, at 22:26, Peter Maydell wrote: >>> >>>> On 3 October 2012 21:01, Blue Swirl <blauwirbel@gmail.com> wrote: >>>>> On Mon, Oct 1, 2012 at 4:20 PM, Anthony Liguori <anthony@codemonkey.ws> wrote: >>>>>> Jan Kiszka <jan.kiszka@siemens.com> writes: >>>>>>> + /* The default accelerator depends on the availability of KVM. */ >>>>>>> + p = kvm_configured ? "kvm" : "tcg"; >>>>>>> } >>>> >>>>>> Blue/Aurelien, any objections? >>>>> >>>>> No, maybe a message could be printed that says that the default has >>>>> changed, for a few releases. >>>> >>>> I've lost track of the conversation, are we currently proposing >>>> the accelerator default to be "kvm" (as per the original patch >>>> you quote here) or "kvm:tcg" ? >>>> >>>> I'm not entirely sure which I prefer from an ARM perspective >>>> For some time to come and for a lot of targets (ie any target >>>> CPU except A15), having a default of "kvm" is going to cause >>>> existing working commandlines to stop working. [I expect that >>>> ARM-host qemu binaries will be built with CONFIG_KVM once ARM >>>> KVM support lands, but the same binary will be run on hosts >>>> without virtualization extensions.] On the other hand, perhaps >>>> there just aren't really very many people who run QEMU on >>>> ARM hosts, and so we can ignore them :-) >>> >>> We get similar problems on PPC. Take the following example: >>> >>> $ qemu-system-ppc -M mpc8544ds -kernel uImage -nographic >> >> But do you really expect people to do this? I have to believe that >> people running on PPC hardware and running qemu-system-ppc most likely >> want to do KVM... > > Sure. But we wouldn't be able to even tell them what went wrong, as we don't have a negotiation mechanism right now that could tell user space "hey, the CPU you selected is unknown to me". Would it help to split out the cpu_model -> CPUClass lookup from cpu_ppc_init() to invoke a hook or inquire a field indicating KVM support? Andreas > > However, if during cpu init we could add such a check and then fall back to tcg mode if accel=kvm:tcg with a warning, that'd be nice user experience. > > We could do the same for ARM. If you do -M beagle on an A15 KVM enabled machine, you would still be able to do so, but KVM tells you it can't emulate an A8 right now. And if in the future KVM learns how to expose an A8 on A15, we could just not bail out and things would magically work. > > Apart from that, I like the idea of kvm:tcg with a warning as the default for qemu. We should still have a qemu-kvm binary in the distro that does accel=kvm so people don't accidentally fall back to tcg mode. > > > Alex >
On 08.10.2012, at 16:03, Andreas Färber wrote: > Am 05.10.2012 04:24, schrieb Alexander Graf: >> >> On 05.10.2012, at 04:17, Anthony Liguori wrote: >> >>> Alexander Graf <agraf@suse.de> writes: >>> >>>> On 03.10.2012, at 22:26, Peter Maydell wrote: >>>> >>>>> On 3 October 2012 21:01, Blue Swirl <blauwirbel@gmail.com> wrote: >>>>>> On Mon, Oct 1, 2012 at 4:20 PM, Anthony Liguori <anthony@codemonkey.ws> wrote: >>>>>>> Jan Kiszka <jan.kiszka@siemens.com> writes: >>>>>>>> + /* The default accelerator depends on the availability of KVM. */ >>>>>>>> + p = kvm_configured ? "kvm" : "tcg"; >>>>>>>> } >>>>> >>>>>>> Blue/Aurelien, any objections? >>>>>> >>>>>> No, maybe a message could be printed that says that the default has >>>>>> changed, for a few releases. >>>>> >>>>> I've lost track of the conversation, are we currently proposing >>>>> the accelerator default to be "kvm" (as per the original patch >>>>> you quote here) or "kvm:tcg" ? >>>>> >>>>> I'm not entirely sure which I prefer from an ARM perspective >>>>> For some time to come and for a lot of targets (ie any target >>>>> CPU except A15), having a default of "kvm" is going to cause >>>>> existing working commandlines to stop working. [I expect that >>>>> ARM-host qemu binaries will be built with CONFIG_KVM once ARM >>>>> KVM support lands, but the same binary will be run on hosts >>>>> without virtualization extensions.] On the other hand, perhaps >>>>> there just aren't really very many people who run QEMU on >>>>> ARM hosts, and so we can ignore them :-) >>>> >>>> We get similar problems on PPC. Take the following example: >>>> >>>> $ qemu-system-ppc -M mpc8544ds -kernel uImage -nographic >>> >>> But do you really expect people to do this? I have to believe that >>> people running on PPC hardware and running qemu-system-ppc most likely >>> want to do KVM... >> >> Sure. But we wouldn't be able to even tell them what went wrong, as we don't have a negotiation mechanism right now that could tell user space "hey, the CPU you selected is unknown to me". > > Would it help to split out the cpu_model -> CPUClass lookup from > cpu_ppc_init() to invoke a hook or inquire a field indicating KVM support? Well, we need to basically determine whether KVM is enabled only after cpu creation of the machine file. Alex
diff --git a/kvm-all.c b/kvm-all.c index 92a7137..4d5f86c 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -103,6 +103,7 @@ struct KVMState #endif }; +bool kvm_configured = true; KVMState *kvm_state; bool kvm_kernel_irqchip; bool kvm_async_interrupts_allowed; diff --git a/kvm-stub.c b/kvm-stub.c index 3c52eb5..86a6451 100644 --- a/kvm-stub.c +++ b/kvm-stub.c @@ -17,6 +17,7 @@ #include "gdbstub.h" #include "kvm.h" +bool kvm_configured; KVMState *kvm_state; bool kvm_kernel_irqchip; bool kvm_async_interrupts_allowed; diff --git a/kvm.h b/kvm.h index dea2998..9936e5f 100644 --- a/kvm.h +++ b/kvm.h @@ -22,6 +22,7 @@ #include <linux/kvm.h> #endif +extern bool kvm_configured; extern int kvm_allowed; extern bool kvm_kernel_irqchip; extern bool kvm_async_interrupts_allowed; diff --git a/vl.c b/vl.c index 8d305ca..f557bd1 100644 --- a/vl.c +++ b/vl.c @@ -2215,8 +2215,8 @@ static int configure_accelerator(void) } if (p == NULL) { - /* Use the default "accelerator", tcg */ - p = "tcg"; + /* The default accelerator depends on the availability of KVM. */ + p = kvm_configured ? "kvm" : "tcg"; } while (!accel_initialised && *p != '\0') {
If we built a target for a host that supports KVM in principle, set the default accelerator to KVM as well. This also means the start of QEMU will fail to start if KVM support turns out to be unavailable at runtime. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- kvm-all.c | 1 + kvm-stub.c | 1 + kvm.h | 1 + vl.c | 4 ++-- 4 files changed, 5 insertions(+), 2 deletions(-)