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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1426844885.32097.36.camel@nilsson.home.kraxel.org>
Date:	Fri, 20 Mar 2015 10:48:05 +0100
From:	Gerd Hoffmann <kraxel@...hat.com>
To:	David Herrmann <dh.herrmann@...il.com>
Cc:	virtio-dev@...ts.oasis-open.org,
	virtualization@...ts.linux-foundation.org, mst@...hat.com,
	Rusty Russell <rusty@...tcorp.com.au>,
	open list <linux-kernel@...r.kernel.org>,
	"open list:ABI/API" <linux-api@...r.kernel.org>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>
Subject: Re: [PATCH 1/1] Add virtio-input driver.


  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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/