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

RiscVPkg: Implement ResetSystem RuntimeService #6

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

Conversation

JohnAZoidberg
Copy link
Collaborator

@JohnAZoidberg JohnAZoidberg commented Feb 13, 2020

When executing the reset command in the UEFI shell, the debug message is printed but the VM gets stuck. Not sure whether that's my implementation fault or OpenSBI&QEMU.

To try this PR, the following patch also needs to be applied to edk2-platforms:

--- i/Platform/SiFive/U5SeriesPkg/FreedomU540HiFiveUnleashedBoard/U540.dsc
+++ w/Platform/SiFive/U5SeriesPkg/FreedomU540HiFiveUnleashedBoard/U540.dsc
@@ -427,7 +427,8 @@
   MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
   MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf {
     <LibraryClasses>
-      ResetSystemLib|MdeModulePkg/Library/BaseResetSystemLibNull/BaseResetSystemLibNull.inf
+      ResetSystemLib|RiscVPkg/Library/ResetSystemLib/ResetSystemLib.inf
   }
   EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf

@changab
Copy link
Owner

changab commented Feb 14, 2020

The same reason, access to MSCRATCH in S-Mode is invalid.
This is what we are going to outline the clear architecture, I will do that.
We should work on those tasks from the bottom to top layer. Otherwise, you may have to revise your code often. I will prioritize it according to the TODO you listed.

@JohnAZoidberg
Copy link
Collaborator Author

Right... that was stupid.
I changed it to make the shutdown ecall.
Now it properly calls into OpenSBI, however that function doesn't do anything: https://github.com/changab/edk2-platforms/blob/master/Platform/SiFive/U5SeriesPkg/FreedomU540HiFiveUnleashedBoard/Library/OpensbiPlatformLib/Platform.c#L182

@JohnAZoidberg
Copy link
Collaborator Author

JohnAZoidberg commented Feb 17, 2020

I think this is good and stable. It only uses the SBI ecall interface.
For other reset types we'll have to think what to do and talk to the RISC-V people.

[Defines]
INF_VERSION = 0x00010005
BASE_NAME = ResetSystemLib
FILE_GUID = 3eff6057-1116-4dcb-837e-c0ef1a120ab1
Copy link
Owner

Choose a reason for hiding this comment

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

This is the newly generated GUID, right? Just make sure this is not duplicate with others.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, of course ;)
I have the feeling you have been burnt by this mistake in the past? 😆

Copy link
Owner

Choose a reason for hiding this comment

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

hah, not really. just people sometime forget to use a new one. :)

@changab
Copy link
Owner

changab commented Feb 17, 2020

Ok. Please keep this in the pocket. Send out to edk2-mailing list once edk2 RISC-V port gets into master branch.

@changab
Copy link
Owner

changab commented Feb 17, 2020

For other reset types we'll have to think what to do and talk to the RISC-V people.

sure. We may have to ask them to create a new extension for shutdown with parameters of reset types, this replaces original Shutdown() API.

@JohnAZoidberg
Copy link
Collaborator Author

Atish has created a draft extension for system reset: avpatel/riscv-sbi-doc@71d1870

We might need to help get this through. He said that it might be easier to get accepted if we present a strong use-case.

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.

2 participants