* [PATCH v2 0/3] CanoKey: Fix xHCI compatibility and CCID ZLP
@ 2022-06-13 12:12 Hongren (Zenithal) Zheng
2022-06-13 12:14 ` [PATCH v2 1/3] hw/usb/canokey: Fix " Hongren (Zenithal) Zheng
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Hongren (Zenithal) Zheng @ 2022-06-13 12:12 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
---
v2:
* use usb_ep_get instead of recording ep_in_pointer
as suggested by kraxel
* CI result for v2 is at
https://gitlab.com/ZenithalHourlyRate/qemu/-/pipelines/562306905
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 | 30 ++++++++++++++++++++++++++----
2 files changed, 26 insertions(+), 14 deletions(-)
--
2.35.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/3] hw/usb/canokey: Fix CCID ZLP
2022-06-13 12:12 [PATCH v2 0/3] CanoKey: Fix xHCI compatibility and CCID ZLP Hongren (Zenithal) Zheng
@ 2022-06-13 12:14 ` Hongren (Zenithal) Zheng
2022-06-13 12:15 ` [PATCH v2 2/3] hw/usb/canokey: fix compatibility of qemu-xhci Hongren (Zenithal) Zheng
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Hongren (Zenithal) Zheng @ 2022-06-13 12:14 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 imported 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] 8+ messages in thread
* [PATCH v2 2/3] hw/usb/canokey: fix compatibility of qemu-xhci
2022-06-13 12:12 [PATCH v2 0/3] CanoKey: Fix xHCI compatibility and CCID ZLP Hongren (Zenithal) Zheng
2022-06-13 12:14 ` [PATCH v2 1/3] hw/usb/canokey: Fix " Hongren (Zenithal) Zheng
@ 2022-06-13 12:15 ` Hongren (Zenithal) Zheng
2022-06-13 12:15 ` [PATCH v2 3/3] docs/system/devices/usb/canokey: remove limitations on qemu-xhci Hongren (Zenithal) Zheng
2022-06-23 15:16 ` [PATCH v2 0/3] CanoKey: Fix xHCI compatibility and CCID ZLP Hongren (Zenithal) Zheng
3 siblings, 0 replies; 8+ messages in thread
From: Hongren (Zenithal) Zheng @ 2022-06-13 12:15 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 | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/hw/usb/canokey.c b/hw/usb/canokey.c
index 86548923eb..8da0d65556 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) {
+ usb_wakeup(usb_ep_get(&key->dev, USB_TOKEN_IN, ep_in), 0);
+ }
/*
* ready for more data in device loop
*
@@ -208,6 +215,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
+ * (useful thanks to both data_in and data_out having been 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 */
--
2.35.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 3/3] docs/system/devices/usb/canokey: remove limitations on qemu-xhci
2022-06-13 12:12 [PATCH v2 0/3] CanoKey: Fix xHCI compatibility and CCID ZLP Hongren (Zenithal) Zheng
2022-06-13 12:14 ` [PATCH v2 1/3] hw/usb/canokey: Fix " Hongren (Zenithal) Zheng
2022-06-13 12:15 ` [PATCH v2 2/3] hw/usb/canokey: fix compatibility of qemu-xhci Hongren (Zenithal) Zheng
@ 2022-06-13 12:15 ` Hongren (Zenithal) Zheng
2022-06-23 15:16 ` [PATCH v2 0/3] CanoKey: Fix xHCI compatibility and CCID ZLP Hongren (Zenithal) Zheng
3 siblings, 0 replies; 8+ messages in thread
From: Hongren (Zenithal) Zheng @ 2022-06-13 12:15 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] 8+ messages in thread
* Re: [PATCH v2 0/3] CanoKey: Fix xHCI compatibility and CCID ZLP
2022-06-13 12:12 [PATCH v2 0/3] CanoKey: Fix xHCI compatibility and CCID ZLP Hongren (Zenithal) Zheng
` (2 preceding siblings ...)
2022-06-13 12:15 ` [PATCH v2 3/3] docs/system/devices/usb/canokey: remove limitations on qemu-xhci Hongren (Zenithal) Zheng
@ 2022-06-23 15:16 ` Hongren (Zenithal) Zheng
2022-06-23 16:21 ` Hongren (Zenithal) Zheng
2022-07-01 10:51 ` Gerd Hoffmann
3 siblings, 2 replies; 8+ messages in thread
From: Hongren (Zenithal) Zheng @ 2022-06-23 15:16 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel, contact, MkfsSion
On Mon, Jun 13, 2022 at 08:12:38PM +0800, Hongren (Zenithal) Zheng wrote:
> ---
> v2:
> * use usb_ep_get instead of recording ep_in_pointer
> as suggested by kraxel
> * CI result for v2 is at
> https://gitlab.com/ZenithalHourlyRate/qemu/-/pipelines/562306905
Hi kraxel, is there any further feedback on this?
BTW, as the commit "add myself as CanoKey maintainer" has been
merged, how should I submit patches on CanoKey to you and QEMU?
For my own patches, I assume I should submit them as PATCHes
to you and the list, and after enough review it will be passed
via you by a git PULL.
For other contributors, before I can send PULL (my key is not signed yet)
I think I should first give Reviewed-by tags then request you
to pass them. After I can send PULL, I think I can just
send it to you (or who will be more appropriate?)
Regards,
>
> 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 | 30 ++++++++++++++++++++++++++----
> 2 files changed, 26 insertions(+), 14 deletions(-)
>
> --
> 2.35.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/3] CanoKey: Fix xHCI compatibility and CCID ZLP
2022-06-23 15:16 ` [PATCH v2 0/3] CanoKey: Fix xHCI compatibility and CCID ZLP Hongren (Zenithal) Zheng
@ 2022-06-23 16:21 ` Hongren (Zenithal) Zheng
2022-07-01 10:51 ` Gerd Hoffmann
1 sibling, 0 replies; 8+ messages in thread
From: Hongren (Zenithal) Zheng @ 2022-06-23 16:21 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel, contact, MkfsSion
On Thu, Jun 23, 2022 at 11:16:39PM +0800, Hongren (Zenithal) Zheng wrote:
> BTW, as the commit "add myself as CanoKey maintainer" has been
> merged, how should I submit patches on CanoKey to you and QEMU?
Also I want to add CanoKey in ChangeLog/7.1 in QEMU wiki.
But the account for wiki should be created by some other
people with existing accounts. I wonder if you or anyone
on the list could kindly do me a favor? Thanks!
My preferred username is Zenithal.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/3] CanoKey: Fix xHCI compatibility and CCID ZLP
2022-06-23 15:16 ` [PATCH v2 0/3] CanoKey: Fix xHCI compatibility and CCID ZLP Hongren (Zenithal) Zheng
2022-06-23 16:21 ` Hongren (Zenithal) Zheng
@ 2022-07-01 10:51 ` Gerd Hoffmann
2022-07-02 10:58 ` Hongren (Zenithal) Zheng
1 sibling, 1 reply; 8+ messages in thread
From: Gerd Hoffmann @ 2022-07-01 10:51 UTC (permalink / raw)
To: Hongren (Zenithal) Zheng; +Cc: qemu-devel, contact, MkfsSion
On Thu, Jun 23, 2022 at 11:16:39PM +0800, Hongren (Zenithal) Zheng wrote:
> On Mon, Jun 13, 2022 at 08:12:38PM +0800, Hongren (Zenithal) Zheng wrote:
>
> > ---
> > v2:
> > * use usb_ep_get instead of recording ep_in_pointer
> > as suggested by kraxel
> > * CI result for v2 is at
> > https://gitlab.com/ZenithalHourlyRate/qemu/-/pipelines/562306905
>
> Hi kraxel, is there any further feedback on this?
>
> BTW, as the commit "add myself as CanoKey maintainer" has been
> merged, how should I submit patches on CanoKey to you and QEMU?
Sending to the list with /me Cc'ed is fine.
> For other contributors, before I can send PULL (my key is not signed yet)
> I think I should first give Reviewed-by tags then request you
> to pass them.
Yes.
Series queued up now.
thanks,
Gerd
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/3] CanoKey: Fix xHCI compatibility and CCID ZLP
2022-07-01 10:51 ` Gerd Hoffmann
@ 2022-07-02 10:58 ` Hongren (Zenithal) Zheng
0 siblings, 0 replies; 8+ messages in thread
From: Hongren (Zenithal) Zheng @ 2022-07-02 10:58 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel, contact, MkfsSion
On Fri, Jul 01, 2022 at 12:51:06PM +0200, Gerd Hoffmann wrote:
> On Thu, Jun 23, 2022 at 11:16:39PM +0800, Hongren (Zenithal) Zheng wrote:
> > On Mon, Jun 13, 2022 at 08:12:38PM +0800, Hongren (Zenithal) Zheng wrote:
> >
> > > ---
> > > v2:
> > > * use usb_ep_get instead of recording ep_in_pointer
> > > as suggested by kraxel
> > > * CI result for v2 is at
> > > https://gitlab.com/ZenithalHourlyRate/qemu/-/pipelines/562306905
> >
> > Hi kraxel, is there any further feedback on this?
> >
> > BTW, as the commit "add myself as CanoKey maintainer" has been
> > merged, how should I submit patches on CanoKey to you and QEMU?
>
> Sending to the list with /me Cc'ed is fine.
>
> > For other contributors, before I can send PULL (my key is not signed yet)
> > I think I should first give Reviewed-by tags then request you
> > to pass them.
>
> Yes.
>
> Series queued up now.
Thanks!
> Also I want to add CanoKey in ChangeLog/7.1 in QEMU wiki.
> But the account for wiki should be created by some other
> people with existing accounts. I wonder if you or anyone
> on the list could kindly do me a favor? Thanks!
>
> My preferred username is Zenithal.
Could you please also add a wiki account for me?
There are two additional patches reviewed by me, and you have been Cc'ed.
I wonder if you could queue them up.
[PATCH v4 1/2] hw: canokey: Remove HS support as not compliant to the spec
[PATCH v4] docs/system/devices/canokey: Document limitations on usb-ehci
>
> thanks,
> Gerd
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-07-02 11:05 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-13 12:12 [PATCH v2 0/3] CanoKey: Fix xHCI compatibility and CCID ZLP Hongren (Zenithal) Zheng
2022-06-13 12:14 ` [PATCH v2 1/3] hw/usb/canokey: Fix " Hongren (Zenithal) Zheng
2022-06-13 12:15 ` [PATCH v2 2/3] hw/usb/canokey: fix compatibility of qemu-xhci Hongren (Zenithal) Zheng
2022-06-13 12:15 ` [PATCH v2 3/3] docs/system/devices/usb/canokey: remove limitations on qemu-xhci Hongren (Zenithal) Zheng
2022-06-23 15:16 ` [PATCH v2 0/3] CanoKey: Fix xHCI compatibility and CCID ZLP Hongren (Zenithal) Zheng
2022-06-23 16:21 ` Hongren (Zenithal) Zheng
2022-07-01 10:51 ` Gerd Hoffmann
2022-07-02 10:58 ` 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.