Conversation
manish-pandey-arm
force-pushed
the
mp/std_operations
branch
from
7c9aa98 to
b5e0915
Compare
source/transfer_list.rst
Outdated
| #. Add `align8(te.hdr_size + te.data_size)` to `te_base_addr`. | ||
|
|
||
|
|
||
| Adding a void TE |
There was a problem hiding this comment.
I kind of agree with this but would go one step further. If we're adding an "Overwriting a TE" section then I think this it's unnecessary to have this operation also embedded within the existing "Adding a TE" section. So I would remove that bit from "Adding a TE", rename it to "Append a TE", and inline this "Overwriting part of the TL with a void TE" operation within the "Overwriting a TE" section.
manish-pandey-arm
force-pushed
the
mp/std_operations
branch
from
b5e0915 to
9b189f0
Compare
manish-pandey-arm
force-pushed
the
mp/std_operations
branch
from
9b189f0 to
ee8818f
Compare
|
This set of changes looks good to me. The overarching comment I have (which is unrelated to this PR) is whether the Standard Operation Section is needed now that we have a library implementation. |
Signed-off-by: Manish Pandey <manish.pandey2@arm.com>
manish-pandey-arm
dismissed stale reviews from jmarinho and sjg20
via
d201a3e
manish-pandey-arm
force-pushed
the
mp/std_operations
branch
2 times, most recently
from
d201a3e to
fcabbcb
Compare
jmarinho
left a comment
jmarinho
left a comment
There was a problem hiding this comment.
small set of nits, but otherwise LGTM
|
|
||
| #. If `has_checksum`, subtract `te.hdr_size + data_size` bytes starting at `te_base_addr` from `tl.checksum` | ||
|
|
||
| #. Set `te.tag_id` (`te_base_addr + 0x0`) to `0x0` (XFERLIST_VOID) |
There was a problem hiding this comment.
I'm in two minds about providing the offsets explicitly (e.g. te_base_addr + 0x3). On one hand it's a useful reference for the implementer, but it may prove a bit of a burden to maintain down the line. As I understand it, the header layout is liable to change. If it does, we'd have to fix all the places in the spec where we reference hard-coded offsets. Given the structure is well defined, I think it's probably sufficient to just refer to it directly (i.e. te.tag_id).
There was a problem hiding this comment.
We can do this universally when migrating to appendinx ?
Steps for adding a void TE was already described in creating a new TE but this operation can be used at multiple places so introduce it as stand alone operation so that it can be referenced elsewhere. Signed-off-by: Manish Pandey <manish.pandey2@arm.com>
Signed-off-by: Manish Pandey <manish.pandey2@arm.com>
manish-pandey-arm
force-pushed
the
mp/std_operations
branch
from
fcabbcb to
53edca2
Compare
Overwrite operation is used to replace content of a given TE Signed-off-by: Manish Pandey <manish.pandey2@arm.com>
Updated the "Standard Operations" section to consistently use `te.hdr_size` instead of the hardcoded `0x8`. Also add a note explaining the assumption of `te.hdr_size = 0x8` and its potential future configurability. This improves consistency, avoids magic numbers, and prepares the format for variable-sized TE headers. Signed-off-by: Manish Pandey <manish.pandey2@arm.com>
manish-pandey-arm
force-pushed
the
mp/std_operations
branch
from
53edca2 to
740dfd0
Compare
|
|
||
| Overwriting a TE | ||
| ^^^^^^^^^^^^^^^^ | ||
|
|
There was a problem hiding this comment.
It would be useful to define or motivate this more clearly. Why would one want to do this?
There was a problem hiding this comment.
Overwriting a TE is a space-efficient way to update the data associated with an existing tag without changing its position in the transfer list. This is useful in scenarios where the tag ID remains valid, and only the payload needs to be refreshed. It avoids the overhead of allocating new space and modifying the TL structure unless the new data exceeds the original size, in which case a replacement strategy is triggered.
@sjg20 Do you these comments in spec or you happy with current version ?
danh-arm
left a comment
danh-arm
left a comment
There was a problem hiding this comment.
Just 1 minor comment that can be addressed later
|
|
||
| .. note:: | ||
| After relocating a TL, implementations should consider scrubbing the old TL memory if it contains | ||
| any secrets that might be accessible to later untrusted software. |
There was a problem hiding this comment.
minor: I think this note also applies in the case of Removing a TE or Overwriting a TE. This could perhaps be duplicated or placed in a more general section?
danh-arm
dismissed stale reviews from jwerner-chromium and sjg20
Comments have been addressed in latest version
6 participants
Provide details around security considerations and add new standard operations like "Removing a TE" and "Overwriting a TE" as mentioned in #62