[PATCH] udev: add rules for qemu guests

[PATCH] udev: add rules for qemu guests

[Gerd Hoffmann]

These patches enable usb autosuspend for the qemu emulated HID devices.
This reduces the cpu load for idle guests with a hid device attached
because the linux kernel will suspend the usb bus then and qemu can stop
running a 1000 Hz to emulate the (active) UHCI controller.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 Makefile.am                      |    6 ++++++
 extras/qemu/99-qemu-usb.rules    |   12 ++++++++++++
 extras/qemu/qemu-usb-autosuspend |    7 +++++++
 3 files changed, 25 insertions(+), 0 deletions(-)
 create mode 100644 extras/qemu/99-qemu-usb.rules
 create mode 100755 extras/qemu/qemu-usb-autosuspend

diff --git a/Makefile.am b/Makefile.am
index 2e3fe39..c09b2a0 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -317,6 +317,12 @@ extras_v4l_id_v4l_id_LDADD = libudev/libudev-private.la
 libexec_PROGRAMS += extras/v4l_id/v4l_id
 dist_udevrules_DATA += extras/v4l_id/60-persistent-v4l.rules
 
+# ------------------------------------------------------------------------------
+# qemu -- qemu/kvm guest tweaks
+# ------------------------------------------------------------------------------
+dist_udevrules_DATA += extras/qemu/99-qemu-usb.rules
+dist_libexec_SCRIPTS += extras/qemu/qemu-usb-autosuspend
+
 if ENABLE_EXTRAS
 # ------------------------------------------------------------------------------
 # conditional extras (need glib, libusb, libacl, ...)
diff --git a/extras/qemu/99-qemu-usb.rules b/extras/qemu/99-qemu-usb.rules
new file mode 100644
index 0000000..35f33e1
--- /dev/null
+++ b/extras/qemu/99-qemu-usb.rules
@@ -0,0 +1,12 @@
+#
+# Enable autosuspend for qemu emulated usb hid devices.
+#
+# Note that there are buggy qemu versions which advertise remote
+# wakeup support but don't actually implement it correctly.  This
+# is the reason why we need a match for the serial number here.
+# The serial number "42" is used to tag the implementations where
+# remote wakeup is working.
+#
+ACTION="add", SUBSYSTEM="usb", ATTR{product}="QEMU USB Mouse", ATTR{serial}="42", RUN+="qemu-usb-autosuspend %p"
+ACTION="add", SUBSYSTEM="usb", ATTR{product}="QEMU USB Tablet", ATTR{serial}="42", RUN+="qemu-usb-autosuspend %p"
+ACTION="add", SUBSYSTEM="usb", ATTR{product}="QEMU USB Keyboard", ATTR{serial}="42", RUN+="qemu-usb-autosuspend %p"
diff --git a/extras/qemu/qemu-usb-autosuspend b/extras/qemu/qemu-usb-autosuspend
new file mode 100755
index 0000000..48761de
--- /dev/null
+++ b/extras/qemu/qemu-usb-autosuspend
@@ -0,0 +1,7 @@
+#!/bin/sh
+path="$1"
+if test -f "/sys${path}/power/control"; then
+	echo "auto" > "/sys${path}/power/control"
+elif test -f "/sys${path}/power/level"; then
+	echo "auto" > "/sys${path}/power/level"
+fi
-- 
1.7.1

Re: [PATCH] udev: add rules for qemu guests

[Kay Sievers]

On Tue, Jan 25, 2011 at 11:44, Gerd Hoffmann <kraxel@redhat.com> wrote:
> These patches enable usb autosuspend for the qemu emulated HID devices.
> This reduces the cpu load for idle guests with a hid device attached
> because the linux kernel will suspend the usb bus then and qemu can stop
> running a 1000 Hz to emulate the (active) UHCI controller.

I guess this should go into the qemu package, not the udev sources.

Kay

Re: [PATCH] udev: add rules for qemu guests

[Gerd Hoffmann]

On 01/25/11 13:01, Kay Sievers wrote:
> On Tue, Jan 25, 2011 at 11:44, Gerd Hoffmann<kraxel@redhat.com>  wrote:
>> These patches enable usb autosuspend for the qemu emulated HID devices.
>> This reduces the cpu load for idle guests with a hid device attached
>> because the linux kernel will suspend the usb bus then and qemu can stop
>> running a 1000 Hz to emulate the (active) UHCI controller.
>
> I guess this should go into the qemu package, not the udev sources.

Well, this should be installed in a *guest*, whereas qemu itself is 
installed on the virtualization *host*.  Placing it in the qemu package 
would have no effect at all ;)

cheers,
   Gerd

Re: [PATCH] udev: add rules for qemu guests

[Kay Sievers]

On Tue, Jan 25, 2011 at 11:44, Gerd Hoffmann <kraxel@redhat.com> wrote:
> These patches enable usb autosuspend for the qemu emulated HID devices.
> This reduces the cpu load for idle guests with a hid device attached
> because the linux kernel will suspend the usb bus then and qemu can stop
> running a 1000 Hz to emulate the (active) UHCI controller.

> +dist_udevrules_DATA += extras/qemu/99-qemu-usb.rules

The number should be 10-90 for the default things, or is there a
dependency on anything else?

> +ACTION="add", SUBSYSTEM="usb", ATTR{product}="QEMU USB Mouse", ATTR{serial}="42", RUN+="qemu-usb-autosuspend %p"

