diff mbox

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

Message ID alpine.DEB.2.02.1305281225440.4799@kaball.uk.xensource.com
State New
Headers show

Commit Message

Stefano Stabellini May 28, 2013, 11:49 a.m. UTC
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>

Comments

Andreas Färber May 28, 2013, 11:57 a.m. UTC | #1
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__);
>
jacek burghardt May 28, 2013, 1:12 p.m. UTC | #2
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
>
Stefano Stabellini May 28, 2013, 3:15 p.m. UTC | #3
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.
diff mbox

Patch

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__);