Patchwork usb: add serial number generator

login
register
mail settings
Submitter Gerd Hoffmann
Date April 20, 2012, 10:55 a.m.
Message ID <1334919350-3707-1-git-send-email-kraxel@redhat.com>
Download mbox | patch
Permalink /patch/154012/
State New
Headers show

Comments

Gerd Hoffmann - April 20, 2012, 10:55 a.m.
This patch adds a function which creates unique serial numbers for usb
devices and puts it into use.  Windows guests tend to become unhappy if
they find two identical usb devices in the system.  Effects range from
non-functional devices (with yellow exclamation mark in device manager)
to BSODs.  Handing out unique serial numbers to devices fixes this.

With this patch applied almost all emulated devices get a generated,
unique serial number.  There are two exceptions:

 * usb-storage devices will prefer a user-specified serial number
   and will only get a generated number in case the serial property
   is unset.
 * usb-hid devices keep the fixed serial number "42" as it is used
   to signal "remote wakeup actually works".
   See commit 7b074a22dab4bdda9864b933f1bc811a3db42845

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/desc.c                 |   32 ++++++++++++++++++++++++++++++++
 hw/usb/desc.h                 |    1 +
 hw/usb/dev-audio.c            |    1 +
 hw/usb/dev-bluetooth.c        |    1 +
 hw/usb/dev-hub.c              |    1 +
 hw/usb/dev-network.c          |    1 +
 hw/usb/dev-serial.c           |    1 +
 hw/usb/dev-smartcard-reader.c |    1 +
 hw/usb/dev-storage.c          |    2 ++
 hw/usb/dev-wacom.c            |    1 +
 10 files changed, 42 insertions(+), 0 deletions(-)
Hans de Goede - April 20, 2012, 12:23 p.m.
Hi,

Looks good, ack.

Regards,

Hans


