| Message ID | f056c7e5-fa74-469c-87f8-0f0925301b2d@web.de |
|---|---|
| State | New |
| Headers | show |
| Series | i386/vmmouse: Properly reset state | expand |
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
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.
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. >
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. >>
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. >>>
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); }