search

au-ts / sddf

A collection of interfaces, libraries and tools for writing device drivers for seL4 that allow accessing devices securely and with low overhead.
Other
17 stars 14 forks source link

#176 (I2C) Remove redundant data tokens from transport layer #210

Closed omeh-a closed 2 days ago

omeh-a commented 1 month ago

This pull request fixes a significant inefficiency in the transport layer - previously, data was transferred by having every write arranged as follows

|WRITE|DATA|[data]|DATA|[data] ... | DATA|

... where each DATA was an identical token indicating that the byte after this is to be treated as a payload to read or write.

This is more severe in the case of a read, where it would be:

|READ|DATA|DATA|DATA|DATA|DATA|DATA|DATA| ..... DATA_END|

This has been refactored to remove this redundancy minimally while retaining the same transport layer semantics - i.e. a buffer is shared for the request and the return data. We now transmit a READ or WRITE followed by a buffer size N between 0 and 255. The following N bytes are treated as payloads. This halves the amount of space required for write operations.

|WRITE|SIZE|DATA0| ... |DATA(n-1)|
|READ|SIZE|(N empty bytes)|

This still means that reads must be padded with empty bytes however, since the buffer that supplies instructions is also used to return data. This works by having the READ share the same semantics as write, but the N bytes after are simply ignored and can be arbitrary. There is still a performance benefit however, since we can simply skip these dead bytes when loading and unloading and this only means we waste some space in our block of allocated memory.

The transport layer can be further improved by decoupling the transmit and return path. A separate shared buffer for return data will enable a minimally-sized buffer to be allocated on request. This can be a discussion for later however :)

omeh-a commented 1 week ago

Disclaimer: the massive series of force pushes was because my git config was wonky and did awful things 😭

TristanCM347 commented 1 week ago 
|WRITE|DATA|[data]|DATA|[data] ... | DATA_END|
|READ|DATA|DATA|DATA|DATA|DATA|DATA|DATA| ..... DATA|

you didn't do this in the code but the last token of each should be swapped because DATA_END is only for READS in your pr description.

omeh-a commented 1 week ago 
|WRITE|DATA|[data]|DATA|[data] ... | DATA_END|
|READ|DATA|DATA|DATA|DATA|DATA|DATA|DATA| ..... DATA|

you didn't do this in the code but the last token of each should be swapped because DATA_END is only for READS in your pr description.

There's actually no DATA_END anymore at all :)