On 04/20/2012 12:55 PM, Gerd Hoffmann wrote:
> This patch adds a function which creates unique serial numbers for usb
> devices and puts it into use.  Windows guests tend to become unhappy if
> they find two identical usb devices in the system.  Effects range from
> non-functional devices (with yellow exclamation mark in device manager)
> to BSODs.  Handing out unique serial numbers to devices fixes this.
>
> With this patch applied almost all emulated devices get a generated,
> unique serial number.  There are two exceptions:
>
>   * usb-storage devices will prefer a user-specified serial number
>     and will only get a generated number in case the serial property
>     is unset.
>   * usb-hid devices keep the fixed serial number "42" as it is used
>     to signal "remote wakeup actually works".
>     See commit 7b074a22dab4bdda9864b933f1bc811a3db42845
>
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
> ---
>   hw/usb/desc.c                 |   32 ++++++++++++++++++++++++++++++++
>   hw/usb/desc.h                 |    1 +
>   hw/usb/dev-audio.c            |    1 +
>   hw/usb/dev-bluetooth.c        |    1 +
>   hw/usb/dev-hub.c              |    1 +
>   hw/usb/dev-network.c          |    1 +
>   hw/usb/dev-serial.c           |    1 +
>   hw/usb/dev-smartcard-reader.c |    1 +
>   hw/usb/dev-storage.c          |    2 ++
>   hw/usb/dev-wacom.c            |    1 +
>   10 files changed, 42 insertions(+), 0 deletions(-)
>
> diff --git a/hw/usb/desc.c b/hw/usb/desc.c
> index 3c77368..e8a3c6a 100644
> --- a/hw/usb/desc.c
> +++ b/hw/usb/desc.c
> @@ -1,3 +1,5 @@
> +#include<ctype.h>
> +
>   #include "hw/usb.h"
>   #include "hw/usb/desc.h"
>   #include "trace.h"
> @@ -412,6 +414,36 @@ void usb_desc_set_string(USBDevice *dev, uint8_t index, const char *str)
>       s->str = g_strdup(str);
>   }
>
> +/*
> + * This function creates a serial number for a usb device.
> + * The serial number should:
> + *   (a) Be unique within the virtual machine.
> + *   (b) Be constant, so you don't get a new one each
> + *       time the guest is started.
> + * So we are using the physical location to generate a serial number
> + * from it.  It has three pieces:  First a fixed, device-specific
> + * prefix.  Second the device path of the host controller (which is
> + * the pci address in most cases).  Third the physical port path.
> + * Results in serial numbers like this: "314159-0000:00:1d.7-3".
> + */
> +void usb_desc_create_serial(USBDevice *dev)
> +{
> +    DeviceState *hcd = dev->qdev.parent_bus->parent;
> +    const USBDesc *desc = usb_device_get_usb_desc(dev);
> +    int index = desc->id.iSerialNumber;
> +    char serial[64];
> +    int dst;
> +
> +    assert(index != 0&&  desc->str[index] != NULL);
> +    dst = snprintf(serial, sizeof(serial), "%s", desc->str[index]);
> +    if (hcd&&  hcd->parent_bus&&  hcd->parent_bus->info->get_dev_path) {
> +        char *path = hcd->parent_bus->info->get_dev_path(hcd);
> +        dst += snprintf(serial+dst, sizeof(serial)-dst, "-%s", path);
> +    }
> +    dst += snprintf(serial+dst, sizeof(serial)-dst, "-%s", dev->port->path);
> +    usb_desc_set_string(dev, index, serial);
> +}
> +
>   const char *usb_desc_get_string(USBDevice *dev, uint8_t index)
>   {
>       USBDescString *s;
> diff --git a/hw/usb/desc.h b/hw/usb/desc.h
> index d164e8f..7cf5442 100644
> --- a/hw/usb/desc.h
> +++ b/hw/usb/desc.h
> @@ -171,6 +171,7 @@ int usb_desc_other(const USBDescOther *desc, uint8_t *dest, size_t len);
>   void usb_desc_init(USBDevice *dev);
>   void usb_desc_attach(USBDevice *dev);
>   void usb_desc_set_string(USBDevice *dev, uint8_t index, const char *str);
> +void usb_desc_create_serial(USBDevice *dev);
>   const char *usb_desc_get_string(USBDevice *dev, uint8_t index);
>   int usb_desc_string(USBDevice *dev, int index, uint8_t *dest, size_t len);
>   int usb_desc_get_descriptor(USBDevice *dev, int value, uint8_t *dest, size_t len);
> diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c
> index 426b95c..79b75fb 100644
> --- a/hw/usb/dev-audio.c
> +++ b/hw/usb/dev-audio.c
> @@ -648,6 +648,7 @@ static int usb_audio_initfn(USBDevice *dev)
>   {
>       USBAudioState *s = DO_UPCAST(USBAudioState, dev, dev);
>
> +    usb_desc_create_serial(dev);
>       usb_desc_init(dev);
>       s->dev.opaque = s;
>       AUD_register_card("usb-audio",&s->card);
> diff --git a/hw/usb/dev-bluetooth.c b/hw/usb/dev-bluetooth.c
> index 195370c..6b74eff 100644
> --- a/hw/usb/dev-bluetooth.c
> +++ b/hw/usb/dev-bluetooth.c
> @@ -494,6 +494,7 @@ static void usb_bt_handle_destroy(USBDevice *dev)
>
>   static int usb_bt_initfn(USBDevice *dev)
>   {
> +    usb_desc_create_serial(dev);
>       usb_desc_init(dev);
>       return 0;
>   }
> diff --git a/hw/usb/dev-hub.c b/hw/usb/dev-hub.c
> index 9c91665..b5962da 100644
> --- a/hw/usb/dev-hub.c
> +++ b/hw/usb/dev-hub.c
> @@ -520,6 +520,7 @@ static int usb_hub_initfn(USBDevice *dev)
>       USBHubPort *port;
>       int i;
>
> +    usb_desc_create_serial(dev);
>       usb_desc_init(dev);
>       s->intr = usb_ep_get(dev, USB_TOKEN_IN, 1);
>       for (i = 0; i<  NUM_PORTS; i++) {
> diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c
> index cff55f2..b238a09 100644
> --- a/hw/usb/dev-network.c
> +++ b/hw/usb/dev-network.c
> @@ -1324,6 +1324,7 @@ static int usb_net_initfn(USBDevice *dev)
>   {
>       USBNetState *s = DO_UPCAST(USBNetState, dev, dev);
>
> +    usb_desc_create_serial(dev);
>       usb_desc_init(dev);
>
>       s->rndis_state = RNDIS_UNINITIALIZED;
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index 8dcac8b..56743ee 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -479,6 +479,7 @@ static int usb_serial_initfn(USBDevice *dev)
>   {
>       USBSerialState *s = DO_UPCAST(USBSerialState, dev, dev);
>
> +    usb_desc_create_serial(dev);
>       usb_desc_init(dev);
>
>       if (!s->cs) {
> diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
> index 8e66675..3b7604e 100644
> --- a/hw/usb/dev-smartcard-reader.c
> +++ b/hw/usb/dev-smartcard-reader.c
> @@ -1189,6 +1189,7 @@ static int ccid_initfn(USBDevice *dev)
>   {
>       USBCCIDState *s = DO_UPCAST(USBCCIDState, dev, dev);
>
> +    usb_desc_create_serial(dev);
>       usb_desc_init(dev);
>       qbus_create_inplace(&s->bus.qbus,&ccid_bus_info,&dev->qdev, NULL);
>       s->intr = usb_ep_get(dev, USB_TOKEN_IN, CCID_INT_IN_EP);
> diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
> index 3d2f244..ae22fb1 100644
> --- a/hw/usb/dev-storage.c
> +++ b/hw/usb/dev-storage.c
> @@ -546,6 +546,8 @@ static int usb_msd_initfn(USBDevice *dev)
>       }
>       if (s->serial) {
>           usb_desc_set_string(dev, STR_SERIALNUMBER, s->serial);
> +    } else {
> +        usb_desc_create_serial(dev);
>       }
>
>       usb_desc_init(dev);
> diff --git a/hw/usb/dev-wacom.c b/hw/usb/dev-wacom.c
> index c1cfd74..3b51d45 100644
> --- a/hw/usb/dev-wacom.c
> +++ b/hw/usb/dev-wacom.c
> @@ -339,6 +339,7 @@ static void usb_wacom_handle_destroy(USBDevice *dev)
>   static int usb_wacom_initfn(USBDevice *dev)
>   {
>       USBWacomState *s = DO_UPCAST(USBWacomState, dev, dev);
> +    usb_desc_create_serial(dev);
>       usb_desc_init(dev);
>       s->changed = 1;
>       return 0;

Patch

diff --git a/hw/usb/desc.c b/hw/usb/desc.c
index 3c77368..e8a3c6a 100644
--- a/hw/usb/desc.c
+++ b/hw/usb/desc.c
@@ -1,3 +1,5 @@ 
+#include <ctype.h>
+
 #include "hw/usb.h"
 #include "hw/usb/desc.h"
 #include "trace.h"
@@ -412,6 +414,36 @@  void usb_desc_set_string(USBDevice *dev, uint8_t index, const char *str)
     s->str = g_strdup(str);
 }
 
+/*
+ * This function creates a serial number for a usb device.
+ * The serial number should:
+ *   (a) Be unique within the virtual machine.
+ *   (b) Be constant, so you don't get a new one each
+ *       time the guest is started.
+ * So we are using the physical location to generate a serial number
+ * from it.  It has three pieces:  First a fixed, device-specific
+ * prefix.  Second the device path of the host controller (which is
+ * the pci address in most cases).  Third the physical port path.
+ * Results in serial numbers like this: "314159-0000:00:1d.7-3".
+ */
+void usb_desc_create_serial(USBDevice *dev)
+{
+    DeviceState *hcd = dev->qdev.parent_bus->parent;
+    const USBDesc *desc = usb_device_get_usb_desc(dev);
+    int index = desc->id.iSerialNumber;
+    char serial[64];
+    int dst;
+
+    assert(index != 0 && desc->str[index] != NULL);
+    dst = snprintf(serial, sizeof(serial), "%s", desc->str[index]);
+    if (hcd && hcd->parent_bus && hcd->parent_bus->info->get_dev_path) {
+        char *path = hcd->parent_bus->info->get_dev_path(hcd);
+        dst += snprintf(serial+dst, sizeof(serial)-dst, "-%s", path);
+    }
+    dst += snprintf(serial+dst, sizeof(serial)-dst, "-%s", dev->port->path);
+    usb_desc_set_string(dev, index, serial);
+}
+
 const char *usb_desc_get_string(USBDevice *dev, uint8_t index)
 {
     USBDescString *s;
diff --git a/hw/usb/desc.h b/hw/usb/desc.h
index d164e8f..7cf5442 100644
--- a/hw/usb/desc.h
+++ b/hw/usb/desc.h
@@ -171,6 +171,7 @@  int usb_desc_other(const USBDescOther *desc, uint8_t *dest, size_t len);
 void usb_desc_init(USBDevice *dev);
 void usb_desc_attach(USBDevice *dev);
 void usb_desc_set_string(USBDevice *dev, uint8_t index, const char *str);
+void usb_desc_create_serial(USBDevice *dev);
 const char *usb_desc_get_string(USBDevice *dev, uint8_t index);
 int usb_desc_string(USBDevice *dev, int index, uint8_t *dest, size_t len);
 int usb_desc_get_descriptor(USBDevice *dev, int value, uint8_t *dest, size_t len);
diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c
index 426b95c..79b75fb 100644
--- a/hw/usb/dev-audio.c
+++ b/hw/usb/dev-audio.c
@@ -648,6 +648,7 @@  static int usb_audio_initfn(USBDevice *dev)
 {
     USBAudioState *s = DO_UPCAST(USBAudioState, dev, dev);
 
+    usb_desc_create_serial(dev);
     usb_desc_init(dev);
     s->dev.opaque = s;
     AUD_register_card("usb-audio", &s->card);
diff --git a/hw/usb/dev-bluetooth.c b/hw/usb/dev-bluetooth.c
index 195370c..6b74eff 100644
--- a/hw/usb/dev-bluetooth.c
+++ b/hw/usb/dev-bluetooth.c
@@ -494,6 +494,7 @@  static void usb_bt_handle_destroy(USBDevice *dev)
 
 static int usb_bt_initfn(USBDevice *dev)
 {
+    usb_desc_create_serial(dev);
     usb_desc_init(dev);
     return 0;
 }
diff --git a/hw/usb/dev-hub.c b/hw/usb/dev-hub.c
index 9c91665..b5962da 100644
--- a/hw/usb/dev-hub.c
+++ b/hw/usb/dev-hub.c
@@ -520,6 +520,7 @@  static int usb_hub_initfn(USBDevice *dev)
     USBHubPort *port;
     int i;
 
+    usb_desc_create_serial(dev);
     usb_desc_init(dev);
     s->intr = usb_ep_get(dev, USB_TOKEN_IN, 1);
     for (i = 0; i < NUM_PORTS; i++) {
diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c
index cff55f2..b238a09 100644
--- a/hw/usb/dev-network.c
+++ b/hw/usb/dev-network.c
@@ -1324,6 +1324,7 @@  static int usb_net_initfn(USBDevice *dev)
 {
     USBNetState *s = DO_UPCAST(USBNetState, dev, dev);
 
+    usb_desc_create_serial(dev);
     usb_desc_init(dev);
 
     s->rndis_state = RNDIS_UNINITIALIZED;
diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index 8dcac8b..56743ee 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -479,6 +479,7 @@  static int usb_serial_initfn(USBDevice *dev)
 {
     USBSerialState *s = DO_UPCAST(USBSerialState, dev, dev);
 
+    usb_desc_create_serial(dev);
     usb_desc_init(dev);
 
     if (!s->cs) {
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 8e66675..3b7604e 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -1189,6 +1189,7 @@  static int ccid_initfn(USBDevice *dev)
 {
     USBCCIDState *s = DO_UPCAST(USBCCIDState, dev, dev);
 
+    usb_desc_create_serial(dev);
     usb_desc_init(dev);
     qbus_create_inplace(&s->bus.qbus, &ccid_bus_info, &dev->qdev, NULL);
     s->intr = usb_ep_get(dev, USB_TOKEN_IN, CCID_INT_IN_EP);
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 3d2f244..ae22fb1 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -546,6 +546,8 @@  static int usb_msd_initfn(USBDevice *dev)
     }
     if (s->serial) {
         usb_desc_set_string(dev, STR_SERIALNUMBER, s->serial);
+    } else {
+        usb_desc_create_serial(dev);
     }
 
     usb_desc_init(dev);
diff --git a/hw/usb/dev-wacom.c b/hw/usb/dev-wacom.c
index c1cfd74..3b51d45 100644
--- a/hw/usb/dev-wacom.c
+++ b/hw/usb/dev-wacom.c
@@ -339,6 +339,7 @@  static void usb_wacom_handle_destroy(USBDevice *dev)
 static int usb_wacom_initfn(USBDevice *dev)
 {
     USBWacomState *s = DO_UPCAST(USBWacomState, dev, dev);
+    usb_desc_create_serial(dev);
     usb_desc_init(dev);
     s->changed = 1;
     return 0;