All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/6] Introduce CanoKey QEMU
@ 2022-05-19 12:35 Hongren (Zenithal) Zheng
  2022-05-19 12:38 ` [PATCH v5 1/6] hw/usb: Add CanoKey Implementation Hongren (Zenithal) Zheng
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Hongren (Zenithal) Zheng @ 2022-05-19 12:35 UTC (permalink / raw)
  To: Gerd Hoffmann, Thomas Huth; +Cc: qemu-devel, contact

- One sentense

With this patch series, QEMU would fully emulate an open-source secure key,
CanoKey, with supports of various features listed below:

 * U2F / FIDO2 with Ed25519 and HMAC-secret
 * OpenPGP Card V3.4 with RSA4096, Ed25519 and more
 * PIV (NIST SP 800-73-4)
 * HOTP / TOTP

- What's New

Although we have seen multiple emulated devices providing different
functionalities for different purposes such as U2F (hw/usb/u2f-emulated.c)
and CAC (hw/usb/ccid-card-emulated.c), modern secure key needs more advanced
protocols like FIDO2 (WebAuthn, in comparison to U2f) and PIV
(in comparison to CAC), which is not implemented previously.

Other features like OpenPGP / TOTP are also not implemented before, at least
as an emulated functionality.

- Why get upstreamed

At Canokeys.org, virtual cards on its own are for testing and debugging
on the key itself. We have implemented various virt-cards including
fido-hid-over-udp and USB/IP on our CI for testing and developer debuging.

As we found emulated U2F and CAC in QEMU mainline, we estimated we could
implement such features as well, which is good for testing since now
we can emulate the whole key as an USB device, and we implemented it!
as presented by this patch series.

The story doesn't end here. As CanoKey QEMU is a fully functional key and it is
inside QEMU, we think this emulated device could reach a wider audience
other than CanoKey developers: projects using secure key can also benefit
from it.

For example, this device can be used in CI for projects using secure key.
Bringing up a VM using QEMU with CanoKey QEMU, now we have an environment
with secure key, and we can test the correctness of the behavior of the code.

Another example is that as it is fully emulated rather than some hardware,
all traces/debug logs can be easily extracted, which is helpful for
developpers to debug.

One note though, using CanoKey QEMU as a daily secure key is not recommended
as the secret key in the emulated key is not protected by hardware.

- Implementation details

CanoKey implements all these platform independent features in canokey-core
https://github.com/canokeys/canokey-core, and leaves the USB implementation
to the platform, thus in this patch series we implemented the USB part
in QEMU platform using QEMU's USB APIs, therefore the emulated CanoKey
can communicate with the guest OS using USB.

Some note though, CanoKey also has a NFC interface, thus we can implement
the NFC part in QEMU and expose CanoKey to the guest as an NFC device.
This is left as future work.

In the meanwhile, unlike other emulated device which has a passthrough
counterpart, CanoKey QEMU does not provide a passthrough mode as a whole
since CanoKey has multiple interfaces which is hard to passthrough.
(Left as future work, passthrough via WebUSB interface)
You may try to use u2f-passthru and ccid-card-passthru to pass the U2F and
CCID (e.g. OpenPGP, PIV) part of a real (or virtual, referring to USB/IP)
CanoKey on the host to the guest.

---
v1 -> v2:
  * Use trace events instead of printf to log canokey.c function call
  * Update debug instructions (trace, pcap) in CanoKey doc
  * Drop commit about legacy -usbdevice usage

v2 -> v3:
  * Fix code style in commit hw/usb/canokey: Add trace events
  * Move docs/canokey.txt to docs/system/devices/canokey.rst

v3 -> v4:
  * Refactor canokey.c into single thread version. This version
    is much easier to understand and review
  * Add more comments
  * Add links to canokey.rst in usb.rst
  * Update MAINTAINERS canokey docs file

v4 -> v5:
  * Fix compile when there is no libcanokey-qemu
  * Separate canokey-qemu repo from canokey-core (doc change)
  * Categorize canokey into Misc devices
  * Add high speed support (thus usb-ehci) can use it
  * State the limitation when used with qemu-xhci

  About the qemu-xhci issue, see patch [PATCH v5 4/6] for more detail
  This patchset passes the CI, see
  https://gitlab.com/ZenithalHourlyRate/qemu/-/pipelines/543016316

Hongren (Zenithal) Zheng (6):
  hw/usb: Add CanoKey Implementation
  hw/usb/canokey: Add trace events
  meson: Add CanoKey
  docs: Add CanoKey documentation
  docs/system/devices/usb: Add CanoKey to USB devices examples
  MAINTAINERS: add myself as CanoKey maintainer

 MAINTAINERS                      |   8 +
 docs/system/device-emulation.rst |   1 +
 docs/system/devices/canokey.rst  | 168 +++++++++++++++++
 docs/system/devices/usb.rst      |   4 +
 hw/usb/Kconfig                   |   5 +
 hw/usb/canokey.c                 | 313 +++++++++++++++++++++++++++++++
 hw/usb/canokey.h                 |  69 +++++++
 hw/usb/meson.build               |   5 +
 hw/usb/trace-events              |  16 ++
 meson.build                      |   6 +
 meson_options.txt                |   2 +
 scripts/meson-buildoptions.sh    |   3 +
 12 files changed, 600 insertions(+)
 create mode 100644 docs/system/devices/canokey.rst
 create mode 100644 hw/usb/canokey.c
 create mode 100644 hw/usb/canokey.h

-- 
2.35.1



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

* [PATCH v5 1/6] hw/usb: Add CanoKey Implementation
  2022-05-19 12:35 [PATCH v5 0/6] Introduce CanoKey QEMU Hongren (Zenithal) Zheng
@ 2022-05-19 12:38 ` Hongren (Zenithal) Zheng
  2022-05-19 12:38 ` [PATCH v5 2/6] hw/usb/canokey: Add trace events Hongren (Zenithal) Zheng
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Hongren (Zenithal) Zheng @ 2022-05-19 12:38 UTC (permalink / raw)
  To: Gerd Hoffmann, Thomas Huth; +Cc: qemu-devel, contact

This commit added a new emulated device called CanoKey to QEMU.

CanoKey implements platform independent features in canokey-core
https://github.com/canokeys/canokey-core, and leaves the USB implementation
to the platform.

In this commit the USB part was implemented in QEMU using QEMU's USB APIs,
therefore the emulated CanoKey can communicate with the guest OS using USB.

Signed-off-by: Hongren (Zenithal) Zheng <i@zenithal.me>
---
 hw/usb/canokey.c | 300 +++++++++++++++++++++++++++++++++++++++++++++++
 hw/usb/canokey.h |  69 +++++++++++
 2 files changed, 369 insertions(+)
 create mode 100644 hw/usb/canokey.c
 create mode 100644 hw/usb/canokey.h

diff --git a/hw/usb/canokey.c b/hw/usb/canokey.c
new file mode 100644
index 0000000000..6cb8b7cdb0
--- /dev/null
+++ b/hw/usb/canokey.c
@@ -0,0 +1,300 @@
+/*
+ * CanoKey QEMU device implementation.
+ *
+ * Copyright (c) 2021-2022 Canokeys.org <contact@canokeys.org>
+ * Written by Hongren (Zenithal) Zheng <i@zenithal.me>
+ *
+ * This code is licensed under the Apache-2.0.
+ */
+
+#include "qemu/osdep.h"
+#include <canokey-qemu.h>
+
+#include "qemu/module.h"
+#include "qapi/error.h"
+#include "hw/usb.h"
+#include "hw/qdev-properties.h"
+#include "desc.h"
+#include "canokey.h"
+
+#define CANOKEY_EP_IN(ep) ((ep) & 0x7F)
+
+#define CANOKEY_VENDOR_NUM     0x20a0
+#define CANOKEY_PRODUCT_NUM    0x42d2
+
+/*
+ * placeholder, canokey-qemu implements its own usb desc
+ * Namely we do not use usb_desc_handle_contorl
+ */
+enum {
+    STR_MANUFACTURER = 1,
+    STR_PRODUCT,
+    STR_SERIALNUMBER
+};
+
+static const USBDescStrings desc_strings = {
+    [STR_MANUFACTURER]     = "canokeys.org",
+    [STR_PRODUCT]          = "CanoKey QEMU",
+    [STR_SERIALNUMBER]     = "0"
+};
+
+static const USBDescDevice desc_device_canokey = {
+    .bcdUSB                        = 0x0,
+    .bMaxPacketSize0               = 16,
+    .bNumConfigurations            = 0,
+    .confs = NULL,
+};
+
+static const USBDesc desc_canokey = {
+    .id = {
+        .idVendor          = CANOKEY_VENDOR_NUM,
+        .idProduct         = CANOKEY_PRODUCT_NUM,
+        .bcdDevice         = 0x0100,
+        .iManufacturer     = STR_MANUFACTURER,
+        .iProduct          = STR_PRODUCT,
+        .iSerialNumber     = STR_SERIALNUMBER,
+    },
+    .full = &desc_device_canokey,
+    .high = &desc_device_canokey,
+    .str  = desc_strings,
+};
+
+
+/*
+ * libcanokey-qemu.so side functions
+ * All functions are called from canokey_emu_device_loop
+ */
+int canokey_emu_stall_ep(void *base, uint8_t ep)
+{
+    CanoKeyState *key = base;
+    uint8_t ep_in = CANOKEY_EP_IN(ep); /* INTR IN has ep 129 */
+    key->ep_in_size[ep_in] = 0;
+    key->ep_in_state[ep_in] = CANOKEY_EP_IN_STALL;
+    return 0;
+}
+
+int canokey_emu_set_address(void *base, uint8_t addr)
+{
+    CanoKeyState *key = base;
+    key->dev.addr = addr;
+    return 0;
+}
+
+int canokey_emu_prepare_receive(
+        void *base, uint8_t ep, uint8_t *pbuf, uint16_t size)
+{
+    CanoKeyState *key = base;
+    key->ep_out[ep] = pbuf;
+    key->ep_out_size[ep] = size;
+    return 0;
+}
+
+int canokey_emu_transmit(
+        void *base, uint8_t ep, const uint8_t *pbuf, uint16_t size)
+{
+    CanoKeyState *key = base;
+    uint8_t ep_in = CANOKEY_EP_IN(ep); /* INTR IN has ep 129 */
+    memcpy(key->ep_in[ep_in] + key->ep_in_size[ep_in],
+            pbuf, size);
+    key->ep_in_size[ep_in] += size;
+    key->ep_in_state[ep_in] = CANOKEY_EP_IN_READY;
+    /*
+     * ready for more data in device loop
+     *
+     * Note: this is a quirk for CanoKey CTAPHID
+     * because it calls multiple emu_transmit in one device_loop
+     * but w/o data_in it would stuck in device_loop
+     * This has no side effect for CCID as it is strictly
+     * OUT then IN transfer
+     * However it has side effect for Control transfer
+     */
+    if (ep_in != 0) {
+        canokey_emu_data_in(ep_in);
+    }
+    return 0;
+}
+
+uint32_t canokey_emu_get_rx_data_size(void *base, uint8_t ep)
+{
+    CanoKeyState *key = base;
+    return key->ep_out_size[ep];
+}
+
+/*
+ * QEMU side functions
+ */
+static void canokey_handle_reset(USBDevice *dev)
+{
+    CanoKeyState *key = CANOKEY(dev);
+    for (int i = 0; i != CANOKEY_EP_NUM; ++i) {
+        key->ep_in_state[i] = CANOKEY_EP_IN_WAIT;
+        key->ep_in_pos[i] = 0;
+        key->ep_in_size[i] = 0;
+    }
+    canokey_emu_reset();
+}
+
+static void canokey_handle_control(USBDevice *dev, USBPacket *p,
+               int request, int value, int index, int length, uint8_t *data)
+{
+    CanoKeyState *key = CANOKEY(dev);
+
+    canokey_emu_setup(request, value, index, length);
+
+    uint32_t dir_in = request & DeviceRequest;
+    if (!dir_in) {
+        /* OUT */
+        if (key->ep_out[0] != NULL) {
+            memcpy(key->ep_out[0], data, length);
+        }
+        canokey_emu_data_out(p->ep->nr, data);
+    }
+
+    canokey_emu_device_loop();
+
+    /* IN */
+    switch (key->ep_in_state[0]) {
+    case CANOKEY_EP_IN_WAIT:
+        p->status = USB_RET_NAK;
+        break;
+    case CANOKEY_EP_IN_STALL:
+        p->status = USB_RET_STALL;
+        break;
+    case CANOKEY_EP_IN_READY:
+        memcpy(data, key->ep_in[0], key->ep_in_size[0]);
+        p->actual_length = key->ep_in_size[0];
+        /* reset state */
+        key->ep_in_state[0] = CANOKEY_EP_IN_WAIT;
+        key->ep_in_size[0] = 0;
+        key->ep_in_pos[0] = 0;
+        break;
+    }
+}
+
+static void canokey_handle_data(USBDevice *dev, USBPacket *p)
+{
+    CanoKeyState *key = CANOKEY(dev);
+
+    uint8_t ep_in = CANOKEY_EP_IN(p->ep->nr);
+    uint8_t ep_out = p->ep->nr;
+    uint32_t in_len;
+    uint32_t out_pos;
+    uint32_t out_len;
+    switch (p->pid) {
+    case USB_TOKEN_OUT:
+        usb_packet_copy(p, key->ep_out_buffer[ep_out], p->iov.size);
+        out_pos = 0;
+        while (out_pos != p->iov.size) {
+            /*
+             * key->ep_out[ep_out] set by prepare_receive
+             * to be a buffer inside libcanokey-qemu.so
+             * key->ep_out_size[ep_out] set by prepare_receive
+             * to be the buffer length
+             */
+            out_len = MIN(p->iov.size - out_pos, key->ep_out_size[ep_out]);
+            memcpy(key->ep_out[ep_out],
+                    key->ep_out_buffer[ep_out] + out_pos, out_len);
+            out_pos += out_len;
+            /* update ep_out_size to actual len */
+            key->ep_out_size[ep_out] = out_len;
+            canokey_emu_data_out(ep_out, NULL);
+        }
+        break;
+    case USB_TOKEN_IN:
+        if (key->ep_in_pos[ep_in] == 0) { /* first time IN */
+            canokey_emu_data_in(ep_in);
+            canokey_emu_device_loop(); /* may call transmit multiple times */
+        }
+        switch (key->ep_in_state[ep_in]) {
+        case CANOKEY_EP_IN_WAIT:
+            /* NAK for early INTR IN */
+            p->status = USB_RET_NAK;
+            break;
+        case CANOKEY_EP_IN_STALL:
+            p->status = USB_RET_STALL;
+            break;
+        case CANOKEY_EP_IN_READY:
+            /* submit part of ep_in buffer to USBPacket */
+            in_len = MIN(key->ep_in_size[ep_in] - key->ep_in_pos[ep_in],
+                    p->iov.size);
+            usb_packet_copy(p,
+                    key->ep_in[ep_in] + key->ep_in_pos[ep_in], in_len);
+            key->ep_in_pos[ep_in] += in_len;
+            /* reset state if all data submitted */
+            if (key->ep_in_pos[ep_in] == key->ep_in_size[ep_in]) {
+                key->ep_in_state[ep_in] = CANOKEY_EP_IN_WAIT;
+                key->ep_in_size[ep_in] = 0;
+                key->ep_in_pos[ep_in] = 0;
+            }
+            break;
+        }
+        break;
+    default:
+        p->status = USB_RET_STALL;
+        break;
+    }
+}
+
+static void canokey_realize(USBDevice *base, Error **errp)
+{
+    CanoKeyState *key = CANOKEY(base);
+
+    if (key->file == NULL) {
+        error_setg(errp, "You must provide file=/path/to/canokey-file");
+        return;
+    }
+
+    usb_desc_init(base);
+
+    for (int i = 0; i != CANOKEY_EP_NUM; ++i) {
+        key->ep_in_state[i] = CANOKEY_EP_IN_WAIT;
+        key->ep_in_size[i] = 0;
+        key->ep_in_pos[i] = 0;
+    }
+
+    if (canokey_emu_init(key, key->file)) {
+        error_setg(errp, "canokey can not create or read %s", key->file);
+        return;
+    }
+}
+
+static void canokey_unrealize(USBDevice *base)
+{
+}
+
+static Property canokey_properties[] = {
+    DEFINE_PROP_STRING("file", CanoKeyState, file),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void canokey_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
+
+    uc->product_desc   = "CanoKey QEMU";
+    uc->usb_desc       = &desc_canokey;
+    uc->handle_reset   = canokey_handle_reset;
+    uc->handle_control = canokey_handle_control;
+    uc->handle_data    = canokey_handle_data;
+    uc->handle_attach  = usb_desc_attach;
+    uc->realize        = canokey_realize;
+    uc->unrealize      = canokey_unrealize;
+    dc->desc           = "CanoKey QEMU";
+    device_class_set_props(dc, canokey_properties);
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+}
+
+static const TypeInfo canokey_info = {
+    .name = TYPE_CANOKEY,
+    .parent = TYPE_USB_DEVICE,
+    .instance_size = sizeof(CanoKeyState),
+    .class_init = canokey_class_init
+};
+
+static void canokey_register_types(void)
+{
+    type_register_static(&canokey_info);
+}
+
+type_init(canokey_register_types)
diff --git a/hw/usb/canokey.h b/hw/usb/canokey.h
new file mode 100644
index 0000000000..24cf304203
--- /dev/null
+++ b/hw/usb/canokey.h
@@ -0,0 +1,69 @@
+/*
+ * CanoKey QEMU device header.
+ *
+ * Copyright (c) 2021-2022 Canokeys.org <contact@canokeys.org>
+ * Written by Hongren (Zenithal) Zheng <i@zenithal.me>
+ *
+ * This code is licensed under the Apache-2.0.
+ */
+
+#ifndef CANOKEY_H
+#define CANOKEY_H
+
+#include "hw/qdev-core.h"
+
+#define TYPE_CANOKEY "canokey"
+#define CANOKEY(obj) \
+    OBJECT_CHECK(CanoKeyState, (obj), TYPE_CANOKEY)
+
+/*
+ * State of Canokey (i.e. hw/canokey.c)
+ */
+
+/* CTRL INTR BULK */
+#define CANOKEY_EP_NUM 3
+/* BULK/INTR IN can be up to 1352 bytes, e.g. get key info */
+#define CANOKEY_EP_IN_BUFFER_SIZE 2048
+/* BULK OUT can be up to 270 bytes, e.g. PIV import cert */
+#define CANOKEY_EP_OUT_BUFFER_SIZE 512
+
+typedef enum {
+    CANOKEY_EP_IN_WAIT,
+    CANOKEY_EP_IN_READY,
+    CANOKEY_EP_IN_STALL
+} CanoKeyEPState;
+
+typedef struct CanoKeyState {
+    USBDevice dev;
+
+    /* IN packets from canokey device loop */
+    uint8_t ep_in[CANOKEY_EP_NUM][CANOKEY_EP_IN_BUFFER_SIZE];
+    /*
+     * See canokey_emu_transmit
+     *
+     * For large INTR IN, receive multiple data from canokey device loop
+     * in this case ep_in_size would increase with every call
+     */
+    uint32_t ep_in_size[CANOKEY_EP_NUM];
+    /*
+     * Used in canokey_handle_data
+     * for IN larger than p->iov.size, we would do multiple handle_data()
+     *
+     * The difference between ep_in_pos and ep_in_size:
+     * We first increase ep_in_size to fill ep_in buffer in device_loop,
+     * then use ep_in_pos to submit data from ep_in buffer in handle_data
+     */
+    uint32_t ep_in_pos[CANOKEY_EP_NUM];
+    CanoKeyEPState ep_in_state[CANOKEY_EP_NUM];
+
+    /* OUT pointer to canokey recv buffer */
+    uint8_t *ep_out[CANOKEY_EP_NUM];
+    uint32_t ep_out_size[CANOKEY_EP_NUM];
+    /* For large BULK OUT, multiple write to ep_out is needed */
+    uint8_t ep_out_buffer[CANOKEY_EP_NUM][CANOKEY_EP_OUT_BUFFER_SIZE];
+
+    /* Properties */
+    char *file; /* canokey-file */
+} CanoKeyState;
+
+#endif /* CANOKEY_H */
-- 
2.35.1



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

* [PATCH v5 2/6] hw/usb/canokey: Add trace events
  2022-05-19 12:35 [PATCH v5 0/6] Introduce CanoKey QEMU Hongren (Zenithal) Zheng
  2022-05-19 12:38 ` [PATCH v5 1/6] hw/usb: Add CanoKey Implementation Hongren (Zenithal) Zheng
