All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] usb-ccid: make ids and descriptor configurable
@ 2023-01-16 15:46 Ripke, Klaus
  2023-01-17  7:04 ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 5+ messages in thread
From: Ripke, Klaus @ 2023-01-16 15:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: kraxel, marcandre.lureau

Signed-off-by: Klaus Ripke <klaus.ripke@secunet.com>

hw/usb/dev-smartcard-reader.c:
Set some static values from ccid_properties.

---
 hw/usb/dev-smartcard-reader.c | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-
reader.c
index 28164d89be..4002157773 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -311,6 +311,11 @@ struct USBCCIDState {
     uint8_t  powered;
     uint8_t  notify_slot_change;
     uint8_t  debug;
+    /* the following are copied to static on initial realize */
+    uint16_t vendor;
+    uint16_t product;
+    uint8_t  maxslot;
+    uint8_t  feat2;
 };
 
 /*
@@ -323,7 +328,11 @@ struct USBCCIDState {
  *   0dc3:1004 Athena Smartcard Solutions, Inc.
  */
 
-static const uint8_t qemu_ccid_descriptor[] = {
+enum {
+    DESC_MAXSLOT = 4,
+    DESC_FEAT2 = 42 /* dwFeatures byte 2 */
+};
+static uint8_t qemu_ccid_descriptor[] = {
         /* Smart Card Device Class Descriptor */
         0x36,       /* u8  bLength; */
         0x21,       /* u8  bDescriptorType; Functional */
@@ -472,7 +481,7 @@ static const USBDescDevice desc_device = {
     },
 };
 
-static const USBDesc desc_ccid = {
+static USBDesc desc_ccid = {
     .id = {
         .idVendor          = CCID_VENDOR_ID,
         .idProduct         = CCID_PRODUCT_ID,
@@ -1295,9 +1304,10 @@ static void ccid_card_realize(DeviceState *qdev,
Error **errp)
     USBCCIDState *s = USB_CCID_DEV(dev);
     Error *local_err = NULL;
 
-    if (card->slot != 0) {
-        error_setg(errp, "usb-ccid supports one slot, can't add %d",
-                   card->slot);
+    DPRINTF(s, D_VERBOSE, "%s: slot %d\n", __func__, card->slot);
+    if (card->slot > qemu_ccid_descriptor[DESC_MAXSLOT]) {
+        error_setg(errp, "usb-ccid supports %d slot, can't add %d",
+                   qemu_ccid_descriptor[DESC_MAXSLOT] + 1, card-
>slot);
         return;
     }
     if (s->card != NULL) {
@@ -1317,6 +1327,14 @@ static void ccid_card_realize(DeviceState *qdev,
Error **errp)
 static void ccid_realize(USBDevice *dev, Error **errp)
 {
     USBCCIDState *s = USB_CCID_DEV(dev);
+    static int initialized;
+    if (!initialized) {
+        desc_ccid.id.idVendor = s->vendor;
+        desc_ccid.id.idProduct = s->product;
+        qemu_ccid_descriptor[DESC_MAXSLOT] = s->maxslot;
+        qemu_ccid_descriptor[DESC_FEAT2] = s->feat2;
+        initialized = !0;
+    }
 
     usb_desc_create_serial(dev);
     usb_desc_init(dev);
@@ -1339,6 +1357,8 @@ static void ccid_realize(USBDevice *dev, Error
**errp)
     ccid_reset_parameters(s);
     ccid_reset(s);
     s->debug = parse_debug_env("QEMU_CCID_DEBUG", D_VERBOSE, s-
>debug);
+    DPRINTF(s, D_VERBOSE, "ccid_realize %d %x %x %x %x\n",
+        initialized, s->vendor, s->product, s->maxslot, s->feat2);
 }
 
 static int ccid_post_load(void *opaque, int version_id)
@@ -1434,9 +1454,14 @@ static const VMStateDescription ccid_vmstate = {
 
 static Property ccid_properties[] = {
     DEFINE_PROP_UINT8("debug", USBCCIDState, debug, 0),
+    DEFINE_PROP_UINT16("vendor", USBCCIDState, vendor,
CCID_VENDOR_ID),
+    DEFINE_PROP_UINT16("product", USBCCIDState, product,
CCID_PRODUCT_ID),
+    DEFINE_PROP_UINT8("maxslot", USBCCIDState, maxslot, 0),
+    DEFINE_PROP_UINT8("feat2", USBCCIDState, feat2, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
+
 static void ccid_class_initfn(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-- 
2.34.1

-- 
Klaus Ripke
Senior Developer
Public Authorities Division
secunet Security Networks AG

Telefon:  +49 201 5454-2982

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] usb-ccid: make ids and descriptor configurable
  2023-01-16 15:46 [PATCH] usb-ccid: make ids and descriptor configurable Ripke, Klaus
@ 2023-01-17  7:04 ` Philippe Mathieu-Daudé
  2023-01-17 13:29   ` Ripke, Klaus
  2023-01-23 16:25   ` Ripke, Klaus
  0 siblings, 2 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-17  7:04 UTC (permalink / raw)
  To: Ripke, Klaus, qemu-devel; +Cc: kraxel, marcandre.lureau

Hi Klaus,

On 16/1/23 16:46, Ripke, Klaus wrote:
> Signed-off-by: Klaus Ripke <klaus.ripke@secunet.com>
> 
> hw/usb/dev-smartcard-reader.c:
> Set some static values from ccid_properties.
> 
> ---
>   hw/usb/dev-smartcard-reader.c | 35 ++++++++++++++++++++++++++++++-----
>   1 file changed, 30 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-
> reader.c
> index 28164d89be..4002157773 100644
> --- a/hw/usb/dev-smartcard-reader.c
> +++ b/hw/usb/dev-smartcard-reader.c
> @@ -311,6 +311,11 @@ struct USBCCIDState {
>       uint8_t  powered;
>       uint8_t  notify_slot_change;
>       uint8_t  debug;
> +    /* the following are copied to static on initial realize */
> +    uint16_t vendor;
> +    uint16_t product;
> +    uint8_t  maxslot;
> +    uint8_t  feat2;
>   };
>   
>   /*
> @@ -323,7 +328,11 @@ struct USBCCIDState {
>    *   0dc3:1004 Athena Smartcard Solutions, Inc.
>    */
>   
> -static const uint8_t qemu_ccid_descriptor[] = {
> +enum {
> +    DESC_MAXSLOT = 4,
> +    DESC_FEAT2 = 42 /* dwFeatures byte 2 */
> +};
> +static uint8_t qemu_ccid_descriptor[] = {

If you create 2 devices with different properties, the
first gets its properties overwritten with the second's
ones.

>           /* Smart Card Device Class Descriptor */
>           0x36,       /* u8  bLength; */
>           0x21,       /* u8  bDescriptorType; Functional */
> @@ -472,7 +481,7 @@ static const USBDescDevice desc_device = {
>       },
>   };
>   
> -static const USBDesc desc_ccid = {
> +static USBDesc desc_ccid = {
>       .id = {
>           .idVendor          = CCID_VENDOR_ID,
>           .idProduct         = CCID_PRODUCT_ID,
> @@ -1295,9 +1304,10 @@ static void ccid_card_realize(DeviceState *qdev,
> Error **errp)
>       USBCCIDState *s = USB_CCID_DEV(dev);
>       Error *local_err = NULL;
>   
> -    if (card->slot != 0) {
> -        error_setg(errp, "usb-ccid supports one slot, can't add %d",
> -                   card->slot);
> +    DPRINTF(s, D_VERBOSE, "%s: slot %d\n", __func__, card->slot);
> +    if (card->slot > qemu_ccid_descriptor[DESC_MAXSLOT]) {
> +        error_setg(errp, "usb-ccid supports %d slot, can't add %d",
> +                   qemu_ccid_descriptor[DESC_MAXSLOT] + 1, card-
>> slot);
>           return;
>       }
>       if (s->card != NULL) {
> @@ -1317,6 +1327,14 @@ static void ccid_card_realize(DeviceState *qdev,
> Error **errp)
>   static void ccid_realize(USBDevice *dev, Error **errp)
>   {
>       USBCCIDState *s = USB_CCID_DEV(dev);
> +    static int initialized;
> +    if (!initialized) {
> +        desc_ccid.id.idVendor = s->vendor;
> +        desc_ccid.id.idProduct = s->product;
> +        qemu_ccid_descriptor[DESC_MAXSLOT] = s->maxslot;
> +        qemu_ccid_descriptor[DESC_FEAT2] = s->feat2;
> +        initialized = !0;
> +    }
>   
>       usb_desc_create_serial(dev);
>       usb_desc_init(dev);
> @@ -1339,6 +1357,8 @@ static void ccid_realize(USBDevice *dev, Error
> **errp)
>       ccid_reset_parameters(s);
>       ccid_reset(s);
>       s->debug = parse_debug_env("QEMU_CCID_DEBUG", D_VERBOSE, s-
>> debug);
> +    DPRINTF(s, D_VERBOSE, "ccid_realize %d %x %x %x %x\n",
> +        initialized, s->vendor, s->product, s->maxslot, s->feat2);
>   }
>   
>   static int ccid_post_load(void *opaque, int version_id)
> @@ -1434,9 +1454,14 @@ static const VMStateDescription ccid_vmstate = {
>   
>   static Property ccid_properties[] = {
>       DEFINE_PROP_UINT8("debug", USBCCIDState, debug, 0),
> +    DEFINE_PROP_UINT16("vendor", USBCCIDState, vendor,
> CCID_VENDOR_ID),
> +    DEFINE_PROP_UINT16("product", USBCCIDState, product,
> CCID_PRODUCT_ID),
> +    DEFINE_PROP_UINT8("maxslot", USBCCIDState, maxslot, 0),
> +    DEFINE_PROP_UINT8("feat2", USBCCIDState, feat2, 0),
>       DEFINE_PROP_END_OF_LIST(),
>   };
>   
> +
>   static void ccid_class_initfn(ObjectClass *klass, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(klass);



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] usb-ccid: make ids and descriptor configurable
  2023-01-17  7:04 ` Philippe Mathieu-Daudé
@ 2023-01-17 13:29   ` Ripke, Klaus
  2023-01-23 16:25   ` Ripke, Klaus
  1 sibling, 0 replies; 5+ messages in thread
From: Ripke, Klaus @ 2023-01-17 13:29 UTC (permalink / raw)
  To: qemu-devel, philmd; +Cc: kraxel, marcandre.lureau

hi Philippe,

On Tue, 2023-01-17 at 08:04 +0100, Philippe Mathieu-Daudé wrote:
> If you create 2 devices with different properties, the
> first gets its properties overwritten with the second's
> ones.
The !initialized guard should prevent that.
In practize you would not create more usb-ccid devices,
but usb-ccid-passthru on the same class,
slots are now supported in card_realize.
Abandoning all the static structures seems too big a change
for a little tweaking, and setting from ENV was refused.

> > +        qemu_ccid_descriptor[DESC_FEAT2] = s->feat2;
Some devices (libcacard vscclient) want 4 here for "Automatic IFSD
exchange".


best Klaus

-- 
Klaus Ripke
Senior Developer
Public Authorities Division
secunet Security Networks AG

Telefon:  +49 201 5454-2982

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] usb-ccid: make ids and descriptor configurable
  2023-01-17  7:04 ` Philippe Mathieu-Daudé
  2023-01-17 13:29   ` Ripke, Klaus
@ 2023-01-23 16:25   ` Ripke, Klaus
  2023-03-01 21:37     ` seeking advice for configuring usb_desc in ccid / dev-smartcard-reader.c Ripke, Klaus
  1 sibling, 1 reply; 5+ messages in thread
From: Ripke, Klaus @ 2023-01-23 16:25 UTC (permalink / raw)
  To: qemu-devel, philmd; +Cc: kraxel, marcandre.lureau

Hi Philippe,

so I guess it's rejected. Any suggestions?

TIA Klaus

On Tue, 2023-01-17 at 08:04 +0100, Philippe Mathieu-Daudé wrote:
> Hi Klaus,
> 
> On 16/1/23 16:46, Ripke, Klaus wrote:
> > Signed-off-by: Klaus Ripke <klaus.ripke@secunet.com>
> > 
> > hw/usb/dev-smartcard-reader.c:
> > Set some static values from ccid_properties.
> > 
> > ---
> >   hw/usb/dev-smartcard-reader.c | 35
> > ++++++++++++++++++++++++++++++-----
> >   1 file changed, 30 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-
> > reader.c
> > index 28164d89be..4002157773 100644
> > --- a/hw/usb/dev-smartcard-reader.c
> > +++ b/hw/usb/dev-smartcard-reader.c
> > @@ -311,6 +311,11 @@ struct USBCCIDState {
> >       uint8_t  powered;
> >       uint8_t  notify_slot_change;
> >       uint8_t  debug;
> > +    /* the following are copied to static on initial realize */
> > +    uint16_t vendor;
> > +    uint16_t product;
> > +    uint8_t  maxslot;
> > +    uint8_t  feat2;
> >   };
> >   
> >   /*
> > @@ -323,7 +328,11 @@ struct USBCCIDState {
> >    *   0dc3:1004 Athena Smartcard Solutions, Inc.
> >    */
> >   
> > -static const uint8_t qemu_ccid_descriptor[] = {
> > +enum {
> > +    DESC_MAXSLOT = 4,
> > +    DESC_FEAT2 = 42 /* dwFeatures byte 2 */
> > +};
> > +static uint8_t qemu_ccid_descriptor[] = {
> 
> If you create 2 devices with different properties, the
> first gets its properties overwritten with the second's
> ones.
> 
> >           /* Smart Card Device Class Descriptor */
> >           0x36,       /* u8  bLength; */
> >           0x21,       /* u8  bDescriptorType; Functional */
> > @@ -472,7 +481,7 @@ static const USBDescDevice desc_device = {
> >       },
> >   };
> >   
> > -static const USBDesc desc_ccid = {
> > +static USBDesc desc_ccid = {
> >       .id = {
> >           .idVendor          = CCID_VENDOR_ID,
> >           .idProduct         = CCID_PRODUCT_ID,
> > @@ -1295,9 +1304,10 @@ static void ccid_card_realize(DeviceState
> > *qdev,
> > Error **errp)
> >       USBCCIDState *s = USB_CCID_DEV(dev);
> >       Error *local_err = NULL;
> >   
> > -    if (card->slot != 0) {
> > -        error_setg(errp, "usb-ccid supports one slot, can't add
> > %d",
> > -                   card->slot);
> > +    DPRINTF(s, D_VERBOSE, "%s: slot %d\n", __func__, card->slot);
> > +    if (card->slot > qemu_ccid_descriptor[DESC_MAXSLOT]) {
> > +        error_setg(errp, "usb-ccid supports %d slot, can't add
> > %d",
> > +                   qemu_ccid_descriptor[DESC_MAXSLOT] + 1, card-
> > > slot);
> >           return;
> >       }
> >       if (s->card != NULL) {
> > @@ -1317,6 +1327,14 @@ static void ccid_card_realize(DeviceState
> > *qdev,
> > Error **errp)
> >   static void ccid_realize(USBDevice *dev, Error **errp)
> >   {
> >       USBCCIDState *s = USB_CCID_DEV(dev);
> > +    static int initialized;
> > +    if (!initialized) {
> > +        desc_ccid.id.idVendor = s->vendor;
> > +        desc_ccid.id.idProduct = s->product;
> > +        qemu_ccid_descriptor[DESC_MAXSLOT] = s->maxslot;
> > +        qemu_ccid_descriptor[DESC_FEAT2] = s->feat2;
> > +        initialized = !0;
> > +    }
> >   
> >       usb_desc_create_serial(dev);
> >       usb_desc_init(dev);
> > @@ -1339,6 +1357,8 @@ static void ccid_realize(USBDevice *dev,
> > Error
> > **errp)
> >       ccid_reset_parameters(s);
> >       ccid_reset(s);
> >       s->debug = parse_debug_env("QEMU_CCID_DEBUG", D_VERBOSE, s-
> > > debug);
> > +    DPRINTF(s, D_VERBOSE, "ccid_realize %d %x %x %x %x\n",
> > +        initialized, s->vendor, s->product, s->maxslot, s->feat2);
> >   }
> >   
> >   static int ccid_post_load(void *opaque, int version_id)
> > @@ -1434,9 +1454,14 @@ static const VMStateDescription ccid_vmstate
> > = {
> >   
> >   static Property ccid_properties[] = {
> >       DEFINE_PROP_UINT8("debug", USBCCIDState, debug, 0),
> > +    DEFINE_PROP_UINT16("vendor", USBCCIDState, vendor,
> > CCID_VENDOR_ID),
> > +    DEFINE_PROP_UINT16("product", USBCCIDState, product,
> > CCID_PRODUCT_ID),
> > +    DEFINE_PROP_UINT8("maxslot", USBCCIDState, maxslot, 0),
> > +    DEFINE_PROP_UINT8("feat2", USBCCIDState, feat2, 0),
> >       DEFINE_PROP_END_OF_LIST(),
> >   };
> >   
> > +
> >   static void ccid_class_initfn(ObjectClass *klass, void *data)
> >   {
> >       DeviceClass *dc = DEVICE_CLASS(klass);
> 

-- 
Klaus Ripke
Senior Developer
Public Authorities Division
secunet Security Networks AG

Telefon:  +49 201 5454-2982

^ permalink raw reply	[flat|nested] 5+ messages in thread

* seeking advice for configuring usb_desc in ccid / dev-smartcard-reader.c
  2023-01-23 16:25   ` Ripke, Klaus
@ 2023-03-01 21:37     ` Ripke, Klaus
  0 siblings, 0 replies; 5+ messages in thread
From: Ripke, Klaus @ 2023-03-01 21:37 UTC (permalink / raw)
  To: qemu-devel, philmd; +Cc: kraxel, marcandre.lureau

Hello


in he/usb/dev-smartcard-reader.c: we need a slightly differing version
of the "Athena Smart Card Reader" as of qemu_ccid_descriptor with two
bytes changed to fixed "extended" values, 14 for max slot and 4 in
feature 2.
This data is shared by all ccid devices through a chain down to
usb_desc (which is klass->usb_desc for all ccid as of now).

Should we best follow the practice of dev-audio and dev-hid by using
another static config, selected by some device property?

Or better dynamically create and modify copies of all structures in
realize?

Or some other way?


many thanks for your help, kind regards
Klaus

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-03-01 21:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-16 15:46 [PATCH] usb-ccid: make ids and descriptor configurable Ripke, Klaus
2023-01-17  7:04 ` Philippe Mathieu-Daudé
2023-01-17 13:29   ` Ripke, Klaus
2023-01-23 16:25   ` Ripke, Klaus
2023-03-01 21:37     ` seeking advice for configuring usb_desc in ccid / dev-smartcard-reader.c Ripke, Klaus

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.