[Qemu-devel] [PATCH 2/2] Drop obsolete nographic timer

[Qemu-devel] [PATCH 2/2] Drop obsolete nographic timer

[Jan Kiszka]

We flush coalesced MMIO in the device models now, and VNC - for which
this was once introduced - is also fine without it as it has its own
refresh timer.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 vl.c |   13 -------------
 1 files changed, 0 insertions(+), 13 deletions(-)

diff --git a/vl.c b/vl.c
index 54d4dd9..db5ed7c 100644
--- a/vl.c
+++ b/vl.c
@@ -247,8 +247,6 @@ int nb_numa_nodes;
 uint64_t node_mem[MAX_NODES];
 uint64_t node_cpumask[MAX_NODES];
 
-static QEMUTimer *nographic_timer;
-
 uint8_t qemu_uuid[16];
 
 static QEMUBootSetHandler *boot_set_handler;
@@ -1221,13 +1219,6 @@ static void gui_update(void *opaque)
     qemu_mod_timer(ds->gui_timer, interval + qemu_get_clock_ms(rt_clock));
 }
 
-static void nographic_update(void *opaque)
-{
-    uint64_t interval = GUI_REFRESH_INTERVAL;
-
-    qemu_mod_timer(nographic_timer, interval + qemu_get_clock_ms(rt_clock));
-}
-
 struct vm_change_state_entry {
     VMChangeStateHandler *cb;
     void *opaque;
@@ -3522,10 +3513,6 @@ int main(int argc, char **argv, char **envp)
         }
         dcl = dcl->next;
     }
-    if (ds->gui_timer == NULL) {
-        nographic_timer = qemu_new_timer_ms(rt_clock, nographic_update, NULL);
-        qemu_mod_timer(nographic_timer, qemu_get_clock_ms(rt_clock));
-    }
     text_consoles_set_display(ds);
 
     if (gdbstub_dev && gdbserver_start(gdbstub_dev) < 0) {
-- 
1.7.3.4

Re: [Qemu-devel] [PATCH 2/2] Drop obsolete nographic timer

[David Gibson]

On Fri, Sep 30, 2011 at 12:31:18PM +0200, Jan Kiszka wrote:
> We flush coalesced MMIO in the device models now, and VNC - for which
> this was once introduced - is also fine without it as it has its own
> refresh timer.

These don't seem to have gone in yet, any word on their progress?  KVM
exits are *really* expensive on powerpc, so this represents a
significant performance problem for us.  I'm hoping this can make the
freeze.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [Qemu-devel] [PATCH 2/2] Drop obsolete nographic timer

[Anthony Liguori]

On 10/13/2011 07:05 PM, David Gibson wrote:
> On Fri, Sep 30, 2011 at 12:31:18PM +0200, Jan Kiszka wrote:
>> We flush coalesced MMIO in the device models now, and VNC - for which
>> this was once introduced - is also fine without it as it has its own
>> refresh timer.
>
> These don't seem to have gone in yet, any word on their progress?  KVM
> exits are *really* expensive on powerpc, so this represents a
> significant performance problem for us.  I'm hoping this can make the
> freeze.
>

Jan, can you resubmit if needed?  I don't have 1/2.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 2/2] Drop obsolete nographic timer

[Blue Swirl]

On Tue, Nov 1, 2011 at 20:10, Anthony Liguori <aliguori@us.ibm.com> wrote:
> On 10/13/2011 07:05 PM, David Gibson wrote:
>>
>> On Fri, Sep 30, 2011 at 12:31:18PM +0200, Jan Kiszka wrote:
>>>
>>> We flush coalesced MMIO in the device models now, and VNC - for which
>>> this was once introduced - is also fine without it as it has its own
>>> refresh timer.
>>
>> These don't seem to have gone in yet, any word on their progress?  KVM
>> exits are *really* expensive on powerpc, so this represents a
>> significant performance problem for us.  I'm hoping this can make the
>> freeze.
>>
>
> Jan, can you resubmit if needed?  I don't have 1/2.

These are already in, see
9870a5e6cd3df584328728b3d822b23cee28c561 and
e9a07334fb6ee08ddd61787c102d36e7e781efef.

>
> Regards,
>
> Anthony Liguori
>
>

Re: [Qemu-devel] [PATCH 2/2] Drop obsolete nographic timer

[Anthony Liguori]

On 11/01/2011 03:15 PM, Blue Swirl wrote:
> On Tue, Nov 1, 2011 at 20:10, Anthony Liguori<aliguori@us.ibm.com>  wrote:
>> On 10/13/2011 07:05 PM, David Gibson wrote:
>>>
>>> On Fri, Sep 30, 2011 at 12:31:18PM +0200, Jan Kiszka wrote:
>>>>
>>>> We flush coalesced MMIO in the device models now, and VNC - for which
>>>> this was once introduced - is also fine without it as it has its own
>>>> refresh timer.
>>>
>>> These don't seem to have gone in yet, any word on their progress?  KVM
>>> exits are *really* expensive on powerpc, so this represents a
>>> significant performance problem for us.  I'm hoping this can make the
>>> freeze.
>>>
>>
>> Jan, can you resubmit if needed?  I don't have 1/2.
>
> These are already in, see
> 9870a5e6cd3df584328728b3d822b23cee28c561 and
> e9a07334fb6ee08ddd61787c102d36e7e781efef.

Thanks.  Guess I should have looked first.

Regards,

Anthony Liguori

>
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>
>

Re: [Qemu-devel] [PATCH 2/2] Drop obsolete nographic timer

[Marek Vasut]

Dear Jan Kiszka,

> We flush coalesced MMIO in the device models now, and VNC - for which
> this was once introduced - is also fine without it as it has its own
> refresh timer.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  vl.c |   13 -------------
>  1 files changed, 0 insertions(+), 13 deletions(-)

This patch seems to break QEMU/PXA emulation for me on the ZipitZ2 platform. The 
serial console became very slow, but only when I type something in. The output 
from the device to the console is ok. After reverting this particular one, the 
console behaves normally.

Thanks for your help!

Best regards,
Marek Vasut

Re: [Qemu-devel] [PATCH 2/2] Drop obsolete nographic timer

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 826 bytes --]