@ 2022-05-19 12:38 ` Hongren (Zenithal) Zheng
  2022-05-19 12:38 ` [PATCH v5 3/6] meson: Add CanoKey Hongren (Zenithal) Zheng
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Hongren (Zenithal) Zheng @ 2022-05-19 12:38 UTC (permalink / raw)
  To: Gerd Hoffmann, Thomas Huth; +Cc: qemu-devel, contact

Signed-off-by: Hongren (Zenithal) Zheng <i@zenithal.me>
---
 hw/usb/canokey.c    | 13 +++++++++++++
 hw/usb/trace-events | 16 ++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/hw/usb/canokey.c b/hw/usb/canokey.c
index 6cb8b7cdb0..4a08b1cbd7 100644
--- a/hw/usb/canokey.c
+++ b/hw/usb/canokey.c
@@ -14,6 +14,7 @@
 #include "qapi/error.h"
 #include "hw/usb.h"
 #include "hw/qdev-properties.h"
+#include "trace.h"
 #include "desc.h"
 #include "canokey.h"
 
@@ -66,6 +67,7 @@ static const USBDesc desc_canokey = {
  */
 int canokey_emu_stall_ep(void *base, uint8_t ep)
 {
+    trace_canokey_emu_stall_ep(ep);
     CanoKeyState *key = base;
     uint8_t ep_in = CANOKEY_EP_IN(ep); /* INTR IN has ep 129 */
     key->ep_in_size[ep_in] = 0;
@@ -75,6 +77,7 @@ int canokey_emu_stall_ep(void *base, uint8_t ep)
 
 int canokey_emu_set_address(void *base, uint8_t addr)
 {
+    trace_canokey_emu_set_address(addr);
     CanoKeyState *key = base;
     key->dev.addr = addr;
     return 0;
@@ -83,6 +86,7 @@ int canokey_emu_set_address(void *base, uint8_t addr)
 int canokey_emu_prepare_receive(
         void *base, uint8_t ep, uint8_t *pbuf, uint16_t size)
 {
+    trace_canokey_emu_prepare_receive(ep, size);
     CanoKeyState *key = base;
     key->ep_out[ep] = pbuf;
     key->ep_out_size[ep] = size;
@@ -92,6 +96,7 @@ int canokey_emu_prepare_receive(
 int canokey_emu_transmit(
         void *base, uint8_t ep, const uint8_t *pbuf, uint16_t size)
 {
+    trace_canokey_emu_transmit(ep, size);
     CanoKeyState *key = base;
     uint8_t ep_in = CANOKEY_EP_IN(ep); /* INTR IN has ep 129 */
     memcpy(key->ep_in[ep_in] + key->ep_in_size[ep_in],
@@ -125,6 +130,7 @@ uint32_t canokey_emu_get_rx_data_size(void *base, uint8_t ep)
  */
 static void canokey_handle_reset(USBDevice *dev)
 {
+    trace_canokey_handle_reset();
     CanoKeyState *key = CANOKEY(dev);
     for (int i = 0; i != CANOKEY_EP_NUM; ++i) {
         key->ep_in_state[i] = CANOKEY_EP_IN_WAIT;
@@ -137,6 +143,7 @@ static void canokey_handle_reset(USBDevice *dev)
 static void canokey_handle_control(USBDevice *dev, USBPacket *p,
                int request, int value, int index, int length, uint8_t *data)
 {
+    trace_canokey_handle_control_setup(request, value, index, length);
     CanoKeyState *key = CANOKEY(dev);
 
     canokey_emu_setup(request, value, index, length);
@@ -144,6 +151,7 @@ static void canokey_handle_control(USBDevice *dev, USBPacket *p,
     uint32_t dir_in = request & DeviceRequest;
     if (!dir_in) {
         /* OUT */
+        trace_canokey_handle_control_out();
         if (key->ep_out[0] != NULL) {
             memcpy(key->ep_out[0], data, length);
         }
@@ -163,6 +171,7 @@ static void canokey_handle_control(USBDevice *dev, USBPacket *p,
     case CANOKEY_EP_IN_READY:
         memcpy(data, key->ep_in[0], key->ep_in_size[0]);
         p->actual_length = key->ep_in_size[0];
+        trace_canokey_handle_control_in(p->actual_length);
         /* reset state */
         key->ep_in_state[0] = CANOKEY_EP_IN_WAIT;
         key->ep_in_size[0] = 0;
@@ -182,6 +191,7 @@ static void canokey_handle_data(USBDevice *dev, USBPacket *p)
     uint32_t out_len;
     switch (p->pid) {
     case USB_TOKEN_OUT:
+        trace_canokey_handle_data_out(ep_out, p->iov.size);
         usb_packet_copy(p, key->ep_out_buffer[ep_out], p->iov.size);
         out_pos = 0;
         while (out_pos != p->iov.size) {
@@ -226,6 +236,7 @@ static void canokey_handle_data(USBDevice *dev, USBPacket *p)
                 key->ep_in_size[ep_in] = 0;
                 key->ep_in_pos[ep_in] = 0;
             }
+            trace_canokey_handle_data_in(ep_in, in_len);
             break;
         }
         break;
@@ -237,6 +248,7 @@ static void canokey_handle_data(USBDevice *dev, USBPacket *p)
 
 static void canokey_realize(USBDevice *base, Error **errp)
 {
+    trace_canokey_realize();
     CanoKeyState *key = CANOKEY(base);
 
     if (key->file == NULL) {
@@ -260,6 +272,7 @@ static void canokey_realize(USBDevice *base, Error **errp)
 
 static void canokey_unrealize(USBDevice *base)
 {
+    trace_canokey_unrealize();
 }
 
 static Property canokey_properties[] = {
diff --git a/hw/usb/trace-events b/hw/usb/trace-events
index 9773cb5330..914ca71668 100644
--- a/hw/usb/trace-events
+++ b/hw/usb/trace-events
@@ -345,3 +345,19 @@ usb_serial_set_baud(int bus, int addr, int baud) "dev %d:%u baud rate %d"
 usb_serial_set_data(int bus, int addr, int parity, int data, int stop) "dev %d:%u parity %c, data bits %d, stop bits %d"
 usb_serial_set_flow_control(int bus, int addr, int index) "dev %d:%u flow control %d"
 usb_serial_set_xonxoff(int bus, int addr, uint8_t xon, uint8_t xoff) "dev %d:%u xon 0x%x xoff 0x%x"
+
+# canokey.c
+canokey_emu_stall_ep(uint8_t ep) "ep %d"
+canokey_emu_set_address(uint8_t addr) "addr %d"
+canokey_emu_prepare_receive(uint8_t ep, uint16_t size) "ep %d size %d"
+canokey_emu_transmit(uint8_t ep, uint16_t size) "ep %d size %d"
+canokey_thread_start(void)
+canokey_thread_stop(void)
+canokey_handle_reset(void)
+canokey_handle_control_setup(int request, int value, int index, int length) "request 0x%04X value 0x%04X index 0x%04X length 0x%04X"
+canokey_handle_control_out(void)
+canokey_handle_control_in(int actual_len) "len %d"
+canokey_handle_data_out(uint8_t ep_out, uint32_t out_len) "ep %d len %d"
+canokey_handle_data_in(uint8_t ep_in, uint32_t in_len) "ep %d len %d"
+canokey_realize(void)
+canokey_unrealize(void)
-- 
2.35.1



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

* [PATCH v5 3/6] meson: Add CanoKey
  2022-05-19 12:35 [PATCH v5 0/6] Introduce CanoKey QEMU Hongren (Zenithal) Zheng
  2022-05-19 12:38 ` [PATCH v5 1/6] hw/usb: Add CanoKey Implementation Hongren (Zenithal) Zheng
  2022-05-19 12:38 ` [PATCH v5 2/6] hw/usb/canokey: Add trace events Hongren (Zenithal) Zheng
@ 2022-05-19 12:38 ` Hongren (Zenithal) Zheng
  2022-05-19 12:39 ` [PATCH v5 4/6] docs: Add CanoKey documentation Hongren (Zenithal) Zheng
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Hongren (Zenithal) Zheng @ 2022-05-19 12:38 UTC (permalink / raw)
  To: Gerd Hoffmann, Thomas Huth; +Cc: qemu-devel, contact

Signed-off-by: Hongren (Zenithal) Zheng <i@zenithal.me>
---
 hw/usb/Kconfig                | 5 +++++
 hw/usb/meson.build            | 5 +++++
 meson.build                   | 6 ++++++
 meson_options.txt             | 2 ++
 scripts/meson-buildoptions.sh | 3 +++
 5 files changed, 21 insertions(+)

diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index 53f8283ffd..ce4f433976 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -119,6 +119,11 @@ config USB_U2F
     default y
     depends on USB
 
+config USB_CANOKEY
+    bool
+    default y
+    depends on USB
+
 config IMX_USBPHY
     bool
     default y
diff --git a/hw/usb/meson.build b/hw/usb/meson.build
index de853d780d..793df42e21 100644
--- a/hw/usb/meson.build
+++ b/hw/usb/meson.build
@@ -63,6 +63,11 @@ if u2f.found()
   softmmu_ss.add(when: 'CONFIG_USB_U2F', if_true: [u2f, files('u2f-emulated.c')])
 endif
 
+# CanoKey
+if canokey.found()
+  softmmu_ss.add(when: 'CONFIG_USB_CANOKEY', if_true: [canokey, files('canokey.c')])
+endif
+
 # usb redirect
 if usbredir.found()
   usbredir_ss = ss.source_set()
diff --git a/meson.build b/meson.build
index 53a4728250..7f90e49ee6 100644
--- a/meson.build
+++ b/meson.build
@@ -1383,6 +1383,12 @@ if have_system
                    method: 'pkg-config',
                    kwargs: static_kwargs)
 endif
+canokey = not_found
+if have_system
+  canokey = dependency('canokey-qemu', required: get_option('canokey'),
+                   method: 'pkg-config',
+                   kwargs: static_kwargs)
+endif
 usbredir = not_found
 if not get_option('usb_redir').auto() or have_system
   usbredir = dependency('libusbredirparser-0.5', required: get_option('usb_redir'),
diff --git a/meson_options.txt b/meson_options.txt
index 29c6b90cec..b41066aa11 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -189,6 +189,8 @@ option('spice_protocol', type : 'feature', value : 'auto',
        description: 'Spice protocol support')
 option('u2f', type : 'feature', value : 'auto',
        description: 'U2F emulation support')
+option('canokey', type : 'feature', value : 'auto',
+       description: 'CanoKey support')
 option('usb_redir', type : 'feature', value : 'auto',
        description: 'libusbredir support')
 option('l2tpv3', type : 'feature', value : 'auto',
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index 5d2172bfb4..af2e7bc8e5 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -76,6 +76,7 @@ meson_options_help() {
   printf "%s\n" '  bpf             eBPF support'
   printf "%s\n" '  brlapi          brlapi character device driver'
   printf "%s\n" '  bzip2           bzip2 support for DMG images'
+  printf "%s\n" '  canokey         CanoKey support'
   printf "%s\n" '  cap-ng          cap_ng support'
   printf "%s\n" '  cloop           cloop image format support'
   printf "%s\n" '  cocoa           Cocoa user interface (macOS only)'
@@ -205,6 +206,8 @@ _meson_option_parse() {
     --disable-brlapi) printf "%s" -Dbrlapi=disabled ;;
     --enable-bzip2) printf "%s" -Dbzip2=enabled ;;
     --disable-bzip2) printf "%s" -Dbzip2=disabled ;;
+    --enable-canokey) printf "%s" -Dcanokey=enabled ;;
+    --disable-canokey) printf "%s" -Dcanokey=disabled ;;
     --enable-cap-ng) printf "%s" -Dcap_ng=enabled ;;
     --disable-cap-ng) printf "%s" -Dcap_ng=disabled ;;
     --enable-capstone) printf "%s" -Dcapstone=enabled ;;
-- 
2.35.1



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

* [PATCH v5 4/6] docs: Add CanoKey documentation
  2022-05-19 12:35 [PATCH v5 0/6] Introduce CanoKey QEMU Hongren (Zenithal) Zheng
                   ` (2 preceding siblings ...)
  2022-05-19 12:38 ` [PATCH v5 3/6] meson: Add CanoKey Hongren (Zenithal) Zheng
@ 2022-05-19 12:39 ` Hongren (Zenithal) Zheng
  2022-06-09  9:56   ` Gerd Hoffmann
  2022-05-19 12:40 ` [PATCH v5 5/6] docs/system/devices/usb: Add CanoKey to USB devices examples Hongren (Zenithal) Zheng
  2022-05-19 12:40 ` [PATCH v5 6/6] MAINTAINERS: add myself as CanoKey maintainer Hongren (Zenithal) Zheng
  5 siblings, 1 reply; 8+ messages in thread
From: Hongren (Zenithal) Zheng @ 2022-05-19 12:39 UTC (permalink / raw)
  To: Gerd Hoffmann, Thomas Huth; +Cc: qemu-devel, contact

Signed-off-by: Hongren (Zenithal) Zheng <i@zenithal.me>
---
 docs/system/device-emulation.rst |   1 +
 docs/system/devices/canokey.rst  | 168 +++++++++++++++++++++++++++++++
 2 files changed, 169 insertions(+)
 create mode 100644 docs/system/devices/canokey.rst

Note on the qemu-xhci issue:

For FIDO2 packets, they follow the pattern below

  Interrupt IN (size 64)
  Interrupt OUT (size 128 with payload)
  Interrupt OUT ACK (size 64)
  Interrupt IN ACK (size 128 with payload)

This can be captured by `fido2-token -I /dev/hidraw0` or
`systemd-cryptenroll --fido2-device=auto`. I doubt this
is a feature of libfido2 that it sends IN before OUT.
I've been coping with this pattern for a long miserable
time, you can see the url below for a example
https://github.com/canokeys/canokey-usbip/blob/e9db44f1aa7f46c7dd7266a682d778921edbeb49/Src/usbip.c#L33-L36

In qemu-xhci, it assumes a pattern like this

  Interrupt IN (size 64)
   -> usb_handle_packet
  Interrupt IN ACK (size 128 with payload (not possible))
   <- usb_handle_packet returns
  Interrupt OUT (size 128 with payload)
   -> the next usb_handle_packet
  Interrupt OUT ACK (size 64)
   <- the next usb_handle_packet returns

However, for device, it can not ack the IN token if the payload
in OUT token has not been passed.

Also, we can not have a trace like this because we
are single threaded (not async) and the next usb_handle_packet
must be called after the return of the first usb-handle_packet
(emulating async interrupt ep with sync thread is painful)

  Interrupt IN (size 64)
   -> usb_handle_packet
  Interrupt OUT (size 128 with payload)
   -> the next usb_handle_packet (not possible)
  Interrupt OUT ACK (size 64)
   <- the next usb_handle_packet returns
  Interrupt IN ACK (size 128 with payload)
   <- the first usb_handle_packet returns

The code works for uhci/ehci in the following way

  Interrupt IN (size 64)
   -> usb_handle_packet
  Interrupt IN NAK (size 64)
   <- usb_handle_packet returns
  ... there are many IN NAK here
  ... uhci/ehci reschedule OUT before IN now
  Interrupt OUT (size 128 with payload)
   -> the next usb_handle_packet
  Interrupt OUT ACK (size 64)
   <- the next usb_handle_packet returns
  Interrupt IN (size 64)
   -> last usb_handle_packet
  Interrupt IN ACK (size 128 with payload)
   <- last usb_handle_packet returns

I think qemu-xhci should retry/schedule the failed IN token after
receiving NAK instead of failing immediately, because interrupt
endpoint is async. However, I'm not so familiar with qemu-xhci
and I tried to debug it but in vain. The debug trace only showed
that after the first IN NAK it retried and then use
`xhci_ep_nuke_xfers` to cancel the IN packet. After
handling the OUT packet, the IN packet is not retried and
the whole QEMU stuck there.

  Interrupt IN (size 64)
   -> usb_handle_packet
  Interrupt IN NAK (size 64)
   <- usb_handle_packet returns
    ... nuke_xfers
  Interrupt IN (size 64)
   -> usb_handle_packet
  Interrupt IN NAK (size 64)
   <- usb_handle_packet returns
  Interrupt OUT (size 128 with payload)
   -> the next usb_handle_packet
  Interrupt OUT ACK (size 64)
   <- the next usb_handle_packet returns
    ... xhci_try_complete_packet
    ... the whole QEMU stuck here as there is no more IN token scheduled

You can see that in canokey_handle_data of hw/usb/canokey.c
I NAKed for early INTR IN, this is what u2f_key_handle_data
also does so I think the bug is not in canokey.c
Also, canokey.c works with uhci/ehci controllers


diff --git a/docs/system/device-emulation.rst b/docs/system/device-emulation.rst
index 3b729b920d..0506006056 100644
--- a/docs/system/device-emulation.rst
+++ b/docs/system/device-emulation.rst
@@ -92,3 +92,4 @@ Emulated Devices
    devices/vhost-user.rst
    devices/virtio-pmem.rst
    devices/vhost-user-rng.rst
+   devices/canokey.rst
diff --git a/docs/system/devices/canokey.rst b/docs/system/devices/canokey.rst
new file mode 100644
index 0000000000..169f99b8eb
--- /dev/null
+++ b/docs/system/devices/canokey.rst
@@ -0,0 +1,168 @@
+.. _canokey:
+
+CanoKey QEMU
+------------
+
+CanoKey [1]_ is an open-source secure key with supports of
+
+* U2F / FIDO2 with Ed25519 and HMAC-secret
+* OpenPGP Card V3.4 with RSA4096, Ed25519 and more [2]_
+* PIV (NIST SP 800-73-4)
+* HOTP / TOTP
+* NDEF
+
+All these platform-independent features are in canokey-core [3]_.
+
+For different platforms, CanoKey has different implementations,
+including both hardware implementions and virtual cards:
+
+* CanoKey STM32 [4]_
+* CanoKey Pigeon [5]_
+* (virt-card) CanoKey USB/IP
+* (virt-card) CanoKey FunctionFS
+
+In QEMU, yet another CanoKey virt-card is implemented.
+CanoKey QEMU exposes itself as a USB device to the guest OS.
+
+With the same software configuration as a hardware key,
+the guest OS can use all the functionalities of a secure key as if
+there was actually an hardware key plugged in.
+
+CanoKey QEMU provides much convenience for debuging:
+
+* libcanokey-qemu supports debuging output thus developers can
+  inspect what happens inside a secure key
+* CanoKey QEMU supports trace event thus event
+* QEMU USB stack supports pcap thus USB packet between the guest
+  and key can be captured and analysed
+
+Then for developers:
+
+* For developers on software with secure key support (e.g. FIDO2, OpenPGP),
+  they can see what happens inside the secure key
+* For secure key developers, USB packets between guest OS and CanoKey
+  can be easily captured and analysed
+
+Also since this is a virtual card, it can be easily used in CI for testing
+on code coping with secure key.
+
+Building
+========
+
+libcanokey-qemu is required to use CanoKey QEMU.
+
+.. code-block:: shell
+
+    git clone https://github.com/canokeys/canokey-qemu
+    mkdir canokey-qemu/build
+    pushd canokey-qemu/build
+
+If you want to install libcanokey-qemu in a different place,
+add ``-DCMAKE_INSTALL_PREFIX=/path/to/your/place`` to cmake below.
+
+.. code-block:: shell
+
+    cmake ..
+    make
+    make install # may need sudo
+    popd
+
+Then configuring and building:
+
+.. code-block:: shell
+
+    # depending on your env, lib/pkgconfig can be lib64/pkgconfig
+    export PKG_CONFIG_PATH=/path/to/your/place/lib/pkgconfig:$PKG_CONFIG_PATH
+    ./configure --enable-canokey && make
+
+Using CanoKey QEMU
+==================
+
+CanoKey QEMU stores all its data on a file of the host specified by the argument
+when invoking qemu.
+
+.. parsed-literal::
+
+    |qemu_system| -usb -device canokey,file=$HOME/.canokey-file
+
+Note: you should keep this file carefully as it may contain your private key!
+
+The first time when the file is used, it is created and initialized by CanoKey,
+afterwards CanoKey QEMU would just read this file.
+
+After the guest OS boots, you can check that there is a USB device.
+
+For example, If the guest OS is an Linux machine. You may invoke lsusb
+and find CanoKey QEMU there:
+
+.. code-block:: shell
+
+    $ lsusb
+    Bus 001 Device 002: ID 20a0:42d4 Clay Logic CanoKey QEMU
+
+You may setup the key as guided in [6]_. The console for the key is at [7]_.
+
+Debuging
+========
+
+CanoKey QEMU consists of two parts, ``libcanokey-qemu.so`` and ``canokey.c``,
+the latter of which resides in QEMU. The former provides core functionality
+of a secure key while the latter provides platform-dependent functions:
+USB packet handling.
+
+If you want to trace what happens inside the secure key, when compiling
+libcanokey-qemu, you should add ``-DQEMU_DEBUG_OUTPUT=ON`` in cmake command
+line:
+
+.. code-block:: shell
+
+    cmake .. -DQEMU_DEBUG_OUTPUT=ON
+
+If you want to trace events happened in canokey.c, use
+
+.. parsed-literal::
+
+    |qemu_system| --trace "canokey_*" \\
+        -usb -device canokey,file=$HOME/.canokey-file
+
+If you want to capture USB packets between the guest and the host, you can:
+
+.. parsed-literal::
+
+    |qemu_system| -usb -device canokey,file=$HOME/.canokey-file,pcap=key.pcap
+
+Limitations
+===========
+
+Currently libcanokey-qemu.so has dozens of global variables as it was originally
+designed for embedded systems. Thus one qemu instance can not have
+multiple CanoKey QEMU running, namely you can not
+
+.. parsed-literal::
+
+    |qemu_system| -usb -device canokey,file=$HOME/.canokey-file \\
+         -device canokey,file=$HOME/.canokey-file2
+
+Also, there is no lock on canokey-file, thus two CanoKey QEMU instance
+can not read one canokey-file at the same time.
+
+Another limitation is that this device is not compatible with ``qemu-xhci``,
+in that this device would hang when there are FIDO2 packets (traffic on
+interrupt endpoints). If you do not use FIDO2 then it works as intended,
+but for full functionality you should use old uhci/ehci bus and attach canokey
+to it, for example
+
+.. parsed-literal::
+
+   |qemu_system| -device piix3-usb-uhci,id=uhci -device canokey,bus=uhci.0
+
+References
+==========
+
+.. [1] `<https://canokeys.org>`_
+.. [2] `<https://docs.canokeys.org/userguide/openpgp/#supported-algorithm>`_
+.. [3] `<https://github.com/canokeys/canokey-core>`_
+.. [4] `<https://github.com/canokeys/canokey-stm32>`_
+.. [5] `<https://github.com/canokeys/canokey-pigeon>`_
+.. [6] `<https://docs.canokeys.org/>`_
+.. [7] `<https://console.canokeys.org/>`_
-- 
2.35.1



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

* [PATCH v5 5/6] docs/system/devices/usb: Add CanoKey to USB devices examples
  2022-05-19 12:35 [PATCH v5 0/6] Introduce CanoKey QEMU Hongren (Zenithal) Zheng
                   ` (3 preceding siblings ...)
  2022-05-19 12:39 ` [PATCH v5 4/6] docs: Add CanoKey documentation Hongren (Zenithal) Zheng
@ 2022-05-19 12:40 ` Hongren (Zenithal) Zheng
  2022-05-19 12:40 ` [PATCH v5 6/6] MAINTAINERS: add myself as CanoKey maintainer Hongren (Zenithal) Zheng
  5 siblings, 0 replies; 8+ messages in thread
From: Hongren (Zenithal) Zheng @ 2022-05-19 12:40 UTC (permalink / raw)
  To: Gerd Hoffmann, Thomas Huth; +Cc: qemu-devel, contact

Signed-off-by: Hongren (Zenithal) Zheng <i@zenithal.me>
---
 docs/system/devices/usb.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/docs/system/devices/usb.rst b/docs/system/devices/usb.rst
index afb7d6c226..872d916758 100644
--- a/docs/system/devices/usb.rst
+++ b/docs/system/devices/usb.rst
@@ -199,6 +199,10 @@ option or the ``device_add`` monitor command. Available devices are:
 ``u2f-{emulated,passthru}``
    Universal Second Factor device
 
+``canokey``
+   An Open-source Secure Key implementing FIDO2, OpenPGP, PIV and more.
+   For more information, see :ref:`canokey`.
+
 Physical port addressing
 ^^^^^^^^^^^^^^^^^^^^^^^^
 
-- 
2.35.1



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

* [PATCH v5 6/6] MAINTAINERS: add myself as CanoKey maintainer
  2022-05-19 12:35 [PATCH v5 0/6] Introduce CanoKey QEMU Hongren (Zenithal) Zheng
                   ` (4 preceding siblings ...)
  2022-05-19 12:40 ` [PATCH v5 5/6] docs/system/devices/usb: Add CanoKey to USB devices examples Hongren (Zenithal) Zheng
@ 2022-05-19 12:40 ` Hongren (Zenithal) Zheng
  5 siblings, 0 replies; 8+ messages in thread
From: Hongren (Zenithal) Zheng @ 2022-05-19 12:40 UTC (permalink / raw)
  To: Gerd Hoffmann, Thomas Huth; +Cc: qemu-devel, contact

Signed-off-by: Hongren (Zenithal) Zheng <i@zenithal.me>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index dff0200f70..03856c558f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2397,6 +2397,14 @@ F: hw/intc/s390_flic*.c
 F: include/hw/s390x/s390_flic.h
 L: qemu-s390x@nongnu.org
 
+CanoKey
+M: Hongren (Zenithal) Zheng <i@zenithal.me>
+S: Maintained
+R: Canokeys.org <contact@canokeys.org>
+F: hw/usb/canokey.c
+F: hw/usb/canokey.h
+F: docs/system/devices/canokey.rst
+
 Subsystems
 ----------
 Overall Audio backends
-- 
2.35.1



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

* Re: [PATCH v5 4/6] docs: Add CanoKey documentation
  2022-05-19 12:39 ` [PATCH v5 4/6] docs: Add CanoKey documentation Hongren (Zenithal) Zheng
@ 2022-06-09  9:56   ` Gerd Hoffmann
  0 siblings, 0 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2022-06-09  9:56 UTC (permalink / raw)
  To: Hongren (Zenithal) Zheng; +Cc: Thomas Huth, qemu-devel, contact

On Thu, May 19, 2022 at 08:39:38PM +0800, Hongren (Zenithal) Zheng wrote:
> Signed-off-by: Hongren (Zenithal) Zheng <i@zenithal.me>
> ---
>  docs/system/device-emulation.rst |   1 +
>  docs/system/devices/canokey.rst  | 168 +++++++++++++++++++++++++++++++
>  2 files changed, 169 insertions(+)
>  create mode 100644 docs/system/devices/canokey.rst
> 
> Note on the qemu-xhci issue:
> 
> For FIDO2 packets, they follow the pattern below
> 
>   Interrupt IN (size 64)
>   Interrupt OUT (size 128 with payload)
>   Interrupt OUT ACK (size 64)
>   Interrupt IN ACK (size 128 with payload)

> In qemu-xhci, it assumes a pattern like this
> 
>   Interrupt IN (size 64)
>    -> usb_handle_packet
>   Interrupt IN ACK (size 128 with payload (not possible))
>    <- usb_handle_packet returns
>   Interrupt OUT (size 128 with payload)
>    -> the next usb_handle_packet
>   Interrupt OUT ACK (size 64)
>    <- the next usb_handle_packet returns

> The code works for uhci/ehci in the following way
> 
>   Interrupt IN (size 64)
>    -> usb_handle_packet
>   Interrupt IN NAK (size 64)
>    <- usb_handle_packet returns
>   ... there are many IN NAK here
>   ... uhci/ehci reschedule OUT before IN now
>   Interrupt OUT (size 128 with payload)
>    -> the next usb_handle_packet
>   Interrupt OUT ACK (size 64)
>    <- the next usb_handle_packet returns
>   Interrupt IN (size 64)
>    -> last usb_handle_packet
>   Interrupt IN ACK (size 128 with payload)
>    <- last usb_handle_packet returns

I think this is just a missing usb_wakeup() call somewhere.  If a
usb device got data it must notify the host adapter that way.

> I think qemu-xhci should retry/schedule the failed IN token after
> receiving NAK instead of failing immediately, because interrupt
> endpoint is async.

uhci/ehci keeps polling the device.  That is pretty much mandatory for
correct emulation due to the way the host adapter hardware is designed.
So things are typically working even without an explicit usb_wakeup()
call.

xhci doesn't poll (which is good because that reduces virtualization
overhead alot) but requires an explicit usb_wakeup() call to make xhci
re-try NACK-ed transfers.

take care,
  Gerd



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

end of thread, other threads:[~2022-06-09 12:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-19 12:35 [PATCH v5 0/6] Introduce CanoKey QEMU Hongren (Zenithal) Zheng
2022-05-19 12:38 ` [PATCH v5 1/6] hw/usb: Add CanoKey Implementation Hongren (Zenithal) Zheng
2022-05-19 12:38 ` [PATCH v5 2/6] hw/usb/canokey: Add trace events Hongren (Zenithal) Zheng
2022-05-19 12:38 ` [PATCH v5 3/6] meson: Add CanoKey Hongren (Zenithal) Zheng
2022-05-19 12:39 ` [PATCH v5 4/6] docs: Add CanoKey documentation Hongren (Zenithal) Zheng
2022-06-09  9:56   ` Gerd Hoffmann
2022-05-19 12:40 ` [PATCH v5 5/6] docs/system/devices/usb: Add CanoKey to USB devices examples Hongren (Zenithal) Zheng
2022-05-19 12:40 ` [PATCH v5 6/6] MAINTAINERS: add myself as CanoKey maintainer Hongren (Zenithal) Zheng

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.