Comments for "https://os.phil-opp.com/hardware-interrupts/"

[utterances-bot]

This is a general purpose comment thread for the “Hardware Interrupts” post.

[ZerothLaw]

Hi, I'd be interested in writing the small scancode_set1 crate! You can reach me at trinioler at gmail.com if you want to continue the discussion out of band. :)

[phil-opp]

@ZerothLaw Awesome, I'll write you an email!

[rybot666]

I’m following your tutorials, and (am currently) creating an API for device drivers. Thanks for the amazing work!

[phil-opp]

@rybot666 You're welcome! What are your plans for the API design?

[donald-pinckney]

I'm curious if you have any plans for how to do APIC, as it doesn't look like there is a crate for it.

[rybot666]

@phil-opp I’m planning on using traits to implement it (I’ve got a Driver trait that holds info, and an InterruptHandler trait that allows drivers to register interrupt handlers). You can have a look, but the source code is a bit of a mess. The API for drivers is in master, because I forgot to make a PR. ;)

EDIT: Forgot the link

[thejpster]

I wrote a crate called pc-keyboard which handles raw access to a UK 105 key PS/2 keyboard generating scan code set 2. Happy for patches to make it work with set 1 and other keyboard layouts.

[phil-opp]

@thejpster Awesome! cc @ZerothLaw

[phil-opp]

@rybot666 Ah, so that devices can register and unregister for certain interrupts at runtime?

[phil-opp]

@donald-pinckney Nothing specific yet, just some vague ideas.

[rybot666]

@phil-opp Exactly. Currently (as in right now) trying to get rid of errors about not being able to send Fn(...) types across threads

[phil-opp]

@rybot666 I think you need to add the Send bound to your register function to allow only function types that can be safely sent across threads.

[rybot666]

@phil-opp Yea, I have and it seems to have fixed that. Although my current device isn't very good (an iPad and the Github editor).

[ZerothLaw]

I wrote a crate called pc-keyboard which handles raw access to a UK 105 key PS/2 keyboard generating scan code set 2. Happy for patches to make it work with set 1 and other keyboard layouts.

Hey @thejpster I'll see about sending you a PR with scancode set 1 sometime soon.

[npip99]

"As already mentioned, the 8259 APIC has been superseded by the APIC"

Perhaps "8259 PIC" was intended here?

[phil-opp]

@npip99 Thanks, fixed in 68f8d23.

[GrayJack]

Isn't bad that the pic8259_simple crate hasn't been updated in 2 years, considering that the owner itself said "I am not qualified to have written this crate"?

[phil-opp]

@GrayJack I understand your concerns, but the crate is rather small and I don't see anything problematic in the code. Also, I contacted the owner before I used the crate for this post, and they said that they're still willing to maintain this crate.

[JoshMcguigan]

There is a small mistake in the code examples in the implementation and enabling interrupts sections. The last code example under Implementation uses the PIC in main.rs, but does not show the import statement. The first code example under Enabling Interrupts shows the import statement as a new line.

[phil-opp]

@JoshMcguigan Thanks! Fixed in 2e16cc4.

[bIgBV]

I am really enjoying this series. I'm currently working on adding a simply network layer utilizing virtio to xv6 and it's been really cool comparing the two implementations. I'm learning a ton from the in depth explanations as well since there in't a lot of information regarding this except for the osdev wiki and the intel manuals.

[phil-opp]

@bIgBV Great to hear that!

[krsoninikhil]