This should probably be a simple ATTR{power/control}="auto"

> --- /dev/null
> +++ b/extras/qemu/qemu-usb-autosuspend
> @@ -0,0 +1,7 @@
> +#!/bin/sh

If possible, we try to avoid shell scripts.

> +path="$1"
> +if test -f "/sys${path}/power/control"; then
> +       echo "auto" > "/sys${path}/power/control"
> +elif test -f "/sys${path}/power/level"; then
> +       echo "auto" > "/sys${path}/power/level"
> +fi

Which one is the actual one? Is the second try only for older kernels?
That could go in the compat rules, udev does not ship rules for older
kernels in the default installation.

Anyway, why isn't all this done in the kernel? Can't this be made
working somehow?

Carrying out configuration, and unconditionally applying it from udev
rules, should be avoided if possible and if the kernel could do it
itself. We ran into serious issues in the past when more recent
kernels did not like the config anymore that udev applied.

Cheers,
Kay

Re: [PATCH] udev: add rules for qemu guests

[Gerd Hoffmann]

On 01/25/11 14:39, Kay Sievers wrote:
> On Tue, Jan 25, 2011 at 11:44, Gerd Hoffmann<kraxel@redhat.com>  wrote:
>> These patches enable usb autosuspend for the qemu emulated HID devices.
>> This reduces the cpu load for idle guests with a hid device attached
>> because the linux kernel will suspend the usb bus then and qemu can stop
>> running a 1000 Hz to emulate the (active) UHCI controller.
>
>> +dist_udevrules_DATA += extras/qemu/99-qemu-usb.rules
>
> The number should be 10-90 for the default things, or is there a
> dependency on anything else?

No dependency at all.  What number do you suggest then?

>> +ACTION="add", SUBSYSTEM="usb", ATTR{product}="QEMU USB Mouse", ATTR{serial}="42", RUN+="qemu-usb-autosuspend %p"
>
> This should probably be a simple ATTR{power/control}="auto"

That should work indeed.  Didn't know you can write sysfs files that way.

>> +path="$1"
>> +if test -f "/sys${path}/power/control"; then
>> +       echo "auto">  "/sys${path}/power/control"
>> +elif test -f "/sys${path}/power/level"; then
>> +       echo "auto">  "/sys${path}/power/level"
>> +fi
>
> Which one is the actual one? Is the second try only for older kernels?

Yes, it is for older kernels.

> That could go in the compat rules, udev does not ship rules for older
> kernels in the default installation.

Ok.  Is there some way to express "power/control doesn't exist but 
power/level does" as udev rule, so I can zap the shell script?

> Anyway, why isn't all this done in the kernel? Can't this be made
> working somehow?

As far I know the kernel doesn't do that by default because too much 
broken hardware is out there which breaks when autosuspend is enabled by 
default.  I saw mentioned somewhere the plan is to have udev enable this 
for known-good devices, so I'm trying that route :)

Havn't checked whenever there is a in-kernel white list for that stuff 
which could be used instead.

cheers,
   Gerd

Re: [PATCH] udev: add rules for qemu guests

[Kay Sievers]

On Tue, Jan 25, 2011 at 15:07, Gerd Hoffmann <kraxel@redhat.com> wrote:
> On 01/25/11 14:39, Kay Sievers wrote:

>>> +dist_udevrules_DATA += extras/qemu/99-qemu-usb.rules
>>
>> The number should be 10-90 for the default things, or is there a
>> dependency on anything else?
>
> No dependency at all.  What number do you suggest then?

Doesn't really matter, we just leave 00-09 and 91-99 for other custom
stuff, to get easily before and after the common things.

>> That could go in the compat rules, udev does not ship rules for older
>> kernels in the default installation.
>
> Ok.  Is there some way to express "power/control doesn't exist but
> power/level does" as udev rule, so I can zap the shell script?

TEST="" should do that.

>> Anyway, why isn't all this done in the kernel? Can't this be made
>> working somehow?
>
> As far I know the kernel doesn't do that by default because too much broken
> hardware is out there which breaks when autosuspend is enabled by default.

Yeah, there is a reason not to do that for all devices, there are too
many broken ones.

> I saw mentioned somewhere the plan is to have udev enable this for
> known-good devices, so I'm trying that route :)

Oh, yeah, nice idea. That's the reason there is no list at all. :)

> Havn't checked whenever there is a in-kernel white list for that stuff which
> could be used instead.

Yeah, would be nice to have some generic way to enable autosuspend
when the hardware (real or emulated one) hints that it supports that.
Not sure how it would look like in detail though.

Kay

Re: [PATCH] udev: add rules for qemu guests

[Gerd Hoffmann]

   Hi,

>> No dependency at all.  What number do you suggest then?
>
> Doesn't really matter, we just leave 00-09 and 91-99 for other custom
> stuff, to get easily before and after the common things.

Fine, so I can pick '42' ;)

>> Ok.  Is there some way to express "power/control doesn't exist but
>> power/level does" as udev rule, so I can zap the shell script?
>
> TEST="" should do that.

Thanks.

>> Havn't checked whenever there is a in-kernel white list for that stuff which
>> could be used instead.
>
> Yeah, would be nice to have some generic way to enable autosuspend
> when the hardware (real or emulated one) hints that it supports that.

Well, there is a descriptor bit for that of course, but unfortunately 
one can't expect that only (physical or virtual) devices without bugs 
actually set that ...

cheers,
   Gerd