usb: fix memory leaks during device disconnect

fixes #2731

repos/dde_linux/run/usb_hid_reconnect.run

@@ -0,0 +1,189 @@
+#
+# USB HID test
+#
+# Test connect and disconnect
+
+if { [have_spec linux] || [have_spec muen] } {
+	puts "Run script does not support Linux and Muen."
+	exit 0
+}
+
+if { [get_cmd_switch --autopilot] && [have_include "power_on/qemu"] } {
+	puts "Run script does not support autopilot mode on Qemu"
+	exit 0
+}
+
+if { [get_cmd_switch --autopilot] && ![have_spec x86_64] } {
+	puts "Run script does not support autopilot mode on this platform"
+	exit 0
+}
+
+#
+# Build
+#
+
+set build_components {
+	core init
+	drivers/timer
+	drivers/usb
+	test/input
+	server/dynamic_rom
+}
+
+lappend_if [have_spec gpio] build_components drivers/gpio
+
+source ${genode_dir}/repos/base/run/platform_drv.inc
+append_platform_drv_build_components
+
+build $build_components
+
+proc gpio_drv { } { if {[have_spec rpi] && [have_spec hw]}  { return hw_gpio_drv }
+                    if {[have_spec rpi] && [have_spec foc]} { return foc_gpio_drv }
+                    return gpio_drv }
+
+create_boot_directory
+
+#
+# Generate config
+#
+
+append config {
+<config>
+	<parent-provides>
+		<service name="ROM"/>
+		<service name="IRQ"/>
+		<service name="IO_MEM"/>
+		<service name="IO_PORT"/>
+		<service name="PD"/>
+		<service name="RM"/>
+		<service name="CPU"/>
+		<service name="LOG"/>
+	</parent-provides>
+	<default-route>
+		<any-service> <parent/> <any-child/> </any-service>
+	</default-route>
+	<default caps="100"/>}
+
+append_if [have_spec gpio] config "
+	<start name=\"[gpio_drv]\">
+		<resource name=\"RAM\" quantum=\"4M\"/>
+		<provides><service name=\"Gpio\"/></provides>
+		<config/>
+	</start>"
+
+append_platform_drv_config
+
+append config {
+	<start name="timer">
+		<resource name="RAM" quantum="1M"/>
+		<provides><service name="Timer"/></provides>
+	</start>
+
+	<start name="usb_drv" caps="120">
+		<resource name="RAM" quantum="10M"/>
+		<provides><service name="Input"/></provides>
+		<config uhci="no" ohci="no" ehci="no" xhci="yes"
+		        capslock_led="rom" numlock_led="rom" scrlock_led="rom"
+		        bios_handoff="no">
+			<hid/>
+		</config>
+		<route>
+			<service name="ROM" label="capslock"> <child name="dynamic_rom"/> </service>
+			<service name="ROM" label="numlock">  <child name="dynamic_rom"/> </service>
+			<service name="ROM" label="scrlock">  <child name="dynamic_rom"/> </service>
+			<service name="ROM">      <parent/>    </service>
+			<service name="CPU">      <parent/>    </service>
+			<service name="PD">       <parent/>    </service>
+			<service name="IO_PORT">  <parent/>    </service>
+			<service name="IO_MEM">   <parent/>    </service>
+			<service name="LOG">      <parent/>    </service>
+			<service name="RM">      <parent/>    </service>
+			<service name="Platform"> <any-child/> </service>
+			<service name="Timer"> <child name="timer"/> </service>
+		</route>
+	</start>
+
+	<start name="dynamic_rom">
+		<resource name="RAM" quantum="4M"/>
+		<provides> <service name="ROM"/> </provides>
+		<config verbose="no">
+			<rom name="capslock">
+				<inline> <capslock enabled="no"/> </inline>
+				<sleep milliseconds="250" />
+				<inline> <capslock enabled="yes"/> </inline>
+				<sleep milliseconds="250" />
+			</rom>
+			<rom name="numlock">
+				<inline> <numlock enabled="no"/> </inline>
+				<sleep milliseconds="500" />
+				<inline> <numlock enabled="yes"/> </inline>
+				<sleep milliseconds="500" />
+			</rom>
+			<rom name="scrlock">
+				<inline> <scrlock enabled="no"/> </inline>
+				<sleep milliseconds="1000" />
+				<inline> <scrlock enabled="yes"/> </inline>
+				<sleep milliseconds="1000" />
+			</rom>
+		</config>
+		<route>
+			<service name="ROM">   <parent/>             </service>
+			<service name="CPU">   <parent/>             </service>
+			<service name="PD">    <parent/>             </service>
+			<service name="LOG">   <parent/>             </service>
+			<service name="Timer"> <child name="timer"/> </service>
+		</route>
+	</start>
+
+	<start name="test-input">
+		<resource name="RAM" quantum="1M"/>
+	</start>
+</config>}
+
+install_config $config
+
+#
+# Boot modules
+#
+
+# generic modules
+set boot_modules {
+	core ld.lib.so init timer usb_drv test-input dynamic_rom
+}
+
+lappend_if [have_spec gpio] boot_modules [gpio_drv]
+
+append_platform_drv_boot_modules
+
+build_boot_image $boot_modules
+
+append qemu_args " -device nec-usb-xhci,id=xhci -device usb-kbd,bus=xhci.0 -nographic"
+
+# autopilot test
+#run_genode_until forever
+# wait for keyboard
+run_genode_until {.*USB HID.*Keyboard.*} 60
+
+set spawn_id $qemu_spawn_id
+
+# send Ctrl-a+c to enter Qemu's monitor mode
+send "\x01\x63"
+
+# wait for monitor to become ready
+run_genode_until {(qemu)} 20 $spawn_id
+
+for {set i 0} {$i < 50} {incr i} {
+
+	# connect keyboard
+	send "device_add usb-kbd,id=ukb1\n"
+
+	# wait for keyboard
+	run_genode_until {.*USB HID.*Keyboard.*\n} 20 $spawn_id
+
+	# disconnect keyboard
+	send "device_del ukb1\n"
+
+	# wait for disconnect
+	run_genode_until {.*USB disconnect.*\n} 20 $spawn_id
+
+}

