Re: [PATCH 1/1] Add virtio-input driver.

[Gerd Hoffmann <kraxel@redhat.com>]

  Hi,

> > +static int virtinput_send_status(struct virtio_input *vi,
> > +                                u16 type, u16 code, s32 value)
> > +{
> > +       struct virtio_input_event *stsbuf;
> > +       struct scatterlist sg[1];
> > +
> > +       stsbuf = kzalloc(sizeof(*stsbuf), GFP_ATOMIC);
> > +       if (!stsbuf)
> > +               return -ENOMEM;
> > +
> > +       stsbuf->type  = cpu_to_le16(type);
> > +       stsbuf->code  = cpu_to_le16(code);
> > +       stsbuf->value = cpu_to_le32(value);
> > +       sg_init_one(sg, stsbuf, sizeof(*stsbuf));
> > +       virtqueue_add_outbuf(vi->sts, sg, 1, stsbuf, GFP_ATOMIC);
> > +       virtqueue_kick(vi->sts);
> 
> GFP_ATOMIC, eww. But everyone does that for input_event() callbacks..

Yea, did it this way because I saw it elsewhere.

> we should fix that for user-space input one day.

Sounds like I have to use GFP_ATOMIC and can't switch to GFP_KERNEL,
correct?

> > +       size = virtinput_cfg_select(vi, VIRTIO_INPUT_CFG_ABS_INFO, abs);
> > +       virtio_cread(vi->vdev, struct virtio_input_config, u.abs.min, &mi);
> > +       virtio_cread(vi->vdev, struct virtio_input_config, u.abs.max, &ma);
> > +       virtio_cread(vi->vdev, struct virtio_input_config, u.abs.fuzz, &fu);
> > +       virtio_cread(vi->vdev, struct virtio_input_config, u.abs.flat, &fl);
> 
> abs.resolution is missing. Please add it, we really also need to add
> it to uinput one day.

Ok.  How should I handle cases where the resolution is either not known
or not fixed?  Just leave it zero?

> > +       vi->idev->name = vi->name;
> > +       vi->idev->phys = vi->phys;
> 
> Can you set vi->idev->uniq to the virtio-bus path?
> 
> > +       vi->idev->id.bustype = BUS_VIRTUAL;
> > +       vi->idev->id.vendor  = 0x0001;
> > +       vi->idev->id.product = 0x0001;
> > +       vi->idev->id.version = 0x0100;
> 
> Please don't hardcode those. All user-space based interaction with
> input-devices relies on those IDs. Can we retrieve it from the host
> just like the name?

Yes, we can.

There will be emulated devices, i.e. the input coming from
vnc/gtk/whatever will be sent to the virtio devices (instead of ps/2 or
usb).  For these we should probably have fixed IDs per device.  There
are keyboard/mouse/tablet at the moment.  Suggestions how to pick IDs?

There will also be pass-through support, i.e. qemu
opening /dev/input/event<nr> and forwarding everything to the guest.
How should that be handled best?  Copy all four from the host?  Even
though the bustype is BUS_USB?  Not sure this actually improves things
because the guest can match the device, or whenever this confuses apps
due to BUS_USB being applied to virtio devices ...

cheers,
  Gerd