On Mon, Sep 05, 2016 at 09:56:03AM +0200, Cédric Le Goater wrote: > On 09/05/2016 04:58 AM, David Gibson wrote: > > On Wed, Aug 31, 2016 at 06:34:10PM +0200, Cédric Le Goater wrote: > >> This is is an abstraction of a POWER8 chip which is a set of cores > >> plus other 'units', like the pervasive unit, the interrupt controller, > >> the memory controller, the on-chip microcontroller, etc. The whole can > >> be seen as a socket. It depends on a cpu model and its characteristics, > >> max cores, specific init are defined in a PnvChipClass. > >> > >> We start with an near empty PnvChip with only a few cpu constants > >> which we will grow in the subsequent patches with the controllers > >> required to run the system. > >> > >> Signed-off-by: Cédric Le Goater <[email protected]> > >> --- > >> > >> Changes since v1: > >> > >> - introduced a PnvChipClass depending on the cpu model. It also > >> provides some chip constants used by devices, like the cpu model hw > >> id (f000f), a enum type (not sure this is useful yet), a custom > >> realize ops for customization. > >> - the num-chips property can be configured on the command line. > >> > >> Maybe this object deserves its own file hw/ppc/pnv_chip.c ? > >> > >> hw/ppc/pnv.c | 154 > >> +++++++++++++++++++++++++++++++++++++++++++++++++++ > >> include/hw/ppc/pnv.h | 71 ++++++++++++++++++++++++ > >> 2 files changed, 225 insertions(+) > >> > >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > >> index 70413e3c5740..06051268e200 100644 > >> --- a/hw/ppc/pnv.c > >> +++ b/hw/ppc/pnv.c > >> @@ -168,6 +168,8 @@ static void ppc_powernv_init(MachineState *machine) > >> char *fw_filename; > >> long fw_size; > >> long kernel_size; > >> + int i; > >> + char *chip_typename; > >> > >> /* allocate RAM */ > >> if (ram_size < (1 * G_BYTE)) { > >> @@ -212,6 +214,153 @@ static void ppc_powernv_init(MachineState *machine) > >> exit(1); > >> } > >> } > >> + > >> + /* Create the processor chips */ > >> + chip_typename = g_strdup_printf(TYPE_PNV_CHIP "-%s", > >> machine->cpu_model); > >> + > >> + pnv->chips = g_new0(PnvChip *, pnv->num_chips); > >> + for (i = 0; i < pnv->num_chips; i++) { > >> + Object *chip = object_new(chip_typename); > >> + object_property_set_int(chip, CHIP_HWID(i), "chip-id", > >> &error_abort); > >> + object_property_set_bool(chip, true, "realized", &error_abort); > > > > I'm guessing these could fail due to bad user supplied parameters, not > > just internal bugs. In which case this should be &error_fatal rather > > than &error_abort. > > yes. > > > > >> + pnv->chips[i] = PNV_CHIP(chip); > >> + } > >> + g_free(chip_typename); > >> +} > >> + > >> +static void pnv_chip_power8nvl_realize(PnvChip *chip, Error **errp) > >> +{ > >> + ; > >> +} > > > > I don't think you should need to define an empty realize function. > > Or is this just a placeholder for things future patches will add? > > yes that was the plan but maybe this is a little early. chip_type > proved to be useful enough for the moment in the full patchset > I maintain. > > P9 will use a xive object when available and not a xics. I think > this is when the real big difference will show up. So I should > make realize() optional. > > >> + > >> +static void pnv_chip_power8nvl_class_init(ObjectClass *klass, void *data) > >> +{ > >> + DeviceClass *dc = DEVICE_CLASS(klass); > >> + PnvChipClass *k = PNV_CHIP_CLASS(klass); > >> + > >> + k->realize = pnv_chip_power8nvl_realize; > >> + k->cpu_model = "POWER8NVL"; > >> + k->chip_type = PNV_CHIP_P8NVL; > > > > With the new chip class per cpu class, does this chip_type field serve > > any purpose any more? > > It does look a bit redundant, I agree. But it is useful for xscom > address translation (P9 is a little different), for xscom devices > in general, for core id to pir conversion. It also does for lpc_irq > support, which applies to P8NVL (and upper I suppose). > > Let's keep it for the moment as it serves its purpose, which is > to handle small differences without too much cost. If this is > going too far, I will propose a set of ops per chip type.
Ok, fair enough.
--
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
