The `len` parameter is size_t (unsigned) while `actual` from
libusb_bulk_transfer() is int (signed). Their direct comparison
on line 331 triggers -Wsign-compare: if `actual` were somehow
negative, the implicit conversion to size_t would produce a
large positive value, making the comparison silently wrong.
Fixes: b4030fabe6 ("usb: Fix checkpatch warning about unnecessary parenthesis")
Signed-off-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
When working with multiple devices and the --serial argument, the serial
numbers must be known.
Add a new command "qdl list" to list the connected devices in EDL mode
and their serial number.
Signed-off-by: lucarin91 <lucarin@protonmail.com>
[bjorn: Replaced --list with list, changed output to only print serial, some stylistic changes]
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Checkpatch was warning about unnecessary parenthesis in the change to
the ZLP handling, but Github apparently doesn't highlight warnings.
Fix this to avoid leaving the building with broken windows.
Fixes: b9ad4ceaf8 ("firehose/usb: Explicitly handle ZLP on USB read transfers")
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
The assumption that every other Firehose read transfer is null is
incorrect. A Zero-Length Packet (ZLP) is only sent when the received
transfer size is a multiple of the USB IN endpoint's Max Packet Size.
This is a way for the device to indicated end-of-transfer.
Unconditionally expecting an empty packet was not an issue with USB 2.0,
where the typical 512-byte sector size matches the bulk IN endpoint's
Max Packet Size. However, with USB SuperSpeed (Max Packet Size = 1024),
attempting to read an empty packet after a 512/1024-byte transfer is
unnecessary and fails (timeout), resulting in errors during storage
device sector size discovery:
```
waiting for programmer...
qdl: failed to read: Resource temporarily unavailable
```
Fix and move ZLP handling to bus-specific code (usb.c).
Reported-by: Danilo Leo <d.leo@arduino.cc>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
During QRB2210 provisioning, a USB write failure has been
observed early in the Firehose phase (first block write).
This issue has been traced to a timeout during the USB
bulk transfer of the Zero-Length Packet (ZLP).
In some conditions, the ZLP transfer may take longer than
the current timeout, up to approximately 1.7 seconds.
The issue specifically occurs after a prior large eMMC write
operation (e.g., during a previous QDL session). It could then
be related to internal eMMC I/O operations or timing delays
affecting the USB ack.
To resolve this issue, we introduce a timeout parameter to the
qdl_write function, consistent with the existing qdl_read, and
we increase the timeout to 10 seconds for Firehose raw binary
write operations to avoid 'false-positive' timeout.
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
The firehose_read() at the start of firehose_run() serves the purpose of
waiting for the programmer to show up and to consume any <log/> messages
spat out, so that we're ready to invoke the <configure/>.
On MSM8916 there are no <log/> entries, so this is a 5 second loop
hitting -ETIMEDOUTs in firehose_read(). On other tested targets, it does
drain the <log/> entries and then hits -ETIMEDOUTs for the remainder of
5 seconds.
On the newer targets, we could perhaps determine when the programmer is
up by looking for <log/> entries, but this doesn't help the MSM8916
model and it doesn't fit with the current implementation.
If we instead tweak the timeouts and error handling of
firehose_configure(), we can speculatively attempt to configure the
programmer until success.
This meets both the observed models, and shows savings between 4 and 4.5
seconds in startup time across the tested devices.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Somewhere along the road, we broke support for the programmer for
msm8916 (and presumably other similar socs). Attempts to operate
firehose against these boards would exclusively result in USB timeouts
on write.
The reason for this is that, in contrast to all other commonly used
targets, the programmer for MSM8916 generates <log/> messages after the
<response/> message and writes are not services until all the <log/>
entries are consumed.
Rework the read loop, to continue reading messages until a timeout or an
error occurs, and then return the response status, if one was previously
found. This cost us 100ms per attempt to read a response, but I've not
found any other way to determine that we've consumed all responses.
Naively doing this does however break <read/>, as immediately following
the response with "rawmode=true" the device starts sending the data.
Adding support for parsing out "rawmode" and immediately terminating the
loop solves this, and reduces the impact of the 100ms penalty (as it no
longer applies to either <program/> or <read/>.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
This extends the Firehose protocol implementation to support the VIP
extension. It implements a state machine that counts the number of
packets sent, then injects the VIP table as the next RAW packet when
the Firehose programmer runs out of provided digests and needs a new table
of digests to validate the next packets. For example:
Packet 0: DigestsTableToSign.bin.mbn (53 digest + 1 digest of next table)
Packet 1: <configure>
Packet 2: <program>
Packet 3: ...
...
Packet 54: ChainedTableOfDigests0.bin (255 digests + digest of next table)
Packet 55: <program>
...
Packet 309: ChainedTableOfDigests1.bin
To enable VIP extension provide a path where previously generated VIP
tables are stored using "--vip-table-path" param:
$ qdl --vip-table-path "<vip-table-path>" prog_firehose_ddr.elf \
rawprogram*.xml patch*.xml
Signed-off-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
Address all obvious coding style issues caught by checkpatch.pl tool.
sha2.c and sha2.h were kept as there are.
Signed-off-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
Add support for Digests Table generation for Validated Image
Programming (VIP), which is activated when Secure Boot is enabled
on the target. VIP controls which packets are allowed to be issued to
the target. Controlling the packets that can be sent to the target is
done through hashing. The target applies a hashing function to all
received data, comparing the resulting hash digest against an existing
digest table in memory. If the calculated hash digest matches the next
entry in the table, the packet (data or command) is accepted; otherwise,
the packet is rejected, and the target halts.
This change introduces logic for VIP table generation.
In the current VIP design the first signed hash table can be a
maximum of 8 KB. Considering that it must be in MBN format, and in
addition to the raw hash table, it also includes an MBN header,
a signature, and certificates, the number of hash digests it can
contain is limited to 54 hashes (a 40-byte MBN header +
a 1696-byte hash table + a 256-byte signature + 6144 bytes of certificates).
All hashes left are stored in the additional ChainedTableOfDigests<n>.bin
files.
To generate table of digests run QDL with --create-digests param,
providing a path to store VIP tables.
As a result 3 types of files are generated:
- DIGEST_TABLE.bin - contains the SHA256 table of digests for all firehose
packets to be sent to the target. It is an intermediary table and is
used only for the subsequent generation of "DigestsToSign.bin" and
"ChainedTableOfDigests.bin" files. It is not used by QDL for VIP
programming.
- DigestsToSign.bin - first 53 digests + digest of ChainedTableOfDigests.bin.
This file has to be converted to MBN format and then signed with sectools:
$ sectools mbn-tool generate --data DigestsToSign.bin --mbn-version 6
--outfile DigestsToSign.bin.mbn
$ sectools secure-image --sign DigestsToSign.bin.mbn --image-id=VIP
Please check the security profile for your SoC to determine which version of
the MBN format should be used.
- ChainedTableOfDigests<n>.bin - contains left digests, split on
multiple files with 255 digests + appended hash of next table.
For example, for 400 packets supposed to be sent to the target, these files
will be generated (all digests are 32 bytes in size):
DIGEST_TABLE.bin
_____________
| Digest 0 |
| Digest 1 |
| etc. |
| |
| Digest 399 |
|_____________|
DigestsTableToSign.bin ChainedTableOfDigests0.bin ChainedTableOfDigests1.bin
___________________ ___________________ ____________
| Digest 0 | | Digest 53 | | Digest 308 |
| Digest 1 | | Digest 54 | | Digest 309 |
| etc. | | etc. | | etc. |
| Digest 52 | | Digest 307 | | Digest 399 |
| Next table digest | | Next table digest | |____________|
|___________________| |___________________|
When QDL is executed with --debug parameter, it will also report
Firehose packet SHA-256 hashes, for example:
FIREHOSE WRITE: <?xml version="1.0"?>
<data><patch SECTOR_SIZE_IN_BYTES="4096" byte_offset="72" filename="DISK"
physical_partition_number="5" size_in_bytes="8"
start_sector="NUM_DISK_SECTORS-1" value="NUM_DISK_SECTORS-5."/></data>
FIREHOSE PACKET SHA256: a27b1459042ea36f654c5eed795730bf73ce37ce5e92e204fe06833e5e5e1749
Signed-off-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
Decouple the flashing logic from the underlying type of communication.
This is needed for introducing simulation mode, where no real flashing is
performed, but firehose packets are used for other tasks, like
VIP table generation.
Signed-off-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
Make build compatible with Windows using MSYS2 MINGW64 compiler. Add a small compatibility file for functions that don't exist in MINGW64.
Signed-off-by: Julien Vanier <jvanier@gmail.com>
Rather than sprinkling the user experience decisions across the
implementation with prints to stdout, stderr, conditional checks for
qdl_debug etc, consolidate these into a single set of ux wrappers.
Transition all callers of printf() and fprintf() to these new wrappers,
without changing the level of content of the printouts.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Every time the USB code attempts to identify and possibly open the EDL
USB device, the libusb device descriptor-object is leaked.
While at it, also introduce qdl_close() to clean up the overall state,
in order to approach not leaking memory in a clean QDL run.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Since commit 760b3dffb0 ("qdl: Rework qdl_write to limit write sizes
to out_maxpktsize") outgoing transfers has been chunked into blocks of
wMaxPacketSize.
As reported by Wiktor Drewniak, Maksim Paimushkin, and Luca Weiss in [1]
there's huge benefits to be found in reverting this change, but out of
caution the limit was untouched.
With the transition to libusb, the throughput dropped another ~15% on my
machine. The numbers for HighSpeed and SuperSpeed are also in the same
ballpark.
With SuperSpeed, benchmarking of different chunk sizes in the megabyte
range shows improvement over these numbers in the range of 15-20x on the
same machine/board combination.
The bug report related to the reduction in size describes a host machine
running out of swiotlb, from the fact that we requested 1MB physically
contiguous memory. It's unclear if attempts was made to correct the
kernel's behavior. libusb does inquiry the capabilities of the kernel
and will split the buffer and submitting multiple URBs at once, as
needed. While no definitive guidance has been found, multiple sources
seems to recommend passing 1-2MB of buffer to libusb at a time. So,
let's move the default chunk size back up to 1MB and hope that libusb
resolves the reported problem.
Additionally, introduce a new option --out-chunk-size, which allow the
user to override the chunk size. This would allow any user to reduce the
size if needed, but also allow the value to be increased as needed.
[1] https://github.com/linux-msm/qdl/pull/39
Reported-by: Wiktor Drewniak
Reported-by: Maksim Paimushkin
Reported-by: Luca Weiss
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
The documentation for libusb_bulk_transfer() specifies that in the event
that the read or write request is split into multiple chunks, some of
these may still be transferred despite a LIBUSB_ERROR_TIMEOUT is
returned.
In the transition to libusb this detail was missed and completed
read transfers are sometimes considred to be timeouts and the data
discarded, obviously resulting in failure to continue.
Consider the "transferred" value in the event of timeout, to avoid this.
Although not yet spotted in testing, the same logic is introduced for
the write path.
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
In the event that the kernel have some other driver attached to the
device the attempt claim of the interface will fail.
Lost in the libusb conversion was a call to USBDEVFS_DISCONNECT to first
detach any such drivers. Reintroduce this by invoking
libusb_detach_kernel_driver().
As with some other libusb functions there are multiple return values
denoting "success", so rely on libusb_claim_interface() to catch the
actual errors.
Reported-by: Maxim Akristiniy
Suggested-by: Maxim Akristiniy
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
libusb commit f0cce43f882d ("core: Fix definition and use of enum
libusb_transfer_type") split the types of split transfer type and
endpoint transfer, introducing LIBUSB_ENDPOINT_TRANSFER_TYPE_BULK.
The result is that when building with older libusb, such the one
available on Ubuntu 20.04 the build fails with the following error:
usb.c:84:16: error: ‘LIBUSB_ENDPOINT_TRANSFER_TYPE_BULK’ undeclared (first use in this function); did you mean ‘LIBUSB_TRANSFER_TYPE_BULK’?
Introduce an alias using the preprocessor to make available the new
define when building against the older version of libusb headers.
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
When working on a host with multiple boards attached being able to
select a specific board by serial number becomes necessary.
In the EDL USB descriptors a device serial number is available as part
of the iProduct string, so this can be used for comparison.
As libusb requires a handle the libusb_open() needs to be moved into the
loop.
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
In order to support selecting board based on serial number the iProduct
field needs to be inspected, with the hand-rolled parsing of the USB
descriptors this becomes cumbersome.
Furthermore the direct use of Linux's USBDEVFS creats an unnecessary
dependency on the host OS being Linux.
It's unclear why libusb wasn't choosen in the first place, perhaps it
relates to the faint memory of 0.1 vs 1.0 packaging issues?
Move to libusb-1.0 in order to resolve these issues, as well as clean up
the code a bit.
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
