Re: [PATCH gnumach] interrupt: Mask, eoi, unmask

[Samuel Thibault]

Samuel Thibault, le lun. 02 oct. 2023 01:56:51 +0200, a ecrit:
> Samuel Thibault, le lun. 02 oct. 2023 01:50:14 +0200, a ecrit:
> > Samuel Thibault, le lun. 02 oct. 2023 01:43:48 +0200, a ecrit:
> > > Damien Zammit, le dim. 01 oct. 2023 23:26:02 +0000, a ecrit:
> > > > I think the logic for this should be:
> > > > 
> > > > When we get irq N, first we mask irq N, then EOI irq N.
> > > > Then call the handler. If there is a user handler for irq N, let the 
> > > > irq_ack
> > > > unmask irq N, otherwise we need to unmask irq N now.
> > > > But don't EOI in the user handlers anymore.
> > > > 
> > > > What do you think?
> > > 
> > > That can be a plan, yes, but what about the legacy irqs, and their
> > > in-kernel handlers?  We need to fix them too. One keypoint there is that
> > > we want to unmask with IF cleared, so that we don't get interrupted
> > > again on unmask, but on iret (so we don't nest).
> > 
> > For the legacy irqs and their in-kernel handlers, we don't really need
> > to mask actually, we can just keep IF cleared, i.e. the spl level raise
> > should be enough. But we want to keep IF cleared when lowering it again,
> > IIRC that's already handled so.
> 
> So in the end, I'd tend to think that it's up to queue_intr to do the
> unmasking, like it does now, while spl is still high and thus we don't
> risk nesting. That way it's the in-kernel intr handler that knows
> whether to mask/unmask or not. I.e. it'd be like this:
> 
> - interrupt.S raises spl (thus IF cleared)
> - interrupt.S EOI
> - interrupt.S calls the handler
>   - for pure in-kernel handlers, they do whatever they want with IF
>     cleared.
>   - when a userland handler is registers, queue_intr masks the irq.
> - interrupt.S lowers spl with splx_cli, thus IF still cleared
> - iret, that clears the IF
> 
> - later on, userland acks the IRQ, that unmasks the irq
> 
> Is that not already very close to what we have currently?

(of course we want to record that reasoning in the source code)

Samuel