Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upstream changes and fixes #204

Open
wants to merge 35 commits into
base: master
Choose a base branch
from
Open

Upstream changes and fixes #204

wants to merge 35 commits into from

Conversation

pesco
Copy link

@pesco pesco commented Jul 26, 2023

Some regular updates by the looks of it, but notably fixes two serious bugs that could alternatively be cherry-picked: b68f3a1 and 84c32dc.

I stumbled over the latter because it makes a device using the bootloader unusable with my OpenBSD machine.

Also includes a drive-by tweak to the README to clarify that we need GNU Make to build.

stevenj and others added 30 commits June 9, 2018 21:48
Same code size, more readable.
pybadge and pygamer fixes; new boards; improved splashscreen display; merge from upstream
adafruit release 3.7.0: pybadge accelerometer, board ids, minor fixes
…Using RPi Serial line as the UART port for bootloader
In a FAT12 or FAT16 filesystem, the first entry's first byte must
match the media descriptor given in the BPB (BIOS Parameter Block,
part of the boot sector).  This gives a kind of check of the disk
geometry: on an unpartitioned disk (like a floppy or ZIP disk), the
BPB can be read with the wrong geometry, but to properly locate the
FAT, the geometry must be correct.

Many OSs don't check that these match, but a few do, such as FreeBSD
(both in the kernel msdos filesystem driver, and in fsck_msdosfs).  If
they don't match, the mount will fail with an unhelpful error message.

Previously, the media descriptor was 0xf8 in the BPB, but 0xf0 in the
FAT.  This patch changes them to 0xf0 everywhere.

There were different values used for different disk geometries before
disk addressing was reformed in the 90s, but since then only 0xf0 and
0xf8 are in common use.  Typically, 0xf0 is used for a filesystem
within an unpartitioned medium (such as a floppy disk), and 0xf8 is
used for a filesystem within a partition (such as a hard disk).

Since the UF2 bootloader emulates an unpartitioned removable medium,
0xf0 seemed appropriate.

See also the description of the media descriptor, offset 0x15, at
https://en.wikipedia.org/w/index.php?title=Design_of_the_FAT_file_system&oldid=920681387#BIOS_Parameter_Block
particularly the last few paragraphs which note that it must match the
FAT ID.
USB mass storage works by wrapping other storage protocols in USB
packets.  There are several defined, but universally the most common
one is SCSI.  While SCSI is big-endian, USB is little-endian.

The wrappers used in the USB mass storage protocol have some
"signature" (magic number) fields that are part of the USB protocol,
and hence little-endian.  Most OSs apparently don't check these
signatures, but some (such as FreeBSD) do.

The previous code encodes these signatures as big-endian.  This patch
fixes them to be little-endian.

(Technical background follows for the curious.  This is oriented to
SCSI under USB Bulk-Only mass storage protocol, which is by far the
predominant protocol used by USB mass storage devices, including the
UF2 bootloader here.)

A CBW (Command Block Wrapper) is a SCSI command (or other protocol,
but typically SCSI) embedded in a USB mass storage packet.  A CSW
(Command Status Wrapper) is a SCSI status (reply) embedded in a USB
mass storage packet.

The CBW/CSW mechanism embeds any SCSI protocol transaction.  The first
command is typically an INQUIRY command (which retrieves metadata
about the drive), but for simplicity, we'll talk about a READ command.

A typical "read" transaction is as folllows:

1. The host sends the device a SCSI read command (SCSI CDB block),
encapsulated in a USB mass storage CBW packet (which is further
encapsulated in USB bulk data packets, addressed to the device's read
endpoint).

