Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 04/22] ppc/xics: add an InterruptStatsProvider interface to ICS and ICP objects

[David Gibson]

On Fri, Feb 24, 2017 at 11:52:01AM +0100, Cédric Le Goater wrote:
> On 02/23/2017 03:15 AM, David Gibson wrote:
> > On Thu, Feb 16, 2017 at 02:47:27PM +0100, Cédric Le Goater wrote:
> >> This is, again, to reduce the use of the list of ICS objects. Let's
> >> make each individual ICS and ICP object an InterruptStatsProvider and
> >> remove this same interface from XICSState.
> >>
> >> Signed-off-by: Cédric Le Goater <address@hidden>
> > 
> > I'm a little hesitant about this, because it means that getting the
> > interrupt stats information is now spread out over the qom tree,
> > whereas previously there was a single location to get a good summary
> > of the systems overall interrupt status.  The previous behaviour seems
> > like it would be more convenient for debugging.
> > 
> > That said, I see the structural advantages of this split.  Hmm.. still
> > thinking..
> 
> This is true. Another argument in favour of what you are saying 
> is the order in which these are printed. See below.  
> 
> What we could do after the cleanup is to make the machine an 
> InterruptStatsProvider to clarify things.

Right.  So "info pic" does at least iterate through all the providers,
but the semi-random order is pretty icky.  I think putting the stats
provider on the machine would be a better idea - I guess it should be
easy enough if the xics code provides a helper.

> 
> Thanks,
> 
> C. 
> 
> 
> (qemu) info pic 
> CPU 18 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 0 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 21 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 2 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 23 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 4 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 11 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 25 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 6 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 13 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 27 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 8 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 15 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 29 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 30 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 17 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 20 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 19 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 1 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> ICS 1000..13ff 0x10018f1ea40
>   1000 MSI ff 00
>   1001 MSI ff 00
>   1002 MSI ff 00
>   1003 MSI ff 00
>   1004 LSI ff 00
>   1005 LSI ff 00
>   1006 LSI ff 00
>   1007 LSI ff 00
>   1008 MSI ff 00
>   1009 MSI ff 00
>   100a MSI ff 00
>   100b MSI ff 00
>   100c MSI ff 00
> CPU 22 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 3 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 10 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 24 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 5 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 12 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 26 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 7 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 14 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 28 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 9 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 16 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> CPU 31 XIRR=00000000 ((nil)) PP=ff MFRR=ff
> 
> 

-- 
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

signature.asc
Description: PGP signature