[Skiboot] [PATCH] mbox: Sanitize interrupts registers

[Benjamin Herrenschmidt]

On Fri, 2017-04-21 at 09:51 +1000, Cyril Bur wrote:
> On Fri, 2017-04-21 at 08:59 +1000, Benjamin Herrenschmidt wrote:
> > If some status interrupts are left unmasked by a previous
> > firmware run (either HostBoot or some other version of skiboot),
> > we fail to clear them and end up with a runaway SerIRQ.
> > 
> > Signed-off-by: Benjamin Herrenschmidt <benh at kernel.crashing.org>
> > ---
> > 
> > Let's merge that now. It will be obsoleted by the v2 protocol
> > implementation but we want a fix out ASAP.
> 
> I think this is still important even after v2 implementation gets
> merged.

Don't we use a different interrupt with v2 anyway ? 

> > 
> >  hw/lpc-mbox.c | 15 +++++++++++----
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/lpc-mbox.c b/hw/lpc-mbox.c
> > index fd107fb..21e6eee 100644
> > --- a/hw/lpc-mbox.c
> > +++ b/hw/lpc-mbox.c
> > @@ -200,11 +200,18 @@ static struct lpc_client mbox_lpc_client = {
> >  
> >  static bool mbox_init_hw(void)
> >  {
> > -	/*
> > -	 * Turns out there isn't anything to do.
> > -	 * It might be a good idea to santise the registers
> > though.
> > -	 * TODO
> > +	/* Disable all status interrupts except attentions */
> > +	bmc_mbox_outb(0x00, MBOX_HOST_INT_EN_0);
> > +	bmc_mbox_outb(MBOX_STATUS_ATTN, MBOX_HOST_INT_EN_1);
> > +
> 
> Would it be wise to also (you do it for the control register
> interrupt):
> 
> 	/* 
> 	 * Cleanup host interrupt status bits except attention, these 	
>  * bits are write 1 to clear.
> 	 */
> 	bmc_mbox_outb(0xff, MBOX_STATUS_0);
> 	bmc_mbox_outb(~MBOX_STATUS_ATTN, MBOX_STATUS_1);

Not as important since we don't enable those... it won't hurt
to clear them but it won't gain anything either. Once we enable them,
the story is different.

So let's do that later with the v2 update.

Another subsequent fix we will need when we start using status
interrupts for message exchanges: Remember how the HW trigger the
interrupt by our own writes to the register ? IE not only when the BMC
writes the response but also when we write the command.

So if/when we switch to using the status interrupt instead of the host
interrupt, we need to keep it disabled until after we have written the
command (but not sent it to BMC yet) and cleared the status bit. Only
then it is safe to enable the interrupt.

> > +	/* Cleanup host interrupt and status */
> > +	bmc_mbox_outb(MBOX_CTRL_INT_STATUS, MBOX_HOST_CTRL);
> > +
> > +	/* Disable host control interrupt for now (will be
> > +	 * re-enabled when needed). Clear BMC interrupts
> >  	 */
> > +	bmc_mbox_outb(MBOX_CTRL_INT_MASK, MBOX_BMC_CTRL);
> > +
> >  	return true;
> >  }
> > 
> > 
> > _______________________________________________
> > Skiboot mailing list
> > Skiboot at lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/skiboot