Patchwork [v3,1/6] xen: do not initialize the interval timer emulator

login
register
mail settings
Submitter Stefano Stabellini
Date Jan. 27, 2012, 6:21 p.m.
Message ID <1327688498-12362-1-git-send-email-stefano.stabellini@eu.citrix.com>
Download mbox | patch
Permalink /patch/138265/
State New
Headers show

Comments

Stefano Stabellini - Jan. 27, 2012, 6:21 p.m.
PIT is emulated by the hypervisor so we don't need to emulate it in Qemu:
this patch prevents Qemu from waking up needlessly at PIT_FREQ on Xen.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 hw/pc.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)
Jan Kiszka - Jan. 27, 2012, 7:09 p.m.
On 2012-01-27 19:21, Stefano Stabellini wrote:
> PIT is emulated by the hypervisor so we don't need to emulate it in Qemu:
> this patch prevents Qemu from waking up needlessly at PIT_FREQ on Xen.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  hw/pc.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pc.c b/hw/pc.c
> index 85304cf..7a7ce98 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -43,6 +43,7 @@
>  #include "ui/qemu-spice.h"
>  #include "memory.h"
>  #include "exec-memory.h"
> +#include "arch_init.h"
>  
>  /* output Bochs bios info messages */
>  //#define DEBUG_BIOS
> @@ -1130,7 +1131,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
>      DriveInfo *fd[MAX_FD];
>      qemu_irq rtc_irq = NULL;
>      qemu_irq *a20_line;
> -    ISADevice *i8042, *port92, *vmmouse, *pit;
> +    ISADevice *i8042, *port92, *vmmouse, *pit = NULL;
>      qemu_irq *cpu_exit_irq;
>  
>      register_ioport_write(0x80, 1, 1, ioport80_write, NULL);
> @@ -1151,7 +1152,9 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
>  
>      qemu_register_boot_set(pc_boot_set, *rtc_state);
>  
> -    pit = pit_init(isa_bus, 0x40, 0);
> +    if (!xen_available()) {
> +        pit = pit_init(isa_bus, 0x40, 0);
> +    }
>      pcspk_init(pit);
>  
>      for(i = 0; i < MAX_SERIAL_PORTS; i++) {

Thus as guest accessing to port 0x61 will be able to crash qemu because
pit is NULL? Or do you emulate that port in the kernel? If not, you
likely want to move pcspk_init() under the same umbrella.

Jan
Stefano Stabellini - Jan. 30, 2012, 11:39 a.m.
On Fri, 27 Jan 2012, Jan Kiszka wrote:
> On 2012-01-27 19:21, Stefano Stabellini wrote:
> > PIT is emulated by the hypervisor so we don't need to emulate it in Qemu:
> > this patch prevents Qemu from waking up needlessly at PIT_FREQ on Xen.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > ---
> >  hw/pc.c |    7 +++++--
> >  1 files changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/pc.c b/hw/pc.c
> > index 85304cf..7a7ce98 100644
> > --- a/hw/pc.c
> > +++ b/hw/pc.c
> > @@ -43,6 +43,7 @@
> >  #include "ui/qemu-spice.h"
> >  #include "memory.h"
> >  #include "exec-memory.h"
> > +#include "arch_init.h"
> >  
> >  /* output Bochs bios info messages */
> >  //#define DEBUG_BIOS
> > @@ -1130,7 +1131,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
> >      DriveInfo *fd[MAX_FD];
> >      qemu_irq rtc_irq = NULL;
> >      qemu_irq *a20_line;
> > -    ISADevice *i8042, *port92, *vmmouse, *pit;
> > +    ISADevice *i8042, *port92, *vmmouse, *pit = NULL;
> >      qemu_irq *cpu_exit_irq;
> >  
> >      register_ioport_write(0x80, 1, 1, ioport80_write, NULL);
> > @@ -1151,7 +1152,9 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
> >  
> >      qemu_register_boot_set(pc_boot_set, *rtc_state);
> >  
> > -    pit = pit_init(isa_bus, 0x40, 0);
> > +    if (!xen_available()) {
> > +        pit = pit_init(isa_bus, 0x40, 0);
> > +    }
> >      pcspk_init(pit);
> >  
> >      for(i = 0; i < MAX_SERIAL_PORTS; i++) {
> 
> Thus as guest accessing to port 0x61 will be able to crash qemu because
> pit is NULL? Or do you emulate that port in the kernel? If not, you
> likely want to move pcspk_init() under the same umbrella.

We already emulate both pit and port 0x61 in xen so a guest won't be
able to crash qemu that easily :)
But now that you make me think about it, it makes sense to move
pcspk_init under the same if, like you suggested.
Thanks,

Stefano
Jan Kiszka - Jan. 30, 2012, 3:13 p.m.
On 2012-01-30 12:39, Stefano Stabellini wrote:
> On Fri, 27 Jan 2012, Jan Kiszka wrote:
>> On 2012-01-27 19:21, Stefano Stabellini wrote:
>>> PIT is emulated by the hypervisor so we don't need to emulate it in Qemu:
>>> this patch prevents Qemu from waking up needlessly at PIT_FREQ on Xen.
>>>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>> ---
>>>  hw/pc.c |    7 +++++--
>>>  1 files changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/pc.c b/hw/pc.c
>>> index 85304cf..7a7ce98 100644
>>> --- a/hw/pc.c
>>> +++ b/hw/pc.c
>>> @@ -43,6 +43,7 @@
>>>  #include "ui/qemu-spice.h"
>>>  #include "memory.h"
>>>  #include "exec-memory.h"
>>> +#include "arch_init.h"
>>>  
>>>  /* output Bochs bios info messages */
>>>  //#define DEBUG_BIOS
>>> @@ -1130,7 +1131,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
>>>      DriveInfo *fd[MAX_FD];
>>>      qemu_irq rtc_irq = NULL;
>>>      qemu_irq *a20_line;
>>> -    ISADevice *i8042, *port92, *vmmouse, *pit;
>>> +    ISADevice *i8042, *port92, *vmmouse, *pit = NULL;
>>>      qemu_irq *cpu_exit_irq;
>>>  
>>>      register_ioport_write(0x80, 1, 1, ioport80_write, NULL);
>>> @@ -1151,7 +1152,9 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
>>>  
>>>      qemu_register_boot_set(pc_boot_set, *rtc_state);
>>>  
>>> -    pit = pit_init(isa_bus, 0x40, 0);
>>> +    if (!xen_available()) {
>>> +        pit = pit_init(isa_bus, 0x40, 0);
>>> +    }
>>>      pcspk_init(pit);
>>>  
>>>      for(i = 0; i < MAX_SERIAL_PORTS; i++) {
>>
>> Thus as guest accessing to port 0x61 will be able to crash qemu because
>> pit is NULL? Or do you emulate that port in the kernel? If not, you
>> likely want to move pcspk_init() under the same umbrella.
> 
> We already emulate both pit and port 0x61 in xen so a guest won't be
> able to crash qemu that easily :)

Which, btw, most likely breaks sound output via the speaker. We used to
fake 0x61 in the kernel as well, but now we properly emulated it in user
space again (well, upcoming qemu patches will, qemu-kvm is broken in
this regard).

> But now that you make me think about it, it makes sense to move
> pcspk_init under the same if, like you suggested.

Provided there is no use for user space, this would be consistent.

Jan

Patch

diff --git a/hw/pc.c b/hw/pc.c
index 85304cf..7a7ce98 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -43,6 +43,7 @@ 
 #include "ui/qemu-spice.h"
 #include "memory.h"
 #include "exec-memory.h"
+#include "arch_init.h"
 
 /* output Bochs bios info messages */
 //#define DEBUG_BIOS
@@ -1130,7 +1131,7 @@  void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
     DriveInfo *fd[MAX_FD];
     qemu_irq rtc_irq = NULL;
     qemu_irq *a20_line;
-    ISADevice *i8042, *port92, *vmmouse, *pit;
+    ISADevice *i8042, *port92, *vmmouse, *pit = NULL;
     qemu_irq *cpu_exit_irq;
 
     register_ioport_write(0x80, 1, 1, ioport80_write, NULL);
@@ -1151,7 +1152,9 @@  void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
 
     qemu_register_boot_set(pc_boot_set, *rtc_state);
 
-    pit = pit_init(isa_bus, 0x40, 0);
+    if (!xen_available()) {
+        pit = pit_init(isa_bus, 0x40, 0);
+    }
     pcspk_init(pit);
 
     for(i = 0; i < MAX_SERIAL_PORTS; i++) {