Strony Zhang wrote:
> J?rgen Keil :
> > And I suspect that's the problem! Since it's using
> > the keyboard in boot protocol mode, it shouldn't
> > try to find out the keyboard's packet size from
> > looking at / parsing the HID descriptor. I suspect
> > that would only be a valid strategy if the boot
> > protocol is disabled.
> >
> From the MacBook Pro internal keyboard report descriptor, the report ID
> 1 is used to identify the keyboard usage.
Yes.
> According to the HID spec, the
> report ID should be transferred as one byte prefix.
Report ID is a one byte prefix, correct. But does the HID spec
tells us if report prefix bytes are supposed to be transmitted
in boot protocol mode?
Appendix B: "Boot Interface Descriptor" contains this:
The HID Subclass 1 defines two descriptors for Boot Devices.
Devices may append additional data to these boot reports, but
the first 8 bytes of keyboard reports[1] and the first 3 bytes of
mouse reports must conform to the format defined by the Boot
Report descriptor in order for the data to be correctly
interpreted by the BIOS. The report may not exceed 8 bytes in
length[2]. The BIOS will ignore any extensions to reports. These
descriptors describe reports that the BIOS expects to see.
However, since the BIOS does not actually read the Report
descriptors, these descriptors do not have to be hard-coded
into the device if an alternative report descriptor is
provided[3]. Instead, descriptors that describe the device reports
in a USB-aware operating system should be included (these may
or may not be the same)[4]. When the HID class driver is loaded,
it will issue a Change Protocol, changing from the boot
protocol to the report protocol after reading the boot
interface's Report descriptor.
Some comments by me:
[1] boot protocol keyboard data always has a size of 8 or more bytes, but...
[2] ... the report may not exceed 8 bytes;
so a size of exactly 8 bytes is the only possible size for a boot
protocol keyboard report.
[3] BIOS does not use HID report descriptors; so BIOS would have no way of
knowing that report prefix bytes are used on an Apple MacBook Pro usb
internal keyboard.
[4] I think this is exactly what happens: Apple doesn't encode the fixed
format boot protocol keyboard format from the HID spec in the HID report
descriptors; they describe the format that is used when boot protocol
mode is disabled.
It seems the goal is to make it as easy as possible for the BIOS to
support USB keyboards, so a fixed data format is defined. And the BIOS
doesn't have to parse HID report descriptors. So it doesn't make much
sense to make is more complicate and transmit report ID prefix bytes ...
> After the internal
> keyboard is set into Boot protocol mode, its 8 bytes data don't include
> the one byte prefix (report ID 0x01).
Correct.
> I tried disabling the boot
> protocol on the Macbook. But I failed to see the one byte report ID
> prefix was included in the keyboard data.
> Could you verify this on your MacBook pro?
Ok, I tried that - I do get the report ID prefix byte.
In kmdb I changed the call in usbkbm_open()
from usbkbm_set_protocol(usbkbmd, SET_BOOT_PROTOCOL)
to usbkbm_set_protocol(usbkbmd, SET_REPORT_PROTOCOL),
and used cfgadm -c disconnect / configure to "hotplug" the internal
usb keyboard.
Now I received data like this from the apple keyboard (here the
hex dump only printed the first 8 bytes, the final two bytes are lost):
10 bytes (exp 8) - 01 02 00 00 00 00 00 00 (Left Shift)
10 bytes (exp 8) - 01 00 00 00 00 00 00 00
10 bytes (exp 8) - 01 01 00 00 00 00 00 00 (Left Control)
10 bytes (exp 8) - 01 00 00 00 00 00 00 00
10 bytes (exp 8) - 01 00 00 04 00 00 00 00 (a)
10 bytes (exp 8) - 01 00 00 00 00 00 00 00
10 bytes (exp 8) - 01 00 00 05 00 00 00 00 (b)
10 bytes (exp 8) - 01 00 00 00 00 00 00 00
10 bytes (exp 8) - 01 01 00 00 00 00 00 00 (Control)
10 bytes (exp 8) - 01 01 00 06 00 00 00 00 (c)
10 bytes (exp 8) - 01 01 00 00 00 00 00 00
10 bytes (exp 8) - 01 00 00 00 00 00 00 00
And once more with a full dump:
10 bytes (exp 8) - 01 02 00 00 00 00 00 00 00 00 (Left Shift)
10 bytes (exp 8) - 01 00 00 00 00 00 00 00 00 00
10 bytes (exp 8) - 01 00 00 00 00 00 00 00 00 01 (Eject key)
10 bytes (exp 8) - 01 00 00 00 00 00 00 00 00 00
10 bytes (exp 8) - 01 00 00 00 00 00 00 00 00 02 (Fn key)
10 bytes (exp 8) - 01 00 00 00 00 00 00 00 00 00
I haven't been able to get reports with the 0x52 report ID prefix
out of the keyboard. I did expect that the Fn-key would send these,
but all the "Fn" key did was set bit 2 in the extra byte at the end of the
standard 0x01 report.
In boot keyboard protocol mode, data on the MacBook pro looks like
this (with the original usbkbm module I got "exp 0"):
# ~jk/src/dtrace/usbkbm.d
8 bytes (exp 8) - 20 00 00 00 00 00 00 00
8 bytes (exp 8) - 00 00 00 00 00 00 00 00
a 8 bytes (exp 8) - 00 00 04 00 00 00 00 00
8 bytes (exp 8) - 00 00 00 00 00 00 00 00
b 8 bytes (exp 8) - 00 00 05 00 00 00 00 00
8 bytes (exp 8) - 00 00 00 00 00 00 00 00
8 bytes (exp 8) - 01 00 00 00 00 00 00 00
^C
8 bytes (exp 8) - 01 00 06 00 00 00 00 00
My dtrace script:
# cat ~jk/src/dtrace/usbkbm.d
#!/usr/sbin/dtrace -s
#pragma D option quiet
BEGIN
{
M_DATA = 0;
}
fbt::usbkbm_rput:entry
{
this->q = (queue_t *)arg0;
this->mp = (mblk_t *)arg1;
this->usbkbmd = (usbkbm_state_t *)this->q->q_ptr;
}
fbt::usbkbm_rput:entry
/this->mp->b_datap->db_type == M_DATA/
{
cp = (unsigned char *)this->mp->b_rptr;
printf("\t%d bytes (exp %d) - %02x %02x %02x %02x %02x %02x %02x
%02x\n",
this->mp->b_wptr - this->mp->b_rptr,
this->usbkbmd->usbkbm_packet_size,
cp[0], cp[1], cp[2], cp[3], cp[4], cp[5], cp[6], cp[7]);
}
> It seems there is not a close relationship between report
> ID and boot protocol on the actual hardware.
Yep. And [4] from Appendix B: "Boot Interface Descriptor" in the HID
spec tells us that there doesn't have to be such a relationship...
> I saw in CR6674852 that one
> byte report ID prefix appeared in the IBM keyboard data.
Interesting, so you're saying that the "Lite-On Technology IBM Enhanced
Performance Wireless USB Keyboard" does send report ID prefix bytes
although the keyboard should be set to boot protocol mode?
Is this keyboard usable at the BIOS level?
What kind of HID report descriptors does that Lite-On wireless usb keyboard
from CR6674852 return?
Do we know for sure that the Lite-On wireless usb keyboard accepted the
switch to boot protocol mode?
> Or the MacBook pro internal USB keyboard does not conform to the HID
> spec at the point.
Hmm, the HID spec could be clearer, but I think one can argue that
the MacBook Pro USB keyboard conforms to the spec.
> > I suspect that when an usb keyboard is used in boot
> > protocol mode, it should just use an expected keyboard
> > data packet size of USB_KBD_DEFAULT_PACKET_SIZE (=8)
> > bytes (without looking at the HID descriptors)!
> >
> The codes of getting the packet size by parsing the report descriptor
> have been there years. If the value parsed from the actual hardware is
> replaced with that defined in spec, it is possible to bring some regression.
Yes, maybe. I suspect that USB keyboard devices with missing boot protocol
support could break. But are such devices supported by usbkbm?
Sanity checking the packet_size value from hidparser_get_packet_size()
seems to be less risky - something like this:
if (hidparser_get_packet_size(usbkbmd->usbkbm_report_descr,
0, HIDPARSER_ITEM_INPUT, (uint32_t *)&packet_size) ==
HIDPARSER_FAILURE || packet_size < 8) {
^^^^^^^^^^^^^^^^^^
USB_DPRINTF_L3(PRINT_MASK_OPEN,
usbkbm_log_handle, "get_packet_size failed"
"setting default packet size(8)");
/* Setting to default packet size = 8 */
usbkbmd->usbkbm_packet_size =
USB_KBD_DEFAULT_PACKET_SIZE;
} else {
usbkbmd->usbkbm_packet_size = packet_size/8;
}
> > dtracing in usbkbm_rput() reveals that the MacBook Pro USB
> > keyboard is sending standard 8 bytes boot protocol packets,
> > but they are dropped on the floor because the usbkbm module
> > thinks that the correct packets size is 0 (zero!) bytes - apparently
> > that size of 0 bytes doesn't make any sense at all.
> >
> The reason why the value is equal to 0 at the line 560 is that the
> report id 0 (default) is passed to hidparser_get_packet_size() as the
> argument. The actual report ID is 1.
Yes, hidparser didn't find INPUT items not having a report id, so the
total size of the packet remains at the initial value of 0 bits.
And I think if the code would magically pass report ID 1 to
hidparser_get_packet_size(), we would get a packet size of 10 bytes;
1 byte report id + 8 byte standard boot keyboard protocol +
1 extra byte containg a bit for an "Eject" key (usage 0xb8 in
usage page 0x0c). I suspect that would not help much, because now
the data packets are discarded because the actual size of 8 bytes !=
the expected size of 10 bytes.
> Therefore, if the close relationship between report ID and boot protocol
> exists, the complete fix seems to be to disable the boot protocol and
> add the report ID support in usbkbm.
OK, support for USB keyboard report protocol (including report id support)
would be nice. But I guess that would result in more regressions than
using boot protocol with the fixed size.
