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

[V2] SbsaQemu-rme: Add support for SBSA-rme platform #243

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mathieupoirier
Copy link

Hi Maintainers,

Please find herein the second revision of this set. Following our converstion from the first iteration, I am using a command line flag to enable RME support. I also took the initiative of adding some documentation to avoid having to look in the configuration files to remember the exact command line option to use.

Thanks for the time,
Mathieu

Add support for SBSA-rme platform by 1) pushing the start of the base
memory to 0x10043000000 to make room to the RMM and 2) increasing the
size of FLASH0 to accomotate the RMM.

Enabling RMM is done by specifying -D ENABLE_RME on the build command
line.  The name of the flag was chosen to reflect what is currently done
on TF-A.

Signed-off-by: Mathieu Poirier <[email protected]>
Platform/Qemu/SbsaQemu/SbsaQemu.dsc Show resolved Hide resolved
@@ -97,6 +97,10 @@ Create a directory $WORKSPACE that would hold source code of the components.
cd $WORKSPACE
build -b RELEASE -a AARCH64 -t GCC5 -p edk2-platforms/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
```
An RME aware SBSA system can be compiled by adding `-D ENABLE_RME` to the
command line above. In that case `bl1.bin` and `fip.bin` also have to be RME
aware. Please refer to TF-A instructions to generate RME enabled binaries.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add link to those instructions.

Copy link
Author

Choose a reason for hiding this comment

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

I am in the process of upstreaming the TF-A part and as such those instructions don't exists yet. I added this here so that we don't forget but I can also remove - it's up to you.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is better to have something like this than nothing ;D


# Space for 32 stacks
gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x1000007c000
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder can we have something like this:

gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|gArmTokenSpaceGuid.PcdSystemMemoryBase+0x7c000

This would remove a need for extra "if".

Copy link
Contributor

Choose a reason for hiding this comment

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

Checked. Such definition works so code can be a bit simpler.

Copy link
Contributor

Choose a reason for hiding this comment

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

@leiflindholm may know why we reserve space for 31 stacks (31 x 0x4000 = 0x7c000) when comment says about 32 of them (so 0x80000).

Copy link
Member

Choose a reason for hiding this comment

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

@hrw https://github.com/tianocore/edk2/blob/master/ArmPlatformPkg/Sec/AArch64/ModuleEntryPoint.S#L33
It's a weird pattern, but it adds up.

Ultimately, I guess we should only reserve the one stack these days? @ardbiesheuvel?

Platform/Qemu/SbsaQemu/SbsaQemu.fdf Show resolved Hide resolved
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.

3 participants