As Rust beginner, this line if let Some(key) = key { in 'Interpreting the Scancodes' section is confusing to me, Some(key) seems like Some(Some('1') if scancode is 0x02. Could a different variable name be used in place of first key like if let Some(n) = key {? Or is it common to use same variable while pattern matching?

[Androthi]

a note for anyone using the latest version (0.5.0) of pc-keyboard, you'll need to make a change.

use pc_keyboard::{Keyboard, ScancodeSet1, DecodedKey, layouts, HandleControl }; // new

...

Mutex::new(Keyboard::new(layouts::Us104Key, ScancodeSet1, HandleControl::Ignore)); //new

which causes some odd behavior in the number pad (might be bug in pc-keyboard.

or

Mutex::new(Keyboard::new(layouts::Us104Key, ScancodeSet1, HandleControl::MapLettersToUnicode));

which works better, but now the control key is a modifier to be used with another key and returns unicode. which means you lose direct access to the control key.

[phil-opp]

@krsoninikhil

As Rust beginner, this line if let Some(key) = key { in 'Interpreting the Scancodes' section is confusing to me,

This feature is called pattern matching and is normally used in combination with a match expression and an enum. However, Rust also supports patterns any many other places, including the if let clause. The if let statement does the following:

• Check whether the right side of the = sign matches the pattern on the left side
• If not, skip the block (or execute the else clause if present)
• If yes, destructure the right side and assign the variables used in the pattern, then execute the if block

You can use any variable name in the pattern, including the same variable name as used on the right side. If you use the same name, the original variable is shadowed and only the new variable can be used inside the block. For more information see the official documentation which includes a shadowed variable in the example (Ok(age) = age).

So to answer the second part of your question: Yes, a different variable name like n would work too (if let Some(n) = key). And yes, shadowing in such situations is somewhat common in Rust because it provides a nice way to keep using the original variable name without needing to unwrap it. But this is entirely personal preference, so if you prefer using a different variable name that's perfectly fine too.

I hope this answers your question! I opened issue https://github.com/phil-opp/blog_os/issues/572 to add an explanation like the above to the post.

[krsoninikhil]

I read about pattern matching and if let from official docs (here) and didn't notice about shadowing. Thanks for taking time to explain it. Adding one line about shadowing in post would definitely help.

[phil-opp]

You're welcome! I will add such a line when I have some spare time.

Edit: Done in ac16f2ed55f6a709a0ed1e7ec6e0150d5240d6d8.

[12101111]

I wrote an "operating system" booting from UEFI that can output characters to the screen (via GOP) and UART. According to the previous article, I made the interrupt work.

I noticed after booting from UEFI, 8259 PIC don't work. I found UEFI firmware will enable APIC instead of 8259 PIC.

It seems I need a lot of work to get the keyboard working (ACPI Table, APIC, USB HID, etc.).

[Ngalstyan4]

Thanks a lot for this series! I am working on an OS research project as a CS undergraduate in Princeton and these have been very useful for me.

I noticed that you always call notify_end_of_interrupt right before the interrupt function returns. The function name indicates that should be the intended way to call it but the function simply sends an interrupt acknowledgement (outw(0x20, 0x20)) in which case do you think it would make more sense to call it right when we enter the interrupt handler function?

When calling it at the end we risk losing further interrupts if the handler takes too long to execute. This can be particularly likely and dangerous in case of timer interrupts as it can result in weird behaviour which is what happened in my case.

[phil-opp]

@12101111 That's unfortunate. I was aware of the missing VGA text buffer support for UEFI, but I didn't know about that the 8259 is not supported. I don't know what the best solution for this is yet, but I will think about it. Thanks for reporting!

[phil-opp]

@Ngalstyan4 You're welcome! I'm glad to hear that it's useful.

As I understand it, the EOI signal tells the interrupt controller that you're done with the interrupt request and ready for the next one. So for the keyboard controller, I think that you should first read the scancode before sending the EOI. For the timer interrupt we don't need additional data from the PIT, so we can send the EOI right at the beginning.

Independently of when you send the EOI signal, your interrupt handler should be as short as possible. If it takes too long to execute (and is not reentrant), you will have missed interrupts even when sending the EOI signal at the beginning. Imagine that the interrupt hander runs 2ms and a timer interrupt occurs every 1ms: then the interrupt controller would need to queue an unlimited number of interrupt requests, which is not possible.

The best solution to this is to keep the interrupt handlers as short as possible. For example, instead of executing your scheduling code in the timer interrupt handler, add a scheduling task to a work list that is executed after the interrupt handler returns.

[aufflick]

Is there a possible deadlock in keyboard_interrupt_handler since notify_end_of_interrupt will get called before the KEYBOARD lock falls out of scope?

[ethindp]

I don't think so. My understanding of Spin::Mutex is that when locked, it will release the mutex after it leaves scope. The only possible way I could see a deadlock occurring is if the PIC is faulty or two interrupts for the keyboard get sent simultaneously. Even then, the handler would be called twice, and both calls woudl happen almost together, and therefore, one would wait while the other finished. Then that one would get handled, kind of like a queue. Using this setup I can't really imagine an actual deadlock occurring in practice. (I rewrote my keyboard handler so this doesn't apply to me, but my handler is fully asynchronous and does involve locks, so if there is a deadlock that can be caused, I'd love to know a definitive answer to this question too.)

On 7/21/19, Mark Aufflick notifications@github.com wrote:

Is there a possible deadlock in keyboard_interrupt_handler since notify_end_of_interrupt will get called before the KEYBOARD lock falls out of scope?

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/phil-opp/blog_os/issues/480#issuecomment-513593696

-- Signed, Ethin D. Probst

[phil-opp]

@aufflick The notify_end_of_interrupt function only signals to the interrupt controller that we're finished handling the interrupt, so that it can send the next one when possible. However, interrupts stay disabled on the CPU by default until the interrupt handler returns. So even if the interrupt controller raises an interrupt right after the notify_end_of_interrupt call, the CPU would not start to handle it until the handler returns.

@ethindp We currently use only a single core and the legacy PIC, so no two interrupts can run at the same time. So a deadlock could occur if we the "keep interrupts enabled" flag for the IDT entry of the keyboard handler.

[ethindp]

Ah. I could also see something liek a deadlock happening once we use an APIC and/or enter SMP mode -- that will be problematic, and I'll have to rewrite my current IDT code. Ug

On 7/22/19, Philipp Oppermann notifications@github.com wrote:

@aufflick The notify_end_of_interrupt function only signals to the interrupt controller that we're finished handling the interrupt, so that it can send the next one when possible. However, interrupts stay disabled on the CPU by default until the interrupt handler returns. So even if the interrupt controller raises an interrupt right after the notify_end_of_interrupt call, the CPU would not start to handle it until the handler returns.

@ethindp We currently use only a single core and the legacy PIC, so no two interrupts can run at the same time. So a deadlock could occur if we the "keep interrupts enabled" flag for the IDT entry of the keyboard handler.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/phil-opp/blog_os/issues/480#issuecomment-513674009

-- Signed, Ethin D. Probst

[phil-opp]

As long as we don't make the keyboard handler nested by reenabling interrupts before it is finished, I don't think it will cause issues. And there is no reason to do so because we want to handle keyboard input sequentially anyway.

[64]

There shouldn’t be a deadlock here even with SMP. If two interrupts happen on different CPUs at the same time, one will take the keyboard lock and the other will be forced to wait until it’s finished.

APIC has a similar EOI mechanism but uses MMIO instead of in/out, as the 8259 PIC does - it shouldn’t be that hard to replace. Basically the main thing you need to avoid is locking something twice from the same CPU.

As a side note, there’s really no reason for the PIC to take a lock here as far as I see it, it’s not mutating anything.

[phil-opp]

As a side note, there’s really no reason for the PIC to take a lock here as far as I see it, it’s not mutating anything.

It's mutating the internal state of the interrupt controller, isn't it?

[64]

Yes but it doesn’t need to - it treats writing to a port as a mutation. It’s not modifying any actual data.

[phil-opp]

I guess that a question of the mental model. What I meant is that the port write has a side effect, even without modifying any data. Before it, the internal state of the interrupt controller is that the interrupt is still processed. Afterwards, the interrupt controller is in a different internal state, namely that the handling of the interrupt has finished. For me this sounds like some internal register of the interrupt controller changes its value as a result of that port write.

[ethindp]

How would we even set up aPIC and SMP support anyway? I've read up on th aPIC and SMP on OSDev but can't: 1) figure out how to read ACPI tables (I know I can read pointers and all that, but if I read the memory at 0x40E (I think that's the right one), will I get a memory address which I then need to read at?). 2) Enable SMP. I've read the Intel SDMs on this, but still don't know how to send the appropriate commands to the processors.

On 7/22/19, Philipp Oppermann notifications@github.com wrote:

I guess that a question of the mental model. What I meant is that the port write has a side effect, even without modifying any data. Before it, the internal state of the interrupt controller is that the interrupt is still processed. Afterwards, the interrupt controller is in a different internal state, namely that the handling of the interrupt has finished. For me this sounds like some internal register of the interrupt controller changes its value as a result of that port write.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/phil-opp/blog_os/issues/480#issuecomment-513830153

-- Signed, Ethin D. Probst

[64]

There is no easy walkthrough that I’m aware of. OSDev wiki explains what you need to do to get the ACPI tables. Here’s a useful reference that helped me with SMP before: https://nemez.net/osdev/lapic.txt

[stuaxo]

This is great, maybe I can finally output a keyboard handler that handles chords.. ie when you hold down DOWN and RIGHT, it will continuously repeat DOWN then RIGHT so diagonal scrolling comes for free.

[JohnDowson]

Is there any reason to implement InterruptIndex::as_usize as usize::from(self.as_u8()) as opposed to self as usize?

[ethindp]

Getting the APIC and all of that working shouldn't be too difficult (the x86 crate lets us set up the XAPIC or X2APIC, as well as sending IPIs. I don't know how to tell the processors where to start execution though. And it doesn't help with ACPI either.

[phil-opp]

@JohnDowson Casting with as is more dangerous because it is also possible to accidentally truncate values. For example, consider 500u32 as u8, which will silently change the number. For this reason, it is generally recommend to use the From/Into traits if possible and the TryFrom/TryInto traits otherwise. This way, it is clear from the code whether a truncation is desired or not. Also, if you switch the integer types at some, the compiler will throw errors instead of silently introducing a potential truncation.

[phil-opp]

@ethindp I plan to look into the APIC more closely soon as part of adding UEFI support. I'll update the blog with some instructions then.

[SvenGroot]

I'm having an issue that as soon as I add the PIC initialize code and enable interrupts, the double fault message doesn't print fully. Instead, it seems to hang in the middle of the InterruptStackFrame structure. If I don't include the structure in the output, it prints the whole thing.

I cloned your blog_os repo and checkout out the post-07 branch, and if I induce a double fault there (by causing a stack overflow in _start), the system apparently triple faults (reboots endlessly), with again only a partial double fault message printed. There too not including the stack frame in the output causes the reboot loop to go away.

Any idea what could be causing this?

[SvenGroot]

Ok, I had a thought after noticing that this didn't happen when running the release profile. I tried increasing the size of the double fault handler's stack, and that solved the issue. It seems that when compiling without optimizations, the Debug formatter for InterruptStackFrame overflows the 4kb stack. What that ends up doing (hang vs. reboot loop) seems to depend on what's at the address it ends up accessing after that.

[phil-opp]

@SvenGroot Thanks for reporting! You're completely right that this is caused by a stack overflow on the double fault stack. See https://github.com/phil-opp/blog_os/issues/449#issuecomment-667638975 for more information. I now pushed https://github.com/phil-opp/blog_os/commit/0425bd3c819bd26910c4e82a7a24c2a5126d4116 and https://github.com/phil-opp/blog_os/commit/817e36c064acbc5edaeabda55161d625e4c24a23 to increase the stack size to 4096 * 5 bytes, which should prevent this issue.