Message ID | 1296678500-19497-7-git-send-email-alevy@redhat.com |
---|---|
State | New |
Headers | show |
On 02/02/2011 02:28 PM, Alon Levy wrote: > I'll fold it before submitting the version to be applied, but > I hope keeping it as a separate patch will make reviewing easier. > Hrm, can you just send out the new patches? It's actually harder to review like this. Regards, Anthony Liguori > Behavioral changes: > * fix abort on client answer after card remove > * enable migration > * remove side affect code from asserts > * return consistent self-powered state > * mask out reserved bits in ccid_set_parameters > * add missing abRFU in SetParameters (no affect on linux guest) > > whitefixes / comments / consts defines: > * remove stale comment > * remove ccid_print_pending_answers if no DEBUG_CCID > * replace printf's with DPRINTF, remove DEBUG_CCID, add verbosity defines > * use error_report > * update copyright (most of the code is not original) > * reword known bug comment > * add missing closing quote in comment > * add missing whitespace on one line > * s/CCID_SetParameter/CCID_SetParameters/ > * add comments > * use define for max packet size > > Comment for "return consistent self-powered state": > > the Configuration Descriptor bmAttributes claims we are self powered, > but we were returning not self powered to USB_REQ_GET_STATUS control message. > > In practice, this message is not sent by a linux 2.6.35.10-74.fc14.x86_64 > guest (not tested on other guests), unless you issue lsusb -v as root (for > example). > > Signed-off-by: Alon Levy<alevy@redhat.com> > --- > hw/ccid.h | 7 +++ > hw/usb-ccid.c | 115 ++++++++++++++++++++++++++++----------------------------- > 2 files changed, 63 insertions(+), 59 deletions(-) > > diff --git a/hw/ccid.h b/hw/ccid.h > index af59070..f5dcfae 100644 > --- a/hw/ccid.h > +++ b/hw/ccid.h > @@ -6,11 +6,16 @@ > typedef struct CCIDCardState CCIDCardState; > typedef struct CCIDCardInfo CCIDCardInfo; > > +/* state of the CCID Card device (i.e. hw/ccid-card-*.c) > + */ > struct CCIDCardState { > DeviceState qdev; > uint32_t slot; // For future use with multiple slot reader. > }; > > +/* callbacks to be used by the CCID device (hw/usb-ccid.c) to call > + * into the smartcard device (hw/ccid-card-*.c) > + */ > struct CCIDCardInfo { > DeviceInfo qdev; > void (*print)(Monitor *mon, CCIDCardState *card, int indent); > @@ -20,6 +25,8 @@ struct CCIDCardInfo { > int (*initfn)(CCIDCardState *card); > }; > > +/* API for smartcard calling the CCID device (used by hw/ccid-card-*.c) > + */ > void ccid_card_send_apdu_to_guest(CCIDCardState *card, uint8_t* apdu, uint32_t len); > void ccid_card_card_removed(CCIDCardState *card); > void ccid_card_card_inserted(CCIDCardState *card); > diff --git a/hw/usb-ccid.c b/hw/usb-ccid.c > index 58f69a6..09e39ac 100644 > --- a/hw/usb-ccid.c > +++ b/hw/usb-ccid.c > @@ -1,15 +1,18 @@ > /* > * CCID Device emulation > * > - * Based on usb-serial.c: > + * Written by Alon Levy, with contributions from Robert Relyea. > + * > + * Based on usb-serial.c, see it's copyright and attributions below. > + * > + * This code is licenced under the GNU LGPL, version 2 or later. > + * > + * ------- > + * > + * usb-serial.c copyright and attribution: > * Copyright (c) 2006 CodeSourcery. > * Copyright (c) 2008 Samuel Thibault<samuel.thibault@ens-lyon.org> > * Written by Paul Brook, reused for FTDI by Samuel Thibault, > - * Reused for CCID by Alon Levy. > - * Contributed to by Robert Relyea > - * Copyright (c) 2010 Red Hat. > - * > - * This code is licenced under the LGPL. > */ > > /* References: > @@ -19,22 +22,16 @@ > * Specification for Integrated Circuit(s) Cards Interface Devices > * > * Endianess note: from the spec (1.3) > - * "Fields that are larger than a byte are stored in little endian > + * "Fields that are larger than a byte are stored in little endian" > * > * KNOWN BUGS > * 1. remove/insert can sometimes result in removed state instead of inserted. > * This is a result of the following: > - * symptom: dmesg shows ERMOTEIO (-121), pcscd shows -99. This happens > - * when we send a too short packet, seen in uhci-usb.c, resulting from > - * a urb requesting SPD and us returning a smaller packet. > + * symptom: dmesg shows ERMOTEIO (-121), pcscd shows -99. This can happen > + * when a short packet is sent, as seen in uhci-usb.c, resulting from a urb > + * from the guest requesting SPD and us returning a smaller packet. > * Not sure which messages trigger this. > * > - * Migration note: > - * > - * All the VMStateDescription's are left here for future use, but > - * not enabled right now since there is no support for USB migration. > - * > - * To enable define ENABLE_MIGRATION > */ > > #include "qemu-common.h" > @@ -44,11 +41,14 @@ > > #include "hw/ccid.h" > > -//#define DEBUG_CCID > - > #define DPRINTF(s, lvl, fmt, ...) \ > do { if (lvl<= s->debug) { printf("usb-ccid: " fmt , ## __VA_ARGS__); } } while (0) > > +#define D_WARN 1 > +#define D_INFO 2 > +#define D_MORE_INFO 3 > +#define D_VERBOSE 4 > + > #define CCID_DEV_NAME "usb-ccid" > > /* The two options for variable sized buffers: > @@ -64,6 +64,8 @@ do { if (lvl<= s->debug) { printf("usb-ccid: " fmt , ## __VA_ARGS__); } } while > #define InterfaceOutClass ((USB_DIR_OUT|USB_TYPE_CLASS|USB_RECIP_INTERFACE)<<8) > #define InterfaceInClass ((USB_DIR_IN |USB_TYPE_CLASS|USB_RECIP_INTERFACE)<<8) > > +#define CCID_MAX_PACKET_SIZE 64 > + > #define CCID_CONTROL_ABORT 0x1 > #define CCID_CONTROL_GET_CLOCK_FREQUENCIES 0x2 > #define CCID_CONTROL_GET_DATA_RATES 0x3 > @@ -209,11 +211,12 @@ typedef struct __attribute__ ((__packed__)) CCID_IccPowerOff { > uint16_t abRFU; > } CCID_IccPowerOff; > > -typedef struct __attribute__ ((__packed__)) CCID_SetParameter { > +typedef struct __attribute__ ((__packed__)) CCID_SetParameters { > CCID_Header hdr; > uint8_t bProtocolNum; > + uint16_t abRFU; > uint8_t abProtocolDataStructure[0]; > -} CCID_SetParameter; > +} CCID_SetParameters; > > typedef struct CCID_Notify_Slot_Change { > uint8_t bMessageType; /* CCID_MESSAGE_TYPE_RDR_to_PC_NotifySlotChange */ > @@ -278,10 +281,6 @@ struct USBCCIDState { > uint16_t migration_target_port; > }; > > -/* Slot specific variables. We emulate a single slot card reader. > - */ > - > - > /* CCID Spec chapter 4: CCID uses a standard device descriptor per Chapter 9, > * "USB Device Framework", section 9.6.1, in the Universal Serial Bus > * Specification. > @@ -416,7 +415,8 @@ static const uint8_t qemu_ccid_config_descriptor[] = { > /* u8 ep_bEndpointAddress; IN Endpoint 1 */ > 0x80 | CCID_INT_IN_EP, > 0x03, /* u8 ep_bmAttributes; Interrupt */ > - 0x40, 0x00, /* u16 ep_wMaxPacketSize; */ > + /* u16 ep_wMaxPacketSize; */ > + CCID_MAX_PACKET_SIZE& 0xff, (CCID_MAX_PACKET_SIZE>> 8), > 0xff, /* u8 ep_bInterval; */ > > /* Bulk-In endpoint */ > @@ -455,32 +455,31 @@ static void ccid_clear_pending_answers(USBCCIDState *s) > > static void ccid_print_pending_answers(USBCCIDState *s) > { > -#ifdef DEBUG_CCID > Answer *answer; > int i, count; > > - printf("usb-ccid: pending answers:"); > + DPRINTF(s, D_VERBOSE, "usb-ccid: pending answers:"); > if (!ccid_has_pending_answers(s)) { > - printf(" empty\n"); > + DPRINTF(s, D_VERBOSE, " empty\n"); > return; > } > for (i = s->pending_answers_start, count=s->pending_answers_num ; > count> 0; count--, i++) { > answer =&s->pending_answers[i % PENDING_ANSWERS_NUM]; > if (count == 1) { > - printf("%d:%d\n", answer->slot, answer->seq); > + DPRINTF(s, D_VERBOSE, "%d:%d\n", answer->slot, answer->seq); > } else { > - printf("%d:%d,", answer->slot, answer->seq); > + DPRINTF(s, D_VERBOSE, "%d:%d,", answer->slot, answer->seq); > } > } > -#endif > } > > static void ccid_add_pending_answer(USBCCIDState *s, CCID_Header *hdr) > { > Answer* answer; > > - assert(s->pending_answers_num++< PENDING_ANSWERS_NUM); > + assert(s->pending_answers_num< PENDING_ANSWERS_NUM); > + s->pending_answers_num++; > answer =&s->pending_answers[(s->pending_answers_end++) % PENDING_ANSWERS_NUM]; > answer->slot = hdr->bSlot; > answer->seq = hdr->bSeq; > @@ -492,7 +491,8 @@ static void ccid_remove_pending_answer(USBCCIDState *s, > { > Answer *answer; > > - assert(s->pending_answers_num--> 0); > + assert(s->pending_answers_num> 0); > + s->pending_answers_num--; > answer =&s->pending_answers[(s->pending_answers_start++) % PENDING_ANSWERS_NUM]; > *slot = answer->slot; > *seq = answer->seq; > @@ -528,16 +528,16 @@ static void* ccid_reserve_recv_buf(USBCCIDState* s, uint16_t len) > { > BulkIn* bulk_in; > > - DPRINTF(s, 4, "%s: QUEUE: reserve %d bytes\n", __func__, len); > + DPRINTF(s, D_VERBOSE, "%s: QUEUE: reserve %d bytes\n", __func__, len); > > /* look for an existing element */ > if (len> BULK_IN_BUF_SIZE) { > - printf("usb-ccid.c: %s: len larger then max (%d>%d). discarding message.\n", > + DPRINTF(s, D_WARN, "usb-ccid.c: %s: len larger then max (%d>%d). discarding message.\n", > __func__, len, BULK_IN_BUF_SIZE); > return NULL; > } > if (s->bulk_in_pending_num>= BULK_IN_PENDING_NUM) { > - printf("usb-ccid.c: %s: No free bulk_in buffers. discarding message.\n", > + DPRINTF(s, D_WARN, "usb-ccid.c: %s: No free bulk_in buffers. discarding message.\n", > __func__); > return NULL; > } > @@ -576,7 +576,7 @@ static int ccid_handle_control(USBDevice *dev, int request, int value, > DPRINTF(s, 1, "got control %x, value %x\n",request, value); > switch (request) { > case DeviceRequest | USB_REQ_GET_STATUS: > - data[0] = (0<< USB_DEVICE_SELF_POWERED) | > + data[0] = (1<< USB_DEVICE_SELF_POWERED) | > (dev->remote_wakeup<< USB_DEVICE_REMOTE_WAKEUP); > data[1] = 0x00; > ret = 2; > @@ -709,7 +709,7 @@ static uint8_t ccid_calc_status(USBCCIDState *s) > bmCommandStatus > */ > uint8_t ret = ccid_card_status(s) | (s->bmCommandStatus<< 6); > - DPRINTF(s, 4, "status = %d\n", ret); > + DPRINTF(s, D_VERBOSE, "status = %d\n", ret); > return ret; > } > > @@ -770,11 +770,9 @@ static void ccid_write_data_block( > p->b.hdr.bSeq = seq; > p->b.bStatus = ccid_calc_status(s); > p->b.bError = s->bError; > -#ifdef DEBUG_CCID > if (p->b.bError) { > - DPRINTF(s, 4, "error %d", p->b.bError); > + DPRINTF(s, D_VERBOSE, "error %d", p->b.bError); > } > -#endif > memcpy(p->abData, data, len); > ccid_reset_error_status(s); > } > @@ -805,12 +803,12 @@ static void ccid_write_data_block_atr(USBCCIDState* s, CCID_Header* recv) > > static void ccid_set_parameters(USBCCIDState *s, CCID_Header *recv) > { > - CCID_SetParameter *ph = (CCID_SetParameter *) recv; > + CCID_SetParameters *ph = (CCID_SetParameters *) recv; > uint32_t len = 0; > - if (ph->bProtocolNum == 0) { > + if ((ph->bProtocolNum& 3) == 0) { > len = 5; > } > - if (ph->bProtocolNum == 1) { > + if ((ph->bProtocolNum& 3) == 1) { > len = 7; > } > if (len == 0) { > @@ -884,7 +882,7 @@ static void ccid_on_apdu_from_guest(USBCCIDState *s, CCID_XferBlock *recv) > if (s->card) { > s->cardinfo->apdu_from_guest(s->card, recv->abData, len); > } else { > - printf("warning: discarded apdu\n"); > + DPRINTF(s, D_WARN, "warning: discarded apdu\n"); > } > } > > @@ -901,15 +899,15 @@ static int ccid_handle_bulk_out(USBCCIDState *s, USBPacket *p) > ccid_header = (CCID_Header*)s->bulk_out_data; > memcpy(s->bulk_out_data + s->bulk_out_pos, p->data, p->len); > s->bulk_out_pos += p->len; > - if (p->len == 64) { > - DPRINTF(s, 4, "usb-ccid: bulk_in: expecting more packets (%d/%d)\n", > + if (p->len == CCID_MAX_PACKET_SIZE) { > + DPRINTF(s, D_VERBOSE, "usb-ccid: bulk_in: expecting more packets (%d/%d)\n", > p->len, ccid_header->dwLength); > return 0; > } > if (s->bulk_out_pos< 10) { > DPRINTF(s, 1, "%s: bad USB_TOKEN_OUT length, should be at least 10 bytes\n", __func__); > } else { > - DPRINTF(s, 3, "%s %x\n", __func__, ccid_header->bMessageType); > + DPRINTF(s, D_MORE_INFO, "%s %x\n", __func__, ccid_header->bMessageType); > switch (ccid_header->bMessageType) { > case CCID_MESSAGE_TYPE_PC_to_RDR_GetSlotStatus: > ccid_write_slot_status(s, ccid_header); > @@ -965,7 +963,7 @@ static int ccid_bulk_in_copy_to_guest(USBCCIDState *s, uint8_t *data, int len) > { > int ret = 0; > > - assert(len>0); > + assert(len> 0); > ccid_bulk_in_get(s); > if (s->current_bulk_in != NULL) { > ret = MIN(s->current_bulk_in->len - s->current_bulk_in->pos, len); > @@ -978,7 +976,7 @@ static int ccid_bulk_in_copy_to_guest(USBCCIDState *s, uint8_t *data, int len) > ret = USB_RET_NAK; /* return when device has no data - usb 2.0 spec Table 8-4 */ > } > if (ret> 0) { > - DPRINTF(s, 3, "%s: %d/%d req/act to guest (BULK_IN)\n", __func__, len, ret); > + DPRINTF(s, D_MORE_INFO, "%s: %d/%d req/act to guest (BULK_IN)\n", __func__, len, ret); > } > if (ret != USB_RET_NAK&& ret< len) { > DPRINTF(s, 1, "%s: returning short (EREMOTEIO) %d< %d\n", __func__, ret, len); > @@ -1015,7 +1013,7 @@ static int ccid_handle_data(USBDevice *dev, USBPacket *p) > ret = 2; > s->notify_slot_change = false; > s->bmSlotICCState&= ~SLOT_0_CHANGED_MASK; > - DPRINTF(s, 2, "handle_data: int_in: notify_slot_change %X, requested len %d\n", > + DPRINTF(s, D_INFO, "handle_data: int_in: notify_slot_change %X, requested len %d\n", > s->bmSlotICCState, len); > } > break; > @@ -1146,7 +1144,11 @@ void ccid_card_card_error(CCIDCardState *card, uint64_t error) > DPRINTF(s, 1, "VSC_Error: %lX\n", s->last_answer_error); > /* TODO: these error's should be more verbose and propogated to the guest. > * */ > - ccid_write_data_block_answer(s, NULL, 0); > + /* we flush all pending answers on CardRemove message in ccid-card-passthru, > + * so check that first to not trigger abort */ > + if (ccid_has_pending_answers(s)) { > + ccid_write_data_block_answer(s, NULL, 0); > + } > } > > void ccid_card_card_inserted(CCIDCardState *card) > @@ -1184,12 +1186,12 @@ static int ccid_card_init(DeviceState *qdev, DeviceInfo *base) > int ret = 0; > > if (card->slot != 0) { > - fprintf(stderr, "Warning: usb-ccid supports one slot, can't add %d", > + error_report("Warning: usb-ccid supports one slot, can't add %d", > card->slot); > return -1; > } > if (s->card != NULL) { > - fprintf(stderr, "Warning: usb-ccid card already full, not adding\n"); > + error_report("Warning: usb-ccid card already full, not adding\n"); > return -1; > } > ret = info->initfn ? info->initfn(card) : ret; > @@ -1233,7 +1235,6 @@ static int ccid_initfn(USBDevice *dev) > return 0; > } > > -#ifdef ENABLE_MIGRATION > static int ccid_post_load(void *opaque, int version_id) > { > USBCCIDState *s = opaque; > @@ -1325,7 +1326,6 @@ static VMStateDescription ccid_vmstate = { > VMSTATE_END_OF_LIST() > } > }; > -#endif // ENABLE_MIGRATION > > static struct USBDeviceInfo ccid_info = { > .product_desc = "QEMU USB CCID", > @@ -1342,12 +1342,9 @@ static struct USBDeviceInfo ccid_info = { > DEFINE_PROP_UINT8("debug", USBCCIDState, debug, 0), > DEFINE_PROP_END_OF_LIST(), > }, > -#ifdef ENABLE_MIGRATION > .qdev.vmsd =&ccid_vmstate, > -#endif > }; > > - > static void ccid_register_devices(void) > { > usb_qdev_register(&ccid_info); >
On Thu, Feb 03, 2011 at 10:48:03AM -0600, Anthony Liguori wrote: > On 02/02/2011 02:28 PM, Alon Levy wrote: > >I'll fold it before submitting the version to be applied, but > >I hope keeping it as a separate patch will make reviewing easier. > > Hrm, can you just send out the new patches? It's actually harder > to review like this. Yes. > > Regards, > > Anthony Liguori > > >Behavioral changes: > > * fix abort on client answer after card remove > > * enable migration > > * remove side affect code from asserts > > * return consistent self-powered state > > * mask out reserved bits in ccid_set_parameters > > * add missing abRFU in SetParameters (no affect on linux guest) > > > >whitefixes / comments / consts defines: > > * remove stale comment > > * remove ccid_print_pending_answers if no DEBUG_CCID > > * replace printf's with DPRINTF, remove DEBUG_CCID, add verbosity defines > > * use error_report > > * update copyright (most of the code is not original) > > * reword known bug comment > > * add missing closing quote in comment > > * add missing whitespace on one line > > * s/CCID_SetParameter/CCID_SetParameters/ > > * add comments > > * use define for max packet size > > > >Comment for "return consistent self-powered state": > > > >the Configuration Descriptor bmAttributes claims we are self powered, > >but we were returning not self powered to USB_REQ_GET_STATUS control message. > > > >In practice, this message is not sent by a linux 2.6.35.10-74.fc14.x86_64 > >guest (not tested on other guests), unless you issue lsusb -v as root (for > >example). > > > >Signed-off-by: Alon Levy<alevy@redhat.com> > >--- > > hw/ccid.h | 7 +++ > > hw/usb-ccid.c | 115 ++++++++++++++++++++++++++++----------------------------- > > 2 files changed, 63 insertions(+), 59 deletions(-) > > > >diff --git a/hw/ccid.h b/hw/ccid.h > >index af59070..f5dcfae 100644 > >--- a/hw/ccid.h > >+++ b/hw/ccid.h > >@@ -6,11 +6,16 @@ > > typedef struct CCIDCardState CCIDCardState; > > typedef struct CCIDCardInfo CCIDCardInfo; > > > >+/* state of the CCID Card device (i.e. hw/ccid-card-*.c) > >+ */ > > struct CCIDCardState { > > DeviceState qdev; > > uint32_t slot; // For future use with multiple slot reader. > > }; > > > >+/* callbacks to be used by the CCID device (hw/usb-ccid.c) to call > >+ * into the smartcard device (hw/ccid-card-*.c) > >+ */ > > struct CCIDCardInfo { > > DeviceInfo qdev; > > void (*print)(Monitor *mon, CCIDCardState *card, int indent); > >@@ -20,6 +25,8 @@ struct CCIDCardInfo { > > int (*initfn)(CCIDCardState *card); > > }; > > > >+/* API for smartcard calling the CCID device (used by hw/ccid-card-*.c) > >+ */ > > void ccid_card_send_apdu_to_guest(CCIDCardState *card, uint8_t* apdu, uint32_t len); > > void ccid_card_card_removed(CCIDCardState *card); > > void ccid_card_card_inserted(CCIDCardState *card); > >diff --git a/hw/usb-ccid.c b/hw/usb-ccid.c > >index 58f69a6..09e39ac 100644 > >--- a/hw/usb-ccid.c > >+++ b/hw/usb-ccid.c > >@@ -1,15 +1,18 @@ > > /* > > * CCID Device emulation > > * > >- * Based on usb-serial.c: > >+ * Written by Alon Levy, with contributions from Robert Relyea. > >+ * > >+ * Based on usb-serial.c, see it's copyright and attributions below. > >+ * > >+ * This code is licenced under the GNU LGPL, version 2 or later. > >+ * > >+ * ------- > >+ * > >+ * usb-serial.c copyright and attribution: > > * Copyright (c) 2006 CodeSourcery. > > * Copyright (c) 2008 Samuel Thibault<samuel.thibault@ens-lyon.org> > > * Written by Paul Brook, reused for FTDI by Samuel Thibault, > >- * Reused for CCID by Alon Levy. > >- * Contributed to by Robert Relyea > >- * Copyright (c) 2010 Red Hat. > >- * > >- * This code is licenced under the LGPL. > > */ > > > > /* References: > >@@ -19,22 +22,16 @@ > > * Specification for Integrated Circuit(s) Cards Interface Devices > > * > > * Endianess note: from the spec (1.3) > >- * "Fields that are larger than a byte are stored in little endian > >+ * "Fields that are larger than a byte are stored in little endian" > > * > > * KNOWN BUGS > > * 1. remove/insert can sometimes result in removed state instead of inserted. > > * This is a result of the following: > >- * symptom: dmesg shows ERMOTEIO (-121), pcscd shows -99. This happens > >- * when we send a too short packet, seen in uhci-usb.c, resulting from > >- * a urb requesting SPD and us returning a smaller packet. > >+ * symptom: dmesg shows ERMOTEIO (-121), pcscd shows -99. This can happen > >+ * when a short packet is sent, as seen in uhci-usb.c, resulting from a urb > >+ * from the guest requesting SPD and us returning a smaller packet. > > * Not sure which messages trigger this. > > * > >- * Migration note: > >- * > >- * All the VMStateDescription's are left here for future use, but > >- * not enabled right now since there is no support for USB migration. > >- * > >- * To enable define ENABLE_MIGRATION > > */ > > > > #include "qemu-common.h" > >@@ -44,11 +41,14 @@ > > > > #include "hw/ccid.h" > > > >-//#define DEBUG_CCID > >- > > #define DPRINTF(s, lvl, fmt, ...) \ > > do { if (lvl<= s->debug) { printf("usb-ccid: " fmt , ## __VA_ARGS__); } } while (0) > > > >+#define D_WARN 1 > >+#define D_INFO 2 > >+#define D_MORE_INFO 3 > >+#define D_VERBOSE 4 > >+ > > #define CCID_DEV_NAME "usb-ccid" > > > > /* The two options for variable sized buffers: > >@@ -64,6 +64,8 @@ do { if (lvl<= s->debug) { printf("usb-ccid: " fmt , ## __VA_ARGS__); } } while > > #define InterfaceOutClass ((USB_DIR_OUT|USB_TYPE_CLASS|USB_RECIP_INTERFACE)<<8) > > #define InterfaceInClass ((USB_DIR_IN |USB_TYPE_CLASS|USB_RECIP_INTERFACE)<<8) > > > >+#define CCID_MAX_PACKET_SIZE 64 > >+ > > #define CCID_CONTROL_ABORT 0x1 > > #define CCID_CONTROL_GET_CLOCK_FREQUENCIES 0x2 > > #define CCID_CONTROL_GET_DATA_RATES 0x3 > >@@ -209,11 +211,12 @@ typedef struct __attribute__ ((__packed__)) CCID_IccPowerOff { > > uint16_t abRFU; > > } CCID_IccPowerOff; > > > >-typedef struct __attribute__ ((__packed__)) CCID_SetParameter { > >+typedef struct __attribute__ ((__packed__)) CCID_SetParameters { > > CCID_Header hdr; > > uint8_t bProtocolNum; > >+ uint16_t abRFU; > > uint8_t abProtocolDataStructure[0]; > >-} CCID_SetParameter; > >+} CCID_SetParameters; > > > > typedef struct CCID_Notify_Slot_Change { > > uint8_t bMessageType; /* CCID_MESSAGE_TYPE_RDR_to_PC_NotifySlotChange */ > >@@ -278,10 +281,6 @@ struct USBCCIDState { > > uint16_t migration_target_port; > > }; > > > >-/* Slot specific variables. We emulate a single slot card reader. > >- */ > >- > >- > > /* CCID Spec chapter 4: CCID uses a standard device descriptor per Chapter 9, > > * "USB Device Framework", section 9.6.1, in the Universal Serial Bus > > * Specification. > >@@ -416,7 +415,8 @@ static const uint8_t qemu_ccid_config_descriptor[] = { > > /* u8 ep_bEndpointAddress; IN Endpoint 1 */ > > 0x80 | CCID_INT_IN_EP, > > 0x03, /* u8 ep_bmAttributes; Interrupt */ > >- 0x40, 0x00, /* u16 ep_wMaxPacketSize; */ > >+ /* u16 ep_wMaxPacketSize; */ > >+ CCID_MAX_PACKET_SIZE& 0xff, (CCID_MAX_PACKET_SIZE>> 8), > > 0xff, /* u8 ep_bInterval; */ > > > > /* Bulk-In endpoint */ > >@@ -455,32 +455,31 @@ static void ccid_clear_pending_answers(USBCCIDState *s) > > > > static void ccid_print_pending_answers(USBCCIDState *s) > > { > >-#ifdef DEBUG_CCID > > Answer *answer; > > int i, count; > > > >- printf("usb-ccid: pending answers:"); > >+ DPRINTF(s, D_VERBOSE, "usb-ccid: pending answers:"); > > if (!ccid_has_pending_answers(s)) { > >- printf(" empty\n"); > >+ DPRINTF(s, D_VERBOSE, " empty\n"); > > return; > > } > > for (i = s->pending_answers_start, count=s->pending_answers_num ; > > count> 0; count--, i++) { > > answer =&s->pending_answers[i % PENDING_ANSWERS_NUM]; > > if (count == 1) { > >- printf("%d:%d\n", answer->slot, answer->seq); > >+ DPRINTF(s, D_VERBOSE, "%d:%d\n", answer->slot, answer->seq); > > } else { > >- printf("%d:%d,", answer->slot, answer->seq); > >+ DPRINTF(s, D_VERBOSE, "%d:%d,", answer->slot, answer->seq); > > } > > } > >-#endif > > } > > > > static void ccid_add_pending_answer(USBCCIDState *s, CCID_Header *hdr) > > { > > Answer* answer; > > > >- assert(s->pending_answers_num++< PENDING_ANSWERS_NUM); > >+ assert(s->pending_answers_num< PENDING_ANSWERS_NUM); > >+ s->pending_answers_num++; > > answer =&s->pending_answers[(s->pending_answers_end++) % PENDING_ANSWERS_NUM]; > > answer->slot = hdr->bSlot; > > answer->seq = hdr->bSeq; > >@@ -492,7 +491,8 @@ static void ccid_remove_pending_answer(USBCCIDState *s, > > { > > Answer *answer; > > > >- assert(s->pending_answers_num--> 0); > >+ assert(s->pending_answers_num> 0); > >+ s->pending_answers_num--; > > answer =&s->pending_answers[(s->pending_answers_start++) % PENDING_ANSWERS_NUM]; > > *slot = answer->slot; > > *seq = answer->seq; > >@@ -528,16 +528,16 @@ static void* ccid_reserve_recv_buf(USBCCIDState* s, uint16_t len) > > { > > BulkIn* bulk_in; > > > >- DPRINTF(s, 4, "%s: QUEUE: reserve %d bytes\n", __func__, len); > >+ DPRINTF(s, D_VERBOSE, "%s: QUEUE: reserve %d bytes\n", __func__, len); > > > > /* look for an existing element */ > > if (len> BULK_IN_BUF_SIZE) { > >- printf("usb-ccid.c: %s: len larger then max (%d>%d). discarding message.\n", > >+ DPRINTF(s, D_WARN, "usb-ccid.c: %s: len larger then max (%d>%d). discarding message.\n", > > __func__, len, BULK_IN_BUF_SIZE); > > return NULL; > > } > > if (s->bulk_in_pending_num>= BULK_IN_PENDING_NUM) { > >- printf("usb-ccid.c: %s: No free bulk_in buffers. discarding message.\n", > >+ DPRINTF(s, D_WARN, "usb-ccid.c: %s: No free bulk_in buffers. discarding message.\n", > > __func__); > > return NULL; > > } > >@@ -576,7 +576,7 @@ static int ccid_handle_control(USBDevice *dev, int request, int value, > > DPRINTF(s, 1, "got control %x, value %x\n",request, value); > > switch (request) { > > case DeviceRequest | USB_REQ_GET_STATUS: > >- data[0] = (0<< USB_DEVICE_SELF_POWERED) | > >+ data[0] = (1<< USB_DEVICE_SELF_POWERED) | > > (dev->remote_wakeup<< USB_DEVICE_REMOTE_WAKEUP); > > data[1] = 0x00; > > ret = 2; > >@@ -709,7 +709,7 @@ static uint8_t ccid_calc_status(USBCCIDState *s) > > bmCommandStatus > > */ > > uint8_t ret = ccid_card_status(s) | (s->bmCommandStatus<< 6); > >- DPRINTF(s, 4, "status = %d\n", ret); > >+ DPRINTF(s, D_VERBOSE, "status = %d\n", ret); > > return ret; > > } > > > >@@ -770,11 +770,9 @@ static void ccid_write_data_block( > > p->b.hdr.bSeq = seq; > > p->b.bStatus = ccid_calc_status(s); > > p->b.bError = s->bError; > >-#ifdef DEBUG_CCID > > if (p->b.bError) { > >- DPRINTF(s, 4, "error %d", p->b.bError); > >+ DPRINTF(s, D_VERBOSE, "error %d", p->b.bError); > > } > >-#endif > > memcpy(p->abData, data, len); > > ccid_reset_error_status(s); > > } > >@@ -805,12 +803,12 @@ static void ccid_write_data_block_atr(USBCCIDState* s, CCID_Header* recv) > > > > static void ccid_set_parameters(USBCCIDState *s, CCID_Header *recv) > > { > >- CCID_SetParameter *ph = (CCID_SetParameter *) recv; > >+ CCID_SetParameters *ph = (CCID_SetParameters *) recv; > > uint32_t len = 0; > >- if (ph->bProtocolNum == 0) { > >+ if ((ph->bProtocolNum& 3) == 0) { > > len = 5; > > } > >- if (ph->bProtocolNum == 1) { > >+ if ((ph->bProtocolNum& 3) == 1) { > > len = 7; > > } > > if (len == 0) { > >@@ -884,7 +882,7 @@ static void ccid_on_apdu_from_guest(USBCCIDState *s, CCID_XferBlock *recv) > > if (s->card) { > > s->cardinfo->apdu_from_guest(s->card, recv->abData, len); > > } else { > >- printf("warning: discarded apdu\n"); > >+ DPRINTF(s, D_WARN, "warning: discarded apdu\n"); > > } > > } > > > >@@ -901,15 +899,15 @@ static int ccid_handle_bulk_out(USBCCIDState *s, USBPacket *p) > > ccid_header = (CCID_Header*)s->bulk_out_data; > > memcpy(s->bulk_out_data + s->bulk_out_pos, p->data, p->len); > > s->bulk_out_pos += p->len; > >- if (p->len == 64) { > >- DPRINTF(s, 4, "usb-ccid: bulk_in: expecting more packets (%d/%d)\n", > >+ if (p->len == CCID_MAX_PACKET_SIZE) { > >+ DPRINTF(s, D_VERBOSE, "usb-ccid: bulk_in: expecting more packets (%d/%d)\n", > > p->len, ccid_header->dwLength); > > return 0; > > } > > if (s->bulk_out_pos< 10) { > > DPRINTF(s, 1, "%s: bad USB_TOKEN_OUT length, should be at least 10 bytes\n", __func__); > > } else { > >- DPRINTF(s, 3, "%s %x\n", __func__, ccid_header->bMessageType); > >+ DPRINTF(s, D_MORE_INFO, "%s %x\n", __func__, ccid_header->bMessageType); > > switch (ccid_header->bMessageType) { > > case CCID_MESSAGE_TYPE_PC_to_RDR_GetSlotStatus: > > ccid_write_slot_status(s, ccid_header); > >@@ -965,7 +963,7 @@ static int ccid_bulk_in_copy_to_guest(USBCCIDState *s, uint8_t *data, int len) > > { > > int ret = 0; > > > >- assert(len>0); > >+ assert(len> 0); > > ccid_bulk_in_get(s); > > if (s->current_bulk_in != NULL) { > > ret = MIN(s->current_bulk_in->len - s->current_bulk_in->pos, len); > >@@ -978,7 +976,7 @@ static int ccid_bulk_in_copy_to_guest(USBCCIDState *s, uint8_t *data, int len) > > ret = USB_RET_NAK; /* return when device has no data - usb 2.0 spec Table 8-4 */ > > } > > if (ret> 0) { > >- DPRINTF(s, 3, "%s: %d/%d req/act to guest (BULK_IN)\n", __func__, len, ret); > >+ DPRINTF(s, D_MORE_INFO, "%s: %d/%d req/act to guest (BULK_IN)\n", __func__, len, ret); > > } > > if (ret != USB_RET_NAK&& ret< len) { > > DPRINTF(s, 1, "%s: returning short (EREMOTEIO) %d< %d\n", __func__, ret, len); > >@@ -1015,7 +1013,7 @@ static int ccid_handle_data(USBDevice *dev, USBPacket *p) > > ret = 2; > > s->notify_slot_change = false; > > s->bmSlotICCState&= ~SLOT_0_CHANGED_MASK; > >- DPRINTF(s, 2, "handle_data: int_in: notify_slot_change %X, requested len %d\n", > >+ DPRINTF(s, D_INFO, "handle_data: int_in: notify_slot_change %X, requested len %d\n", > > s->bmSlotICCState, len); > > } > > break; > >@@ -1146,7 +1144,11 @@ void ccid_card_card_error(CCIDCardState *card, uint64_t error) > > DPRINTF(s, 1, "VSC_Error: %lX\n", s->last_answer_error); > > /* TODO: these error's should be more verbose and propogated to the guest. > > * */ > >- ccid_write_data_block_answer(s, NULL, 0); > >+ /* we flush all pending answers on CardRemove message in ccid-card-passthru, > >+ * so check that first to not trigger abort */ > >+ if (ccid_has_pending_answers(s)) { > >+ ccid_write_data_block_answer(s, NULL, 0); > >+ } > > } > > > > void ccid_card_card_inserted(CCIDCardState *card) > >@@ -1184,12 +1186,12 @@ static int ccid_card_init(DeviceState *qdev, DeviceInfo *base) > > int ret = 0; > > > > if (card->slot != 0) { > >- fprintf(stderr, "Warning: usb-ccid supports one slot, can't add %d", > >+ error_report("Warning: usb-ccid supports one slot, can't add %d", > > card->slot); > > return -1; > > } > > if (s->card != NULL) { > >- fprintf(stderr, "Warning: usb-ccid card already full, not adding\n"); > >+ error_report("Warning: usb-ccid card already full, not adding\n"); > > return -1; > > } > > ret = info->initfn ? info->initfn(card) : ret; > >@@ -1233,7 +1235,6 @@ static int ccid_initfn(USBDevice *dev) > > return 0; > > } > > > >-#ifdef ENABLE_MIGRATION > > static int ccid_post_load(void *opaque, int version_id) > > { > > USBCCIDState *s = opaque; > >@@ -1325,7 +1326,6 @@ static VMStateDescription ccid_vmstate = { > > VMSTATE_END_OF_LIST() > > } > > }; > >-#endif // ENABLE_MIGRATION > > > > static struct USBDeviceInfo ccid_info = { > > .product_desc = "QEMU USB CCID", > >@@ -1342,12 +1342,9 @@ static struct USBDeviceInfo ccid_info = { > > DEFINE_PROP_UINT8("debug", USBCCIDState, debug, 0), > > DEFINE_PROP_END_OF_LIST(), > > }, > >-#ifdef ENABLE_MIGRATION > > .qdev.vmsd =&ccid_vmstate, > >-#endif > > }; > > > >- > > static void ccid_register_devices(void) > > { > > usb_qdev_register(&ccid_info); >
diff --git a/hw/ccid.h b/hw/ccid.h index af59070..f5dcfae 100644 --- a/hw/ccid.h +++ b/hw/ccid.h @@ -6,11 +6,16 @@ typedef struct CCIDCardState CCIDCardState; typedef struct CCIDCardInfo CCIDCardInfo; +/* state of the CCID Card device (i.e. hw/ccid-card-*.c) + */ struct CCIDCardState { DeviceState qdev; uint32_t slot; // For future use with multiple slot reader. }; +/* callbacks to be used by the CCID device (hw/usb-ccid.c) to call + * into the smartcard device (hw/ccid-card-*.c) + */ struct CCIDCardInfo { DeviceInfo qdev; void (*print)(Monitor *mon, CCIDCardState *card, int indent); @@ -20,6 +25,8 @@ struct CCIDCardInfo { int (*initfn)(CCIDCardState *card); }; +/* API for smartcard calling the CCID device (used by hw/ccid-card-*.c) + */ void ccid_card_send_apdu_to_guest(CCIDCardState *card, uint8_t* apdu, uint32_t len); void ccid_card_card_removed(CCIDCardState *card); void ccid_card_card_inserted(CCIDCardState *card); diff --git a/hw/usb-ccid.c b/hw/usb-ccid.c index 58f69a6..09e39ac 100644 --- a/hw/usb-ccid.c +++ b/hw/usb-ccid.c @@ -1,15 +1,18 @@ /* * CCID Device emulation * - * Based on usb-serial.c: + * Written by Alon Levy, with contributions from Robert Relyea. + * + * Based on usb-serial.c, see it's copyright and attributions below. + * + * This code is licenced under the GNU LGPL, version 2 or later. + * + * ------- + * + * usb-serial.c copyright and attribution: * Copyright (c) 2006 CodeSourcery. * Copyright (c) 2008 Samuel Thibault <samuel.thibault@ens-lyon.org> * Written by Paul Brook, reused for FTDI by Samuel Thibault, - * Reused for CCID by Alon Levy. - * Contributed to by Robert Relyea - * Copyright (c) 2010 Red Hat. - * - * This code is licenced under the LGPL. */ /* References: @@ -19,22 +22,16 @@ * Specification for Integrated Circuit(s) Cards Interface Devices * * Endianess note: from the spec (1.3) - * "Fields that are larger than a byte are stored in little endian + * "Fields that are larger than a byte are stored in little endian" * * KNOWN BUGS * 1. remove/insert can sometimes result in removed state instead of inserted. * This is a result of the following: - * symptom: dmesg shows ERMOTEIO (-121), pcscd shows -99. This happens - * when we send a too short packet, seen in uhci-usb.c, resulting from - * a urb requesting SPD and us returning a smaller packet. + * symptom: dmesg shows ERMOTEIO (-121), pcscd shows -99. This can happen + * when a short packet is sent, as seen in uhci-usb.c, resulting from a urb + * from the guest requesting SPD and us returning a smaller packet. * Not sure which messages trigger this. * - * Migration note: - * - * All the VMStateDescription's are left here for future use, but - * not enabled right now since there is no support for USB migration. - * - * To enable define ENABLE_MIGRATION */ #include "qemu-common.h" @@ -44,11 +41,14 @@ #include "hw/ccid.h" -//#define DEBUG_CCID - #define DPRINTF(s, lvl, fmt, ...) \ do { if (lvl <= s->debug) { printf("usb-ccid: " fmt , ## __VA_ARGS__); } } while (0) +#define D_WARN 1 +#define D_INFO 2 +#define D_MORE_INFO 3 +#define D_VERBOSE 4 + #define CCID_DEV_NAME "usb-ccid" /* The two options for variable sized buffers: @@ -64,6 +64,8 @@ do { if (lvl <= s->debug) { printf("usb-ccid: " fmt , ## __VA_ARGS__); } } while #define InterfaceOutClass ((USB_DIR_OUT|USB_TYPE_CLASS|USB_RECIP_INTERFACE)<<8) #define InterfaceInClass ((USB_DIR_IN |USB_TYPE_CLASS|USB_RECIP_INTERFACE)<<8) +#define CCID_MAX_PACKET_SIZE 64 + #define CCID_CONTROL_ABORT 0x1 #define CCID_CONTROL_GET_CLOCK_FREQUENCIES 0x2 #define CCID_CONTROL_GET_DATA_RATES 0x3 @@ -209,11 +211,12 @@ typedef struct __attribute__ ((__packed__)) CCID_IccPowerOff { uint16_t abRFU; } CCID_IccPowerOff; -typedef struct __attribute__ ((__packed__)) CCID_SetParameter { +typedef struct __attribute__ ((__packed__)) CCID_SetParameters { CCID_Header hdr; uint8_t bProtocolNum; + uint16_t abRFU; uint8_t abProtocolDataStructure[0]; -} CCID_SetParameter; +} CCID_SetParameters; typedef struct CCID_Notify_Slot_Change { uint8_t bMessageType; /* CCID_MESSAGE_TYPE_RDR_to_PC_NotifySlotChange */ @@ -278,10 +281,6 @@ struct USBCCIDState { uint16_t migration_target_port; }; -/* Slot specific variables. We emulate a single slot card reader. - */ - - /* CCID Spec chapter 4: CCID uses a standard device descriptor per Chapter 9, * "USB Device Framework", section 9.6.1, in the Universal Serial Bus * Specification. @@ -416,7 +415,8 @@ static const uint8_t qemu_ccid_config_descriptor[] = { /* u8 ep_bEndpointAddress; IN Endpoint 1 */ 0x80 | CCID_INT_IN_EP, 0x03, /* u8 ep_bmAttributes; Interrupt */ - 0x40, 0x00, /* u16 ep_wMaxPacketSize; */ + /* u16 ep_wMaxPacketSize; */ + CCID_MAX_PACKET_SIZE & 0xff, (CCID_MAX_PACKET_SIZE >> 8), 0xff, /* u8 ep_bInterval; */ /* Bulk-In endpoint */ @@ -455,32 +455,31 @@ static void ccid_clear_pending_answers(USBCCIDState *s) static void ccid_print_pending_answers(USBCCIDState *s) { -#ifdef DEBUG_CCID Answer *answer; int i, count; - printf("usb-ccid: pending answers:"); + DPRINTF(s, D_VERBOSE, "usb-ccid: pending answers:"); if (!ccid_has_pending_answers(s)) { - printf(" empty\n"); + DPRINTF(s, D_VERBOSE, " empty\n"); return; } for (i = s->pending_answers_start, count=s->pending_answers_num ; count > 0; count--, i++) { answer = &s->pending_answers[i % PENDING_ANSWERS_NUM]; if (count == 1) { - printf("%d:%d\n", answer->slot, answer->seq); + DPRINTF(s, D_VERBOSE, "%d:%d\n", answer->slot, answer->seq); } else { - printf("%d:%d,", answer->slot, answer->seq); + DPRINTF(s, D_VERBOSE, "%d:%d,", answer->slot, answer->seq); } } -#endif } static void ccid_add_pending_answer(USBCCIDState *s, CCID_Header *hdr) { Answer* answer; - assert(s->pending_answers_num++ < PENDING_ANSWERS_NUM); + assert(s->pending_answers_num < PENDING_ANSWERS_NUM); + s->pending_answers_num++; answer = &s->pending_answers[(s->pending_answers_end++) % PENDING_ANSWERS_NUM]; answer->slot = hdr->bSlot; answer->seq = hdr->bSeq; @@ -492,7 +491,8 @@ static void ccid_remove_pending_answer(USBCCIDState *s, { Answer *answer; - assert(s->pending_answers_num-- > 0); + assert(s->pending_answers_num > 0); + s->pending_answers_num--; answer = &s->pending_answers[(s->pending_answers_start++) % PENDING_ANSWERS_NUM]; *slot = answer->slot; *seq = answer->seq; @@ -528,16 +528,16 @@ static void* ccid_reserve_recv_buf(USBCCIDState* s, uint16_t len) { BulkIn* bulk_in; - DPRINTF(s, 4, "%s: QUEUE: reserve %d bytes\n", __func__, len); + DPRINTF(s, D_VERBOSE, "%s: QUEUE: reserve %d bytes\n", __func__, len); /* look for an existing element */ if (len > BULK_IN_BUF_SIZE) { - printf("usb-ccid.c: %s: len larger then max (%d>%d). discarding message.\n", + DPRINTF(s, D_WARN, "usb-ccid.c: %s: len larger then max (%d>%d). discarding message.\n", __func__, len, BULK_IN_BUF_SIZE); return NULL; } if (s->bulk_in_pending_num >= BULK_IN_PENDING_NUM) { - printf("usb-ccid.c: %s: No free bulk_in buffers. discarding message.\n", + DPRINTF(s, D_WARN, "usb-ccid.c: %s: No free bulk_in buffers. discarding message.\n", __func__); return NULL; } @@ -576,7 +576,7 @@ static int ccid_handle_control(USBDevice *dev, int request, int value, DPRINTF(s, 1, "got control %x, value %x\n",request, value); switch (request) { case DeviceRequest | USB_REQ_GET_STATUS: - data[0] = (0 << USB_DEVICE_SELF_POWERED) | + data[0] = (1 << USB_DEVICE_SELF_POWERED) | (dev->remote_wakeup << USB_DEVICE_REMOTE_WAKEUP); data[1] = 0x00; ret = 2; @@ -709,7 +709,7 @@ static uint8_t ccid_calc_status(USBCCIDState *s) bmCommandStatus */ uint8_t ret = ccid_card_status(s) | (s->bmCommandStatus << 6); - DPRINTF(s, 4, "status = %d\n", ret); + DPRINTF(s, D_VERBOSE, "status = %d\n", ret); return ret; } @@ -770,11 +770,9 @@ static void ccid_write_data_block( p->b.hdr.bSeq = seq; p->b.bStatus = ccid_calc_status(s); p->b.bError = s->bError; -#ifdef DEBUG_CCID if (p->b.bError) { - DPRINTF(s, 4, "error %d", p->b.bError); + DPRINTF(s, D_VERBOSE, "error %d", p->b.bError); } -#endif memcpy(p->abData, data, len); ccid_reset_error_status(s); } @@ -805,12 +803,12 @@ static void ccid_write_data_block_atr(USBCCIDState* s, CCID_Header* recv) static void ccid_set_parameters(USBCCIDState *s, CCID_Header *recv) { - CCID_SetParameter *ph = (CCID_SetParameter *) recv; + CCID_SetParameters *ph = (CCID_SetParameters *) recv; uint32_t len = 0; - if (ph->bProtocolNum == 0) { + if ((ph->bProtocolNum & 3) == 0) { len = 5; } - if (ph->bProtocolNum == 1) { + if ((ph->bProtocolNum & 3) == 1) { len = 7; } if (len == 0) { @@ -884,7 +882,7 @@ static void ccid_on_apdu_from_guest(USBCCIDState *s, CCID_XferBlock *recv) if (s->card) { s->cardinfo->apdu_from_guest(s->card, recv->abData, len); } else { - printf("warning: discarded apdu\n"); + DPRINTF(s, D_WARN, "warning: discarded apdu\n"); } } @@ -901,15 +899,15 @@ static int ccid_handle_bulk_out(USBCCIDState *s, USBPacket *p) ccid_header = (CCID_Header*)s->bulk_out_data; memcpy(s->bulk_out_data + s->bulk_out_pos, p->data, p->len); s->bulk_out_pos += p->len; - if (p->len == 64) { - DPRINTF(s, 4, "usb-ccid: bulk_in: expecting more packets (%d/%d)\n", + if (p->len == CCID_MAX_PACKET_SIZE) { + DPRINTF(s, D_VERBOSE, "usb-ccid: bulk_in: expecting more packets (%d/%d)\n", p->len, ccid_header->dwLength); return 0; } if (s->bulk_out_pos < 10) { DPRINTF(s, 1, "%s: bad USB_TOKEN_OUT length, should be at least 10 bytes\n", __func__); } else { - DPRINTF(s, 3, "%s %x\n", __func__, ccid_header->bMessageType); + DPRINTF(s, D_MORE_INFO, "%s %x\n", __func__, ccid_header->bMessageType); switch (ccid_header->bMessageType) { case CCID_MESSAGE_TYPE_PC_to_RDR_GetSlotStatus: ccid_write_slot_status(s, ccid_header); @@ -965,7 +963,7 @@ static int ccid_bulk_in_copy_to_guest(USBCCIDState *s, uint8_t *data, int len) { int ret = 0; - assert(len>0); + assert(len > 0); ccid_bulk_in_get(s); if (s->current_bulk_in != NULL) { ret = MIN(s->current_bulk_in->len - s->current_bulk_in->pos, len); @@ -978,7 +976,7 @@ static int ccid_bulk_in_copy_to_guest(USBCCIDState *s, uint8_t *data, int len) ret = USB_RET_NAK; /* return when device has no data - usb 2.0 spec Table 8-4 */ } if (ret > 0) { - DPRINTF(s, 3, "%s: %d/%d req/act to guest (BULK_IN)\n", __func__, len, ret); + DPRINTF(s, D_MORE_INFO, "%s: %d/%d req/act to guest (BULK_IN)\n", __func__, len, ret); } if (ret != USB_RET_NAK && ret < len) { DPRINTF(s, 1, "%s: returning short (EREMOTEIO) %d < %d\n", __func__, ret, len); @@ -1015,7 +1013,7 @@ static int ccid_handle_data(USBDevice *dev, USBPacket *p) ret = 2; s->notify_slot_change = false; s->bmSlotICCState &= ~SLOT_0_CHANGED_MASK; - DPRINTF(s, 2, "handle_data: int_in: notify_slot_change %X, requested len %d\n", + DPRINTF(s, D_INFO, "handle_data: int_in: notify_slot_change %X, requested len %d\n", s->bmSlotICCState, len); } break; @@ -1146,7 +1144,11 @@ void ccid_card_card_error(CCIDCardState *card, uint64_t error) DPRINTF(s, 1, "VSC_Error: %lX\n", s->last_answer_error); /* TODO: these error's should be more verbose and propogated to the guest. * */ - ccid_write_data_block_answer(s, NULL, 0); + /* we flush all pending answers on CardRemove message in ccid-card-passthru, + * so check that first to not trigger abort */ + if (ccid_has_pending_answers(s)) { + ccid_write_data_block_answer(s, NULL, 0); + } } void ccid_card_card_inserted(CCIDCardState *card) @@ -1184,12 +1186,12 @@ static int ccid_card_init(DeviceState *qdev, DeviceInfo *base) int ret = 0; if (card->slot != 0) { - fprintf(stderr, "Warning: usb-ccid supports one slot, can't add %d", + error_report("Warning: usb-ccid supports one slot, can't add %d", card->slot); return -1; } if (s->card != NULL) { - fprintf(stderr, "Warning: usb-ccid card already full, not adding\n"); + error_report("Warning: usb-ccid card already full, not adding\n"); return -1; } ret = info->initfn ? info->initfn(card) : ret; @@ -1233,7 +1235,6 @@ static int ccid_initfn(USBDevice *dev) return 0; } -#ifdef ENABLE_MIGRATION static int ccid_post_load(void *opaque, int version_id) { USBCCIDState *s = opaque; @@ -1325,7 +1326,6 @@ static VMStateDescription ccid_vmstate = { VMSTATE_END_OF_LIST() } }; -#endif // ENABLE_MIGRATION static struct USBDeviceInfo ccid_info = { .product_desc = "QEMU USB CCID", @@ -1342,12 +1342,9 @@ static struct USBDeviceInfo ccid_info = { DEFINE_PROP_UINT8("debug", USBCCIDState, debug, 0), DEFINE_PROP_END_OF_LIST(), }, -#ifdef ENABLE_MIGRATION .qdev.vmsd = &ccid_vmstate, -#endif }; - static void ccid_register_devices(void) { usb_qdev_register(&ccid_info);
I'll fold it before submitting the version to be applied, but I hope keeping it as a separate patch will make reviewing easier. Behavioral changes: * fix abort on client answer after card remove * enable migration * remove side affect code from asserts * return consistent self-powered state * mask out reserved bits in ccid_set_parameters * add missing abRFU in SetParameters (no affect on linux guest) whitefixes / comments / consts defines: * remove stale comment * remove ccid_print_pending_answers if no DEBUG_CCID * replace printf's with DPRINTF, remove DEBUG_CCID, add verbosity defines * use error_report * update copyright (most of the code is not original) * reword known bug comment * add missing closing quote in comment * add missing whitespace on one line * s/CCID_SetParameter/CCID_SetParameters/ * add comments * use define for max packet size Comment for "return consistent self-powered state": the Configuration Descriptor bmAttributes claims we are self powered, but we were returning not self powered to USB_REQ_GET_STATUS control message. In practice, this message is not sent by a linux 2.6.35.10-74.fc14.x86_64 guest (not tested on other guests), unless you issue lsusb -v as root (for example). Signed-off-by: Alon Levy <alevy@redhat.com> --- hw/ccid.h | 7 +++ hw/usb-ccid.c | 115 ++++++++++++++++++++++++++++----------------------------- 2 files changed, 63 insertions(+), 59 deletions(-)