Re: [Qemu-devel] [PATCH RFC] ps2: set the keybord output buffer size as the same as kernel

[Gonglei (Arei)]

> On Mi, 2014-04-23 at 08:06 +0000, Gonglei (Arei) wrote:
> > Hi, Gerd and Juan.
> >
> > Thanks for your guides about the confuse live migration about changing the
> keyboard buffer size.
> > According your suggestion, I got two solutions to address the issue:
> >
> > - Keep the data array 256 bytes long, change the rptr/wptr/count/data array
> at post_load(), both
> >  Ps/2 keyboard and mouse. This solution can be compatible with older qemu
> versions, which can
> >  do live migration each other.
> 
> > -Change the data array to 16 bytes, still save as PS2_QUEUE_SIZE. Reset the
> rptr/wptr/count at
> > post_load(), both ps/2 keyboard and mouse. Add VMSTATE_UNUSED(256-16)
> in struct vmstate_ps2_common.
> > This solution just save the 16 bytes buffer and drop the rest, So we can't
> migrate vm to older qemu versions.
> > But migration from old qemu to new qemu is ok.
> 
> I think long term we want #2, but using #1 as step inbetween for a few
> releases until 16 byte buffer size is widely used will might be useful.
> 
> Completely separate question:  Have you figured what the root cause for
> the bug is?  

When the linux kernel booting, will init the i8042 controller 
(drivers/input/serio/i8042.c)
i8042_init() 
        |-> i8042_controller_check()

static int i8042_controller_check(void)
{
        if (i8042_flush() == I8042_BUFFER_SIZE) {
                pr_err("No controller found\n");
                return -ENODEV;
        }

        return 0;
}

With this 16 byte buffer in drivers/input/serio/i8042.h:

#define I8042_BUFFER_SIZE       16

and this piece of code in drivers/input/serio/i8042.c:

/*
 * i8042_flush() flushes all data that may be in the *keyboard* and *mouse* 
buffers
 * of the i8042 down the toilet.
 */

static int i8042_flush(void)
{
        unsigned long flags;
        unsigned char data, str;
        int i = 0;

        spin_lock_irqsave(&i8042_lock, flags);

        while (((str = i8042_read_status()) & I8042_STR_OBF) && (i < 
I8042_BUFFER_SIZE)) {
                udelay(50);
                data = i8042_read_data();
                i++;
                dbg("%02x <- i8042 (flush, %s)", data,
                        str & I8042_STR_AUXDATA ? "aux" : "kbd");
        }

        spin_unlock_irqrestore(&i8042_lock, flags);

        return i;
}

So, if we type anything on keyboard (or move mouse) over 16 bytes while 
Linux kernel is still booting, Linux kernel will get confused. And as a result, 
decides there is no controller found.

>While wading through the code I've figured the queue size
> isn't (directly) exposed to the guest.  
When we type the keyboard or move mouse, the ps2_queue() will be called,
Account the wptr and count value, and fill the data array. Then the qemu 
will update irq. In kbd_update_irq() the kbd(i8042) state will be set:
        s->status |= KBD_STAT_OBF;
        s->outport |= KBD_OUT_OBF;  
then send irq to announce Guest OS.
The process correspond with the above linux kernel code, i8042_flush(void).

> And it's used for both mouse and
> kbd. For the kbd 16 byte buffer is probably ok,
  
Yes, the i8042 controller used by both ps/2 keyboard and ps/2 mouse. So, 
The buffer size is shared by ps/2 kbd and mouse.

> but for mouse events we
> probably want keep some more space to buffer events ...
>
Maybe you are right, and I worry about it too. At present,
but I haven't feel uncomfortable by VNC client then before.

Best regards,
-Gonglei