On 2012-03-10 07:19, Marek Vasut wrote:
> Dear Jan Kiszka,
> 
>> We flush coalesced MMIO in the device models now, and VNC - for which
>> this was once introduced - is also fine without it as it has its own
>> refresh timer.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  vl.c |   13 -------------
>>  1 files changed, 0 insertions(+), 13 deletions(-)
> 
> This patch seems to break QEMU/PXA emulation for me on the ZipitZ2 platform. The 
> serial console became very slow, but only when I type something in. The output 
> from the device to the console is ok. After reverting this particular one, the 
> console behaves normally.

Removing that timer removal likely just revealed some formerly hidden
bug. Can you share the command line used for starting and some test image?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [Qemu-devel] [PATCH 2/2] Drop obsolete nographic timer

[Marek Vasut]

[-- Attachment #1: Type: Text/Plain, Size: 1018 bytes --]

Dear Jan Kiszka,

> On 2012-03-10 07:19, Marek Vasut wrote:
> > Dear Jan Kiszka,
> > 
> >> We flush coalesced MMIO in the device models now, and VNC - for which
> >> this was once introduced - is also fine without it as it has its own
> >> refresh timer.
> >> 
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >> ---
> >> 
> >>  vl.c |   13 -------------
> >>  1 files changed, 0 insertions(+), 13 deletions(-)
> > 
> > This patch seems to break QEMU/PXA emulation for me on the ZipitZ2
> > platform. The serial console became very slow, but only when I type
> > something in. The output from the device to the console is ok. After
> > reverting this particular one, the console behaves normally.
> 
> Removing that timer removal likely just revealed some formerly hidden
> bug. Can you share the command line used for starting and some test image?
> 
> Jan

qemu-system-arm -M z2 -pflash flash.img -serial null -serial null -serial stdio 
-display none

flash.img attached (XZ-ed)

Best regards,
Marek Vasut

[-- Attachment #2: flash.img.xz --]
[-- Type: application/x-xz, Size: 73684 bytes --]

Re: [Qemu-devel] [PATCH 2/2] Drop obsolete nographic timer

[Marek Vasut]

Dear Marek Vasut,

> Dear Jan Kiszka,
> 
> > On 2012-03-10 07:19, Marek Vasut wrote:
> > > Dear Jan Kiszka,
> > > 
> > >> We flush coalesced MMIO in the device models now, and VNC - for which
> > >> this was once introduced - is also fine without it as it has its own
> > >> refresh timer.
> > >> 
> > >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > >> ---
> > >> 
> > >>  vl.c |   13 -------------
> > >>  1 files changed, 0 insertions(+), 13 deletions(-)
> > > 
> > > This patch seems to break QEMU/PXA emulation for me on the ZipitZ2
> > > platform. The serial console became very slow, but only when I type
> > > something in. The output from the device to the console is ok. After
> > > reverting this particular one, the console behaves normally.
> > 
> > Removing that timer removal likely just revealed some formerly hidden
> > bug. Can you share the command line used for starting and some test
> > image?
> > 
> > Jan
> 
> qemu-system-arm -M z2 -pflash flash.img -serial null -serial null -serial
> stdio -display none
> 
> flash.img attached (XZ-ed)
> 

Bump?

Best regards,
Marek Vasut

Re: [Qemu-devel] [PATCH 2/2] Drop obsolete nographic timer

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1621 bytes --]

On 2012-03-15 19:12, Marek Vasut wrote:
> Dear Marek Vasut,
> 
>> Dear Jan Kiszka,
>>
>>> On 2012-03-10 07:19, Marek Vasut wrote:
>>>> Dear Jan Kiszka,
>>>>
>>>>> We flush coalesced MMIO in the device models now, and VNC - for which
>>>>> this was once introduced - is also fine without it as it has its own
>>>>> refresh timer.
>>>>>
>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>> ---
>>>>>
>>>>>  vl.c |   13 -------------
>>>>>  1 files changed, 0 insertions(+), 13 deletions(-)
>>>>
>>>> This patch seems to break QEMU/PXA emulation for me on the ZipitZ2
>>>> platform. The serial console became very slow, but only when I type
>>>> something in. The output from the device to the console is ok. After
>>>> reverting this particular one, the console behaves normally.
>>>
>>> Removing that timer removal likely just revealed some formerly hidden
>>> bug. Can you share the command line used for starting and some test
>>> image?
>>>
>>> Jan
>>
>> qemu-system-arm -M z2 -pflash flash.img -serial null -serial null -serial
>> stdio -display none
>>
>> flash.img attached (XZ-ed)
>>
> 
> Bump?

I had a brief look: The guest is apparently polling the uart, IRQ
delivery is disabled. But it also receives no timer ticks. I haven't
checked how it progresses, maybe there is some loop counter used.

What's the origin of this test image? Have you checked it works properly
on real silicon? I do not know how this platform looks like in reality,
how it should behave and should be programmed (by the guest), so I
really can't compare and analyze, sorry.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [Qemu-devel] [PATCH 2/2] Drop obsolete nographic timer

[Marek Vasut]

Dear Jan Kiszka,

> On 2012-03-15 19:12, Marek Vasut wrote:
> > Dear Marek Vasut,
> > 
> >> Dear Jan Kiszka,
> >> 
> >>> On 2012-03-10 07:19, Marek Vasut wrote:
> >>>> Dear Jan Kiszka,
> >>>> 
> >>>>> We flush coalesced MMIO in the device models now, and VNC - for which
> >>>>> this was once introduced - is also fine without it as it has its own
> >>>>> refresh timer.
> >>>>> 
> >>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >>>>> ---
> >>>>> 
> >>>>>  vl.c |   13 -------------
> >>>>>  1 files changed, 0 insertions(+), 13 deletions(-)
> >>>> 
> >>>> This patch seems to break QEMU/PXA emulation for me on the ZipitZ2
> >>>> platform. The serial console became very slow, but only when I type
> >>>> something in. The output from the device to the console is ok. After
> >>>> reverting this particular one, the console behaves normally.
> >>> 
> >>> Removing that timer removal likely just revealed some formerly hidden
> >>> bug. Can you share the command line used for starting and some test
> >>> image?
> >>> 
> >>> Jan
> >> 
> >> qemu-system-arm -M z2 -pflash flash.img -serial null -serial null
> >> -serial stdio -display none
> >> 
> >> flash.img attached (XZ-ed)
> > 
> > Bump?
> 
> I had a brief look: The guest is apparently polling the uart, IRQ
> delivery is disabled.

Yes, it is.

> But it also receives no timer ticks. I haven't
> checked how it progresses, maybe there is some loop counter used.

What do you mean? How does the timer implementation in pxa-uboot work? udelay() 
calls on the guest (pxa-uboot) simply poll the timer until it reaches certain 
value. There's no paralelism going on at all.

> 
> What's the origin of this test image? Have you checked it works properly
> on real silicon?

Yep.

> I do not know how this platform looks like in reality,
> how it should behave and should be programmed (by the guest), so I
> really can't compare and analyze, sorry.

I'm the maintainer, nice to meet you ;-) Real silicon is a small clamshell pc-
like device. But this breaks all xscale stuff in qemu.

> 
> Jan

Best regards,
Marek Vasut

Re: [Qemu-devel] [PATCH 2/2] Drop obsolete nographic timer

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2564 bytes --]

