Re: [PATCH] usb-ccid: make ids and descriptor configurable

[Ripke, Klaus]

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