search

rust-osdev / x86_64

Library to program x86_64 hardware.
https://docs.rs/x86_64
Apache License 2.0
758 stars 130 forks source link

Allow user provided function for Missing Entries in IDT #167

Closed vinaychandra closed 2 years ago

vinaychandra commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

Looks good to me. Couple of comments.

phil-opp commented 4 years ago

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.

rybot666 commented 4 years ago

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 commented 4 years ago

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 https://github.com/rust-osdev/x86_64/pull/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.