repos/dde_linux/src/lib/usb/dummies.c

@@ -345,15 +345,12 @@ struct device *device_create(struct class *cls, struct device *parent,
                              dev_t devt, void *drvdata,
                              const char *fmt, ...) { TRACE; return NULL; }
 void device_destroy(struct class *cls, dev_t devt) { TRACE; }
-void device_unregister(struct device *dev) { TRACE; }
 void device_lock(struct device *dev) { TRACE; }
 int  device_trylock(struct device *dev) { TRACE; return 0; }
 void device_unlock(struct device *dev) { TRACE; }
 void device_initialize(struct device *dev) { TRACE; }
 int  device_attach(struct device *dev) { TRACE; return 0; }
-int  device_is_registered(struct device *dev) { TRACE; return 0; }
 int  device_bind_driver(struct device *dev) { TRACE; return 0; }
-void device_release_driver(struct device *dev) { TRACE; }
 void device_enable_async_suspend(struct device *dev) { TRACE; }
 void device_set_wakeup_capable(struct device *dev, bool capable) { TRACE; }
 int  device_create_bin_file(struct device *dev,
@@ -367,10 +364,6 @@ void device_remove_file(struct device *dev,
 int device_for_each_child(struct device *dev, void *data,
                           int (*fn)(struct device *dev, void *data)) { TRACE; return 0; }
 
-
-void put_device(struct device *dev) { TRACE; }
-struct device *get_device(struct device *dev) { TRACE; return dev; }
-
 void driver_unregister(struct device_driver *drv) { TRACE; }
 int  driver_attach(struct device_driver *drv) { TRACE; return 0; }
 int  driver_create_file(struct device_driver *driver,

repos/dde_linux/src/lib/usb/include/lx_emul.h

@@ -1029,6 +1029,7 @@ struct device {
 	void                          *driver_data;
 	struct device_node            *of_node;
 	struct device_dma_parameters  *dma_parms;
+	unsigned                       ref;
 };
 
 struct device_attribute {

repos/dde_linux/src/lib/usb/input/evdev.cc

@@ -25,7 +25,6 @@
 
 /* Genode includes */
 #include <base/log.h>
-#include <base/debug.h>
 #include <base/registry.h>
 #include <util/reconstructible.h>
 
@@ -345,7 +344,7 @@ class Keyboard_led
 			usb_control_msg(_usb_device(), usb_sndctrlpipe(_usb_device(), 0),
 			                0x9, USB_TYPE_CLASS  | USB_RECIP_INTERFACE, 0x200,
 			                _interface()->cur_altsetting->desc.bInterfaceNumber,
-			                buf, 1, 0);
+			                buf, 1, 500);
 			kfree(buf);
 		}
 };
@@ -364,6 +363,10 @@ class Usb::Led
 		                 Lx::scheduler() };
 
 		completion _config_update;
+		completion _led_update;
+
+		enum Update_state { NONE, UPDATE, BLOCKED };
+		Update_state _update_state { NONE };
 
 		Led_state _capslock { Lx_kit::env().env(), "capslock" },
 		          _numlock  { Lx_kit::env().env(), "numlock"  },
@@ -390,9 +393,19 @@ class Usb::Led
 			Led *led = (Led *)l;
 
 			while (true) {
+				/* config update from EP */
 				wait_for_completion(&led->_config_update);
+
+				led->_update_state = UPDATE;
+
 				_registry.for_each([&] (Keyboard_led &keyboard) {
 					led->update(keyboard); });
+
+				/* wake up other task that waits for regestry access */
+				if (led->_update_state == BLOCKED)
+					complete(&led->_led_update);
+
+				led->_update_state = NONE;
 			}
 		}
 
@@ -401,6 +414,7 @@ class Usb::Led
 		Led()
 		{
 			init_completion(&_config_update);
+			init_completion(&_led_update);
 			Genode::Signal_transmitter(_config_handler).submit();
 		}
 
@@ -414,6 +428,18 @@ class Usb::Led
 
 			keyboard.update(leds);
 		}
+
+		/*
+		 * wait for completion of registry and led state updates
+		 */
+		void wait_for_registry()
+		{
+			/* task in _run function might receive multiple updates */
+			while ((_update_state == UPDATE)) {
+				_update_state = BLOCKED;
+				wait_for_completion(&_led_update);
+			}
+		}
 };
 
 
@@ -425,6 +451,13 @@ static int led_connect(struct input_handler *handler, struct input_dev *dev,
 {
 	Keyboard_led *keyboard = new (Lx_kit::env().heap()) Keyboard_led(_registry, dev);
 	_led->update(*keyboard);
+
+	input_handle *handle = (input_handle *)kzalloc(sizeof(input_handle), 0);
+	handle->dev     = input_get_device(dev);
+	handle->handler = handler;
+
+	input_register_handle(handle);
+
 	return 0;
 }
 
@@ -433,10 +466,16 @@ static void led_disconnect(struct input_handle *handle)
 {
 	input_dev *dev = handle->dev;
 
+	_led->wait_for_registry();
+
 	_registry.for_each([&] (Keyboard_led &keyboard) {
 		if (keyboard.match(dev))
 			destroy(Lx_kit::env().heap(), &keyboard);
 	});
+
+	input_unregister_handle(handle);
+	input_put_device(dev);
+	kfree(handle);
 }
 
 

repos/dde_linux/src/lib/usb/lx_emul.cc

@@ -352,13 +352,10 @@ class Driver : public Genode::List<Driver>::Element
 		{
 			dev->driver = _drv;
 
-			if (dev->bus->probe) {
+			if (dev->bus->probe)
-				lx_log(DEBUG_DRIVER, "Probing device bus %p", dev->bus->probe);
 				return dev->bus->probe(dev);
-			} else if (_drv->probe) {
+			else if (_drv->probe)
-				lx_log(DEBUG_DRIVER, "Probing driver: %s %p", _drv->name, _drv->probe);
 				return _drv->probe(dev);
-			}
 
 			return 0;
 		}
@@ -394,19 +391,65 @@ int device_add(struct device *dev)
 
 void device_del(struct device *dev)
 {
-	lx_log(DEBUG_DRIVER, "Remove device %p", dev);
 	if (dev->driver && dev->driver->remove)
 		dev->driver->remove(dev);
+
+	if (dev->bus && dev->bus->remove)
+		dev->bus->remove(dev);
 }
 
 
 int device_register(struct device *dev)
 {
-	//XXX: initialize DMA pools (see device_initialize)
 	return device_add(dev);
 }
 
 
+void device_unregister(struct device *dev)
+{
+	device_del(dev);
+	put_device(dev);
+}
+
+
+int device_is_registered(struct device *dev)
+{
+	return 1;
+}
+
+
+void device_release_driver(struct device *dev)
+{
+	/* is usb_unbind_interface(dev); */
+	if (dev->driver->remove)
+		dev->driver->remove(dev);
+
+	dev->driver = nullptr;
+}
+
+
+void put_device(struct device *dev)
+{
+	if (dev->ref) {
+		dev->ref--;
+		return;
+	}
+
+	if (dev->release)
+		dev->release(dev);
+	else if (dev->type && dev->type->release)
+		dev->type->release(dev);
+}
+
+
+struct device *get_device(struct device *dev)
+{
+	dev->ref++;
+
+	return dev;
+}
+
+
 void *dev_get_drvdata(const struct device *dev)
 {
 	return dev->driver_data;
@@ -421,6 +464,7 @@ int dev_set_drvdata(struct device *dev, void *data)
 
 const char *dev_name(const struct device *dev) { return dev->name; }
 
+
 /*******************************
  ** asm-generic/bitops/find.h **
  *******************************/
@@ -964,10 +1008,8 @@ long __wait_completion(struct completion *work, unsigned long timeout)
 
 	while (!work->done) {
 
-		if (j && j <= jiffies) {
+		if (j && j <= jiffies)
-			lx_log(1, "timeout jiffies %lu", jiffies);
 			return 0;
-		}
 
 		Lx::Task *task = Lx::scheduler().current();
 		work->task = (void *)task;