[PATCH v4 1/2] hw: canokey: Remove HS support as not compliant to the spec

[PATCH v4 1/2] hw: canokey: Remove HS support as not compliant to the spec

[MkfsSion]

Canokey core currently using 16 bytes as maximum packet size for
control endpoint, but to run the device in high-speed a 64 bytes
maximum packet size is required according to USB 2.0 specification.
Since we don't acutally need to run the device in high-speed, simply
don't assign high member in USBDesc.

When canokey-qemu is used with xhci, xhci would drive canokey
in high speed mode, since the bcdUSB in canokey-core is 2.1,
yet canokey-core set bMaxPacketSize0 to be 16, this is out
of the spec as the spec said that ``The allowable maximum
control transfer data payload sizes...for high-speed devices,
it is 64 bytes''.

In this case, usb device validation in Windows 10 LTSC 2021
as the guest would fail. It would complain
USB\DEVICE_DESCRIPTOR_VALIDATION_FAILURE.

Note that bcdUSB only identifies the spec version the device
complies, but it has no indication of its speed. So it is
allowed for the device to run in FS but comply the 2.1 spec.

To solve the issue we decided to just drop the high
speed support. This only affects usb-ehci as usb-ehci would
complain speed mismatch when FS device is attached to a HS port.
That's why the .high member was initialized in the first place.
Meanwhile, xhci is not affected as it works well with FS device.
Since everyone is now using xhci, it does no harm to most users.

Suggested-by: Hongren (Zenithal) Zheng <i@zenithal.me>
Signed-off-by: YuanYang Meng <mkfssion@mkfssion.com>
---
 hw/usb/canokey.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/usb/canokey.c b/hw/usb/canokey.c
index 4a08b1cbd7..6a7ab965a5 100644
--- a/hw/usb/canokey.c
+++ b/hw/usb/canokey.c
@@ -56,7 +56,6 @@ static const USBDesc desc_canokey = {
         .iSerialNumber     = STR_SERIALNUMBER,
     },
     .full = &desc_device_canokey,
-    .high = &desc_device_canokey,
     .str  = desc_strings,
 };
 
-- 
2.36.1

[PATCH v4] docs/system/devices/canokey: Document limitations on usb-ehci

[MkfsSion]

Suggested-by: Hongren (Zenithal) Zheng <i@zenithal.me>
Signed-off-by: YuanYang Meng <mkfssion@mkfssion.com>
---
  v4:
    Adopt Zenithal's suggestion of repharsing the limitation 

 docs/system/devices/canokey.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/docs/system/devices/canokey.rst b/docs/system/devices/canokey.rst
index 169f99b8eb..db33b59e9a 100644
--- a/docs/system/devices/canokey.rst
+++ b/docs/system/devices/canokey.rst
@@ -156,6 +156,11 @@ to it, for example
 
    |qemu_system| -device piix3-usb-uhci,id=uhci -device canokey,bus=uhci.0
 
+The other limitation is that this device is not compatible with ``usb-ehci``
+since the device only provides the full-speed mode. However, when a
+full-speed device attach to a high-speed port, ``usb-ehci`` would complain
+about speed mismatch.
+
 References
 ==========
 
-- 
2.36.1

Re: [PATCH v4] docs/system/devices/canokey: Document limitations on usb-ehci

[Hongren Zheng]

On Sat, Jun 25, 2022 at 10:26:51PM +0800, MkfsSion wrote:
> Suggested-by: Hongren (Zenithal) Zheng <i@zenithal.me>
> Signed-off-by: YuanYang Meng <mkfssion@mkfssion.com>
> ---
>   v4:
>     Adopt Zenithal's suggestion of repharsing the limitation 
> 
>  docs/system/devices/canokey.rst | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/docs/system/devices/canokey.rst b/docs/system/devices/canokey.rst
> index 169f99b8eb..db33b59e9a 100644
> --- a/docs/system/devices/canokey.rst
> +++ b/docs/system/devices/canokey.rst
> @@ -156,6 +156,11 @@ to it, for example
>  
>     |qemu_system| -device piix3-usb-uhci,id=uhci -device canokey,bus=uhci.0
>  
> +The other limitation is that this device is not compatible with ``usb-ehci``
> +since the device only provides the full-speed mode. However, when a
> +full-speed device attach to a high-speed port, ``usb-ehci`` would complain
> +about speed mismatch.
> +
>  References
>  ==========
>  
> -- 
> 2.36.1
>

Reviewed-by: Hongren (Zenithal) Zheng <i@zenithal.me>

kraxel could you please also queue this up for PULL, thanks!

Re: [PATCH v4 1/2] hw: canokey: Remove HS support as not compliant to the spec

[Hongren Zheng]

On Sat, Jun 25, 2022 at 10:21:37PM +0800, MkfsSion wrote:
> Canokey core currently using 16 bytes as maximum packet size for
> control endpoint, but to run the device in high-speed a 64 bytes
> maximum packet size is required according to USB 2.0 specification.
> Since we don't acutally need to run the device in high-speed, simply
> don't assign high member in USBDesc.
> 
> When canokey-qemu is used with xhci, xhci would drive canokey
> in high speed mode, since the bcdUSB in canokey-core is 2.1,
> yet canokey-core set bMaxPacketSize0 to be 16, this is out
> of the spec as the spec said that ``The allowable maximum
> control transfer data payload sizes...for high-speed devices,
> it is 64 bytes''.
> 
> In this case, usb device validation in Windows 10 LTSC 2021
> as the guest would fail. It would complain
> USB\DEVICE_DESCRIPTOR_VALIDATION_FAILURE.
> 
> Note that bcdUSB only identifies the spec version the device
> complies, but it has no indication of its speed. So it is
> allowed for the device to run in FS but comply the 2.1 spec.
> 
> To solve the issue we decided to just drop the high
> speed support. This only affects usb-ehci as usb-ehci would
> complain speed mismatch when FS device is attached to a HS port.
> That's why the .high member was initialized in the first place.
> Meanwhile, xhci is not affected as it works well with FS device.
> Since everyone is now using xhci, it does no harm to most users.
> 
> Suggested-by: Hongren (Zenithal) Zheng <i@zenithal.me>
> Signed-off-by: YuanYang Meng <mkfssion@mkfssion.com>
> ---
>  hw/usb/canokey.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/hw/usb/canokey.c b/hw/usb/canokey.c
> index 4a08b1cbd7..6a7ab965a5 100644
> --- a/hw/usb/canokey.c
> +++ b/hw/usb/canokey.c
> @@ -56,7 +56,6 @@ static const USBDesc desc_canokey = {
>          .iSerialNumber     = STR_SERIALNUMBER,
>      },
>      .full = &desc_device_canokey,
> -    .high = &desc_device_canokey,
>      .str  = desc_strings,
>  };
>  
> -- 
> 2.36.1
>

Reviewed-by: Hongren (Zenithal) Zheng <i@zenithal.me>

kraxel could you please pick this patch and queue
this up for PULL, thanks!