From patchwork Tue May 28 13:44:18 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Stefano Stabellini X-Patchwork-Id: 246865 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 873A32C030E for ; Tue, 28 May 2013 23:45:38 +1000 (EST) Received: from localhost ([::1]:53143 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UhKDY-0003q1-MR for incoming@patchwork.ozlabs.org; Tue, 28 May 2013 09:45:36 -0400 Received: from eggs.gnu.org ([208.118.235.92]:44460) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UhKD9-0003pu-Ub for qemu-devel@nongnu.org; Tue, 28 May 2013 09:45:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UhKD3-0006JC-G2 for qemu-devel@nongnu.org; Tue, 28 May 2013 09:45:11 -0400 Received: from smtp.citrix.com ([66.165.176.89]:22025) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UhKD3-0006Iw-BG for qemu-devel@nongnu.org; Tue, 28 May 2013 09:45:05 -0400 X-IronPort-AV: E=Sophos;i="4.87,758,1363132800"; d="scan'208";a="27725743" Received: from accessns.citrite.net (HELO FTLPEX01CL01.citrite.net) ([10.9.154.239]) by FTLPIPO01.CITRIX.COM with ESMTP/TLS/AES128-SHA; 28 May 2013 13:44:34 +0000 Received: from ukmail1.uk.xensource.com (10.80.16.128) by smtprelay.citrix.com (10.13.107.78) with Microsoft SMTP Server id 14.2.342.3; Tue, 28 May 2013 09:44:34 -0400 Received: from kaball.uk.xensource.com ([10.80.2.59]) by ukmail1.uk.xensource.com with esmtp (Exim 4.69) (envelope-from ) id 1UhKCY-0000yz-2i; Tue, 28 May 2013 14:44:34 +0100 Date: Tue, 28 May 2013 14:44:18 +0100 From: Stefano Stabellini X-X-Sender: sstabellini@kaball.uk.xensource.com To: =?UTF-8?Q?Andreas_F=C3=A4rber?= In-Reply-To: <51A49BB9.4020302@suse.de> Message-ID: References: <20130527161227.2bd4c3a3@nial.usersys.redhat.com> <51A49BB9.4020302@suse.de> User-Agent: Alpine 2.02 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-ID: X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 66.165.176.89 Cc: Stefano Stabellini , qemu-devel@nongnu.org, "xen-devel@lists.xen.org" , kraxel@redhat.com, Paolo Bonzini , Igor Mammedov , jacek burghardt Subject: Re: [Qemu-devel] [Xen-devel] target-i386: Introduce ICC bus/device/bridge X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On Tue, 28 May 2013, Andreas Färber wrote: > Am 28.05.2013 13:49, schrieb Stefano Stabellini: > > On Mon, 27 May 2013, Igor Mammedov wrote: > >> On Fri, 24 May 2013 08:56:14 -0600 > >> jacek burghardt wrote: > >> > >>> I wonder if anyone has patch that allows to tlak to icc bus introduced in > >>> qemu upstream ? > >>> https://github.com/qemu/qemu/commit/f0513d2c0156799e0c75a108ab9a049eea4f9607 > >>> > >>> icc-bridge will serve as a parent for icc-bus and provide > >>> mmio mapping services to child icc-devices. > >>> * icc-device will replace SysBusDevice as a parent of APIC > >>> and IOAPIC devices. > >> > >> looking at xen_init_pv() it creates dummy CPU which appears > >> not to be used by PV guest. What was the purpose in creating it? > >> > > > > I think that the purpose used to be keeping the rest of QEMU happy, > > because it used to assume that a CPU was being emulated. As a matter of > > fact the Xen PV machine does not actually emulate anything, especially > > it doesn't emulate the CPU. > > Nowadays QEMU is capable of handling machines without cpus, so I suggest > > removing this code from xen_init_pv altogether. > > > > jacek, can you please confirm that the patch below solves your problem? > > That's based on top of your other patches though, right? Accessing > first_cpu with no CPU created is doomed to fail. That is done for HVM guests (pc_xen_hvm_init and xen-all.c), while this code is for PV guests (xen_machine_pv.c). > qtest does create CPUs, it just doesn't execute them. > > Not arguing against this, just cautioning that additional NULL checks > may be needed elsewhere. That's a good point. I actually tested it and seems to work fine but I wouldn't want to paint me into a corner by setting up a new use case that nobody else is testing. On the other hand I dislike having to create a "useless" x86 cpu, that at the moment doesn't even work because cpu_x86_init seems to be broken by the lack of the ICC bridge. Overall if the QEMU machine model supports it (and it looks like it does), I would rather choose the "no cpu" option. See below the uglier alternative: diff --git a/hw/i386/xen_machine_pv.c b/hw/i386/xen_machine_pv.c index f829a52..260b211 100644 --- a/hw/i386/xen_machine_pv.c +++ b/hw/i386/xen_machine_pv.c @@ -22,6 +22,7 @@ * THE SOFTWARE. */ +#include "hw/cpu/icc_bus.h" #include "hw/hw.h" #include "hw/i386/pc.h" #include "hw/boards.h" @@ -35,10 +36,9 @@ static void xen_init_pv(QEMUMachineInitArgs *args) 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; + DeviceState *icc_bridge; /* Initialize a dummy CPU */ if (cpu_model == NULL) { @@ -48,9 +48,10 @@ static void xen_init_pv(QEMUMachineInitArgs *args) cpu_model = "qemu32"; #endif } - cpu = cpu_x86_init(cpu_model); - cs = CPU(cpu); - cs->halted = 1; + icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE); + object_property_add_child(qdev_get_machine(), "icc-bridge", + OBJECT(icc_bridge), NULL); + pc_cpus_init(cpu_model, icc_bridge); /* Initialize backend core & drivers */ if (xen_be_init() != 0) {