* [PATCH 0/3] CanoKey: Fix xHCI compatibility and CCID ZLP
@ 2022-06-12 18:06 Hongren (Zenithal) Zheng
2022-06-12 18:09 ` [PATCH 1/3] hw/usb/canokey: Fix " Hongren (Zenithal) Zheng
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Hongren (Zenithal) Zheng @ 2022-06-12 18:06 UTC (permalink / raw
To: Gerd Hoffmann; +Cc: qemu-devel, contact, MkfsSion
In patch v5 [1] of Introduce CanoKey QEMU I said that canokey-qemu
was incompatible with qemu-xhci.
kraxel kindly suggested[2] that it should be the problem of usb_wakeup
So I fixed it in this patch set.
Now that the v5 patch has been in the process of git PULL [3],
I think it would be better to post a new patch set instead
of sending out v6, which would make maintainer's tree back and forth.
This patch should be applied after [1].
As for the CCID ZLP issue, it is described in the comment and commit
message.
I added a commit in https://github.com/canokeys/canokey-qemu
to export the EP num in the header, so hw/usb/canokey.c in qemu
could use it for CTAPHID quirks. If you want to compile
this version when --enable-canokey, make sure to use the latest
libcanokey-qemu.so
The CI result for this PATCH is at [4]. The failure for
amd64-debian-container seems irrelevent to this patchset.
[1] https://lore.kernel.org/qemu-devel/YoY5k0PQny8WtAHi@Sun/
[2] https://lore.kernel.org/qemu-devel/20220609095659.ulgk64bx3nlqzs2k@sirius.home.kraxel.org/
[3] https://lore.kernel.org/qemu-devel/20220610092043.1874654-1-kraxel@redhat.com/
[4] https://gitlab.com/ZenithalHourlyRate/qemu/-/pipelines/561801062
Hongren (Zenithal) Zheng (3):
hw/usb/canokey: Fix CCID ZLP
hw/usb/canokey: fix compatibility of qemu-xhci
docs/system/devices/usb/canokey: remove limitations on qemu-xhci
docs/system/devices/canokey.rst | 10 ----------
hw/usb/canokey.c | 35 +++++++++++++++++++++++++++++----
hw/usb/canokey.h | 1 +
3 files changed, 32 insertions(+), 14 deletions(-)
--
2.35.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/3] hw/usb/canokey: Fix CCID ZLP
2022-06-12 18:06 [PATCH 0/3] CanoKey: Fix xHCI compatibility and CCID ZLP Hongren (Zenithal) Zheng
@ 2022-06-12 18:09 ` Hongren (Zenithal) Zheng
2022-06-12 18:10 ` [PATCH 2/3] hw/usb/canokey: fix compatibility of qemu-xhci Hongren (Zenithal) Zheng
2022-06-12 18:10 ` [PATCH 3/3] docs/system/devices/usb/canokey: remove limitations on qemu-xhci Hongren (Zenithal) Zheng
2 siblings, 0 replies; 5+ messages in thread
From: Hongren (Zenithal) Zheng @ 2022-06-12 18:09 UTC (permalink / raw
To: Gerd Hoffmann; +Cc: qemu-devel, contact, MkfsSion
CCID could send zero-length packet (ZLP)
if we invoke two data_in, two packets would be concated
and we could not distinguish them.
The CANOKEY_EMU_EP_CTAPHID is exported from canokey-qemu.h
Reported-by: MkfsSion <myychina28759@gmail.com>
Signed-off-by: Hongren (Zenithal) Zheng <i@zenithal.me>
---
hw/usb/canokey.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/hw/usb/canokey.c b/hw/usb/canokey.c
index 4a08b1cbd7..86548923eb 100644
--- a/hw/usb/canokey.c
+++ b/hw/usb/canokey.c
@@ -109,11 +109,10 @@ int canokey_emu_transmit(
* 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
+ * This has side effect for CCID since CCID can send ZLP
+ * This also has side effect for Control transfer
*/
- if (ep_in != 0) {
+ if (ep_in == CANOKEY_EMU_EP_CTAPHID) {
canokey_emu_data_in(ep_in);
}
return 0;
--
2.35.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] hw/usb/canokey: fix compatibility of qemu-xhci
2022-06-12 18:06 [PATCH 0/3] CanoKey: Fix xHCI compatibility and CCID ZLP Hongren (Zenithal) Zheng
2022-06-12 18:09 ` [PATCH 1/3] hw/usb/canokey: Fix " Hongren (Zenithal) Zheng
@ 2022-06-12 18:10 ` Hongren (Zenithal) Zheng
2022-06-13 10:31 ` Gerd Hoffmann
2022-06-12 18:10 ` [PATCH 3/3] docs/system/devices/usb/canokey: remove limitations on qemu-xhci Hongren (Zenithal) Zheng
2 siblings, 1 reply; 5+ messages in thread
From: Hongren (Zenithal) Zheng @ 2022-06-12 18:10 UTC (permalink / raw
To: Gerd Hoffmann; +Cc: qemu-devel, contact, MkfsSion
XHCI wont poll interrupt IN endpoint if NAKed, and needs wakeup
Suggested-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Hongren (Zenithal) Zheng <i@zenithal.me>
---
hw/usb/canokey.c | 28 ++++++++++++++++++++++++++++
hw/usb/canokey.h | 1 +
2 files changed, 29 insertions(+)
diff --git a/hw/usb/canokey.c b/hw/usb/canokey.c
index 86548923eb..e5fa4a5ad2 100644
--- a/hw/usb/canokey.c
+++ b/hw/usb/canokey.c
@@ -103,6 +103,13 @@ int canokey_emu_transmit(
pbuf, size);
key->ep_in_size[ep_in] += size;
key->ep_in_state[ep_in] = CANOKEY_EP_IN_READY;
+ /*
+ * wake up controller if we NAKed IN token before
+ * Note: this is a quirk for CanoKey CTAPHID
+ */
+ if (ep_in == CANOKEY_EMU_EP_CTAPHID &&
+ key->ep_in_pointer[ep_in] != NULL)
+ usb_wakeup(key->ep_in_pointer[ep_in], 0);
/*
* ready for more data in device loop
*
@@ -135,6 +142,7 @@ static void canokey_handle_reset(USBDevice *dev)
key->ep_in_state[i] = CANOKEY_EP_IN_WAIT;
key->ep_in_pos[i] = 0;
key->ep_in_size[i] = 0;
+ key->ep_in_pointer[i] = NULL;
}
canokey_emu_reset();
}
@@ -163,6 +171,8 @@ static void canokey_handle_control(USBDevice *dev, USBPacket *p,
switch (key->ep_in_state[0]) {
case CANOKEY_EP_IN_WAIT:
p->status = USB_RET_NAK;
+ /* store pointer here for later emu_transmit wakeup */
+ key->ep_in_pointer[0] = p->ep;
break;
case CANOKEY_EP_IN_STALL:
p->status = USB_RET_STALL;
@@ -208,6 +218,22 @@ static void canokey_handle_data(USBDevice *dev, USBPacket *p)
key->ep_out_size[ep_out] = out_len;
canokey_emu_data_out(ep_out, NULL);
}
+ /*
+ * Note: this is a quirk for CanoKey CTAPHID
+ *
+ * There is one code path that uses this device loop
+ * INTR IN -> useful data_in and useless device_loop -> NAKed
+ * INTR OUT -> useful device loop -> transmit -> wakeup
+ * (this one thanks to both data_in and data_out being called)
+ * the next INTR IN -> actual data to guest
+ *
+ * if there is no such device loop, there would be no further
+ * INTR IN, no device loop, no transmit hence no usb_wakeup
+ * then qemu would hang
+ */
+ if (ep_in == CANOKEY_EMU_EP_CTAPHID) {
+ canokey_emu_device_loop(); /* may call transmit multiple times */
+ }
break;
case USB_TOKEN_IN:
if (key->ep_in_pos[ep_in] == 0) { /* first time IN */
@@ -218,6 +244,8 @@ static void canokey_handle_data(USBDevice *dev, USBPacket *p)
case CANOKEY_EP_IN_WAIT:
/* NAK for early INTR IN */
p->status = USB_RET_NAK;
+ /* store pointer here for later emu_transmit wakeup */
+ key->ep_in_pointer[ep_in] = p->ep;
break;
case CANOKEY_EP_IN_STALL:
p->status = USB_RET_STALL;
diff --git a/hw/usb/canokey.h b/hw/usb/canokey.h
index 24cf304203..7261f81e80 100644
--- a/hw/usb/canokey.h
+++ b/hw/usb/canokey.h
@@ -55,6 +55,7 @@ typedef struct CanoKeyState {
*/
uint32_t ep_in_pos[CANOKEY_EP_NUM];
CanoKeyEPState ep_in_state[CANOKEY_EP_NUM];
+ USBEndpoint *ep_in_pointer[CANOKEY_EP_NUM];
/* OUT pointer to canokey recv buffer */
uint8_t *ep_out[CANOKEY_EP_NUM];
--
2.35.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/3] docs/system/devices/usb/canokey: remove limitations on qemu-xhci
2022-06-12 18:06 [PATCH 0/3] CanoKey: Fix xHCI compatibility and CCID ZLP Hongren (Zenithal) Zheng
2022-06-12 18:09 ` [PATCH 1/3] hw/usb/canokey: Fix " Hongren (Zenithal) Zheng
2022-06-12 18:10 ` [PATCH 2/3] hw/usb/canokey: fix compatibility of qemu-xhci Hongren (Zenithal) Zheng
@ 2022-06-12 18:10 ` Hongren (Zenithal) Zheng
2 siblings, 0 replies; 5+ messages in thread
From: Hongren (Zenithal) Zheng @ 2022-06-12 18:10 UTC (permalink / raw
To: Gerd Hoffmann; +Cc: qemu-devel, contact, MkfsSion
Signed-off-by: Hongren (Zenithal) Zheng <i@zenithal.me>
---
docs/system/devices/canokey.rst | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/docs/system/devices/canokey.rst b/docs/system/devices/canokey.rst
index 169f99b8eb..c2c58ae3e7 100644
--- a/docs/system/devices/canokey.rst
+++ b/docs/system/devices/canokey.rst
@@ -146,16 +146,6 @@ multiple CanoKey QEMU running, namely you can not
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
==========
--
2.35.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/3] hw/usb/canokey: fix compatibility of qemu-xhci
2022-06-12 18:10 ` [PATCH 2/3] hw/usb/canokey: fix compatibility of qemu-xhci Hongren (Zenithal) Zheng
@ 2022-06-13 10:31 ` Gerd Hoffmann
0 siblings, 0 replies; 5+ messages in thread
From: Gerd Hoffmann @ 2022-06-13 10:31 UTC (permalink / raw
To: Hongren (Zenithal) Zheng; +Cc: qemu-devel, contact, MkfsSion
Hi,
> case CANOKEY_EP_IN_WAIT:
> /* NAK for early INTR IN */
> p->status = USB_RET_NAK;
> + /* store pointer here for later emu_transmit wakeup */
> + key->ep_in_pointer[ep_in] = p->ep;
There is no need to fish the ep pointer out of usb packets.
You can just use usb_ep_get() instead.
take care,
Gerd
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-06-13 10:34 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-06-12 18:06 [PATCH 0/3] CanoKey: Fix xHCI compatibility and CCID ZLP Hongren (Zenithal) Zheng
2022-06-12 18:09 ` [PATCH 1/3] hw/usb/canokey: Fix " Hongren (Zenithal) Zheng
2022-06-12 18:10 ` [PATCH 2/3] hw/usb/canokey: fix compatibility of qemu-xhci Hongren (Zenithal) Zheng
2022-06-13 10:31 ` Gerd Hoffmann
2022-06-12 18:10 ` [PATCH 3/3] docs/system/devices/usb/canokey: remove limitations on qemu-xhci 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.