Re: [PATCH 06/20] xics: Create sPAPR specific ICS subtype

[David Gibson]

On Fri, Sep 27, 2019 at 06:05:56PM +0200, Greg Kurz wrote:
> On Thu, 26 Sep 2019 10:56:46 +1000
> David Gibson <address@hidden> wrote:
> 
> > On Wed, Sep 25, 2019 at 10:55:35AM +0200, Cédric Le Goater wrote:
> > > On 25/09/2019 10:40, Greg Kurz wrote:
> > > > On Wed, 25 Sep 2019 16:45:20 +1000
> > > > David Gibson <address@hidden> wrote:
> > > > 
> > > >> We create a subtype of TYPE_ICS specifically for sPAPR.  For now all 
> > > >> this
> > > >> does is move the setup of the PAPR specific hcalls and RTAS calls to
> > > >> the realize() function for this, rather than requiring the PAPR code to
> > > >> explicitly call xics_spapr_init().  In future it will have some more
> > > >> function.
> > > >>
> > > >> Signed-off-by: David Gibson <address@hidden>
> > > >> ---
> > > > 
> > > > LGTM, but for extra safety I would also introduce a SpaprIcsState 
> > > > typedef
> > > 
> > > why ? we have ICS_SPAPR() for the checks already.
> > 
> > Eh.. using typedefs when we haven't actually extended a base type
> > isn't common QOM practice.  Yes, it's not as typesafe as it could be,
> > but I'm not really inclined to go to the extra effort here.
> 
> I'll soon need to extend the base type with a nr_servers field,

Uh.. nr_servers doesn't seem like it belongs in the base ICS type.
That really would conflict with the pnv usage where the ICS is
supposed to just represent the ICS, not the xics as a whole.  If you
need nr_servers information here, it seems like pulling it via a
method in XICSFabric would make more sense.

> and while here with an fd field as well in order to get rid of
> the ugly global in xics.c. I'll go to the extra effort :)

That could go in the derived type.  We already kind of conflate ICS
and XICS-as-a-whole for the PAPR subtype, and the KVM xics is PAPR
only anyway.

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