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 SMC calls forwarding framework #21

Closed
wants to merge 3 commits into from

Conversation

malus-brandywine
Copy link

@malus-brandywine malus-brandywine commented Oct 3, 2023

Added:

  • a framework for forwarding SMC calls to Secure Monitor,
  • SiP Service entry in the framework.

Added: - a framework for forwarding SMC calls to Secure Monitor,
- SiP Service entry in the framework.

Signed-off-by: Nataliya Korovkina <[email protected]>
@malus-brandywine
Copy link
Author

Design note: smc.c file provided an SMC router handle_smc(), which called individual Service handlers.

Now there's the place in the file, separated by conditional compilation, to list handlers of Services that require "physical" access (vs. emulation) to EL3 components. At the moment, only SiP Service handler was declared.

There's also common default service handler (unconditional forwarder) which can be assigned to be a handler for any of mentioned Services.

Also, by default, the Service handlers are initialized with NULL in the library, so vmm application should explicitly pick the default handler or implement own one.

wheither a default handler that unconditionally forwards calls to Secure Monitor:

extern handle_service_type handle_xxx;
handle_service_type handle_xxx = default_handle_service;
Copy link
Collaborator

Choose a reason for hiding this comment

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

In my opinion the user of the library should not have to do this, and instead just have a wrapper for setting the handler.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also thank you for writing comments for how to use the API.

Copy link
Author

Choose a reason for hiding this comment

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

In my opinion the user of the library should not have to do this, and instead just have a wrapper for setting the handler.

Agreed, that would be easier to use.

Copy link
Author

Choose a reason for hiding this comment

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

API changed

bool default_handle_service(uint64_t vcpu_id, seL4_UserContext *regs, uint64_t fn_number)
{

seL4_CPtr smc_cap = SMC_CAP;
Copy link
Collaborator

Choose a reason for hiding this comment

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

While it isn't the current case for the rest of the library, I would like the library to be agnostic to the seL4 environment it is in, this means that it should be up to the user to provide the library with what capability to use for SMC calls.

If it's not too hard for you, it'd be good if specifying the SMC cap was part of the API.

Copy link
Author

Choose a reason for hiding this comment

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

Right, good catch, I'll change it.

Copy link
Author

Choose a reason for hiding this comment

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

Done

Added:
- a framework for forwarding SMC calls to Secure Monitor,
- SiP Service entry in the framework.

Signed-off-by: Nataliya Korovkina <[email protected]>
@malus-brandywine
Copy link
Author

Number of added names (functions, vars) somewhat noticeable (and may increase eventually over time), so I added prefixes smc_ to everything newly added, otherwise it makes a zoo.

typedef bool (* smc_handle_service_type)(size_t, seL4_UserContext *, uint64_t);

/* Default handler of SiP service calls */
bool smc_default_handle_sip(size_t vcpu_id, seL4_UserContext *regs, uint64_t fn_number);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is no longer needed right?

Copy link
Author

Choose a reason for hiding this comment

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

Precisely! Fixed.

    Added:
    - a framework for forwarding SMC calls to Secure Monitor,
    - SiP Service entry in the framework.

Signed-off-by: Nataliya Korovkina <[email protected]>
@Ivan-Velickovic Ivan-Velickovic mentioned this pull request Oct 6, 2023
@malus-brandywine
Copy link
Author

Ping. Ping.

@Ivan-Velickovic
Copy link
Collaborator

Ping. Ping.

Will get to it in the next couple days, thanks for the reminder.

@Ivan-Velickovic
Copy link
Collaborator

Will get to it in the next couple days, thanks for the reminder.

Well looks like a couple days turned into a couple months, sorry about that.

I've made #119 which takes your changes and rebases, I made some minor changes as well.

Thanks for the PR and I'll try be more responsive next time :)

@malus-brandywine
Copy link
Author

Thank you!

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