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

Add RW memory layout TE #36

Merged
merged 4 commits into from
Mar 13, 2024

Conversation

harrisonmutai-arm
Copy link
Contributor

@harrisonmutai-arm harrisonmutai-arm commented Feb 9, 2024

XFERLIST_RW_MEM_LAYOUT64 is used to specify an extent of read-write memory that a client can use. Note, in future if other memory types need representation seperate TE's should be defined (left in the layout description as a point of discussion).


This type defines the layout of a region of read-write memory that a client can
allocate into its memory map. If other memory types are required (i.e.
read-only memory), separate TE's should be defined.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds very specific to a single project (TF-A?), so it should probably go in the TE section for that. At least I wouldn't know from this description what to do with this entry if I encountered it in my firmware.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're probably right. The structure this TE represents is fairly simple:

typedef struct meminfo {
	uintptr_t total_base;
	size_t total_size;
} meminfo_t;

This to me seems adaptable to other contexts, although i'm yet to find any concrete examples. In any case, I've moved it to TF-A's namespace. If a more general use case materialises then we can consider adding it back.

source/transfer_list.rst Outdated Show resolved Hide resolved
XFERLIST_RW_MEM_LAYOUT64 is used to specify an extent of read-write memory
that a client can use. The TE pertains to TF-A and is relevant only for
64-bit architectures.

Signed-off-by: Harrison Mutai <harrison.mutai@arm.com>
extent of memory it has available to perform read-write operations on. BL2 maps
the memory described by the layout into its memory map during platform setup. If
other memory types are required (i.e. read-only memory) separate TE's should be
defined.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be useful to generalise this to include any 'next phase'??

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean? I refer to its usage in BL2 here only as a concrete example, but it very well could be used in other places.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, then please put 'For example, ...' in a separate paragraph so it is clear

**Read-Write Memory Layout Entry Layout (XFERLIST_RW_MEM_LAYOUT64)**

This entry type holds a structure that describes the layout of a read-write
memory region. TF-A defines this structure and uses it to convey to BL2 the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the sentences after the first be written as an example? It seems that this information could be used elsewhere

extent of memory it has available to perform read-write operations on. BL2 maps
the memory described by the layout into its memory map during platform setup. If
other memory types are required (i.e. read-only memory) separate TE's should be
defined.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, then please put 'For example, ...' in a separate paragraph so it is clear

Signed-off-by: Harrison Mutai <harrison.mutai@arm.com>
Copy link
Contributor

@danh-arm danh-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good. Just minor comments.

source/transfer_list.rst Outdated Show resolved Hide resolved

* - size
- 8
- hdr_size + 8
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same again. use 0x8.

Signed-off-by: Harrison Mutai <harrison.mutai@arm.com>
danh-arm
danh-arm previously approved these changes Mar 11, 2024
sjg20
sjg20 previously approved these changes Mar 12, 2024
Copy link
Contributor

@sjg20 sjg20 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (with nit)

source/transfer_list.rst Outdated Show resolved Hide resolved
Signed-off-by: Harrison Mutai <harrison.mutai@arm.com>
@manish-pandey-arm manish-pandey-arm merged commit 83d2fbb into FirmwareHandoff:main Mar 13, 2024
2 checks passed
coreboot-org-bot pushed a commit to coreboot/arm-trusted-firmware that referenced this pull request Apr 26, 2024
`TL_TAG_RW_MEM_LAYOUT64` encapsulates a structure used to represent the
layout of a region of memory on 64-bit platforms [2]. In TF-A this is
used to represent the `meminfo_t` structure passed between BL1 and BL2,
which provides BL2 with information about the space it has available in
BL2. The `TL_TAG_TB_FW_CONFIG` entry type encapsulates the trusted
bootloader firmware configuration [1].

[1] FirmwareHandoff/firmware_handoff#37
[2] FirmwareHandoff/firmware_handoff#36

Change-Id: I1e0eeec2ec204e469896490d42a9dce9b1b2f209
Signed-off-by: Harrison Mutai <harrison.mutai@arm.com>
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.

None yet

5 participants