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

Document new PT_AARCH64_MEMTAG_MTE segment #148

Merged
merged 5 commits into from Aug 11, 2022
Merged

Document new PT_AARCH64_MEMTAG_MTE segment #148

merged 5 commits into from Aug 11, 2022

Conversation

luis-machado-arm
Copy link
Contributor

Add PT_AARCH64_MEMTAG_MTE to the table of AARCH64 segment types and document
its purpose.

Add PT_AARCH64_MEMTAG_MTE to the table of AARCH64 segment types and document
its purpose.

A segment of type ``PT_AARCH64_ARCHEXT`` (if present) contains information describing the architecture capabilities required by the executable file. Not all platform ABIs require this segment; the Linux ABI does not. If the segment is present it must appear before segment of type ``PT_LOAD``.

``PT_AARCH64_UNWIND`` (if present) describes the location of a program’s exception unwind tables.

``PT_AARCH64_MEMTAG_MTE`` segments (if present) hold MTE memory tags for a particular memory range. The data is packed as 2 MTE tags per byte. There can be multiple ``PT_AARCH64_MEMTAG_MTE`` segments in a core file.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to check my understanding this Program Header is only expected to be present in a Core File? Which is why it would be still be appropriate to write "Reserved for" in the table above.

If that is the case, I think it would be worth mentioning that the segment type can only appear in Core files. Is there a canonical reference to the Core file format we can add as a reference, I found that the ELF type is ET_CORE but it wasn't a very authoratitive reference.

I think it may be better to describe the format of the program header in a different document. For example if Core files are Linux/Android specific then maybe the sysvabi64 (https://github.com/ARM-software/abi-aa/blob/main/sysvabi64/sysvabi64.rst) is a better place for the details, or perhaps even the GDB documentation? You'd then only need to reference that from here.

One thing I can't work out is how the memory range is deduced. Is it [p_vaddr, p_vaddr + p_memsz) and the location of the tags is [p_offset, p_offset + p_filesz)? May be worth being explicit. Could also be useful to describe the order within the byte and whether it is always the same or different for big-endian.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to check my understanding this Program Header is only expected to be present in a Core File? Which is why it would be still be appropriate to write "Reserved for" in the table above.

That's correct, right now we only want to use it for core files. There has been some ideas about having memory tag data that would be used to initialize memory on program load, but that's is only an idea. I didn't want to commit to locking this to core files. What do you think?

If that is the case, I think it would be worth mentioning that the segment type can only appear in Core files. Is there a canonical reference to the Core file format we can add as a reference, I found that the ELF type is ET_CORE but it wasn't a very authoratitive reference.

Unfortunately core file formats are not very well documented. It seems like nobody wants to own it.

I think it may be better to describe the format of the program header in a different document. For example if Core files are Linux/Android specific then maybe the sysvabi64 (https://github.com/ARM-software/abi-aa/blob/main/sysvabi64/sysvabi64.rst) is a better place for the details, or perhaps even the GDB documentation? You'd then only need to reference that from here.

This particular format is already documented in the Linux Kernel (Documentation/arm64/memory-tagging-extension.rst - Core Dump Support).

One thing I can't work out is how the memory range is deduced. Is it [p_vaddr, p_vaddr + p_memsz) and the location of the tags is [p_offset, p_offset + p_filesz)? May be worth being explicit. Could also be useful to describe the order within the byte and whether it is always the same or different for big-endian.

The memory range is [p_vaddr, p_vaddr + p_memsz), and the tags are within the [p_offset, p_offset + p_filesz) range. So your understanding is correct.

The order within the byte is always the same, and it goes from the first memory tag granule from the memory range to the last.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's correct, right now we only want to use it for core files. There has been some ideas about having memory tag data that would be used to initialize memory on program load, but that's is only an idea. I didn't want to commit to locking this to core files. What do you think?

I think the benefit of saying only for core files, or at least "at present, only for core files" is that it means that readers not concerned with core files can look away now. When I first looked at it I put my static linker glasses on and thought how would the linker generate these program headers, for example would the compiler generate sections that the linker would aggregate, or would the linker generate the sections from static relocations?

My guess is that we'd be much more likely to use dynamic relocations (even when static-linking) to initialise tags, much the same way that capabilities are initialized in Morello.

My preference would be to just reserve the Program Header in this document and reference the Linux kernel documentation for the details. For example:

PT_AARCH64_MEMTAG_MTE segments (if present) hold MTE memory tags for a particular memory range. At present they are defined for core dump files of type ET_CORE. A description of the program header and contents can be found in [MTEEXTENSIONS_].

With MTEEXTENSIONS defined in the references section, pointing to https://www.kernel.org/doc/html/latest/arm64/memory-tagging-extension.html#core-dump-support

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the input Peter. Let me rework it.

Choose a reason for hiding this comment

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

I think the text should be PT\_AARCH64\_MEMTAG\_MTE (ie a '\' before the final '_'). Otherwise, looks good to me.

Add PT_AARCH64_MEMTAG_MTE to the table of AARCH64 segment types and document
its purpose.
Add PT_AARCH64_MEMTAG_MTE to the table of AARCH64 segment types and document
its purpose.
Add PT_AARCH64_MEMTAG_MTE to the table of AARCH64 segment types and document
its purpose.
Copy link
Contributor

@smithp35 smithp35 left a comment

Choose a reason for hiding this comment

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

Thanks for the update. I'm happy with the changes.

All the '_' characters need to use '\' in the table of segment
types.
@luis-machado-arm
Copy link
Contributor Author

Sorry. I missed the bit Richard mentioned above. Fixed now.

@luis-machado-arm
Copy link
Contributor Author

Hi Peter/Richard. Could either of you approve this change again, after the typo fix?

Copy link
Contributor

@smithp35 smithp35 left a comment

Choose a reason for hiding this comment

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

LGTM. I think the previous approval still stood, but happy to do it again just to be sure.

@MaskRay
Copy link
Contributor

MaskRay commented Jun 2, 2022

There is a glibc patch adding PT_AARCH64_MEMTAG_MTE (https://sourceware.org/pipermail/libc-alpha/2022-June/139314.html). The Linux kernel added the value earlier. ISTM it'd be better for the ABI change to be pushed earlier than kernel/glibc/etc.

@stuij stuij merged commit 7cb0695 into ARM-software:main Aug 11, 2022
@stuij
Copy link
Member

stuij commented Aug 11, 2022

Sorry for the lag on this @luis-machado-arm. Just noticed this one was still lingering in the system.

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