* [Qemu-devel] [PATCH] i386/vmmouse: Properly reset state
@ 2019-07-21 8:58 Jan Kiszka
2019-08-25 14:58 ` Jan Kiszka
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2019-07-21 8:58 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini, Richard Henderson, Eduardo Habkost
From: Jan Kiszka <jan.kiszka@siemens.com>
nb_queue was not zeroed so that we no longer delivered events if a
previous guest left the device in an overflow state.
The state of absolute does not matter as the next vmmouse_update_handler
call will align it again.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/i386/vmmouse.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/i386/vmmouse.c b/hw/i386/vmmouse.c
index 5d2d278be4..e335bd07da 100644
--- a/hw/i386/vmmouse.c
+++ b/hw/i386/vmmouse.c
@@ -257,6 +257,7 @@ static void vmmouse_reset(DeviceState *d)
VMMouseState *s = VMMOUSE(d);
s->queue_size = VMMOUSE_QUEUE_SIZE;
+ s->nb_queue = 0;
vmmouse_disable(s);
}
--
2.16.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] i386/vmmouse: Properly reset state
2019-07-21 8:58 [Qemu-devel] [PATCH] i386/vmmouse: Properly reset state Jan Kiszka
@ 2019-08-25 14:58 ` Jan Kiszka
2019-08-27 19:49 ` Eduardo Habkost
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2019-08-25 14:58 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini, Richard Henderson, Eduardo Habkost
On 21.07.19 10:58, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> nb_queue was not zeroed so that we no longer delivered events if a
> previous guest left the device in an overflow state.
>
> The state of absolute does not matter as the next vmmouse_update_handler
> call will align it again.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> hw/i386/vmmouse.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/i386/vmmouse.c b/hw/i386/vmmouse.c
> index 5d2d278be4..e335bd07da 100644
> --- a/hw/i386/vmmouse.c
> +++ b/hw/i386/vmmouse.c
> @@ -257,6 +257,7 @@ static void vmmouse_reset(DeviceState *d)
> VMMouseState *s = VMMOUSE(d);
>
> s->queue_size = VMMOUSE_QUEUE_SIZE;
> + s->nb_queue = 0;
>
> vmmouse_disable(s);
> }
> --
> 2.16.4
>
>
Ping - or who is looking after this?
Jan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] i386/vmmouse: Properly reset state
2019-08-25 14:58 ` Jan Kiszka
@ 2019-08-27 19:49 ` Eduardo Habkost
2019-08-29 18:00 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 6+ messages in thread
From: Eduardo Habkost @ 2019-08-27 19:49 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Paolo Bonzini, qemu-devel, Richard Henderson
On Sun, Aug 25, 2019 at 04:58:18PM +0200, Jan Kiszka wrote:
> On 21.07.19 10:58, Jan Kiszka wrote:
> > From: Jan Kiszka <jan.kiszka@siemens.com>
> >
> > nb_queue was not zeroed so that we no longer delivered events if a
> > previous guest left the device in an overflow state.
> >
> > The state of absolute does not matter as the next vmmouse_update_handler
> > call will align it again.
> >
> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > ---
> > hw/i386/vmmouse.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/hw/i386/vmmouse.c b/hw/i386/vmmouse.c
> > index 5d2d278be4..e335bd07da 100644
> > --- a/hw/i386/vmmouse.c
> > +++ b/hw/i386/vmmouse.c
> > @@ -257,6 +257,7 @@ static void vmmouse_reset(DeviceState *d)
> > VMMouseState *s = VMMOUSE(d);
> >
> > s->queue_size = VMMOUSE_QUEUE_SIZE;
> > + s->nb_queue = 0;
> >
> > vmmouse_disable(s);
> > }
> > --
> > 2.16.4
> >
> >
>
> Ping - or who is looking after this?
Despite being in hw/i386, I think we can say vmmouse.c doesn't
have a maintainer. Last time someone changed vmmouse.c in a
meaningful way (not just adapting to API changes or removing
duplicate code) was in 2012.
But the change makes sense to me, so:
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
I'll queue it.
--
Eduardo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] i386/vmmouse: Properly reset state
2019-08-27 19:49 ` Eduardo Habkost
@ 2019-08-29 18:00 ` Philippe Mathieu-Daudé
2019-08-29 18:12 ` Jan Kiszka
0 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-08-29 18:00 UTC (permalink / raw)
To: Eduardo Habkost, Jan Kiszka; +Cc: Paolo Bonzini, qemu-devel, Richard Henderson
Hi Jan,
On 8/27/19 9:49 PM, Eduardo Habkost wrote:
> On Sun, Aug 25, 2019 at 04:58:18PM +0200, Jan Kiszka wrote:
>> On 21.07.19 10:58, Jan Kiszka wrote:
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> nb_queue was not zeroed so that we no longer delivered events if a
>>> previous guest left the device in an overflow state.
>>>
>>> The state of absolute does not matter as the next vmmouse_update_handler
>>> call will align it again.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>> hw/i386/vmmouse.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/hw/i386/vmmouse.c b/hw/i386/vmmouse.c
>>> index 5d2d278be4..e335bd07da 100644
>>> --- a/hw/i386/vmmouse.c
>>> +++ b/hw/i386/vmmouse.c
>>> @@ -257,6 +257,7 @@ static void vmmouse_reset(DeviceState *d)
>>> VMMouseState *s = VMMOUSE(d);
>>>
>>> s->queue_size = VMMOUSE_QUEUE_SIZE;
>>> + s->nb_queue = 0;
Don't we also need to reset the status in case vmmouse_get_status() is
called directly after reset?
s->status = 0;
With it:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>
>>> vmmouse_disable(s);
>>> }
>>> --
>>> 2.16.4
>>>
>>>
>>
>> Ping - or who is looking after this?
>
> Despite being in hw/i386, I think we can say vmmouse.c doesn't
> have a maintainer. Last time someone changed vmmouse.c in a
> meaningful way (not just adapting to API changes or removing
> duplicate code) was in 2012.
>
Well it does has a few:
$ ./scripts/get_maintainer.pl -f hw/i386/vmmouse.c
"Michael S. Tsirkin" <mst@redhat.com> (supporter:PC)
Marcel Apfelbaum <marcel.apfelbaum@gmail.com> (supporter:PC)
Paolo Bonzini <pbonzini@redhat.com> (maintainer:X86 TCG CPUs)
Richard Henderson <rth@twiddle.net> (maintainer:X86 TCG CPUs)
Eduardo Habkost <ehabkost@redhat.com> (maintainer:X86 TCG CPUs)
However the correct section should rather be "PC Chipset".
> But the change makes sense to me, so:
>
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
>
> I'll queue it.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] i386/vmmouse: Properly reset state
2019-08-29 18:00 ` Philippe Mathieu-Daudé
@ 2019-08-29 18:12 ` Jan Kiszka
2019-08-29 18:38 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2019-08-29 18:12 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Eduardo Habkost
Cc: Paolo Bonzini, qemu-devel, Richard Henderson
On 29.08.19 20:00, Philippe Mathieu-Daudé wrote:
> Hi Jan,
>
> On 8/27/19 9:49 PM, Eduardo Habkost wrote:
>> On Sun, Aug 25, 2019 at 04:58:18PM +0200, Jan Kiszka wrote:
>>> On 21.07.19 10:58, Jan Kiszka wrote:
>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>
>>>> nb_queue was not zeroed so that we no longer delivered events if a
>>>> previous guest left the device in an overflow state.
>>>>
>>>> The state of absolute does not matter as the next vmmouse_update_handler
>>>> call will align it again.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>> ---
>>>> hw/i386/vmmouse.c | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/hw/i386/vmmouse.c b/hw/i386/vmmouse.c
>>>> index 5d2d278be4..e335bd07da 100644
>>>> --- a/hw/i386/vmmouse.c
>>>> +++ b/hw/i386/vmmouse.c
>>>> @@ -257,6 +257,7 @@ static void vmmouse_reset(DeviceState *d)
>>>> VMMouseState *s = VMMOUSE(d);
>>>>
>>>> s->queue_size = VMMOUSE_QUEUE_SIZE;
>>>> + s->nb_queue = 0;
>
> Don't we also need to reset the status in case vmmouse_get_status() is
> called directly after reset?
>
> s->status = 0;
>
Thanks for checking. We call vmmouse_disable() here, and that sets status to
0xffff anyway.
Jan
> With it:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
>>>>
>>>> vmmouse_disable(s);
>>>> }
>>>> --
>>>> 2.16.4
>>>>
>>>>
>>>
>>> Ping - or who is looking after this?
>>
>> Despite being in hw/i386, I think we can say vmmouse.c doesn't
>> have a maintainer. Last time someone changed vmmouse.c in a
>> meaningful way (not just adapting to API changes or removing
>> duplicate code) was in 2012.
>>
>
> Well it does has a few:
>
> $ ./scripts/get_maintainer.pl -f hw/i386/vmmouse.c
> "Michael S. Tsirkin" <mst@redhat.com> (supporter:PC)
> Marcel Apfelbaum <marcel.apfelbaum@gmail.com> (supporter:PC)
> Paolo Bonzini <pbonzini@redhat.com> (maintainer:X86 TCG CPUs)
> Richard Henderson <rth@twiddle.net> (maintainer:X86 TCG CPUs)
> Eduardo Habkost <ehabkost@redhat.com> (maintainer:X86 TCG CPUs)
>
> However the correct section should rather be "PC Chipset".
>
>> But the change makes sense to me, so:
>>
>> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
>>
>> I'll queue it.
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] i386/vmmouse: Properly reset state
2019-08-29 18:12 ` Jan Kiszka
@ 2019-08-29 18:38 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-08-29 18:38 UTC (permalink / raw)
To: Jan Kiszka, Eduardo Habkost; +Cc: Paolo Bonzini, qemu-devel, Richard Henderson
On 8/29/19 8:12 PM, Jan Kiszka wrote:
> On 29.08.19 20:00, Philippe Mathieu-Daudé wrote:
>> Hi Jan,
>>
>> On 8/27/19 9:49 PM, Eduardo Habkost wrote:
>>> On Sun, Aug 25, 2019 at 04:58:18PM +0200, Jan Kiszka wrote:
>>>> On 21.07.19 10:58, Jan Kiszka wrote:
>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>
>>>>> nb_queue was not zeroed so that we no longer delivered events if a
>>>>> previous guest left the device in an overflow state.
>>>>>
>>>>> The state of absolute does not matter as the next
>>>>> vmmouse_update_handler
>>>>> call will align it again.
>>>>>
>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>> ---
>>>>> hw/i386/vmmouse.c | 1 +
>>>>> 1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/hw/i386/vmmouse.c b/hw/i386/vmmouse.c
>>>>> index 5d2d278be4..e335bd07da 100644
>>>>> --- a/hw/i386/vmmouse.c
>>>>> +++ b/hw/i386/vmmouse.c
>>>>> @@ -257,6 +257,7 @@ static void vmmouse_reset(DeviceState *d)
>>>>> VMMouseState *s = VMMOUSE(d);
>>>>>
>>>>> s->queue_size = VMMOUSE_QUEUE_SIZE;
>>>>> + s->nb_queue = 0;
>>
>> Don't we also need to reset the status in case vmmouse_get_status() is
>> called directly after reset?
>>
>> s->status = 0;
>>
>
> Thanks for checking. We call vmmouse_disable() here, and that sets
> status to
> 0xffff anyway.
I missed that, you are correct :)
So:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>>
>>>>> vmmouse_disable(s);
>>>>> }
>>>>> --
>>>>> 2.16.4
>>>>>
>>>>>
>>>>
>>>> Ping - or who is looking after this?
>>>
>>> Despite being in hw/i386, I think we can say vmmouse.c doesn't
>>> have a maintainer. Last time someone changed vmmouse.c in a
>>> meaningful way (not just adapting to API changes or removing
>>> duplicate code) was in 2012.
>>>
>>
>> Well it does has a few:
>>
>> $ ./scripts/get_maintainer.pl -f hw/i386/vmmouse.c
>> "Michael S. Tsirkin" <mst@redhat.com> (supporter:PC)
>> Marcel Apfelbaum <marcel.apfelbaum@gmail.com> (supporter:PC)
>> Paolo Bonzini <pbonzini@redhat.com> (maintainer:X86 TCG CPUs)
>> Richard Henderson <rth@twiddle.net> (maintainer:X86 TCG CPUs)
>> Eduardo Habkost <ehabkost@redhat.com> (maintainer:X86 TCG CPUs)
>>
>> However the correct section should rather be "PC Chipset".
>>
>>> But the change makes sense to me, so:
>>>
>>> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
>>>
>>> I'll queue it.
>>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-08-29 18:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-21 8:58 [Qemu-devel] [PATCH] i386/vmmouse: Properly reset state Jan Kiszka
2019-08-25 14:58 ` Jan Kiszka
2019-08-27 19:49 ` Eduardo Habkost
2019-08-29 18:00 ` Philippe Mathieu-Daudé
2019-08-29 18:12 ` Jan Kiszka
2019-08-29 18:38 ` Philippe Mathieu-Daudé
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).