On 2012-03-16 01:52, Marek Vasut wrote:
> Dear Jan Kiszka,
> 
>> On 2012-03-15 19:12, Marek Vasut wrote:
>>> Dear Marek Vasut,
>>>
>>>> Dear Jan Kiszka,
>>>>
>>>>> On 2012-03-10 07:19, Marek Vasut wrote:
>>>>>> Dear Jan Kiszka,
>>>>>>
>>>>>>> We flush coalesced MMIO in the device models now, and VNC - for which
>>>>>>> this was once introduced - is also fine without it as it has its own
>>>>>>> refresh timer.
>>>>>>>
>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>> ---
>>>>>>>
>>>>>>>  vl.c |   13 -------------
>>>>>>>  1 files changed, 0 insertions(+), 13 deletions(-)
>>>>>>
>>>>>> This patch seems to break QEMU/PXA emulation for me on the ZipitZ2
>>>>>> platform. The serial console became very slow, but only when I type
>>>>>> something in. The output from the device to the console is ok. After
>>>>>> reverting this particular one, the console behaves normally.
>>>>>
>>>>> Removing that timer removal likely just revealed some formerly hidden
>>>>> bug. Can you share the command line used for starting and some test
>>>>> image?
>>>>>
>>>>> Jan
>>>>
>>>> qemu-system-arm -M z2 -pflash flash.img -serial null -serial null
>>>> -serial stdio -display none
>>>>
>>>> flash.img attached (XZ-ed)
>>>
>>> Bump?
>>
>> I had a brief look: The guest is apparently polling the uart, IRQ
>> delivery is disabled.
> 
> Yes, it is.
> 
>> But it also receives no timer ticks. I haven't
>> checked how it progresses, maybe there is some loop counter used.
> 
> What do you mean? How does the timer implementation in pxa-uboot work? udelay() 
> calls on the guest (pxa-uboot) simply poll the timer until it reaches certain 
> value. There's no paralelism going on at all.

