Re: [Qemu-devel] Re: [PATCH 13/15] Add POWER7 support for ppc

[David Gibson]

On Sat, Feb 12, 2011 at 05:09:39PM +0100, Alexander Graf wrote:
> On 12.02.2011, at 15:54, David Gibson wrote:
[snip]
> > +    /* Don't generate spurious events */
> > +    if ((cur_level == 1 && level == 0) || (cur_level == 0 && level != 0)) {
> 
> Did you hit this? Qemu's irq framework should already ensure that
> property. I'm also not sure it's actually correct - if a level
> interrupt is on, the guest would get another interrupt injected, no?
> That would be cur_level ==1 && level == 1 IIUC.

[snip]
> > +        case POWER7_INPUT_CKSTP:
> 
> POWER7 has checkstop?

[snip]
> > +        case POWER7_INPUT_HRESET:
> 
> Does this ever get triggered? POWER7 is run in lpar only, so there is no 
> hreset, right?

[snip]
> > +        case POWER7_INPUT_TBEN:
> > +            LOG_IRQ("%s: set the TBEN state to %d\n", __func__,
> > +                        level);
> > +            /* XXX: TODO */
> 
> Hrm - what is this?

Ah, drat.  I forgot about this.  The POWER7 interrupt stuff I copied
from 970 and them modified minimally to get it working.  I meant to
get around to auditing this stuff to see what was actually relevant to
POWER7.  I'll address this for the next version.

[snip]
> > +#if !defined(CONFIG_USER_ONLY)
> > +    env->slb_nr = 32;
> 
> POWER7 has 64, no? Please check this :).

Nope.  POWER4 and POWER5 have 64, but POWER7 has 32.  This one I did
check and change.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson