From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59489) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XRevi-0000RC-Kf for qemu-devel@nongnu.org; Wed, 10 Sep 2014 06:15:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XRevW-0005tu-S6 for qemu-devel@nongnu.org; Wed, 10 Sep 2014 06:15:14 -0400 Received: from mail-qg0-x229.google.com ([2607:f8b0:400d:c04::229]:34800) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XRevW-0005ta-Oi for qemu-devel@nongnu.org; Wed, 10 Sep 2014 06:15:02 -0400 Received: by mail-qg0-f41.google.com with SMTP id a108so5149014qge.0 for ; Wed, 10 Sep 2014 03:15:02 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <541024A1.1050708@redhat.com> Date: Wed, 10 Sep 2014 12:14:57 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1410265809-27247-1-git-send-email-pbonzini@redhat.com> <1410265809-27247-9-git-send-email-pbonzini@redhat.com> <87sik12b9l.fsf@troll.troll> In-Reply-To: <87sik12b9l.fsf@troll.troll> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 08/10] pckbd: adding new fields to vmstate List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: quintela@redhat.com Cc: amit.shah@redhat.com, qemu-devel@nongnu.org, Pavel.Dovgaluk@ispras.ru, dgilbert@redhat.com Il 09/09/2014 15:07, Juan Quintela ha scritto: > Paolo Bonzini wrote: >> From: Pavel Dovgalyuk >> >> This patch adds outport to VMState to allow correct saving and restoring >> the state of PC keyboard controller. >> >> Signed-off-by: Pavel Dovgalyuk >> Signed-off-by: Paolo Bonzini > > Acked-by: Juan Quintela >> --- >> hw/input/pckbd.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 51 insertions(+) >> >> diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c >> index 2ab8c87..2b0cd3d 100644 >> --- a/hw/input/pckbd.c >> +++ b/hw/input/pckbd.c >> @@ -131,6 +131,7 @@ typedef struct KBDState { >> uint8_t status; >> uint8_t mode; >> uint8_t outport; >> + bool outport_present; > > I don't like this one, but .... > > >> /* Bitmask of devices with data available. */ >> uint8_t pending; >> void *kbd; >> @@ -367,18 +368,68 @@ static void kbd_reset(void *opaque) >> s->mode = KBD_MODE_KBD_INT | KBD_MODE_MOUSE_INT; >> s->status = KBD_STAT_CMD | KBD_STAT_UNLOCKED; >> s->outport = KBD_OUT_RESET | KBD_OUT_A20; >> + s->outport_present = false; >> +} >> + >> +static uint8_t kbd_outport_default(KBDState *s) >> +{ >> + return KBD_OUT_RESET | KBD_OUT_A20 >> + | (s->status & KBD_STAT_OBF ? KBD_OUT_OBF : 0) >> + | (s->status & KBD_STAT_MOUSE_OBF ? KBD_OUT_MOUSE_OBF : 0); >> +} >> + >> +static int kbd_outport_post_load(void *opaque, int version_id) >> +{ >> + KBDState *s = opaque; >> + s->outport_present = true; >> + return 0; >> +} >> + >> +static const VMStateDescription vmstate_kbd_outport = { >> + .name = "pckbd_outport", >> + .version_id = 1, >> + .minimum_version_id = 1, >> + .post_load = kbd_outport_post_load, >> + .fields = (VMStateField[]) { >> + VMSTATE_UINT8(outport, KBDState), >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + >> +static bool kbd_outport_needed(void *opaque) >> +{ >> + KBDState *s = opaque; >> + return s->outport != kbd_outport_default(s); >> +} >> + >> +static int kbd_post_load(void *opaque, int version_id) >> +{ >> + KBDState *s = opaque; > > Only solution that I thought of is putting here: > > > s->outport |= > | (s->status & KBD_STAT_OBF ? KBD_OUT_OBF : 0) > | (s->status & KBD_STAT_MOUSE_OBF ? KBD_OUT_MOUSE_OBF : 0); > > > But I am not sure if that bits can be off if status bits are on. Yes, they can---the outport can be written by the guest (see outport_write). That was my first thought as well. :) Paolo > Thinking about it, why it is that it is not necessary to have on > postload something like that? > > PD: no, I don't claim to understand how the pc keyboard work ... > > Later, Juan. > >