thr3ads.net - Xen devel - target-i386: Introduce ICC bus/device/bridge [May 2013]

If this information is useful, please help other people find it:
Share via:

jacek burghardt

2013-May-24 14:56 UTC

target-i386: Introduce ICC bus/device/bridge

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.


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

Igor Mammedov

2013-May-27 14:12 UTC

head link

Re: target-i386: Introduce ICC bus/device/bridge

On Fri, 24 May 2013 08:56:14 -0600
jacek burghardt <jaceksburghardt@gmail.com> 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?

jacek burghardt

2013-May-28 01:19 UTC

head link

Re: target-i386: Introduce ICC bus/device/bridge

Do you think this change will solv the issue?
No need for xen_vcpu_init anymore:

- the RTC emulator doesn''t have any periodic timers continuously
running
even in absence of guest interactions anymore;

- qemu_dummy_start_vcpu takes care of disabling TCG for us, so we don''t
need to do it manually here.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 hw/i386/pc_piix.c |    1 -
 xen-all.c         |   23 -----------------------
 2 files changed, 0 insertions(+), 24 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 43ab480..1ef4cbf 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -324,7 +324,6 @@ static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
         hw_error("xen hardware virtual machine initialisation
failed");
     }
     pc_init_pci_no_kvmclock(args);
-    xen_vcpu_init();
 }
 #endif

diff --git a/xen-all.c b/xen-all.c
index 539a154..bc308f4 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -574,29 +574,6 @@ void qmp_xen_set_global_dirty_log(bool enable, Error
**errp)
     }
 }

-/* VCPU Operations, MMIO, IO ring ... */
-
-static void xen_reset_vcpu(void *opaque)
-{
-    CPUState *cpu = opaque;
-
-    cpu->halted = 1;
-}
-
-void xen_vcpu_init(void)
-{
-    if (first_cpu != NULL) {
-        CPUState *cpu = ENV_GET_CPU(first_cpu);
-
-        qemu_register_reset(xen_reset_vcpu, cpu);
-        xen_reset_vcpu(cpu);
-    }
-    /* if rtc_clock is left to default (host_clock), disable it */
-    if (rtc_clock == host_clock) {
-        qemu_clock_enable(rtc_clock, false);
-    }
-}
-
 /* get the ioreq packets from share mem */
 static ioreq_t *cpu_get_ioreq_from_shared_memory(XenIOState *state, int
vcpu)
 {


On Mon, May 27, 2013 at 8:12 AM, Igor Mammedov <imammedo@redhat.com>
wrote:
> On Fri, 24 May 2013 08:56:14 -0600
> jacek burghardt <jaceksburghardt@gmail.com> 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?
>

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

Stefano Stabellini

2013-May-28 11:49 UTC

head link

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

On Mon, 27 May 2013, Igor Mammedov wrote:> On Fri, 24 May 2013 08:56:14 -0600
> jacek burghardt <jaceksburghardt@gmail.com> 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?


---


xen_machine_pv: do not create a dummy CPU in machine->init

QEMU can cope with machines without cpus.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff --git a/hw/i386/xen_machine_pv.c b/hw/i386/xen_machine_pv.c
index f829a52..b02ffac 100644
--- a/hw/i386/xen_machine_pv.c
+++ b/hw/i386/xen_machine_pv.c
@@ -31,27 +31,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__);

Andreas Färber

2013-May-28 11:57 UTC

head link

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

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@gmail.com> 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.

qtest does create CPUs, it just doesn''t execute them.

Not arguing against this, just cautioning that additional NULL checks
may be needed elsewhere.

Cheers,
Andreas
> ---
> 
> 
> xen_machine_pv: do not create a dummy CPU in machine->init
> 
> QEMU can cope with machines without cpus.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> diff --git a/hw/i386/xen_machine_pv.c b/hw/i386/xen_machine_pv.c
> index f829a52..b02ffac 100644
> --- a/hw/i386/xen_machine_pv.c
> +++ b/hw/i386/xen_machine_pv.c
> @@ -31,27 +31,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

jacek burghardt

2013-May-28 13:12 UTC

head link

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

is it possible then to have proper icc buss added to the created cpu. I
only see invalid icc bridge when starting pvm but it works fine with hvm no
errors. I can test  i just need to figure out how to patch downloaded
qemu-git at the time of creation of package for arch linux


On Tue, May 28, 2013 at 5:57 AM, Andreas Färber <afaerber@suse.de> 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@gmail.com> 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.
>
> qtest does create CPUs, it just doesn''t execute them.
>
> Not arguing against this, just cautioning that additional NULL checks
> may be needed elsewhere.
>
> Cheers,
> Andreas
>
> > ---
> >
> >
> > xen_machine_pv: do not create a dummy CPU in machine->init
> >
> > QEMU can cope with machines without cpus.
> >
> > Signed-off-by: Stefano Stabellini
<stefano.stabellini@eu.citrix.com>
> >
> > diff --git a/hw/i386/xen_machine_pv.c b/hw/i386/xen_machine_pv.c
> > index f829a52..b02ffac 100644
> > --- a/hw/i386/xen_machine_pv.c
> > +++ b/hw/i386/xen_machine_pv.c
> > @@ -31,27 +31,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
>

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

Stefano Stabellini

2013-May-28 13:44 UTC

head link

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

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@gmail.com> 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@lists.xen.org
http://lists.xen.org/xen-devel

Paolo Bonzini

2013-May-28 14:12 UTC

head link

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

Il 28/05/2013 15:44, Stefano Stabellini ha scritto:> 
> 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.
I also prefer this.  I wonder if this change would make xen_machine_pv.c
work for any architecture rather than just i386, at least for xen_mode
== XEN_ATTACH.

Paolo

Stefano Stabellini

2013-May-28 15:14 UTC

head link

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

On Tue, 28 May 2013, Paolo Bonzini wrote:> Il 28/05/2013 15:44, Stefano Stabellini ha scritto:
> > 
> > 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.
> 
> I also prefer this.  I wonder if this change would make xen_machine_pv.c
> work for any architecture rather than just i386, at least for xen_mode
> == XEN_ATTACH.
Right, that is might goal. I want to have xen_machine_pv on ARM.

Stefano Stabellini

2013-May-28 15:15 UTC

head link

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

On Tue, 28 May 2013, jacek burghardt wrote:> is it possible then to have proper icc buss added to the created cpu. I
only see invalid icc bridge when starting pvm but it works
> fine with hvm no errors. I can test  i just need to figure out how to patch
downloaded qemu-git at the time of creation of package
> for arch linux 
HVM guests and PV guests use two different machine types in QEMU, PV
guests don''t actually need one.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Xen devel - May 2013 - target-i386: Introduce ICC bus/device/bridge

target-i386: Introduce ICC bus/device/bridge

Re: target-i386: Introduce ICC bus/device/bridge

Re: target-i386: Introduce ICC bus/device/bridge

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

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

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

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

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

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

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