I'm starting to understand the issue: The serial port can only accept a
single byte. Once this arrived, serial_can_receive1 returns 0, and the
backend fd is not longer polled by the io-thread. This should change
again as soon as the guest read that byte. But qemu_chr_accept_input,
which is properly called by the serial device, didn't kick the io-thread
in some way. I solved it this way now:

diff --git a/qemu-char.c b/qemu-char.c
index 9a5be75..a589a84 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -177,6 +177,7 @@ void qemu_chr_accept_input(CharDriverState *s)
 {
     if (s->chr_accept_input)
         s->chr_accept_input(s);
+    qemu_notify_event();
 }

 void qemu_chr_fe_printf(CharDriverState *s, const char *fmt, ...)


But I'm not yet sure if this is correct. Comments welcome!

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [Qemu-devel] [PATCH 2/2] Drop obsolete nographic timer

[Paolo Bonzini]

Il 16/03/2012 09:17, Jan Kiszka ha scritto:
> I'm starting to understand the issue: The serial port can only accept a
> single byte. Once this arrived, serial_can_receive1 returns 0, and the
> backend fd is not longer polled by the io-thread. This should change
> again as soon as the guest read that byte. But qemu_chr_accept_input,
> which is properly called by the serial device, didn't kick the io-thread
> in some way. I solved it this way now:
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index 9a5be75..a589a84 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -177,6 +177,7 @@ void qemu_chr_accept_input(CharDriverState *s)
>  {
>      if (s->chr_accept_input)
>          s->chr_accept_input(s);
> +    qemu_notify_event();
>  }
> 
>  void qemu_chr_fe_printf(CharDriverState *s, const char *fmt, ...)
> 
> 
> But I'm not yet sure if this is correct. Comments welcome!

I think so.  qemu_chr_accept_input signals that qemu_chr_be_can_write
could have changed, which means that the can_read handler could have
changed and has to be reevaluated.  qemu_notify_event is the right way
to do so.

Paolo