Hi all, I have few simple fixes for Xen for the development tree as well as QEMU 1.5 and older releases: - the first patch is just a cleanup (that is needed to simplify preprocessor dependencies); - the second patch avoids setting nonblocking on Xen (as is already done for KVM); These two patches should be backported to QEMU 1.5 and older stable branches. - the last patch fixes the "Invalid icc-bridge value" error that prevents the Xen PV machine from starting. This patch should be backported to QEMU 1.5. Stefano Stabellini (3): xen: simplify xen_enabled main_loop: do not set nonblocking if xen_enabled() xen_machine_pv: do not create a dummy CPU in machine->init hw/i386/xen_machine_pv.c | 16 ---------------- include/hw/xen/xen.h | 4 ---- vl.c | 2 +- 3 files changed, 1 insertions(+), 21 deletions(-) - Stefano
No need for preprocessor conditionals in xen_enabled: xen_allowed is always defined. Please backport this patch to stable branches (it is needed by the following patch to remove the dependency on a preprocessor identifier defined by config-target.h). Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> CC: qemu-stable@nongnu.org --- include/hw/xen/xen.h | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-) diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h index 7451c5a..b42b0fd 100644 --- a/include/hw/xen/xen.h +++ b/include/hw/xen/xen.h @@ -25,11 +25,7 @@ extern bool xen_allowed; static inline bool xen_enabled(void) { -#if defined(CONFIG_XEN_BACKEND) && defined(CONFIG_XEN) return xen_allowed; -#else - return 0; -#endif } int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num); -- 1.7.2.5
Stefano Stabellini
2013-May-29 11:34 UTC
[PATCH v2 2/3] main_loop: do not set nonblocking if xen_enabled()
Please backport this patch to stable branches. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> CC: qemu-stable@nongnu.org --- vl.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/vl.c b/vl.c index 510d2c2..47ab45d 100644 --- a/vl.c +++ b/vl.c @@ -2022,7 +2022,7 @@ static void main_loop(void) int64_t ti; #endif do { - nonblocking = !kvm_enabled() && last_io > 0; + nonblocking = !kvm_enabled() && !xen_enabled() && last_io > 0; #ifdef CONFIG_PROFILER ti = profile_getclock(); #endif -- 1.7.2.5
Stefano Stabellini
2013-May-29 11:34 UTC
[PATCH v2 3/3] xen_machine_pv: do not create a dummy CPU in machine->init
This fixes a regression introduced by: commit 62fc403f11523169eb4264de31279745f48e3ecc Author: Igor Mammedov <imammedo@redhat.com> Date: Mon Apr 29 18:54:13 2013 +0200 target-i386: Attach ICC bus to CPU on its creation X86CPU should have parent bus so it could provide bus for child APIC. The commit makes it mandatory to pass a valid ICC bus to cpu_x86_create, but cpu_x86_init just passes NULL to it. xen_machine_pv uses cpu_x86_init, therefore it has been broken. This patch fixes the problem by removing the dummy CPU creation altogether from xen_init_pv, relying on the fact that QEMU can now cope with a machine without an emulated CPU. This fix should be backported to QEMU 1.5. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> CC: imammedo@redhat.com CC: qemu-stable@nongnu.org --- hw/i386/xen_machine_pv.c | 16 ---------------- 1 files changed, 0 insertions(+), 16 deletions(-) diff --git a/hw/i386/xen_machine_pv.c b/hw/i386/xen_machine_pv.c index f829a52..9f2e291 100644 --- a/hw/i386/xen_machine_pv.c +++ b/hw/i386/xen_machine_pv.c @@ -23,7 +23,6 @@ */ #include "hw/hw.h" -#include "hw/i386/pc.h" #include "hw/boards.h" #include "hw/xen/xen_backend.h" #include "xen_domainbuild.h" @@ -31,27 +30,12 @@ static void xen_init_pv(QEMUMachineInitArgs *args) { - const char *cpu_model = args->cpu_model; const char *kernel_filename = args->kernel_filename; const char *kernel_cmdline = args->kernel_cmdline; const char *initrd_filename = args->initrd_filename; - X86CPU *cpu; - CPUState *cs; DriveInfo *dinfo; int i; - /* Initialize a dummy CPU */ - if (cpu_model == NULL) { -#ifdef TARGET_X86_64 - cpu_model = "qemu64"; -#else - cpu_model = "qemu32"; -#endif - } - cpu = cpu_x86_init(cpu_model); - cs = CPU(cpu); - cs->halted = 1; - /* Initialize backend core & drivers */ if (xen_be_init() != 0) { fprintf(stderr, "%s: xen backend core setup failed\n", __FUNCTION__); -- 1.7.2.5
Andreas Färber
2013-May-29 12:11 UTC
Re: [PATCH v2 3/3] xen_machine_pv: do not create a dummy CPU in machine->init
Am 29.05.2013 13:34, schrieb Stefano Stabellini:> This fixes a regression introduced by: > > commit 62fc403f11523169eb4264de31279745f48e3ecc > Author: Igor Mammedov <imammedo@redhat.com> > Date: Mon Apr 29 18:54:13 2013 +0200 > > target-i386: Attach ICC bus to CPU on its creation > > X86CPU should have parent bus so it could provide bus for child APIC. > > The commit makes it mandatory to pass a valid ICC bus to cpu_x86_create, > but cpu_x86_init just passes NULL to it.This is not entirely accurate: The ICC bus is only needed for softmmu, the envisioned remaining use case of cpu_x86_init() was bsd/linux-user. And sorry for not catching this use case, I was in a hurry for the Hard Freeze.> xen_machine_pv uses cpu_x86_init, therefore it has been broken. > > This patch fixes the problem by removing the dummy CPU creation > altogether from xen_init_pv, relying on the fact that QEMU can now cope > with a machine without an emulated CPU. > > This fix should be backported to QEMU 1.5. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > CC: imammedo@redhat.com > CC: qemu-stable@nongnu.orgChange looks okay, Reviewed-by: Andreas Färber <afaerber@suse.de> But for the future please avoid "this patch" in the commit message (because later on it''s a "commit", not a patch) and also please put additional notices such as for backporting below the --- line. Cheers, Andreas> --- > hw/i386/xen_machine_pv.c | 16 ---------------- > 1 files changed, 0 insertions(+), 16 deletions(-) > > diff --git a/hw/i386/xen_machine_pv.c b/hw/i386/xen_machine_pv.c > index f829a52..9f2e291 100644 > --- a/hw/i386/xen_machine_pv.c > +++ b/hw/i386/xen_machine_pv.c > @@ -23,7 +23,6 @@ > */ > > #include "hw/hw.h" > -#include "hw/i386/pc.h" > #include "hw/boards.h" > #include "hw/xen/xen_backend.h" > #include "xen_domainbuild.h" > @@ -31,27 +30,12 @@ > > static void xen_init_pv(QEMUMachineInitArgs *args) > { > - const char *cpu_model = args->cpu_model; > const char *kernel_filename = args->kernel_filename; > const char *kernel_cmdline = args->kernel_cmdline; > const char *initrd_filename = args->initrd_filename; > - X86CPU *cpu; > - CPUState *cs; > DriveInfo *dinfo; > int i; > > - /* Initialize a dummy CPU */ > - if (cpu_model == NULL) { > -#ifdef TARGET_X86_64 > - cpu_model = "qemu64"; > -#else > - cpu_model = "qemu32"; > -#endif > - } > - cpu = cpu_x86_init(cpu_model); > - cs = CPU(cpu); > - cs->halted = 1; > - > /* Initialize backend core & drivers */ > if (xen_be_init() != 0) { > fprintf(stderr, "%s: xen backend core setup failed\n", __FUNCTION__); >-- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Paolo Bonzini
2013-May-29 12:29 UTC
Re: [PATCH v2 3/3] xen_machine_pv: do not create a dummy CPU in machine->init
Il 29/05/2013 14:11, Andreas Färber ha scritto:>> xen_machine_pv uses cpu_x86_init, therefore it has been broken. >> >> This patch fixes the problem by removing the dummy CPU creation >> altogether from xen_init_pv, relying on the fact that QEMU can now cope >> with a machine without an emulated CPU. >> >> This fix should be backported to QEMU 1.5. >> >> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> CC: imammedo@redhat.com >> CC: qemu-stable@nongnu.org > > Change looks okay, > > Reviewed-by: Andreas Färber <afaerber@suse.de> > > But for the future please avoid "this patch" in the commit message > (because later on it''s a "commit", not a patch) and also please put > additional notices such as for backporting below the --- line.I read it as a warning to backporters that the patch, even though it may apply, was not tested and may even be wrong in 1.4. Paolo
Stefano Stabellini
2013-May-29 13:09 UTC
Re: [PATCH v2 3/3] xen_machine_pv: do not create a dummy CPU in machine->init
On Wed, 29 May 2013, Paolo Bonzini wrote:> Il 29/05/2013 14:11, Andreas Färber ha scritto: > >> xen_machine_pv uses cpu_x86_init, therefore it has been broken. > >> > >> This patch fixes the problem by removing the dummy CPU creation > >> altogether from xen_init_pv, relying on the fact that QEMU can now cope > >> with a machine without an emulated CPU. > >> > >> This fix should be backported to QEMU 1.5. > >> > >> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > >> CC: imammedo@redhat.com > >> CC: qemu-stable@nongnu.org > > > > Change looks okay, > > > > Reviewed-by: Andreas Färber <afaerber@suse.de> > > > > But for the future please avoid "this patch" in the commit message > > (because later on it''s a "commit", not a patch) and also please put > > additional notices such as for backporting below the --- line. > > I read it as a warning to backporters that the patch, even though it may > apply, was not tested and may even be wrong in 1.4.Yes, that''s right. In any case Andreas has a point about moving notes about backports or patch revision changes under the --- line, I just have to find a way to make it work with guilt (that would also cut those notes from the commit messages therefore when I generate the patches they would go missing).
Igor Mammedov
2013-May-29 14:35 UTC
Re: [PATCH v2 3/3] xen_machine_pv: do not create a dummy CPU in machine->init
On Wed, 29 May 2013 14:29:51 +0200 Paolo Bonzini <pbonzini@redhat.com> wrote:> Il 29/05/2013 14:11, Andreas Färber ha scritto: > >> xen_machine_pv uses cpu_x86_init, therefore it has been broken. > >> > >> This patch fixes the problem by removing the dummy CPU creation > >> altogether from xen_init_pv, relying on the fact that QEMU can now cope > >> with a machine without an emulated CPU. > >> > >> This fix should be backported to QEMU 1.5. > >> > >> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > >> CC: imammedo@redhat.com > >> CC: qemu-stable@nongnu.org > > > > Change looks okay, > > > > Reviewed-by: Andreas Färber <afaerber@suse.de> > > > > But for the future please avoid "this patch" in the commit message > > (because later on it''s a "commit", not a patch) and also please put > > additional notices such as for backporting below the --- line. > > I read it as a warning to backporters that the patch, even though it may > apply, was not tested and may even be wrong in 1.4.it only needs to be backported to 1.5, 1.4 is not affected.> > Paolo >
On Wed, 29 May 2013, Stefano Stabellini wrote:> Hi all, > I have few simple fixes for Xen for the development tree as well as QEMU > 1.5 and older releases: > > > - the first patch is just a cleanup (that is needed to simplify > preprocessor dependencies); > - the second patch avoids setting nonblocking on Xen (as is already done > for KVM); > These two patches should be backported to QEMU 1.5 and older stable > branches. > > - the last patch fixes the "Invalid icc-bridge value" error that > prevents the Xen PV machine from starting. > This patch should be backported to QEMU 1.5. > > > > Stefano Stabellini (3): > xen: simplify xen_enabled > main_loop: do not set nonblocking if xen_enabled() > xen_machine_pv: do not create a dummy CPU in machine->initI received a "Reviewed-by" on the third patch (thanks Andreas) that is in the only non-trivial one, the other two are obvious and fairly xen specific even if they touch generic code. I''ll assume that you are all happy with them and I am going to go ahead with a PULL request unless you shout in the next couple of days :)