This is an archive of the discontinued LLVM Phabricator instance.

[X86] Enable compilation of user interrupt handlers.
ClosedPublic

Authored by tianqing on Apr 1 2021, 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.Apr 1 2021, 1:23 AM
tianqing requested review of this revision.Apr 1 2021, 1:23 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptApr 1 2021, 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.EditedApr 1 2021, 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.Apr 6 2021, 1:20 AM

Update handling of -mcmodel=kernel.

LGMT. Thank you!

craig.topper added inline comments.Apr 6 2021, 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.Apr 7 2021, 7:42 PM
tianqing marked an inline comment as done.Apr 11 2021, 7:45 PM

Does anyone has further comments?

LuoYuanke accepted this revision.Apr 12 2021, 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.Apr 12 2021, 10:17 PM
This revision was landed with ongoing or failed builds.Apr 22 2021, 8:44 PM
This revision was automatically updated to reflect the committed changes.