Re: [Qemu-devel] [PATCH v3 1/6] xen: do not initialize the interval timer emulator

[Jan Kiszka <jan.kiszka@siemens.com>]

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

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux