[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Xen-devel] target-i386: Introduce ICC bus/device/bridg
From: |
Stefano Stabellini |
Subject: |
Re: [Qemu-devel] [Xen-devel] target-i386: Introduce ICC bus/device/bridge |
Date: |
Tue, 28 May 2013 14:44:18 +0100 |
User-agent: |
Alpine 2.02 (DEB 1266 2009-07-14) |
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 <address@hidden> 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) {
Re: [Qemu-devel] [Xen-devel] target-i386: Introduce ICC bus/device/bridge, jacek burghardt, 2013/05/28