[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] RE: [PATCH v13 26/35] x86/fred: FRED entry/exit and dispatch code
On December 6, 2023 11:19:26 AM PST, "Li, Xin3" <xin3.li@xxxxxxxxx> wrote: >> >>> + case X86_TRAP_OF: >> >>> + exc_overflow(regs); >> >>> + return; >> >>> + >> >>> + /* INT3 */ >> >>> + case X86_TRAP_BP: >> >>> + exc_int3(regs); >> >>> + return; >> >> ... neither OF nor BP will ever enter fred_intx() because they're >> >> type SWEXC not SWINT. >> > Per FRED spec 5.0, section 7.3 Software Interrupts and Related >> > Instructions: >> > INT n (opcode CD followed by an immediate byte): There are 256 such >> > software interrupt instructions, one for each value n of the immediate >> > byte (0–255). >> > >> > And appendix B Event Stack Levels: >> > If the event is an execution of INT n (opcode CD n for 8-bit value n), >> > the event stack level is 0. The event type is 4 (software interrupt) >> > and the vector is n. >> > >> > So int $0x4 and int $0x3 (use asm(".byte 0xCD, 0x03")) get here. >> > >> > But into (0xCE) and int3 (0xCC) do use event type SWEXC. >> > >> > BTW, into is NOT allowed in 64-bit mode but "int $0x4" is allowed. >> >> There is certainly fun to be had with CD 03 and CD 04 byte patterns, but if >> you >> meant to mean those here, then the comments are wrong. >> >> Vectors 3 and 4 are installed with DPL3 because that is necessary to make CC >> and >> CE function in userspace. It also suggests that the SWINT vs SWEXC >> distinction >> was retrofitted to architecture after the 286, because exceptions don't >> check DPL >> and ICEBP delivers #DB from userspace even when Vector 1 has a DPL of 0. >> >> While CC is for most cases indistinguishable from CD 03, CE behaves entirely >> differently to CD 04. CD 04 doesn't #UD in 64bit mode, and will trigger >> exc_overflow() irrespective of the state of EFLAGS.OF. >> >> >> The SDM goes out of it's way to say not to use the CD 03 byte pattern (and it >> does take effort to emit this byte pattern - e.g. GAS will silently >> translate "int $3" >> to "int3"), and there's no plausible way software is using CD 04 in place of >> CE. >> >> So why do we care about containing to make mistakes of the IDT era work in a >> FRED world? > >First, I agree with you because it makes things simple and neat. > >However, the latest SDM and FRED spec 5.0 both doesn't disallow it, so it >becomes an OS implementation choice. > >> >> Is there anything (other than perhaps the selftests) which would even notice? > >I'm just conservative :) > >If a user app can do it with IDT, we should still allow it when FRED is >enabled. But if all key stakeholders don't care whatever gets broken >due to the change and agree to change it. > >> >>> + instrumentation_end(); >> >>> + irqentry_exit(regs, state); >> >>> + } else { >> >>> + common_interrupt(regs, vector); >> >>> + } >> >>> +} >> >>> + >> >>> +static noinstr void fred_exception(struct pt_regs *regs, unsigned >> >>> +long error_code) { >> >>> + /* Optimize for #PF. That's the only exception which matters >> >>> +performance >> >> wise */ >> >>> + if (likely(regs->fred_ss.vector == X86_TRAP_PF)) { >> >>> + exc_page_fault(regs, error_code); >> >>> + return; >> >>> + } >> >>> + >> >>> + switch (regs->fred_ss.vector) { >> >>> + case X86_TRAP_DE: return exc_divide_error(regs); >> >>> + case X86_TRAP_DB: return fred_exc_debug(regs); >> >>> + case X86_TRAP_BP: return exc_int3(regs); >> >>> + case X86_TRAP_OF: return exc_overflow(regs); >> >> Depending on what you want to do with BP/OF vs fred_intx(), this may >> >> need adjusting. >> >> >> >> If you are cross-checking type and vector, then these should be >> >> rejected for not being of type HWEXC. >> > You're right, the event type needs to be SWEXC for into and int3. >> > >> > However, would it be overkilling? Assuming hardware and VMM are sane. >> >> You either care about cross checking, or not. Right now, this patch is a >> mix of the >> two approaches. >> >> In my opinion, cross-checking is the better approach, because it means that >> violations of the assumptions get noticed more quickly, and hopefully by >> whomever is working on the new feature which alters the assumptions. > >Yeah, I can make the change. > >Thanks! > Xin > The intent is to not break userspace even if userspace does something fundamentally stupid.
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |