Re: [Xen-devel] [Qemu-devel] target-i386: Introduce ICC bus/device/bridge

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

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 <jaceksburghardt@xxxxxxxxx> 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) {

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel