On Mon, Sep 30, 2019 at 07:00:43PM +0200, Greg Kurz wrote: > On Mon, 30 Sep 2019 18:45:30 +1000 > David Gibson wrote: > > > On Fri, Sep 27, 2019 at 06:05:56PM +0200, Greg Kurz wrote: > > > On Thu, 26 Sep 2019 10:56:46 +1000 > > > David Gibson 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 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 > > > > > >> --- > > > > > > > > > > > > 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. > > Of course ! I re-used the wording "extended a base type" of your sentence, > that I understand as "a subtype extends a base type with some more data". > I'm talking about the sPAPR ICS subtype here, not the base ICS type. Ah, ok. > > 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. > > > > Exactly, so that's why I was thinking about adding nr_servers there, > but it could go to XICSFabric as well I guess. -- 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