Messages in this thread | ![/](/images/icornerl.gif) | | Date | Wed, 10 Apr 2024 09:44:05 -0700 | Subject | Re: [PATCH v1 1/1] x86/entry/64: Skip SYSRET validation tests when FRED is enabled | From | Xin Li <> |
| |
On 4/9/2024 4:35 PM, H. Peter Anvin wrote: > Now, for a FRED system the return value is ignored anyway, so the only > benefit of this is skipping the test, correct?
Yes, and as syscalls are hot paths, we should save some cycles.
> > I do observe than what is left of do_fast_syscall_32 is a single > assignment followed by __do_fast_syscall_32(), which could be turned > into a tailcall.
Ah, I didn't realize this is a better way, will change.
> > Another thing that has been added lately is a bunch of tests on the int > $0x80 path. This is a slow path *on legacy hardware*, but on FRED > systems it is the *fast* path for IA32 emulation. These tests are also > totally unnecessary *AND IN FACT, WRONG* on a FRED system, as FRED > distinguishes external interrupts from software interrupts. > > Furthermore, under FRED interrupt 0x80 is available as a hardware > interrupt (since there is no reason to block it out.) Therefore, going > and poking the APIC as in int80_is_external() is INCORRECT and possibly > fatal. > > Again, the easiest way to fix that is to follow what XenPV does in > int80_is_external(), but there is more unnecessary stuff: > !user_mode(regs) cannot happen on FRED, and the stuff in the > int80_emulation assembly function should, if it is needed at all on any > FRED-compatible hardware (I don't believe so) should be done in the FRED > user mode assembly entry stub. > > Thus, it might be better to strip down do_int80_emulation() to a lean > fred_int80_emulation().
Yeah, FRED is fundamentally better with its nature. Will do!
Thanks! Xin
> > -hpa > > On 4/2/24 23:24, Xin Li (Intel) wrote: >> Don't do SYSRET validation tests when FRED is enabled, since ERETU is >> the only legit instruction to return to user level. >> >> Signed-off-by: Xin Li (Intel) <xin@zytor.com> >> --- >> arch/x86/entry/common.c | 12 ++++++++++-- >> 1 file changed, 10 insertions(+), 2 deletions(-)
| ![\](/images/icornerr.gif) |