2. If the device is able to fulfill the request, it sends the host the
requested data, with no encapsulation at the USB mass storage level.
(These packets are still encapsulated at the protocol layer, addressed
from the device's write endpoint).

3. If the device is unable to fulfill the request, it sends a USB
STALL packet (which is at the protocol level, lower than the mass
storage level).

4. Regardless of whether the device was able to fulfill the request or
not, it sends a SCSI status packet, encapsulated in a USB mass storage
CSW packet (which is further encapsulated in USB bulk data packets,
addressed from the device's write endpoint).

If everything works well, the host now can unambiguously tell whether
the command succeeded or failed, based on the presence / absence of
the STALL packet, and interprets the next packet as a SCSI status
packet (embedded in a CSW wrapper).

If everything isn't working well, the host and device can get out of
sync in these state transitions.  To ensure that everything is working
properly, every SCSI command or status is wrapped in a packet specific
to the USB mass storage protocol: a CBW for a command, or CSW for a
status (reply).

The CBW starts with the magic number 0x43425355 (little-endian,
"USBC"), followed by a tag used to identify this command.  The CSW
starts with the magic number 0x53425355 (little-endian, "USBS"),
followed by the tag of the command being responded to.

(The endianness of the tag is irrelevant, since it's only a four-byte
sequence that needs to be the same in the CSW as in the initiating
CBW.)

Since the CBW's and CSW's magic numbers are typically irrelevant, they
can usually be ignored.  Most OSs' implementations of the USB mass
storage protocol ignore them.  However, they serve as a good check to
ensure that the host and device are in sync; hence, some OSs (such as
FreeBSD) do validate them.

Since the CBW and CSW are defined as part of the USB spec, their
signatures are little-endian, even though the values in the wrapped
packets are big-endian.

As described above, these signatures are ignored by many OSs, but are
validated by FreeBSD.

I'm surprised that try_read_cbw ever worked if USE_MSC_CHECKS was
enabled.  I patched the code within USE_MSC_CHECKS, but haven't
actually tested it: I only tested the default build.
Fix CSW signatures, and unify FAT media descriptors
Add configuration for PewPew M4 board
ATSAME54P20A SVD bugfix and additions
The MM1_m0 does not exist.
Delete boards/robohatmm1_m0 directory
thanks @JackCasual
The Makefile automatically includes `Makefile.user`, not `Makefile.local`
as was mentioned in `README.md`.
Proposal to allow a GPIO to hold the bootloader from running the app, if its in the correct state
…readme

Fix Makefile.user reference in README.md
…ice/1fa2c020-270c-46ee-8e9b-564614838e23

Adding Microsoft SECURITY.MD
@pesco
Copy link
Author

pesco commented Jul 27, 2023

Other points of note:

  • Adds a "hold bootloader pin" function similar (but not identical) to the one found in add BOOT_LOAD_PIN, update BL from SRAM and Jeff Probe Board #203.

  • Upstream includes a wait state fix for SAMD51 (569c1d8) that overlaps/conflicts with 246e826 here. I chose to keep the "automatic wait states" (VMCTRL->CTRLA.bit.AUTOWS = 1) version from here, but please verify - I don't know what's correct/better.

  • There was a conflict wrt. the PewPew M4 which got independently added here (6211469) and upstream (c08c68a1), I think? The config_data entry for DISPLAY_CFG0 ended up as 0x20180 here and 0x80 upstream. To my eye the former looks like a typo or something, but I can't be sure - please verify.

  • Removes robohatmm1_m0 which, according to the commit message, does not exist. (?)

  • Introduces a few new macros (PINIP, PINCFG, PINMUX).

  • Adds Microsoft's SECURITY.md (boilerplate).

[EDIT by @dhalbert - fixed some commit links to point to upstream]

@dhalbert dhalbert self-requested a review July 27, 2023 11:47
@dhalbert
Copy link

  • Upstream includes a wait state fix for SAMD51 (569c1d8) that overlaps/conflicts with 246e826 here. I chose to keep the "automatic wait states" (VMCTRL->CTRLA.bit.AUTOWS = 1) version from here, but please verify - I don't know what's correct/better.

I prefer the auto-wait-state, and was thinking of PR'ing it to upstream. I pinged the upstream PR author to ask what they prefer.

@dhalbert
Copy link

  • There was a conflict wrt. the PewPew M4 which got independently added here (6211469) and upstream (c08c68a1), I think? The config_data entry for DISPLAY_CFG0 ended up as 0x20180 here and 0x80 upstream. To my eye the former looks like a typo or something, but I can't be sure - please verify.

@deshipu Could you take a look at this?

@deshipu
Copy link

deshipu commented Aug 19, 2023

  • There was a conflict wrt. the PewPew M4 which got independently added here (6211469) and upstream (c08c68a1), I think? The config_data entry for DISPLAY_CFG0 ended up as 0x20180 here and 0x80 upstream. To my eye the former looks like a typo or something, but I can't be sure - please verify.

@deshipu Could you take a look at this?

This is because I originally prototyped with a different display than I ended up using in the boards I sold. Both values are correct, depending on which display you have, the difference is in the horizontal offset – the value 0x20180 is what was sold in the end, so I suppose this is what people will want. It can be changed with the uf2patcher tool if needed.

This board was originally prototyped with a different display than what ended
up being sold. Both values are correct, depending on which display is used -
the difference is in the horizontal offset. The value 0x20180 is correct for
the board as it was sold, so that is likely what users want.

The value can be changed with the uf2patcher tool if needed.
@pesco
Copy link
Author

pesco commented Aug 21, 2023

the value 0x20180 is what was sold in the end, so I suppose this is what people will want.

Thanks for clarifying! I've added another commit to change it back, paraphrasing your explanation in the commit message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants