[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 4/7] pnv/chiptod: Add POWER9/10 chiptod model
From: |
Nicholas Piggin |
Subject: |
Re: [PATCH 4/7] pnv/chiptod: Add POWER9/10 chiptod model |
Date: |
Fri, 24 Nov 2023 16:14:44 +1000 |
On Fri Nov 24, 2023 at 3:49 AM AEST, Cédric Le Goater wrote:
> On 11/23/23 11:30, Nicholas Piggin wrote:
> > The ChipTOD (for Time-Of-Day) is a pervasive facility that keeps the
> > clocks on multiple chips consistent which can keep TOD (time-of-day),
> > synchronise it across multiple chips, and can move that TOD to or from
> > the core timebase units.
>
> May be rephrase a bit the sentence above. I find it difficult to read.
>
> > This driver implements basic emulation of chiptod registers sufficient
>
> it's a model.
+1
> > +static void chiptod_power9_send_remotes(PnvChipTOD *chiptod)
> > +{
> > + PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
>
> Using qdev_get_machine() in a model is always a bit annoying. There is
> work in progress currently to have a single QEMU binary for all arches
> and when done, the "machine" would encompass something bigger including
> the OCC, BMC, etc.
We need a way to get from one chip to another, fundamentally. I
didn't see a better way, I suppose we could build a PnvHost container
that includes all PnvChips or something.
> Do we know how XSCOM propagates the new state to the chiptop on other
> chips ? is it some sort of broadcast on the bus ? Could we model it ?
> I am only asking, not to be done now.
It's a bit hard to work out. Real ChipTOD is vastly more complicated
than this model :P
It seems that ChipTOD can master PIB scoms to other chips, but also
this TTYPE transactions look like they have a command that goes
on the powerbus via ADU.
> > +static void chiptod_power10_invalidate_remotes(PnvChipTOD *chiptod)
> > +{
> > + PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
> > + int i;
> > +
> > + for (i = 0; i < pnv->num_chips; i++) {
> > + Pnv10Chip *chip10 = PNV10_CHIP(pnv->chips[i]);
> > + if (&chip10->chiptod != chiptod) {
> > + chip10->chiptod.tod_state = tod_not_set;
> > + }
> > + }
> > +}
>
> Could we avoid 4 routines doing the same thing and introduce :
>
> chiptod_power*_set_tod_state(PnvChipTOD *chiptod, enum tod_state new)
>
> ?
>
> We could even introcude a PnvChipClass::get_chiptod handler. Might be
> overkill though.
Good idea...
> > + case TOD_TX_TTYPE_5_REG:
> > + if (is_power9) {
> > + chiptod_power9_invalidate_remotes(chiptod);
> > + } else {
> > + chiptod_power10_invalidate_remotes(chiptod);
> > + }
>
> With PnvChipTODClass::invalidate_remotes and PnvChipTODClass::send_remotes
> handlers, or ::set_state, this routine would not need a 'is_power9' parameter
> and the model would not need 2 different xscom_ops. Can it be done ?
... yes! ChipTODClass seems to be a good place for it.
> > +static void pnv_chiptod_realize(DeviceState *dev, Error **errp)
> > +{
> > + PnvChipTOD *chiptod = PNV_CHIPTOD(dev);
> > + PnvChipTODClass *pctc = PNV_CHIPTOD_GET_CLASS(chiptod);
> > +
> > + if (chiptod->primary) {
> > + chiptod->pss_mss_ctrl_reg |= PPC_BIT(1); /* TOD is master */
> > + }
> > +
> > + /* Drawer is master (we do not simulate multi-drawer) */
> > + chiptod->pss_mss_ctrl_reg |= PPC_BIT(2);
> > + chiptod->tod_state = tod_running;
>
> Shouldn't these initial values be set in a reset handler ?
Yes, and actually reset state is tod_error so fixed that too.
Thanks,
Nick
- [PATCH 0/7] ppc: pnv ChipTOD and various timebase fixes, Nicholas Piggin, 2023/11/23
- [PATCH 1/7] target/ppc: Rename TBL to TB on 64-bit, Nicholas Piggin, 2023/11/23
- [PATCH 2/7] target/ppc: Improve timebase register defines naming, Nicholas Piggin, 2023/11/23
- [PATCH 3/7] target/ppc: Fix move-to timebase SPR access permissions, Nicholas Piggin, 2023/11/23
- [PATCH 5/7] pnv/chiptod: Implement the ChipTOD to Core transfer, Nicholas Piggin, 2023/11/23
- [PATCH 4/7] pnv/chiptod: Add POWER9/10 chiptod model, Nicholas Piggin, 2023/11/23
- [PATCH 6/7] target/ppc: Implement core timebase state machine and TFMR, Nicholas Piggin, 2023/11/23
- [PATCH 7/7] target/ppc: Add SMT support to time facilities, Nicholas Piggin, 2023/11/23
- Re: [PATCH 0/7] ppc: pnv ChipTOD and various timebase fixes, Cédric Le Goater, 2023/11/23