Page MenuHomePhabricator

[X86] Enable compilation of user interrupt handlers.
AcceptedPublic

Authored by tianqing on Thu, Apr 1, 1:23 AM.

Details

Summary

Add __uintr_frame structure and use UIRET instruction for functions with
x86 interrupt calling convention when UINTR is present.

Diff Detail

Event Timeline

tianqing created this revision.Thu, Apr 1, 1:23 AM
tianqing requested review of this revision.Thu, Apr 1, 1:23 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptThu, Apr 1, 1:23 AM

A user interrupt is different than a regular interrupt right? It doesn't make sense that we would change the behavior of the interrupt calling convention just because the the user interrupt instructions are enabled. That would occur just from passing a -march for a newer CPU wouldn't it?

LuoYuanke added a comment.EditedThu, Apr 1, 8:10 AM

A user interrupt is different than a regular interrupt right? It doesn't make sense that we would change the behavior of the interrupt calling convention just because the the user interrupt instructions are enabled. That would occur just from passing a -march for a newer CPU wouldn't it?

Maybe need support another attribute "attribute ((user_interrupt))" for functions? However this is what gcc does (https://gcc.godbolt.org/z/8ojTMG6bT).

A user interrupt is different than a regular interrupt right? It doesn't make sense that we would change the behavior of the interrupt calling convention just because the the user interrupt instructions are enabled. That would occur just from passing a -march for a newer CPU wouldn't it?

Maybe need support another attribute "attribute ((user_interrupt))" for functions? However this is what gcc does (https://gcc.godbolt.org/z/8ojTMG6bT).

Since there won't be both user interrupt handler and kernel interrupt handler in the source, there is no need for another
attribute. We discussed that kernel might need to use UINTR instructions. We decided that kernel could use inline asm
statements if needed.

A user interrupt is different than a regular interrupt right? It doesn't make sense that we would change the behavior of the interrupt calling convention just because the the user interrupt instructions are enabled. That would occur just from passing a -march for a newer CPU wouldn't it?

Maybe need support another attribute "attribute ((user_interrupt))" for functions? However this is what gcc does (https://gcc.godbolt.org/z/8ojTMG6bT).

Since there won't be both user interrupt handler and kernel interrupt handler in the source, there is no need for another
attribute. We discussed that kernel might need to use UINTR instructions. We decided that kernel could use inline asm
statements if needed.

So if write kernel code and compile with -march=haswell today, I get IRET. If tomorrow I change my command line to -march=sapphirerapids, now my kernel interrupt code generates user interrupt instructions. That seems surprising.

A user interrupt is different than a regular interrupt right? It doesn't make sense that we would change the behavior of the interrupt calling convention just because the the user interrupt instructions are enabled. That would occur just from passing a -march for a newer CPU wouldn't it?

Maybe need support another attribute "attribute ((user_interrupt))" for functions? However this is what gcc does (https://gcc.godbolt.org/z/8ojTMG6bT).

Since there won't be both user interrupt handler and kernel interrupt handler in the source, there is no need for another
attribute. We discussed that kernel might need to use UINTR instructions. We decided that kernel could use inline asm
statements if needed.

So if write kernel code and compile with -march=haswell today, I get IRET. If tomorrow I change my command line to -march=sapphirerapids, now my kernel interrupt code generates user interrupt instructions. That seems surprising.

-mcmodel=kernel should disable uiret.:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99870

A user interrupt is different than a regular interrupt right? It doesn't make sense that we would change the behavior of the interrupt calling convention just because the the user interrupt instructions are enabled. That would occur just from passing a -march for a newer CPU wouldn't it?

Maybe need support another attribute "attribute ((user_interrupt))" for functions? However this is what gcc does (https://gcc.godbolt.org/z/8ojTMG6bT).

Since there won't be both user interrupt handler and kernel interrupt handler in the source, there is no need for another
attribute. We discussed that kernel might need to use UINTR instructions. We decided that kernel could use inline asm
statements if needed.

So if write kernel code and compile with -march=haswell today, I get IRET. If tomorrow I change my command line to -march=sapphirerapids, now my kernel interrupt code generates user interrupt instructions. That seems surprising.

-mcmodel=kernel should disable uiret.:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99870

That makes sense. Can we put that in this patch?

A user interrupt is different than a regular interrupt right? It doesn't make sense that we would change the behavior of the interrupt calling convention just because the the user interrupt instructions are enabled. That would occur just from passing a -march for a newer CPU wouldn't it?

Maybe need support another attribute "attribute ((user_interrupt))" for functions? However this is what gcc does (https://gcc.godbolt.org/z/8ojTMG6bT).

Since there won't be both user interrupt handler and kernel interrupt handler in the source, there is no need for another
attribute. We discussed that kernel might need to use UINTR instructions. We decided that kernel could use inline asm
statements if needed.

So if write kernel code and compile with -march=haswell today, I get IRET. If tomorrow I change my command line to -march=sapphirerapids, now my kernel interrupt code generates user interrupt instructions. That seems surprising.

-mcmodel=kernel should disable uiret.:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99870

That makes sense. Can we put that in this patch?

The feedback is that don't enable UINTR for kernel build.

tianqing updated this revision to Diff 335426.Tue, Apr 6, 1:20 AM

Update handling of -mcmodel=kernel.

LGMT. Thank you!

craig.topper added inline comments.Tue, Apr 6, 9:57 AM
llvm/test/CodeGen/X86/x86-64-intrcc-uintr.ll
2

Please use update_llc_test_checks.py

tianqing updated this revision to Diff 335980.Wed, Apr 7, 7:42 PM
tianqing marked an inline comment as done.Sun, Apr 11, 7:45 PM

Does anyone has further comments?

LuoYuanke accepted this revision.Mon, Apr 12, 10:17 PM

LGTM. But wait one or two days to see if there is more comments from Craig and HJ.

This revision is now accepted and ready to land.Mon, Apr 12, 10:17 PM