On Thu, Apr 29, 2021 at 08:59:14PM +0100, Marc Zyngier wrote: > AOn Thu, 29 Apr 2021 18:51:59 +0100, > Ricardo Koller <ricarkol@xxxxxxxxxx> wrote: > > > > On Fri, Apr 23, 2021 at 09:58:24AM +0100, Marc Zyngier wrote: > > > Hi Ricardo, > > > > > > Thanks for starting this. > > > > > > On Fri, 23 Apr 2021 05:03:49 +0100, > > > Ricardo Koller <ricarkol@xxxxxxxxxx> wrote: > > > > +.pushsection ".entry.text", "ax" > > > > +.balign 0x800 > > > > +.global vectors > > > > +vectors: > > > > +.popsection > > > > + > > > > +/* > > > > + * Build an exception handler for vector and append a jump to it into > > > > + * vectors (while making sure that it's 0x80 aligned). > > > > + */ > > > > +.macro HANDLER, el, label, vector > > > > +handler\()\vector: > > > > + save_registers \el > > > > + mov x0, sp > > > > + mov x1, \vector > > > > + bl route_exception > > > > + restore_registers \el > > > > + > > > > +.pushsection ".entry.text", "ax" > > > > +.balign 0x80 > > > > + b handler\()\vector > > > > +.popsection > > > > +.endm > > > > > > That's an interesting construct, wildly different from what we are > > > using elsewhere in the kernel, but hey, I like change ;-). It'd be > > > good to add a comment to spell out that anything that emits into > > > .entry.text between the declaration of 'vectors' and the end of this > > > file will break everything. > > > > > > > + > > > > +.global ex_handler_code > > > > +ex_handler_code: > > > > + HANDLER 1, sync, 0 // Synchronous EL1t > > > > + HANDLER 1, irq, 1 // IRQ EL1t > > > > + HANDLER 1, fiq, 2 // FIQ EL1t > > > > + HANDLER 1, error, 3 // Error EL1t > > > > > > Can any of these actually happen? As far as I can see, the whole > > > selftest environment seems to be designed around EL1h. > > > > > > > They can happen. KVM defaults to use EL1h: > > That's not a KVM decision. That's an architectural requirement. Reset > is an exception, exception use the handler mode. > That makes sense, thanks for the clarification. > > > > #define VCPU_RESET_PSTATE_EL1 (PSR_MODE_EL1h | PSR_A_BIT | PSR_I_BIT | \ > > > > but then a guest can set the SPSel to 0: > > > > asm volatile("msr spsel, #0"); > > > > and this happens: > > > > Unexpected exception guest (vector:0x0, ec:0x25) > > > > I think it should still be a valid situation: some test might want to > > try it. > > Sure, but that's not what this test (in patch #2) is doing, is it? > If, as I believe, this is an unexpected situation, why not handle it > separately? I'm not advocating one way or another, but it'd be good to > understand the actual scope of the exception handling in this > infrastructure. > > If you plan to allow tests to run in the EL1t environment, where do > you decide to switch back to EL1t after taking the exception in EL1h? > Are the tests supposed to implement both stack layouts? > > Overall, I'm worried that nobody is going to use this layout *unless* > it becomes mandated. > > Thanks, > > M. > > -- > Without deviation from the norm, progress is not possible. Got it, I see your point. Yes, I'm definitely not planning to use it. Will just treat those vectors as "invalid". Thanks again, Ricardo