fix: correct errors using machine.flash

Author: mikesmitty

src/machine/usb/msc/disk.go

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"fmt"
 	"machine"
-	"runtime/interrupt"
 	"time"
 )
 
@@ -17,14 +16,17 @@ var (
 func (m *msc) RegisterBlockDevice(dev machine.BlockDevice) {
 	m.dev = dev
 
-	m.blockRatio = 1
-	m.blockSize = uint32(m.dev.WriteBlockSize())
-	if m.blockSize < 512 {
-		// If the block size is less than 512 bytes, we'll scale it up to 512 for compatibility
-		m.blockRatio = 512 / m.blockSize
-		m.blockSize = 512
+	if cap(m.blockCache) != int(dev.WriteBlockSize()) {
+		m.blockCache = make([]byte, dev.WriteBlockSize())
+		m.buf = make([]byte, dev.WriteBlockSize())
 	}
-	m.blockCount = uint32(m.dev.Size()) / m.blockSize
+
+	m.blockSizeRaw = uint32(m.dev.WriteBlockSize())
+	m.blockCount = uint32(m.dev.Size()) / m.blockSizeUSB
+	// Read/write/erase operations must be aligned to the underlying hardware blocks. In order to align
+	// them we assume the provided block device is aligned to the end of the underlying hardware block
+	// device and offset all reads/writes by the remaining bytes that don't make up a full block.
+	m.blockOffset = uint32(m.dev.Size()) % m.blockSizeUSB
 	// FIXME: Figure out what to do if the emulated write block size is larger than the erase block size
 
 	// Set VPD UNMAP fields
@@ -33,7 +35,7 @@ func (m *msc) RegisterBlockDevice(dev machine.BlockDevice) {
 			// 0xb0 - 5.4.5 Block Limits VPD page (B0h)
 			if len(vpdPages[i].Data) >= 28 {
 				// Set the OPTIMAL UNMAP GRANULARITY (write blocks per erase block)
-				granularity := uint32(dev.EraseBlockSize()) / m.blockSize
+				granularity := uint32(dev.EraseBlockSize()) / m.blockSizeUSB
 				binary.BigEndian.PutUint32(vpdPages[i].Data[24:28], granularity)
 			}
 			if len(vpdPages[i].Data) >= 32 {
@@ -44,7 +46,7 @@ func (m *msc) RegisterBlockDevice(dev machine.BlockDevice) {
 				// https://www.seagate.com/files/staticfiles/support/docs/manual/Interface%20manuals/100293068j.pdf
 
 				// We assume the block device is aligned to the end of the underlying block device
-				blockOffset := uint32(dev.EraseBlockSize()) % m.blockSize
+				blockOffset := uint32(dev.EraseBlockSize()) % m.blockSizeUSB
 				binary.BigEndian.PutUint32(vpdPages[i].Data[28:32], blockOffset)
 			}
 			break
@@ -56,7 +58,7 @@ var _ machine.BlockDevice = (*RecorderDisk)(nil)
 
 // RecorderDisk is a block device that records actions taken on it
 type RecorderDisk struct {
-	data map[int64][]byte
+	dev  machine.BlockDevice
 	log  []RecorderRecord
 	last time.Time
 	time time.Time
@@ -80,9 +82,9 @@ const (
 )
 
 // NewRecorderDisk creates a new RecorderDisk instance
-func NewRecorderDisk(count int) *RecorderDisk {
+func NewRecorderDisk(dev machine.BlockDevice, count int) *RecorderDisk {
 	d := &RecorderDisk{
-		data: make(map[int64][]byte),
+		dev:  dev,
 		log:  make([]RecorderRecord, 0, count),
 		last: time.Now(),
 	}
@@ -91,95 +93,64 @@ func NewRecorderDisk(count int) *RecorderDisk {
 			OpCode: RecorderOpCodeRead,
 			Offset: 0,
 			Length: 0,
-			Data:   make([]byte, 0, 64),
+			Data:   make([]byte, dev.WriteBlockSize()),
 			Time:   0,
 		})
 	}
 	return d
 }
 
 func (d *RecorderDisk) Size() int64 {
-	return 4096 * int64(d.WriteBlockSize()) // 2MB
+	return d.dev.Size()
 }
 
 func (d *RecorderDisk) WriteBlockSize() int64 {
-	return 512 // 512 bytes
+	return d.dev.WriteBlockSize()
 }
 
 func (d *RecorderDisk) EraseBlockSize() int64 {
-	return 2048 // 4 blocks of 512 bytes
+	return d.dev.EraseBlockSize()
 }
 
 func (d *RecorderDisk) EraseBlocks(startBlock, numBlocks int64) error {
 	d.Record(RecorderOpCodeEraseBlocks, startBlock, int(numBlocks), []byte{})
-	if interrupt.In() {
-		// Flash erase commands are not allowed in interrupt context
-		panic("EraseBlocks attempted in interrupt context")
-	}
-	return nil
+	return d.dev.EraseBlocks(startBlock, numBlocks)
 }
 
 func (d *RecorderDisk) ReadAt(buffer []byte, offset int64) (int, error) {
-	d.Record(RecorderOpCodeRead, offset, len(buffer), []byte{})
-	sector := offset / d.WriteBlockSize()
-	if sector < 0 || offset+int64(len(buffer)) > d.Size() {
-		return 0, fmt.Errorf("read out of bounds: %d", offset)
-	}
-
-	sectorCount := int64(len(buffer)) / d.WriteBlockSize()
-	for i := int64(0); i < sectorCount; i++ {
-		if _, ok := d.data[sector+i]; ok {
-			n := int(offset % d.WriteBlockSize())
-			copy(buffer, d.data[sector+i][n:])
-		}
-	}
-
-	return len(buffer), nil
+	n, err := d.dev.ReadAt(buffer, offset)
+	d.Record(RecorderOpCodeRead, offset, n, buffer)
+	return n, err
 }
 
 func (d *RecorderDisk) WriteAt(buffer []byte, offset int64) (int, error) {
-	if interrupt.In() {
-		// Flash writes aren't possible in interrupt context
-		panic("WriteAt attempted in interrupt context")
-	}
-	if offset < 0 || offset+int64(len(buffer)) > d.Size() {
-		return 0, errWriteOutOfBounds
-	}
-
-	d.Record(RecorderOpCodeWrite, offset, len(buffer), buffer)
-
-	sector := offset / d.WriteBlockSize()
-	sectorCount := int64(len(buffer)) / d.WriteBlockSize()
-	for i := int64(0); i < sectorCount; i++ {
-		_, ok := d.data[sector+i]
-		if !ok {
-			d.data[sector+i] = make([]byte, d.WriteBlockSize())
-		}
-		n := int(offset % d.WriteBlockSize())
-		copy(d.data[sector+i][n:], buffer)
-	}
-	return len(buffer), nil
+	n, err := d.dev.WriteAt(buffer, offset)
+	d.Record(RecorderOpCodeWrite, offset, n, buffer)
+	return n, err
 }
 
 func (d *RecorderDisk) Record(opCode RecorderOpCode, offset int64, length int, data []byte) {
-	n := len(d.log)
-	if n == 0 {
-		return
-	} else if n == cap(d.log) {
-		for i := 0; i < n-1; i++ {
-			d.log[i] = d.log[i+1]
-		}
+	n := len(d.log) - 1
+	// Shift the log entries up to make room for a new entry
+	for i := 0; i < n; i++ {
+		d.log[i].OpCode = d.log[i+1].OpCode
+		d.log[i].Offset = d.log[i+1].Offset
+		d.log[i].Length = d.log[i+1].Length
+		d.log[i].Data = d.log[i].Data[:len(d.log[i+1].Data)]
+		copy(d.log[i].Data, d.log[i+1].Data)
+		d.log[i].Time = d.log[i+1].Time
+		d.log[i].valid = d.log[i+1].valid
 	}
 
 	// Append the new record
-	d.log[n-1].OpCode = opCode
-	d.log[n-1].Offset = offset
-	d.log[n-1].Length = length
-	d.log[n-1].Data = d.log[n-1].Data[:len(data)]
-	copy(d.log[n-1].Data, data)
-	d.log[n-1].Time = time.Since(d.time).Microseconds()
+	d.log[n].OpCode = opCode
+	d.log[n].Offset = offset
+	d.log[n].Length = length
+	d.log[n].Data = d.log[n].Data[:len(data)]
+	copy(d.log[n].Data, data)
+	d.log[n].Time = time.Since(d.time).Microseconds()
 	d.time = d.time.Add(time.Since(d.time))
-	d.log[n-1].valid = true
+	d.log[n].valid = true
 }
 
 func (d *RecorderDisk) ClearLog() {
@@ -193,7 +164,7 @@ func (d *RecorderDisk) GetLog() []RecorderRecord {
 	return d.log
 }
 
-func (r RecorderRecord) String() (string, bool) {
+func (r *RecorderRecord) String() (string, bool) {
 	opCode := "Unknown"
 	switch r.OpCode {
 	case RecorderOpCodeRead:
@@ -203,5 +174,5 @@ func (r RecorderRecord) String() (string, bool) {
 	case RecorderOpCodeEraseBlocks:
 		opCode = "EraseBlocks"
 	}
-	return fmt.Sprintf("%s: %05d+%02d t:%d| % 0x", opCode, r.Offset, r.Length, r.Time, r.Data), r.valid
+	return fmt.Sprintf("%s: %05d+%02d t:%d | % 0x", opCode, r.Offset, r.Length, r.Time, r.Data), r.valid
 }

src/machine/usb/msc/msc.go

@@ -28,6 +28,7 @@ var MSC *msc
 
 type msc struct {
 	buf           []byte     // Buffer for incoming/outgoing data
+	blockCache    []byte     // Buffer for block read/write data
 	taskQueued    bool       // Flag to indicate if the buffer has a task queued
 	rxStalled     bool       // Flag to indicate if the RX endpoint is stalled
 	txStalled     bool       // Flag to indicate if the TX endpoint is stalled
@@ -42,12 +43,13 @@ type msc struct {
 	cswBuf      []byte // CSW response buffer
 	state       mscState
 
-	maxLUN     uint8 // Maximum Logical Unit Number (n-1 for n LUNs)
-	dev        machine.BlockDevice
-	blockCount uint32 // Number of blocks in the device
-	blockRatio uint32 // Number of real blocks per emulated block
-	blockSize  uint32 // Write block size of the device
-	readOnly   bool
+	maxLUN       uint8 // Maximum Logical Unit Number (n-1 for n LUNs)
+	dev          machine.BlockDevice
+	blockCount   uint32 // Number of blocks in the device
+	blockOffset  uint32 // Byte offset of the first block in the device for aligned writes
+	blockSizeUSB uint32 // Write block size as presented to the host over USB
+	blockSizeRaw uint32 // Write block size of the underlying device hardware
+	readOnly     bool
 
 	vendorID   [8]byte  // Max 8 ASCII characters
 	productID  [16]byte // Max 16 ASCII characters
@@ -85,7 +87,10 @@ func newMSC(dev machine.BlockDevice) *msc {
 	// Size our buffer to match the maximum packet size of the IN endpoint
 	maxPacketSize := descriptor.EndpointEP8IN.GetMaxPacketSize()
 	m := &msc{
-		buf:           make([]byte, maxPacketSize),
+		// Some platforms require reads/writes to be aligned to the full underlying hardware block
+		blockCache:    make([]byte, dev.WriteBlockSize()),
+		blockSizeUSB:  512,
+		buf:           make([]byte, dev.WriteBlockSize()),
 		cswBuf:        make([]byte, csw.MsgLen),
 		cbw:           &CBW{Data: make([]byte, 31)},
 		maxPacketSize: uint32(maxPacketSize),
@@ -135,12 +140,13 @@ func (m *msc) processTasks() {
 			cmd := m.cbw.SCSICmd()
 			switch cmd.CmdType() {
 			case scsi.CmdWrite:
-				m.scsiWrite(m.buf)
+				m.scsiWrite(cmd, m.buf)
 			case scsi.CmdUnmap:
 				m.scsiUnmap(m.buf)
 			}
 
-			// If we were waiting for the task buffer to empty, clear the flag
+			// Acknowledge the received data from the host
+			m.queuedBytes = 0
 			m.taskQueued = false
 			machine.AckEndpointRxMessage(usb.MSC_ENDPOINT_OUT)
 		}

src/machine/usb/msc/scsi.go

@@ -23,7 +23,7 @@ func (m *msc) scsiCmdBegin(b []byte) {
 
 	if m.totalBytes > 0 && m.cbw.isOut() {
 		// Reject any other multi-packet commands
-		if m.totalBytes > uint32(cap(m.buf)) {
+		if m.totalBytes > m.maxPacketSize {
 			m.sendScsiError(csw.StatusFailed, scsi.SenseIllegalRequest, scsi.SenseCodeInvalidCmdOpCode)
 			return
 		} else {
@@ -87,7 +87,7 @@ func (m *msc) scsiDataTransfer(b []byte) bool {
 			m.sendScsiError(csw.StatusFailed, scsi.SenseDataProtect, scsi.SenseCodeWriteProtected)
 			return true
 		}
-		return m.scsiQueueTask(b)
+		return m.scsiQueueTask(cmdType, b)
 	}
 
 	// Update our sent bytes count to include the just-confirmed bytes
@@ -129,7 +129,7 @@ func (m *msc) scsiCmdReadCapacity(cmd scsi.Cmd) {
 	// Last LBA address (big endian)
 	binary.BigEndian.PutUint32(m.buf[:4], m.blockCount-1)
 	// Block size (big endian)
-	binary.BigEndian.PutUint32(m.buf[4:8], m.blockSize)
+	binary.BigEndian.PutUint32(m.buf[4:8], m.blockSizeUSB)
 }
 
 func (m *msc) scsiCmdReadFormatCapacity(cmd scsi.Cmd) {
@@ -142,7 +142,7 @@ func (m *msc) scsiCmdReadFormatCapacity(cmd scsi.Cmd) {
 	// Number of blocks (big endian)
 	binary.BigEndian.PutUint32(m.buf[4:8], m.blockCount)
 	// Block size (24-bit, big endian)
-	binary.BigEndian.PutUint32(m.buf[8:12], m.blockSize)
+	binary.BigEndian.PutUint32(m.buf[8:12], m.blockSizeUSB)
 	// Descriptor Type - formatted media
 	m.buf[8] = 2
 }
@@ -210,6 +210,7 @@ func (m *msc) scsiCmdRequestSense() {
 	// Set the buffer size to the SCSI sense message size and clear
 	m.resetBuffer(scsi.RequestSenseRespLen)
 	m.queuedBytes = scsi.RequestSenseRespLen
+	m.totalBytes = scsi.RequestSenseRespLen
 
 	// 0x70 - current error, 0x71 - deferred error (not used)
 	m.buf[0] = 0xF0 // 0x70 for current error plus 0x80 for valid flag bit
@@ -241,28 +242,39 @@ func (m *msc) scsiCmdUnmap(cmd scsi.Cmd) {
 	}
 }
 
-func (m *msc) scsiQueueTask(b []byte) bool {
+func (m *msc) scsiQueueTask(cmdType scsi.CmdType, b []byte) bool {
 	// Check if the incoming data is larger than our buffer
-	if len(b) > cap(m.buf) {
+	if int(m.queuedBytes)+len(b) > cap(m.buf) {
 		m.sendScsiError(csw.StatusFailed, scsi.SenseIllegalRequest, scsi.SenseCodeInvalidFieldInCDB)
 		return true
 	}
 
-	// Save the incoming data in our buffer for processing outside of interrupt context
+	// Save the incoming data in our buffer for processing outside of interrupt context.
 	if m.taskQueued {
 		// If we already have a full task queue we can't accept this data
 		m.sendScsiError(csw.StatusFailed, scsi.SenseAbortedCommand, scsi.SenseCodeMsgReject)
 		return true
 	}
 
 	// Copy the queued task data into our buffer
-	m.buf = m.buf[:len(b)]
-	copy(m.buf, b)
-	m.queuedBytes = uint32(len(b))
-	m.taskQueued = true
+	start := m.queuedBytes
+	end := start + uint32(len(b))
+	m.buf = m.buf[:end]
+	copy(m.buf[start:end], b)
+	m.queuedBytes += uint32(len(b))
 
-	// Don't acknowledge the incoming data until we can process it
-	return false
+	switch cmdType {
+	case scsi.CmdWrite:
+		// If we're writing data wait until we have a full write block of data that can be processed.
+		if m.queuedBytes == uint32(cap(m.blockCache)) {
+			m.taskQueued = true
+		}
+	case scsi.CmdUnmap:
+		m.taskQueued = true
+	}
+
+	// Don't acknowledge the incoming data until we can process it.
+	return !m.taskQueued
 }
 
 func (m *msc) sendScsiError(status csw.Status, key scsi.Sense, code scsi.SenseCode) {