On Wed Jun 14, 2023 at 6:54 PM AEST, Cédric Le Goater wrote: > On 6/14/23 07:14, Nicholas Piggin wrote: > > On Tue Jun 6, 2023 at 11:59 PM AEST, Cédric Le Goater wrote: > >> On 6/4/23 01:36, Nicholas Piggin wrote: > >>> This adds support for chiptod and core timebase state machine models in > >>> the powernv POWER9 and POWER10 models. > >>> > >>> This does not actually change the time or the value in TB registers > >>> (because they are alrady synced in QEMU), but it does go through the > >>> motions. It is enough to be able to run skiboot's chiptod initialisation > >>> code that synchronises core timebases (after a patch to prevent skiboot > >>> skipping chiptod for QEMU, posted to skiboot mailing list). > >>> > >>> Sorry there was some delay since the last posting. There is a bit more > >>> interest in this recently but feedback and comments from RFC was not > >>> forgotten and is much appreciated. > >>> > >>> https://lists.gnu.org/archive/html/qemu-ppc/2022-08/msg00324.html > >>> > >>> I think I accounted for everything except moving register defines to the > >>> .h file. I'm on the fence about that but if they are only used in the .c > >>> file I think it's okay to keep them there for now. I cut out a lot of > >>> unused ones so it's not so cluttered now. > >>> > >>> Lots of other changes and fixes since that RFC. Notably: > >>> - Register names changed to match the workbook names instead of skiboot. > >>> - TFMR moved to timebase_helper.c from misc_helper.c > >>> - More comprehensive model and error checking, particularly of TFMR. > >>> - POWER10 with multi-chip support. > >>> - chiptod and core timebase linked via specific state instead of TFMR. > >> > >> > >> The chiptod units are not exposed to the OS, it is all handled at FW > >> level AFAIK. Could the OPAL people provide some feedback on the low level > >> models ? > > > > Well, no takers so far. I guess I'm a OPAL people :) > >> I did some of the P10 chiptod addressing in skiboot, at least. This > > patch does work with the skiboot chiptod driver at least. > > cool, with 2 chips ?
Yes it worked with 2 chips. > > I would eventually like to add in the ability to actually change the > > TB with it, and inject errors and generate HMIs because that's an area > > that seem to be a bit lacking (most of such testing seemed to be done > > on real hardware using special time facility corruption injection). > > The MCE injection was a nice addon > > > https://lore.kernel.org/qemu-devel/20200325144147.221875-1-npig...@gmail.com/ > https://lore.kernel.org/qemu-devel/20211013214042.618918-1-...@kaod.org/ > > I hope it will get more attention if you resend. Will take another look. > > But yes for now it is a bit difficult to verify it does much useful > > aside from booting skiboot (+ patch to enable chiptod on QEMU I posted > > recently). > > It's difficult to review PATCH 4 without some good knowledge of HW. I know > you do but you can not review your own patches ! That said, the impact is > limited to PowerNV machines, I guess we are fine. Yeah. I appreciate all the review so far. It's pretty complicated even with the workbook. I might be able to add a simpler and higher-level description of the basic init sequence and states. You would still have to trust me, but it might make it easier to see what's going on. Thanks, Nick
