Skip to content

Allow user provided function for Missing Entries in IDT #167

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

Closed
vinaychandra opened this issue Jul 5, 2020 · 5 comments · Fixed by #285
Closed

Allow user provided function for Missing Entries in IDT #167

vinaychandra opened this issue Jul 5, 2020 · 5 comments · Fixed by #285

Comments

@vinaychandra
Copy link
Contributor

Current IDT has missing which is the default interrupt handler. It would be super useful if it can be replaced with a user provided function along with an index.
This would help users in finding out which interrupt index is being thrown instead of creating a different handler for each of the possible indices.

@phil-opp
Copy link
Member

phil-opp commented Jul 6, 2020

Good suggestion! Unfortunately, the only way to do this that I'm aware of is to create wrapper functions for each index that then call into a common default handler function, passing the index as argument. This means that we can't do this through a normal method call. It is possible using a macro, though, as the macro can generate the required wrapper functions at compile time.

I created a prototype of such a proc macro in the idt-set-default-handler branch. I tried it for my blog_os project and it seems to work fine. These are the relevant parts of the code:

https://github.com/phil-opp/blog_os/blob/705435da8a476318bd480a5fb2b77a6a52f4c4e5/src/interrupts.rs#L43

https://github.com/phil-opp/blog_os/blob/705435da8a476318bd480a5fb2b77a6a52f4c4e5/src/interrupts.rs#L52-L62

Could you try whether this works for you?

@vinaychandra
Copy link
Contributor Author

Looks good to me. Couple of comments.

  • Can we document the parameters
  • Can the range be used for values < 32? It would be nice to document that behavior as well.

@phil-opp
Copy link
Member

phil-opp commented Jul 7, 2020

Yes, this definitely needs to be documented, I just wanted to finish the API design first.

Can the range be used for values < 32? It would be nice to document that behavior as well.

I just pushed a new version that is aware of the x86_64-specific exceptions and their error codes. This means that it now also works for values < 32. I also rewrote the macro to make the range parameter optional. If it is omitted, it defaults to setting all entries.

@katietheqt-old
Copy link
Member

I'm not sure how I feel about using a proc macro for this. It means we have to depend on the heavyweight syn crate during compilation, which is in some ways against the goals of this project. Is it infeasable to do this with a normal macro? They expand at compile time right?

@phil-opp
Copy link
Member

phil-opp commented Jul 7, 2020

I tried to use a normal macro first, but I couldn't find a good solution with it. The problem is that there are no compile time loops in normal macros to my knowledge, so would need to invoke the macro up to 256 times to create all the required wrapper functions. Maybe recursive macro expansion could work for this case, but it would still be a mess.

That being said, I fully understand your concerns about compile time. I just opened a draft pull request at #168 in order to evaluate the compile time impact of this on CI. If it is too big, we can think about making it an off-by-default cargo feature, or we find a better approach using normal macros.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants