-
Notifications
You must be signed in to change notification settings - Fork 7
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
IOMMU cleanup #76
base: master
Are you sure you want to change the base?
IOMMU cleanup #76
Conversation
Two IOMMU headers are not really needed. Move the contents of iommu_defs.h into iommu.h. Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
The code to disable mem protection and setup the IOMMU should be called on both the Linux and Multiboot2 launch paths. Both paths now call this common function. Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
This cleans up the code in main.c a bit and makes future changes to this functionality cleaner too. Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
|
||
#define MEMPROT_CR 0x384 | ||
|
||
#define MEMPROT_EN 1<<0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs brackets, otherwise ~MEMPROT_EN
becomes (~1)<<0
, same for DEV_CR_SL_DEV_EN_MASK
above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, I copied the problem to the new MEMPROT_EN define. So DEV_CR_SL_DEV_EN_MASK was already had the issue so how was this working? The disabling of DEV on the APU systems should have failed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was explicitly bracketed in the point it was used: https://github.com/TrenchBoot/landing-zone/pull/76/files#diff-06fe7dc01e61244778e4a79b20e826b83073d3dc18d726b5fa7a1ec5cf87a156R157
Doing it in the header file instead is safer, too many parentheses only hurt eyes but not the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes I see. I guess I should make everything consistent.
/* | ||
* Now in 64b mode, paging is setup. This is the launching point. We can | ||
* now do what we want. First order of business is to setup IOMMU to cover | ||
* all memory. At the end, trampoline to the PM entry point which will | ||
* include the Secure Launch stub. | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is obsolete. Amongst other things, we might be in 32bit mode. Seeing as the hunk has moved by virtue of the introduction of iommu_setup(), I'd just drop it.
Some IOMMU code cleanup: